Merge branch 'master' of github.com:open-keychain/open-keychain

5 files changed, 171 insertions, 123 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKey.java
index 7a63a7a42..8fb3402b2 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKey.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKey.java
@@ -18,6 +18,7 @@
 
 package org.sufficientlysecure.keychain.pgp;
 
+import org.spongycastle.bcpg.sig.KeyFlags;
 import org.spongycastle.openpgp.PGPPublicKey;
 import org.spongycastle.openpgp.operator.jcajce.JcePublicKeyKeyEncryptionMethodGenerator;
 import org.sufficientlysecure.keychain.util.IterableIterator;
@@ -46,12 +47,61 @@ public class CanonicalizedPublicKey extends UncachedPublicKey {
         return new IterableIterator<String>(mPublicKey.getUserIDs());
     }
 
-    public KeyRing getKeyRing() {
-        return mRing;
+    JcePublicKeyKeyEncryptionMethodGenerator getPubKeyEncryptionGenerator() {
+        return new JcePublicKeyKeyEncryptionMethodGenerator(mPublicKey);
     }
 
-    JcePublicKeyKeyEncryptionMethodGenerator getPubKeyEncryptionGenerator() {
-        return  new JcePublicKeyKeyEncryptionMethodGenerator(mPublicKey);
+    public boolean canSign() {
+        // if key flags subpacket is available, honor it!
+        if (getKeyUsage() != null) {
+            return (getKeyUsage() & KeyFlags.SIGN_DATA) != 0;
+        }
+
+        if (UncachedKeyRing.isSigningAlgo(mPublicKey.getAlgorithm())) {
+            return true;
+        }
+
+        return false;
+    }
+
+    public boolean canCertify() {
+        // if key flags subpacket is available, honor it!
+        if (getKeyUsage() != null) {
+            return (getKeyUsage() & KeyFlags.CERTIFY_OTHER) != 0;
+        }
+
+        if (UncachedKeyRing.isSigningAlgo(mPublicKey.getAlgorithm())) {
+            return true;
+        }
+
+        return false;
     }
 
+    public boolean canEncrypt() {
+        // if key flags subpacket is available, honor it!
+        if (getKeyUsage() != null) {
+            return (getKeyUsage() & (KeyFlags.ENCRYPT_COMMS | KeyFlags.ENCRYPT_STORAGE)) != 0;
+        }
+
+        // RSA_GENERAL, RSA_ENCRYPT, ELGAMAL_ENCRYPT, ELGAMAL_GENERAL, ECDH
+        if (UncachedKeyRing.isEncryptionAlgo(mPublicKey.getAlgorithm())) {
+            return true;
+        }
+
+        return false;
+    }
+
+    public boolean canAuthenticate() {
+        // if key flags subpacket is available, honor it!
+        if (getKeyUsage() != null) {
+            return (getKeyUsage() & KeyFlags.AUTHENTICATION) != 0;
+        }
+
+        return false;
+    }
+
+    /** Same method as superclass, but we make it public. */
+    public Integer getKeyUsage() {
+        return super.getKeyUsage();
+    }
 }
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 7bf16791d..8651760c0 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
@@ -22,7 +22,6 @@ import org.spongycastle.bcpg.ArmoredOutputStream;
 import org.spongycastle.bcpg.PublicKeyAlgorithmTags;
 import org.spongycastle.bcpg.SignatureSubpacketTags;
 import org.spongycastle.bcpg.sig.KeyFlags;
-import org.spongycastle.openpgp.PGPKeyFlags;
 import org.spongycastle.openpgp.PGPKeyRing;
 import org.spongycastle.openpgp.PGPObjectFactory;
 import org.spongycastle.openpgp.PGPPublicKey;
@@ -616,43 +615,56 @@ public class UncachedKeyRing {
                         continue;
                     }
 
-                    // if this certificate says it allows signing for the key
-                    if (zert.getHashedSubPackets() != null &&
-                            zert.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.KEY_FLAGS)) {
-
-                        int flags = ((KeyFlags) zert.getHashedSubPackets()
-                                .getSubpacket(SignatureSubpacketTags.KEY_FLAGS)).getFlags();
-                        if ((flags & PGPKeyFlags.CAN_SIGN) == PGPKeyFlags.CAN_SIGN) {
-                            boolean ok = false;
-                            // it MUST have an embedded primary key binding signature
-                            try {
-                                PGPSignatureList list = zert.getUnhashedSubPackets().getEmbeddedSignatures();
-                                for (int i = 0; i < list.size(); i++) {
-                                    WrappedSignature subsig = new WrappedSignature(list.get(i));
-                                    if (subsig.getSignatureType() == PGPSignature.PRIMARYKEY_BINDING) {
-                                        subsig.init(key);
-                                        if (subsig.verifySignature(masterKey, key)) {
-                                            ok = true;
-                                        } else {
-                                            log.add(LogType.MSG_KC_SUB_PRIMARY_BAD, indent);
-                                            badCerts += 1;
-                                            continue uids;
-                                        }
+                    boolean needsPrimaryBinding = false;
+
+                    // If the algorithm is even suitable for signing
+                    if (isSigningAlgo(key.getAlgorithm())) {
+
+                        // If this certificate says it allows signing for the key
+                        if (zert.getHashedSubPackets() != null &&
+                                zert.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.KEY_FLAGS)) {
+                            int flags = ((KeyFlags) zert.getHashedSubPackets()
+                                    .getSubpacket(SignatureSubpacketTags.KEY_FLAGS)).getFlags();
+                            if ((flags & KeyFlags.SIGN_DATA) == KeyFlags.SIGN_DATA) {
+                                needsPrimaryBinding = true;
+                            }
+                        } else {
+                            // If there are no key flags, we STILL require this because the key can sign!
+                            needsPrimaryBinding = true;
+                        }
+
+                    }
+
+                    // If this key can sign, it MUST have a primary key binding certificate
+                    if (needsPrimaryBinding) {
+                        boolean ok = false;
+                        if (zert.getUnhashedSubPackets() != null) try {
+                            // Check all embedded signatures, if any of them fits
+                            PGPSignatureList list = zert.getUnhashedSubPackets().getEmbeddedSignatures();
+                            for (int i = 0; i < list.size(); i++) {
+                                WrappedSignature subsig = new WrappedSignature(list.get(i));
+                                if (subsig.getSignatureType() == PGPSignature.PRIMARYKEY_BINDING) {
+                                    subsig.init(key);
+                                    if (subsig.verifySignature(masterKey, key)) {
+                                        ok = true;
+                                    } else {
+                                        log.add(LogType.MSG_KC_SUB_PRIMARY_BAD, indent);
+                                        badCerts += 1;
+                                        continue uids;
                                     }
                                 }
-                            } catch (Exception e) {
-                                log.add(LogType.MSG_KC_SUB_PRIMARY_BAD_ERR, indent);
-                                badCerts += 1;
-                                continue;
-                            }
-                            // if it doesn't, get rid of this!
-                            if (!ok) {
-                                log.add(LogType.MSG_KC_SUB_PRIMARY_NONE, indent);
-                                badCerts += 1;
-                                continue;
                             }
+                        } catch (Exception e) {
+                            log.add(LogType.MSG_KC_SUB_PRIMARY_BAD_ERR, indent);
+                            badCerts += 1;
+                            continue;
+                        }
+                        // if it doesn't, get rid of this!
+                        if (!ok) {
+                            log.add(LogType.MSG_KC_SUB_PRIMARY_NONE, indent);
+                            badCerts += 1;
+                            continue;
                         }
-
                     }
 
                     // if we already have a cert, and this one is older: skip it
@@ -708,6 +720,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 & KeyFlags.SIGN_DATA) == KeyFlags.SIGN_DATA) {
+                    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 & KeyFlags.ENCRYPT_STORAGE) == KeyFlags.ENCRYPT_STORAGE
+                        || (flags & KeyFlags.ENCRYPT_COMMS) == KeyFlags.ENCRYPT_COMMS
+                    )) {
+                    log.add(LogType.MSG_KC_SUB_ALGO_BAD_ENCRYPT, indent);
+                }
+            }
+
             // re-add certification
             modified = PGPPublicKey.addCertification(modified, selfCert);
             // add revocation, if any
@@ -939,4 +969,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..bb9c7d51c 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;
@@ -232,92 +228,12 @@ public class UncachedPublicKey {
         return getAlgorithm() == PGPPublicKey.ECDH || getAlgorithm() == PGPPublicKey.ECDSA;
     }
 
-    /**
-     * Get all key usage flags.
-     * If at least one key flag subpacket is present return these.
-     * If no subpacket is present it returns null.
-     */
-    @SuppressWarnings("unchecked")
-    public Integer getKeyUsage() {
-        if (mCacheUsage == null) {
-            for (PGPSignature sig : new IterableIterator<PGPSignature>(mPublicKey.getSignatures())) {
-                if (mPublicKey.isMasterKey() && sig.getKeyID() != mPublicKey.getKeyID()) {
-                    continue;
-                }
-
-                PGPSignatureSubpacketVector hashed = sig.getHashedSubPackets();
-                if (hashed != null && hashed.getSubpacket(SignatureSubpacketTags.KEY_FLAGS) != null) {
-                    // init if at least one key flag subpacket has been found
-                    if (mCacheUsage == null) {
-                        mCacheUsage = 0;
-                    }
-                    mCacheUsage |= hashed.getKeyFlags();
-                }
-            }
-        }
-        return mCacheUsage;
-    }
-
-    public boolean canCertify() {
-        // if key flags subpacket is available, honor it!
-        if (getKeyUsage() != null) {
-            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;
-    }
-
-    public boolean canSign() {
-        // if key flags subpacket is available, honor it!
-        if (getKeyUsage() != null) {
-            return (getKeyUsage() & KeyFlags.SIGN_DATA) != 0;
-        }
-
-        if (mPublicKey.getAlgorithm() == PGPPublicKey.RSA_GENERAL
-                || mPublicKey.getAlgorithm() == PGPPublicKey.RSA_SIGN
-                || mPublicKey.getAlgorithm() == PGPPublicKey.ECDSA) {
-            return true;
-        }
-
-        return false;
-    }
-
-    public boolean canEncrypt() {
-        // if key flags subpacket is available, honor it!
-        if (getKeyUsage() != null) {
-            return (getKeyUsage() & (KeyFlags.ENCRYPT_COMMS | KeyFlags.ENCRYPT_STORAGE)) != 0;
-        }
-
-        // RSA_GENERAL, RSA_ENCRYPT, ELGAMAL_ENCRYPT, ELGAMAL_GENERAL, ECDH
-        if (mPublicKey.isEncryptionKey()) {
-            return true;
-        }
-
-        return false;
-    }
-
-    public boolean canAuthenticate() {
-        // if key flags subpacket is available, honor it!
-        if (getKeyUsage() != null) {
-            return (getKeyUsage() & KeyFlags.AUTHENTICATION) != 0;
-        }
-
-        return false;
-    }
-
     public byte[] getFingerprint() {
         return mPublicKey.getFingerprint();
     }
 
-    // TODO This method should have package visibility - no access outside the pgp package!
     // (It's still used in ProviderHelper at this point)
-    public PGPPublicKey getPublicKey() {
+    PGPPublicKey getPublicKey() {
         return mPublicKey;
     }
 
@@ -355,4 +271,33 @@ public class UncachedPublicKey {
         }
     }
 
+    /** Get all key usage flags.
+     * If at least one key flag subpacket is present return these. If no
+     * subpacket is present it returns null.
+     *
+     * Note that this method has package visiblity because it is used in test
+     * cases. Certificates of UncachedPublicKey instances can NOT be assumed to
+     * be verified, so the result of this method should not be used in other
+     * places!
+     */
+    @SuppressWarnings("unchecked")
+    Integer getKeyUsage() {
+        if (mCacheUsage == null) {
+            for (PGPSignature sig : new IterableIterator<PGPSignature>(mPublicKey.getSignatures())) {
+                if (mPublicKey.isMasterKey() && sig.getKeyID() != mPublicKey.getKeyID()) {
+                    continue;
+                }
+
+                PGPSignatureSubpacketVector hashed = sig.getHashedSubPackets();
+                if (hashed != null && hashed.getSubpacket(SignatureSubpacketTags.KEY_FLAGS) != null) {
+                    // init if at least one key flag subpacket has been found
+                    if (mCacheUsage == null) {
+                        mCacheUsage = 0;
+                    }
+                    mCacheUsage |= hashed.getKeyFlags();
+                }
+            }
+        }
+        return mCacheUsage;
+    }
 }
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),
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index 14d0eba9b..f9ae6f029 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -684,6 +684,8 @@
     <string name="msg_kc_sub_revoke_bad">"Removing bad subkey revocation certificate"</string>
     <string name="msg_kc_sub_revoke_dup">"Removing redundant subkey revocation certificate"</string>
     <string name="msg_kc_sub_unknown_algo">"Subkey uses an unknown algorithm, not importing…"</string>
+    <string name="msg_kc_sub_algo_bad_encrpyt">"Subkey has encryption usage flag, but algorithm is not suitable for encryption."</string>
+    <string name="msg_kc_sub_algo_bad_sign">"Subkey has signing usage flag, but algorithm is not suitable for signing."</string>
     <string name="msg_kc_success">"Keyring canonicalization successful, no changes"</string>
     <plurals name="msg_kc_success_bad">
         <item quantity="one">"Keyring canonicalization successful, removed one erroneous certificate"</item>