Merge branch 'master' into encrypted-export

4 files changed, 83 insertions, 39 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/SignEncryptOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/SignEncryptOperation.java
index 843a55389..2ca74063c 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/SignEncryptOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/SignEncryptOperation.java
@@ -17,6 +17,16 @@
 
 package org.sufficientlysecure.keychain.operations;
 
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.FileNotFoundException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.concurrent.atomic.AtomicBoolean;
+
 import android.content.Context;
 import android.net.Uri;
 import android.support.annotation.NonNull;
@@ -40,21 +50,13 @@ import org.sufficientlysecure.keychain.util.InputData;
 import org.sufficientlysecure.keychain.util.Log;
 import org.sufficientlysecure.keychain.util.ProgressScaler;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.FileNotFoundException;
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.util.ArrayDeque;
-import java.util.ArrayList;
-import java.util.concurrent.atomic.AtomicBoolean;
 
-/** This is a high-level operation, which encapsulates one or more sign/encrypt
+/**
+ * This is a high-level operation, which encapsulates one or more sign/encrypt
  * operations, using URIs or byte arrays as input and output.
  *
  * This operation is fail-fast: If any sign/encrypt sub-operation fails or returns
  * a pending result, it will terminate.
- *
  */
 public class SignEncryptOperation extends BaseOperation<SignEncryptParcel> {
 
@@ -63,6 +65,7 @@ public class SignEncryptOperation extends BaseOperation<SignEncryptParcel> {
         super(context, providerHelper, progressable, cancelled);
     }
 
+
     @NonNull
     public SignEncryptResult execute(SignEncryptParcel input, CryptoInputParcel cryptoInput) {
 
@@ -115,7 +118,7 @@ public class SignEncryptOperation extends BaseOperation<SignEncryptParcel> {
                     log.add(LogType.MSG_SE_INPUT_URI, 1);
                     Uri uri = inputUris.removeFirst();
                     try {
-                        InputStream is = mContext.getContentResolver().openInputStream(uri);
+                        InputStream is = FileHelper.openInputStreamSafe(mContext.getContentResolver(), uri);
                         long fileSize = FileHelper.getFileSize(mContext, uri, 0);
                         String filename = FileHelper.getFilename(mContext, uri);
                         inputData = new InputData(is, fileSize, filename);
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignatureChecker.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignatureChecker.java
index 4067372a1..ed5566bc1 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignatureChecker.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignatureChecker.java
@@ -133,7 +133,7 @@ class PgpSignatureChecker {
                         KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(sigKeyId)
                 );
                 CanonicalizedPublicKey keyCandidate = signingRing.getPublicKey(sigKeyId);
-                if ( ! signingKey.canSign()) {
+                if ( ! keyCandidate.canSign()) {
                     continue;
                 }
                 signatureIndex = i;
@@ -156,7 +156,7 @@ class PgpSignatureChecker {
                         KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(sigKeyId)
                 );
                 CanonicalizedPublicKey keyCandidate = signingRing.getPublicKey(sigKeyId);
-                if ( ! signingKey.canSign()) {
+                if ( ! keyCandidate.canSign()) {
                     continue;
                 }
                 signatureIndex = i;
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFilesFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFilesFragment.java
index 3d1629ffb..bfe9ea290 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFilesFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFilesFragment.java
@@ -18,7 +18,6 @@
 package org.sufficientlysecure.keychain.ui;
 
 
-import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Date;
@@ -453,29 +452,9 @@ public class EncryptFilesFragment
 
     }
 
-    /**
-     * Checks that the input uris are not linked to our own internal storage.
-     * This prevents the encryption of our own database (-> export of whole database)
-     */
-    private void securityCheckInternalStorage() {
-        for (FilesAdapter.ViewModel model : mFilesAdapter.mDataset) {
-            File fileInput = new File(model.inputUri.getPath());
-            try {
-                // the canonical path of the file must not start with /data/data/org.sufficientlysecure.keychain/
-                if (fileInput.getCanonicalPath().startsWith(getActivity().getApplicationInfo().dataDir)) {
-                    throw new RuntimeException("Encrypting OpenKeychain's private files is not allowed!");
-                }
-            } catch (IOException e) {
-                Log.e(Constants.TAG, "Getting canonical path failed!", e);
-            }
-        }
-    }
-
-    /**
-     * Prepares mOutputUris, either directly and returns false, or indirectly
-     * which returns true and will call cryptoOperation after mOutputUris has
-     * been set at a later point.
-     */
+    // prepares mOutputUris, either directly and returns false, or indirectly
+    // which returns true and will call cryptoOperation after mOutputUris has
+    // been set at a later point.
     private boolean prepareOutputStreams() {
 
         switch (mAfterEncryptAction) {
@@ -551,8 +530,6 @@ public class EncryptFilesFragment
 
         }
 
-        securityCheckInternalStorage();
-
         return actionsParcel;
 
     }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileHelper.java
index 902af3932..671275823 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileHelper.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/FileHelper.java
@@ -22,6 +22,7 @@ import java.io.BufferedInputStream;
 import java.io.BufferedOutputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
@@ -32,6 +33,7 @@ import android.content.ActivityNotFoundException;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
+import android.content.res.AssetFileDescriptor;
 import android.database.Cursor;
 import android.graphics.Bitmap;
 import android.graphics.Point;
@@ -39,13 +41,19 @@ import android.net.Uri;
 import android.os.Build;
 import android.os.Build.VERSION_CODES;
 import android.os.Environment;
+import android.os.ParcelFileDescriptor;
 import android.provider.DocumentsContract;
 import android.provider.OpenableColumns;
 import android.support.v4.app.Fragment;
+import android.system.ErrnoException;
+import android.system.Os;
+import android.system.StructStat;
 import android.widget.Toast;
 
+import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
 
+import static android.system.OsConstants.S_IROTH;
 
 /** This class offers a number of helper functions for saving documents.
  *
@@ -261,6 +269,62 @@ public class FileHelper {
         return true;
     }
 
+    /**
+     * Tests whether a file is readable by others
+     */
+    @TargetApi(VERSION_CODES.LOLLIPOP)
+    public static boolean S_IROTH(int mode) {
+        return (mode & S_IROTH) == S_IROTH;
+    }
+
+    /**
+     * A replacement for ContentResolver.openInputStream() that does not allow the usage of
+     * "file" Uris that point to private files owned by the application only.
+     *
+     * This is not allowed:
+     * am start -a android.intent.action.SEND -t text/plain -n
+     * "org.sufficientlysecure.keychain.debug/org.sufficientlysecure.keychain.ui.EncryptFilesActivity" --eu
+     * android.intent.extra.STREAM
+     * file:///data/data/org.sufficientlysecure.keychain.debug/databases/openkeychain.db
+     *
+     * @throws FileNotFoundException
+     */
+    @TargetApi(VERSION_CODES.LOLLIPOP)
+    public static InputStream openInputStreamSafe(ContentResolver resolver, Uri uri)
+            throws FileNotFoundException {
+
+        // Not supported on Android < 5
+        if (Build.VERSION.SDK_INT < VERSION_CODES.LOLLIPOP) {
+            return resolver.openInputStream(uri);
+        }
+
+        String scheme = uri.getScheme();
+        if (ContentResolver.SCHEME_FILE.equals(scheme)) {
+            ParcelFileDescriptor pfd = ParcelFileDescriptor.open(
+                    new File(uri.getPath()), ParcelFileDescriptor.parseMode("r"));
+
+            try {
+                final StructStat st = Os.fstat(pfd.getFileDescriptor());
+                if (!S_IROTH(st.st_mode)) {
+                    Log.e(Constants.TAG, "File is not readable by others, aborting!");
+                    throw new FileNotFoundException("Unable to create stream");
+                }
+            } catch (ErrnoException e) {
+                Log.e(Constants.TAG, "fstat() failed: " + e);
+                throw new FileNotFoundException("fstat() failed");
+            }
+
+            AssetFileDescriptor fd = new AssetFileDescriptor(pfd, 0, -1);
+            try {
+                return fd.createInputStream();
+            } catch (IOException e) {
+                throw new FileNotFoundException("Unable to create stream");
+            }
+        } else {
+            return resolver.openInputStream(uri);
+        }
+    }
+
     public interface FileDialogCallback {
         void onFileSelected(File file, boolean checked);
     }