be more mindful of algorithm and usage flag interaction

Fixes #895

3 files changed, 42 insertions, 16 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
index 0e9377890..b4842b0a5 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
@@ -619,8 +619,7 @@ public class UncachedKeyRing {
                     boolean needsPrimaryBinding = false;
 
                     // If the algorithm is even suitable for signing
-                    if (key.getAlgorithm() != PublicKeyAlgorithmTags.ELGAMAL_ENCRYPT
-                            && key.getAlgorithm() != PublicKeyAlgorithmTags.RSA_ENCRYPT) {
+                    if (isSigningAlgo(key.getAlgorithm())) {
 
                         // If this certificate says it allows signing for the key
                         if (zert.getHashedSubPackets() != null &&
@@ -722,6 +721,24 @@ public class UncachedKeyRing {
                 continue;
             }
 
+            // If we have flags, check if the algorithm supports all of them
+            if (selfCert.getHashedSubPackets() == null
+                    && selfCert.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.KEY_FLAGS)) {
+                int flags = ((KeyFlags) selfCert.getHashedSubPackets().getSubpacket(SignatureSubpacketTags.KEY_FLAGS)).getFlags();
+                int algo = key.getAlgorithm();
+                // If this is a signing key, but not a signing algorithm, warn the user
+                if (!isSigningAlgo(algo) && (flags & PGPKeyFlags.CAN_SIGN) == PGPKeyFlags.CAN_SIGN) {
+                    log.add(LogType.MSG_KC_SUB_ALGO_BAD_SIGN, indent);
+                }
+                // If this is an encryption key, but not an encryption algorithm, warn the user
+                if (!isEncryptionAlgo(algo) && (
+                           (flags & PGPKeyFlags.CAN_ENCRYPT_COMMS) == PGPKeyFlags.CAN_ENCRYPT_COMMS
+                        || (flags & PGPKeyFlags.CAN_ENCRYPT_STORAGE) == PGPKeyFlags.CAN_ENCRYPT_STORAGE
+                    )) {
+                    log.add(LogType.MSG_KC_SUB_ALGO_BAD_ENCRYPT, indent);
+                }
+            }
+
             // re-add certification
             modified = PGPPublicKey.addCertification(modified, selfCert);
             // add revocation, if any
@@ -953,4 +970,23 @@ public class UncachedKeyRing {
         }
     }
 
+
+    /** Returns true if the algorithm is of a type which is suitable for signing. */
+    static boolean isSigningAlgo(int algorithm) {
+        return algorithm == PGPPublicKey.RSA_GENERAL
+                || algorithm == PGPPublicKey.RSA_SIGN
+                || algorithm == PGPPublicKey.DSA
+                || algorithm == PGPPublicKey.ELGAMAL_GENERAL
+                || algorithm == PGPPublicKey.ECDSA;
+    }
+
+    /** Returns true if the algorithm is of a type which is suitable for encryption. */
+    static boolean isEncryptionAlgo(int algorithm) {
+        return algorithm == PGPPublicKey.RSA_GENERAL
+                || algorithm == PGPPublicKey.RSA_ENCRYPT
+                || algorithm == PGPPublicKey.ELGAMAL_ENCRYPT
+                || algorithm == PGPPublicKey.ELGAMAL_GENERAL
+                || algorithm == PGPPublicKey.ECDH;
+    }
+
 }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java
index 7f08d121e..74af9fc97 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java
@@ -18,9 +18,6 @@
 
 package org.sufficientlysecure.keychain.pgp;
 
-import org.spongycastle.asn1.ASN1ObjectIdentifier;
-import org.spongycastle.asn1.nist.NISTNamedCurves;
-import org.spongycastle.asn1.teletrust.TeleTrusTNamedCurves;
 import org.spongycastle.bcpg.ECPublicBCPGKey;
 import org.spongycastle.bcpg.SignatureSubpacketTags;
 import org.spongycastle.bcpg.sig.KeyFlags;
@@ -28,7 +25,6 @@ import org.spongycastle.openpgp.PGPPublicKey;
 import org.spongycastle.openpgp.PGPSignature;
 import org.spongycastle.openpgp.PGPSignatureSubpacketVector;
 import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProvider;
-import org.spongycastle.util.Strings;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.util.IterableIterator;
 import org.sufficientlysecure.keychain.util.Log;
@@ -264,12 +260,6 @@ public class UncachedPublicKey {
             return (getKeyUsage() & KeyFlags.CERTIFY_OTHER) != 0;
         }
 
-        if (mPublicKey.getAlgorithm() == PGPPublicKey.RSA_GENERAL
-                || mPublicKey.getAlgorithm() == PGPPublicKey.RSA_SIGN
-                || mPublicKey.getAlgorithm() == PGPPublicKey.ECDSA) {
-            return true;
-        }
-
         return false;
     }
 
@@ -279,9 +269,7 @@ public class UncachedPublicKey {
             return (getKeyUsage() & KeyFlags.SIGN_DATA) != 0;
         }
 
-        if (mPublicKey.getAlgorithm() == PGPPublicKey.RSA_GENERAL
-                || mPublicKey.getAlgorithm() == PGPPublicKey.RSA_SIGN
-                || mPublicKey.getAlgorithm() == PGPPublicKey.ECDSA) {
+        if (UncachedKeyRing.isSigningAlgo(mPublicKey.getAlgorithm())) {
             return true;
         }
 
@@ -295,7 +283,7 @@ public class UncachedPublicKey {
         }
 
         // RSA_GENERAL, RSA_ENCRYPT, ELGAMAL_ENCRYPT, ELGAMAL_GENERAL, ECDH
-        if (mPublicKey.isEncryptionKey()) {
+        if (UncachedKeyRing.isEncryptionAlgo(mPublicKey.getAlgorithm())) {
             return true;
         }
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/results/OperationResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/results/OperationResult.java
index b0a255162..0a4d9649f 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/results/OperationResult.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/results/OperationResult.java
@@ -341,6 +341,8 @@ public abstract class OperationResult implements Parcelable {
         MSG_KC_SUB_REVOKE_BAD (LogLevel.WARN, R.string.msg_kc_sub_revoke_bad),
         MSG_KC_SUB_REVOKE_DUP (LogLevel.DEBUG, R.string.msg_kc_sub_revoke_dup),
         MSG_KC_SUB_UNKNOWN_ALGO (LogLevel.WARN, R.string.msg_kc_sub_unknown_algo),
+        MSG_KC_SUB_ALGO_BAD_ENCRYPT (LogLevel.WARN, R.string.msg_kc_sub_algo_bad_encrpyt),
+        MSG_KC_SUB_ALGO_BAD_SIGN (LogLevel.WARN, R.string.msg_kc_sub_algo_bad_sign),
         MSG_KC_SUCCESS_BAD (LogLevel.OK, R.plurals.msg_kc_success_bad),
         MSG_KC_SUCCESS_BAD_AND_RED (LogLevel.OK, R.string.msg_kc_success_bad_and_red),
         MSG_KC_SUCCESS_REDUNDANT (LogLevel.OK, R.plurals.msg_kc_success_redundant),