make getSignId a secret key operation, and respect unavailable keys

This one should remedy #811, but waiting for a test

8 files changed, 38 insertions, 112 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java
index 554899843..f43cbbeef 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java
@@ -78,7 +78,7 @@ public abstract class CanonicalizedKeyRing extends KeyRing {
 
     public long getEncryptId() throws PgpGeneralException {
         for(CanonicalizedPublicKey key : publicKeyIterator()) {
-            if(key.canEncrypt()) {
+            if (key.canEncrypt() && key.isValid()) {
                 return key.getKeyId();
             }
         }
@@ -94,24 +94,6 @@ public abstract class CanonicalizedKeyRing extends KeyRing {
         }
     }
 
-    public long getSignId() throws PgpGeneralException {
-        for(CanonicalizedPublicKey key : publicKeyIterator()) {
-            if(key.canSign()) {
-                return key.getKeyId();
-            }
-        }
-        throw new PgpGeneralException("No valid signing key found!");
-    }
-
-    public boolean hasSign() throws PgpGeneralException {
-        try {
-            getSignId();
-            return true;
-        } catch (PgpGeneralException e) {
-            return false;
-        }
-    }
-
     public void encode(OutputStream stream) throws IOException {
         getRing().encode(stream);
     }
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 8fb3402b2..3539a4ceb 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKey.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKey.java
@@ -104,4 +104,10 @@ public class CanonicalizedPublicKey extends UncachedPublicKey {
     public Integer getKeyUsage() {
         return super.getKeyUsage();
     }
+
+    /** Returns whether this key is valid, ie not expired or revoked. */
+    public boolean isValid() {
+        return !isRevoked() && !isExpired();
+    }
+
 }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java
index 48a2aaeb6..e20155cc6 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKeyRing.java
@@ -30,6 +30,8 @@ import org.spongycastle.openpgp.operator.PBESecretKeyDecryptor;
 import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
+import org.sufficientlysecure.keychain.provider.KeychainContract;
+import org.sufficientlysecure.keychain.provider.ProviderHelper;
 import org.sufficientlysecure.keychain.util.IterableIterator;
 import org.sufficientlysecure.keychain.util.Log;
 
@@ -74,43 +76,18 @@ public class CanonicalizedSecretKeyRing extends CanonicalizedKeyRing {
         return new CanonicalizedSecretKey(this, mRing.getSecretKey(id));
     }
 
-    /** Getter that returns the subkey that should be used for signing. */
-    CanonicalizedSecretKey getSigningSubKey() throws PgpGeneralException {
-        PGPSecretKey key = mRing.getSecretKey(getSignId());
-        if(key != null) {
-            CanonicalizedSecretKey cKey = new CanonicalizedSecretKey(this, key);
-            if(!cKey.canSign()) {
-                throw new PgpGeneralException("key error");
+    /** Returns the key id which should be used for signing.
+     *
+     * This method returns keys which are actually available (ie. secret available, and not stripped,
+     * revoked, or expired), hence only works on keyrings where a secret key is available!
+     */
+    public long getSecretSignId() throws PgpGeneralException {
+        for(CanonicalizedSecretKey key : secretKeyIterator()) {
+            if (key.canSign() && key.isValid() && key.getSecretKeyType().isUsable()) {
+                return key.getKeyId();
             }
-            return cKey;
-        }
-        // TODO handle with proper exception
-        throw new PgpGeneralException("no signing key available");
-    }
-
-    public boolean hasPassphrase() {
-        PGPSecretKey secretKey = null;
-        boolean foundValidKey = false;
-        for (Iterator keys = mRing.getSecretKeys(); keys.hasNext(); ) {
-            secretKey = (PGPSecretKey) keys.next();
-            if (!secretKey.isPrivateKeyEmpty()) {
-                foundValidKey = true;
-                break;
-            }
-        }
-        if(!foundValidKey) {
-            return false;
-        }
-
-        try {
-            PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder()
-                    .setProvider("SC").build("".toCharArray());
-            PGPPrivateKey testKey = secretKey.extractPrivateKey(keyDecryptor);
-            return testKey == null;
-        } catch(PGPException e) {
-            // this means the crc check failed -> passphrase required
-            return true;
         }
+        throw new PgpGeneralException("no valid signing key available");
     }
 
     public IterableIterator<CanonicalizedSecretKey> secretKeyIterator() {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/KeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/KeyRing.java
index 3ef4b336e..b682378e9 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/KeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/KeyRing.java
@@ -56,10 +56,6 @@ public abstract class KeyRing {
 
     abstract public boolean hasEncrypt() throws PgpGeneralException;
 
-    abstract public long getSignId() throws PgpGeneralException;
-
-    abstract public boolean hasSign() throws PgpGeneralException;
-
     abstract public int getVerified() throws PgpGeneralException;
 
     private static final Pattern USER_ID_PATTERN = Pattern.compile("^(.*?)(?: \\((.*)\\))?(?: <(.*)>)?$");
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 bb9c7d51c..c4cacaca7 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java
@@ -51,12 +51,9 @@ public class UncachedPublicKey {
 
     /** The revocation signature is NOT checked here, so this may be false! */
     public boolean isRevoked() {
-        for (PGPSignature sig : new IterableIterator<PGPSignature>(
-                mPublicKey.getSignaturesOfType(isMasterKey() ? PGPSignature.KEY_REVOCATION
-                                                             : PGPSignature.SUBKEY_REVOCATION))) {
-            return true;
-        }
-        return false;
+        return mPublicKey.getSignaturesOfType(isMasterKey()
+                ? PGPSignature.KEY_REVOCATION
+                : PGPSignature.SUBKEY_REVOCATION).hasNext();
     }
 
     public Date getCreationTime() {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java
index cfb4a915e..ad3ebae5f 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java
@@ -136,64 +136,32 @@ public class CachedPublicKeyRing extends KeyRing {
     @Override
     public long getEncryptId() throws PgpGeneralException {
         try {
-            Cursor subkeys = getSubkeys();
-            if (subkeys != null) {
-                try {
-                    while (subkeys.moveToNext()) {
-                        if (subkeys.getInt(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.CAN_ENCRYPT)) != 0) {
-                            return subkeys.getLong(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.KEY_ID));
-                        }
-                    }
-                } finally {
-                    subkeys.close();
-                }
-            }
-        } catch(Exception e) {
-            throw new PgpGeneralException(e);
-        }
-        throw new PgpGeneralException("No encrypt key found");
-    }
-
-    @Override
-    public boolean hasEncrypt() throws PgpGeneralException {
-        try {
             Object data = mProviderHelper.getGenericData(mUri,
-                    KeychainContract.KeyRings.HAS_ENCRYPT,
+                    KeyRings.HAS_ENCRYPT,
                     ProviderHelper.FIELD_TYPE_INTEGER);
-            return (Long) data > 0;
+            return (Long) data;
         } catch(ProviderHelper.NotFoundException e) {
             throw new PgpGeneralException(e);
         }
     }
 
     @Override
-    public long getSignId() throws PgpGeneralException {
-        try {
-            Cursor subkeys = getSubkeys();
-            if (subkeys != null) {
-                try {
-                    while (subkeys.moveToNext()) {
-                        if (subkeys.getInt(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.CAN_SIGN)) != 0) {
-                            return subkeys.getLong(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.KEY_ID));
-                        }
-                    }
-                } finally {
-                    subkeys.close();
-                }
-            }
-        } catch(Exception e) {
-            throw new PgpGeneralException(e);
-        }
-        throw new PgpGeneralException("No sign key found");
+    public boolean hasEncrypt() throws PgpGeneralException {
+        return getEncryptId() != 0;
     }
 
-    @Override
-    public boolean hasSign() throws PgpGeneralException {
+    /** Returns the key id which should be used for signing.
+     *
+     * This method returns keys which are actually available (ie. secret available, and not stripped,
+     * revoked, or expired), hence only works on keyrings where a secret key is available!
+     *
+     */
+    public long getSecretSignId() throws PgpGeneralException {
         try {
             Object data = mProviderHelper.getGenericData(mUri,
-                    KeychainContract.KeyRings.HAS_SIGN,
+                    KeyRings.HAS_SIGN,
                     ProviderHelper.FIELD_TYPE_INTEGER);
-            return (Long) data > 0;
+            return (Long) data;
         } catch(ProviderHelper.NotFoundException e) {
             throw new PgpGeneralException(e);
         }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
index b7ee453d5..ed1f19dbd 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
@@ -293,7 +293,7 @@ public class OpenPgpService extends RemoteService {
                 // Find the appropriate subkey to sign with
                 CachedPublicKeyRing signingRing =
                         new ProviderHelper(this).getCachedPublicKeyRing(accSettings.getKeyId());
-                final long sigSubKeyId = signingRing.getSignId();
+                final long sigSubKeyId = signingRing.getSecretSignId();
 
                 // sign-only
                 PgpSignEncrypt.Builder builder = new PgpSignEncrypt.Builder(
@@ -405,7 +405,7 @@ public class OpenPgpService extends RemoteService {
                     // Find the appropriate subkey to sign with
                     CachedPublicKeyRing signingRing =
                             new ProviderHelper(this).getCachedPublicKeyRing(accSettings.getKeyId());
-                    final long sigSubKeyId = signingRing.getSignId();
+                    final long sigSubKeyId = signingRing.getSecretSignId();
 
                     String passphrase;
                     if (data.hasExtra(OpenPgpApi.EXTRA_PASSPHRASE)) {
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
index bb966fec8..0ee3c7093 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java
@@ -294,7 +294,7 @@ public class KeychainIntentService extends IntentService implements Progressable
                         // Find the appropriate subkey to sign with
                         CachedPublicKeyRing signingRing =
                                 new ProviderHelper(this).getCachedPublicKeyRing(sigMasterKeyId);
-                        long sigSubKeyId = signingRing.getSignId();
+                        long sigSubKeyId = signingRing.getSecretSignId();
 
                         // Set signature settings
                         builder.setSignatureMasterKeyId(sigMasterKeyId)