Fix Crash when no encryption subkey is available, Issue #1817

Signed-off-by: Durgesh <007durgesh219@gmail.com>

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java

@@ -728,6 +728,7 @@ public abstract class OperationResult implements Parcelable {
         MSG_PSE_ERROR_PGP (LogLevel.ERROR, R.string.msg_pse_error_pgp),
         MSG_PSE_ERROR_SIG (LogLevel.ERROR, R.string.msg_pse_error_sig),
         MSG_PSE_ERROR_UNLOCK (LogLevel.ERROR, R.string.msg_pse_error_unlock),
+        MSG_PSE_ERROR_REVOKED_OR_EXPIRED (LogLevel.ERROR, R.string.msg_pse_error_revoked_or_expired),
         MSG_PSE_KEY_OK (LogLevel.OK, R.string.msg_pse_key_ok),
         MSG_PSE_KEY_UNKNOWN (LogLevel.DEBUG, R.string.msg_pse_key_unknown),
         MSG_PSE_KEY_WARN (LogLevel.WARN, R.string.msg_pse_key_warn),

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptInputParcel.java

@@ -38,7 +38,6 @@ public class PgpSignEncryptInputParcel implements Parcelable {
     protected Long mSignatureSubKeyId = null;
     protected int mSignatureHashAlgorithm = PgpSecurityConstants.OpenKeychainHashAlgorithmTags.USE_DEFAULT;
     protected long mAdditionalEncryptId = Constants.key.none;
-    protected boolean mFailOnMissingEncryptionKeyIds = false;
     protected String mCharset;
     protected boolean mCleartextSignature;
     protected boolean mDetachedSignature = false;
@@ -65,7 +64,6 @@ public class PgpSignEncryptInputParcel implements Parcelable {
         mSignatureSubKeyId = source.readInt() == 1 ? source.readLong() : null;
         mSignatureHashAlgorithm = source.readInt();
         mAdditionalEncryptId = source.readLong();
-        mFailOnMissingEncryptionKeyIds = source.readInt() == 1;
         mCharset = source.readString();
         mCleartextSignature = source.readInt() == 1;
         mDetachedSignature = source.readInt() == 1;
@@ -96,7 +94,6 @@ public class PgpSignEncryptInputParcel implements Parcelable {
         }
         dest.writeInt(mSignatureHashAlgorithm);
         dest.writeLong(mAdditionalEncryptId);
-        dest.writeInt(mFailOnMissingEncryptionKeyIds ? 1 : 0);
         dest.writeString(mCharset);
         dest.writeInt(mCleartextSignature ? 1 : 0);
         dest.writeInt(mDetachedSignature ? 1 : 0);
@@ -113,10 +110,6 @@ public class PgpSignEncryptInputParcel implements Parcelable {
         this.mCharset = mCharset;
     }
 
-    public boolean isFailOnMissingEncryptionKeyIds() {
-        return mFailOnMissingEncryptionKeyIds;
-    }
-
     public long getAdditionalEncryptId() {
         return mAdditionalEncryptId;
     }
@@ -207,11 +200,6 @@ public class PgpSignEncryptInputParcel implements Parcelable {
         return this;
     }
 
-    public PgpSignEncryptInputParcel setFailOnMissingEncryptionKeyIds(boolean failOnMissingEncryptionKeyIds) {
-        mFailOnMissingEncryptionKeyIds = failOnMissingEncryptionKeyIds;
-        return this;
-    }
-
     public PgpSignEncryptInputParcel setCleartextSignature(boolean cleartextSignature) {
         this.mCleartextSignature = cleartextSignature;
         return this;

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncryptOperation.java

@@ -168,10 +168,17 @@ public class PgpSignEncryptOperation extends BaseOperation {
             try {
                 long signingMasterKeyId = input.getSignatureMasterKeyId();
                 long signingSubKeyId = input.getSignatureSubKeyId();
-                {
+
-                    CanonicalizedSecretKeyRing signingKeyRing =
+                CanonicalizedSecretKeyRing signingKeyRing =
-                            mProviderHelper.getCanonicalizedSecretKeyRing(signingMasterKeyId);
+                        mProviderHelper.getCanonicalizedSecretKeyRing(signingMasterKeyId);
-                    signingKey = signingKeyRing.getSecretKey(input.getSignatureSubKeyId());
+                signingKey = signingKeyRing.getSecretKey(input.getSignatureSubKeyId());
+
+
+                // Make sure key is not expired or revoked
+                if (signingKeyRing.isExpired() || signingKeyRing.isRevoked()
+                        || signingKey.isExpired() || signingKey.isRevoked()) {
+                    log.add(LogType.MSG_PSE_ERROR_REVOKED_OR_EXPIRED, indent);
+                    return new PgpSignEncryptResult(PgpSignEncryptResult.RESULT_ERROR, log);
                 }
 
                 // Make sure we are allowed to sign here!
@@ -281,16 +288,17 @@ public class PgpSignEncryptOperation extends BaseOperation {
                         if (encryptSubKeyIds.isEmpty()) {
                             log.add(LogType.MSG_PSE_KEY_WARN, indent + 1,
                                     KeyFormattingUtils.convertKeyIdToHex(id));
-                            if (input.isFailOnMissingEncryptionKeyIds()) {
+                            return new PgpSignEncryptResult(PgpSignEncryptResult.RESULT_ERROR, log);
-                                return new PgpSignEncryptResult(PgpSignEncryptResult.RESULT_ERROR, log);
+                        }
-                            }
+                        // Make sure key is not expired or revoked
+                        if (keyRing.isExpired() || keyRing.isRevoked()) {
+                            log.add(LogType.MSG_PSE_ERROR_REVOKED_OR_EXPIRED, indent);
+                            return new PgpSignEncryptResult(PgpSignEncryptResult.RESULT_ERROR, log);
                         }
                     } catch (ProviderHelper.NotFoundException e) {
                         log.add(LogType.MSG_PSE_KEY_UNKNOWN, indent + 1,
                                 KeyFormattingUtils.convertKeyIdToHex(id));
-                        if (input.isFailOnMissingEncryptionKeyIds()) {
+                        return new PgpSignEncryptResult(PgpSignEncryptResult.RESULT_ERROR, log);
-                            return new PgpSignEncryptResult(PgpSignEncryptResult.RESULT_ERROR, log);
-                        }
                     }
                 }
             }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java

@@ -336,8 +336,7 @@ public class OpenPgpService extends Service {
                     .setVersionHeader(null)
                     .setCompressionAlgorithm(compressionId)
                     .setSymmetricEncryptionAlgorithm(PgpSecurityConstants.OpenKeychainSymmetricKeyAlgorithmTags.USE_DEFAULT)
-                    .setEncryptionMasterKeyIds(keyIds)
+                    .setEncryptionMasterKeyIds(keyIds);
-                    .setFailOnMissingEncryptionKeyIds(true);
 
             if (sign) {
 

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/KeyAdapter.java

@@ -66,6 +66,7 @@ public class KeyAdapter extends CursorAdapter {
             KeyRings.HAS_DUPLICATE_USER_ID,
             KeyRings.FINGERPRINT,
             KeyRings.CREATION,
+            KeyRings.HAS_ENCRYPT
     };
 
     public static final int INDEX_MASTER_KEY_ID = 1;
@@ -77,6 +78,7 @@ public class KeyAdapter extends CursorAdapter {
     public static final int INDEX_HAS_DUPLICATE_USER_ID = 7;
     public static final int INDEX_FINGERPRINT = 8;
     public static final int INDEX_CREATION = 9;
+    public static final int INDEX_HAS_ENCRYPT = 10;
 
     public KeyAdapter(Context context, Cursor c, int flags) {
         super(context, c, flags);
@@ -289,6 +291,7 @@ public class KeyAdapter extends CursorAdapter {
         public final KeyRing.UserId mUserId;
         public final long mKeyId;
         public final boolean mHasDuplicate;
+        public final boolean mHasEncrypt;
         public final Date mCreation;
         public final String mFingerprint;
         public final boolean mIsSecret, mIsRevoked, mIsExpired, mIsVerified;
@@ -299,6 +302,7 @@ public class KeyAdapter extends CursorAdapter {
             mUserIdFull = userId;
             mKeyId = cursor.getLong(INDEX_MASTER_KEY_ID);
             mHasDuplicate = cursor.getLong(INDEX_HAS_DUPLICATE_USER_ID) > 0;
+            mHasEncrypt = cursor.getInt(INDEX_HAS_ENCRYPT) != 0;
             mCreation = new Date(cursor.getLong(INDEX_CREATION) * 1000);
             mFingerprint = KeyFormattingUtils.convertFingerprintToHex(
                     cursor.getBlob(INDEX_FINGERPRINT));
@@ -315,6 +319,7 @@ public class KeyAdapter extends CursorAdapter {
             mUserIdFull = userId;
             mKeyId = ring.getMasterKeyId();
             mHasDuplicate = false;
+            mHasEncrypt = key.getKeyRing().getEncryptIds().size() > 0;
             mCreation = key.getCreationTime();
             mFingerprint = KeyFormattingUtils.convertFingerprintToHex(
                     ring.getFingerprint());

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/widget/EncryptKeyCompletionView.java

@@ -21,6 +21,7 @@ package org.sufficientlysecure.keychain.ui.widget;
 
 import android.content.Context;
 import android.database.Cursor;
+import android.graphics.Color;
 import android.graphics.Rect;
 import android.net.Uri;
 import android.os.Bundle;
@@ -83,6 +84,11 @@ public class EncryptKeyCompletionView extends TokenCompleteTextView<KeyItem>
         LayoutInflater l = LayoutInflater.from(getContext());
         View view = l.inflate(R.layout.recipient_box_entry, null);
         ((TextView) view.findViewById(android.R.id.text1)).setText(keyItem.getReadableName());
+
+        if (keyItem.mIsRevoked || !keyItem.mHasEncrypt || keyItem.mIsExpired) {
+            ((TextView) view.findViewById(android.R.id.text1)).setTextColor(Color.RED);
+        }
+
         return view;
     }
 

OpenKeychain/src/main/res/values/strings.xml

@@ -1259,6 +1259,7 @@
     <string name="msg_pse_error_pgp">"Internal OpenPGP error!"</string>
     <string name="msg_pse_error_sig">"Encountered OpenPGP signature exception!"</string>
     <string name="msg_pse_error_unlock">"Unknown error unlocking key!"</string>
+    <string name="msg_pse_error_revoked_or_expired">"Revoked/Expired key cannot be used for sign or encryption"</string>
     <string name="msg_pse_key_ok">"Encrypting for key: %s"</string>
     <string name="msg_pse_key_unknown">"Missing key for encryption: %s"</string>
     <string name="msg_pse_key_warn">"Bad key for encryption: %s"</string>