wrapped-key-ring: redesign underlying CachedKeyRing

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

@@ -3,45 +3,66 @@ package org.sufficientlysecure.keychain.pgp;
 public abstract class CachedKeyRing {
 
     private final long mMasterKeyId;
-    private final boolean mCanCertify;
-    private final byte[] mFingerprint;
     private final String mUserId;
+    private final boolean mHasAnySecret;
+    private final boolean mIsRevoked;
+    private final boolean mCanCertify;
+    private final long mHasEncryptId;
+    private final long mHasSignId;
     private final int mVerified;
-    private final boolean mHasSecret;
 
-    protected CachedKeyRing(long masterKeyId, boolean canCertify,
+    protected CachedKeyRing(long masterKeyId, String userId, boolean hasAnySecret,
-            byte[] fingerprint, String userId, int verified, boolean hasSecret)
+                            boolean isRevoked, boolean canCertify, long hasEncryptId, long hasSignId,
+                            int verified)
     {
         mMasterKeyId = masterKeyId;
-        mCanCertify = canCertify;
-        mFingerprint = fingerprint;
         mUserId = userId;
+        mHasAnySecret = hasAnySecret;
+        mIsRevoked = isRevoked;
+        mCanCertify = canCertify;
+        mHasEncryptId = hasEncryptId;
+        mHasSignId = hasSignId;
         mVerified = verified;
-        mHasSecret = hasSecret;
-    }
-
-    public byte[] getFingerprint() {
-        return mFingerprint;
-    }
-
-    public String getPrimaryUserId() {
-        return mUserId;
     }
 
     public long getMasterKeyId() {
         return mMasterKeyId;
     }
 
-    public int getVerified() {
+    public String getPrimaryUserId() {
-        return mVerified;
+        return mUserId;
+    }
+
+    public boolean hasAnySecret() {
+        return mHasAnySecret;
+    }
+
+    public boolean isRevoked() {
+        return mIsRevoked;
     }
 
     public boolean canCertify() {
         return mCanCertify;
     }
 
-    public boolean hasSecret() {
+    public long getEncryptId() {
-        return mHasSecret;
+        return mHasEncryptId;
+    }
+
+    public boolean hasEncrypt() {
+        return mHasEncryptId != 0;
+    }
+
+    public long getSignId() {
+        return mHasSignId;
+    }
+
+    public boolean hasSign() {
+        return mHasSignId != 0;
+    }
+
+    public int getVerified() {
+        return mVerified;
     }
 
 }

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

@@ -23,14 +23,14 @@ public class CachedPublicKeyRing extends CachedKeyRing {
     private PGPPublicKeyRing mRing;
     private final byte[] mPubKey;
 
-    public CachedPublicKeyRing(long masterKeyId, int keySize, boolean isRevoked,
+    public CachedPublicKeyRing(long masterKeyId, String userId, boolean hasAnySecret,
-                                boolean canCertify, long creation, long expiry, int algorithm,
+                               boolean isRevoked, boolean canCertify, long hasEncryptId, long hasSignId,
-                                byte[] fingerprint, String userId, int verified, boolean hasSecret,
+                               int verified, byte[] blob)
-                                byte[] pubkey)
     {
-        super(masterKeyId, canCertify, fingerprint, userId, verified, hasSecret);
+        super(masterKeyId, userId, hasAnySecret, isRevoked, canCertify,
+                hasEncryptId, hasSignId, verified);
 
-        mPubKey = pubkey;
+        mPubKey = blob;
     }
 
     PGPPublicKeyRing getRing() {
@@ -52,46 +52,18 @@ public class CachedPublicKeyRing extends CachedKeyRing {
         return new CachedPublicKey(this, getRing().getPublicKey(id));
     }
 
-    public CachedPublicKey getFirstSignSubkey() throws PgpGeneralException {
+    /** Getter that returns the subkey that should be used for signing. */
-        // only return master key if no other signing key is available
+    CachedPublicKey getEncryptionSubKey() throws PgpGeneralException {
-        CachedPublicKey masterKey = null;
+        PGPPublicKey key = getRing().getPublicKey(getEncryptId());
-        for (PGPPublicKey k : new IterableIterator<PGPPublicKey>(getRing().getPublicKeys())) {
+        if(key != null) {
-            CachedPublicKey key = new CachedPublicKey(this, k);
+            CachedPublicKey cKey = new CachedPublicKey(this, key);
-            if (key.isRevoked() || key.canSign() || key.isExpired()) {
+            if(!cKey.canEncrypt()) {
-                continue;
+                throw new PgpGeneralException("key error");
-            }
-            if (key.isMasterKey()) {
-                masterKey = key;
-            } else {
-                return key;
             }
+            return cKey;
         }
-        if(masterKey == null) {
+        // TODO handle with proper exception
-            // TODO proper exception
+        throw new PgpGeneralException("no encryption key available");
-            throw new PgpGeneralException("key not found");
-        }
-        return masterKey;
-    }
-
-    public CachedPublicKey getFirstEncryptSubkey() throws PgpGeneralException {
-        // only return master key if no other encryption key is available
-        CachedPublicKey masterKey = null;
-        for (PGPPublicKey k : new IterableIterator<PGPPublicKey>(getRing().getPublicKeys())) {
-            CachedPublicKey key = new CachedPublicKey(this, k);
-            if (key.isRevoked() || key.canEncrypt() || key.isExpired()) {
-                continue;
-            }
-            if (key.isMasterKey()) {
-                masterKey = key;
-            } else {
-                return key;
-            }
-        }
-        if(masterKey == null) {
-            // TODO proper exception
-            throw new PgpGeneralException("key not found");
-        }
-        return masterKey;
     }
 
     public boolean verifySubkeyBinding(CachedPublicKey cachedSubkey) {
@@ -189,4 +161,24 @@ public class CachedPublicKeyRing extends CachedKeyRing {
         return validPrimaryKeyBinding;
     }
 
+    public IterableIterator<CachedPublicKey> iterator() {
+        final Iterator<PGPPublicKey> it = getRing().getPublicKeys();
+        return new IterableIterator<CachedPublicKey>(new Iterator<CachedPublicKey>() {
+            @Override
+            public boolean hasNext() {
+                return it.hasNext();
+            }
+
+            @Override
+            public CachedPublicKey next() {
+                return new CachedPublicKey(CachedPublicKeyRing.this, it.next());
+            }
+
+            @Override
+            public void remove() {
+                it.remove();
+            }
+        });
+    }
+
 }

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

@@ -1,18 +1,15 @@
 package org.sufficientlysecure.keychain.pgp;
 
-import org.spongycastle.bcpg.sig.KeyFlags;
 import org.spongycastle.openpgp.PGPException;
 import org.spongycastle.openpgp.PGPPrivateKey;
-import org.spongycastle.openpgp.PGPPublicKey;
 import org.spongycastle.openpgp.PGPSecretKey;
 import org.spongycastle.openpgp.PGPSecretKeyRing;
-import org.spongycastle.openpgp.PGPSignature;
-import org.spongycastle.openpgp.PGPSignatureSubpacketVector;
 import org.spongycastle.openpgp.operator.PBESecretKeyDecryptor;
 import org.spongycastle.openpgp.operator.jcajce.JcaPGPDigestCalculatorProviderBuilder;
 import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder;
 import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyEncryptorBuilder;
 import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
 import org.sufficientlysecure.keychain.util.IterableIterator;
 
 import java.io.IOException;
@@ -23,12 +20,13 @@ public class CachedSecretKeyRing extends CachedKeyRing {
 
     private PGPSecretKeyRing mRing;
 
-    public CachedSecretKeyRing(long masterKeyId, int keySize, boolean isRevoked,
+    public CachedSecretKeyRing(long masterKeyId, String userId, boolean hasAnySecret,
-                               boolean canCertify, long creation, long expiry, int algorithm,
+                               boolean isRevoked, boolean canCertify, long hasEncryptId, long hasSignId,
-                               byte[] fingerprint, String userId, int verified, boolean hasSecret,
+                               int verified, byte[] blob)
-                               byte[] blob)
     {
-        super(masterKeyId, canCertify, fingerprint, userId, verified, hasSecret);
+        super(masterKeyId, userId, hasAnySecret,
+                isRevoked, canCertify, hasEncryptId, hasSignId,
+                verified);
 
         mRing = (PGPSecretKeyRing) PgpConversionHelper.BytesToPGPKeyRing(blob);
     }
@@ -45,8 +43,18 @@ public class CachedSecretKeyRing extends CachedKeyRing {
         return new CachedSecretKey(this, mRing.getSecretKey(id));
     }
 
-    public IterableIterator<CachedSecretKey> iterator() {
+    /** Getter that returns the subkey that should be used for signing. */
-        return new IterableIterator<CachedSecretKey>(mRing.getSecretKeys());
+    CachedSecretKey getSigningSubKey() throws PgpGeneralException {
+        PGPSecretKey key = mRing.getSecretKey(getSignId());
+        if(key != null) {
+            CachedSecretKey cKey = new CachedSecretKey(this, key);
+            if(!cKey.canSign()) {
+                throw new PgpGeneralException("key error");
+            }
+            return cKey;
+        }
+        // TODO handle with proper exception
+        throw new PgpGeneralException("no signing key available");
     }
 
     public boolean hasPassphrase() {
@@ -74,50 +82,6 @@ public class CachedSecretKeyRing extends CachedKeyRing {
         }
     }
 
-    /** This returns the subkey that should be used for signing.
-     * At this point, this is simply the first suitable subkey.
-     */
-    CachedSecretKey getSigningSubKey() {
-        for (PGPSecretKey key : new IterableIterator<PGPSecretKey>(mRing.getSecretKeys())) {
-            if (isSigningKey(key.getPublicKey())) {
-                return new CachedSecretKey(this, key);
-            }
-        }
-        // TODO exception
-        return null;
-    }
-
-    @SuppressWarnings("unchecked")
-    public static boolean isSigningKey(PGPPublicKey key) {
-        if (key.getVersion() <= 3) {
-            return true;
-        }
-
-        // special case
-        if (key.getAlgorithm() == PGPPublicKey.RSA_SIGN) {
-            return true;
-        }
-
-        for (PGPSignature sig : new IterableIterator<PGPSignature>(key.getSignatures())) {
-            if (key.isMasterKey() && sig.getKeyID() != key.getKeyID()) {
-                continue;
-            }
-            PGPSignatureSubpacketVector hashed = sig.getHashedSubPackets();
-
-            if (hashed != null && (hashed.getKeyFlags() & KeyFlags.SIGN_DATA) != 0) {
-                return true;
-            }
-
-            PGPSignatureSubpacketVector unhashed = sig.getUnhashedSubPackets();
-
-            if (unhashed != null && (unhashed.getKeyFlags() & KeyFlags.SIGN_DATA) != 0) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
     public UncachedSecretKeyRing changeSecretKeyPassphrase(String oldPassphrase,
                                                      String newPassphrase)
             throws IOException, PGPException, NoSuchProviderException {
@@ -141,4 +105,24 @@ public class CachedSecretKeyRing extends CachedKeyRing {
 
     }
 
+    public IterableIterator<CachedSecretKey> iterator() {
+        final Iterator<PGPSecretKey> it = mRing.getSecretKeys();
+        return new IterableIterator<CachedSecretKey>(new Iterator<CachedSecretKey>() {
+            @Override
+            public boolean hasNext() {
+                return it.hasNext();
+            }
+
+            @Override
+            public CachedSecretKey next() {
+                return new CachedSecretKey(CachedSecretKeyRing.this, it.next());
+            }
+
+            @Override
+            public void remove() {
+                it.remove();
+            }
+        });
+    }
+
 }

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

@@ -270,14 +270,15 @@ public class PgpSignEncrypt {
         /* Get keys for signature generation for later usage */
         CachedSecretKey signingKey = null;
         if (enableSignature) {
-            CachedSecretKeyRing signingKeyRing = null;
+            CachedSecretKeyRing signingKeyRing;
             try {
                 signingKeyRing = mProviderHelper.getCachedSecretKeyRing(mSignatureMasterKeyId);
             } catch (ProviderHelper.NotFoundException e) {
                 throw new NoSigningKeyException();
             }
-            signingKey = signingKeyRing.getSigningSubKey();
+            try {
-            if (signingKey == null) {
+                signingKey = signingKeyRing.getSigningSubKey();
+            } catch(PgpGeneralException e) {
                 throw new NoSigningKeyException();
             }
 
@@ -319,7 +320,7 @@ public class PgpSignEncrypt {
                     try {
                         CachedPublicKeyRing keyRing = mProviderHelper.getCachedPublicKeyRing(
                                 KeyRings.buildUnifiedKeyRingUri(Long.toString(id)));
-                        CachedPublicKey key = keyRing.getFirstEncryptSubkey();
+                        CachedPublicKey key = keyRing.getEncryptionSubKey();
                         cPk.addMethod(key.getPubKeyEncryptionGenerator());
                     } catch (PgpGeneralException e) {
                         Log.e(Constants.TAG, "key not found!", e);

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java

@@ -38,6 +38,7 @@ import org.spongycastle.openpgp.PGPSecretKeyRing;
 import org.spongycastle.openpgp.PGPSignature;
 import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProvider;
 import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.pgp.CachedKeyRing;
 import org.sufficientlysecure.keychain.pgp.CachedSecretKeyRing;
 import org.sufficientlysecure.keychain.pgp.CachedPublicKeyRing;
 import org.sufficientlysecure.keychain.pgp.PgpConversionHelper;
@@ -193,86 +194,55 @@ public class ProviderHelper {
     }
 
     public CachedPublicKeyRing getCachedPublicKeyRing(long id) throws NotFoundException {
-        return getCachedPublicKeyRing(KeyRings.buildUnifiedKeyRingUri(Long.toString(id)));
+        return (CachedPublicKeyRing) getCachedKeyRing(
+                KeyRings.buildUnifiedKeyRingUri(Long.toString(id)), false);
     }
 
     public CachedPublicKeyRing getCachedPublicKeyRing(Uri queryUri) throws NotFoundException {
-        Cursor cursor = mContentResolver.query(queryUri,
+        return (CachedPublicKeyRing) getCachedKeyRing(queryUri, false);
-                new String[] {
-                    KeyRings.MASTER_KEY_ID, KeyRings.KEY_SIZE,
-                    KeyRings.IS_REVOKED, KeyRings.CAN_CERTIFY,
-                    KeyRings.CREATION, KeyRings.EXPIRY,
-                    KeyRings.ALGORITHM, KeyRings.FINGERPRINT,
-                    KeyRings.USER_ID, KeyRings.VERIFIED,
-                    KeyRings.HAS_SECRET, KeyRings.PUBKEY_DATA
-                }, null, null, null);
-        try {
-            if (cursor != null && cursor.moveToFirst()) {
-                long masterKeyId = cursor.getLong(0);
-                int keySize = cursor.getInt(1);
-                boolean isRevoked = cursor.getInt(2) > 0;
-                boolean canCertify = cursor.getInt(3) > 0;
-                long creation = cursor.getLong(4);
-                long expiry = cursor.getLong(5);
-                int algorithm = cursor.getInt(6);
-                byte[] fingerprint = cursor.getBlob(7);
-                String userId = cursor.getString(8);
-                int verified = cursor.getInt(9);
-                boolean hasSecret = cursor.getInt(10) > 0;
-                byte[] pubkey = cursor.getBlob(11);
-                return new CachedPublicKeyRing(
-                        masterKeyId, keySize, isRevoked, canCertify,
-                        creation, expiry, algorithm, fingerprint,
-                        userId, verified, hasSecret, pubkey
-                );
-            } else {
-                throw new NotFoundException("Key not found!");
-            }
-        } finally {
-            if (cursor != null) {
-                cursor.close();
-            }
-        }
     }
 
     public CachedSecretKeyRing getCachedSecretKeyRing(long id) throws NotFoundException {
-        return getCachedSecretKeyRing(KeyRings.buildUnifiedKeyRingUri(Long.toString(id)));
+        return (CachedSecretKeyRing) getCachedKeyRing(
+                KeyRings.buildUnifiedKeyRingUri(Long.toString(id)), true);
     }
 
     public CachedSecretKeyRing getCachedSecretKeyRing(Uri queryUri) throws NotFoundException {
+        return (CachedSecretKeyRing) getCachedKeyRing(queryUri, true);
+    }
+
+
+    private CachedKeyRing getCachedKeyRing(Uri queryUri, boolean secret) throws NotFoundException {
         Cursor cursor = mContentResolver.query(queryUri,
                 new String[] {
-                        KeyRings.MASTER_KEY_ID, KeyRings.KEY_SIZE,
+                    // we pick from cache:
-                        KeyRings.IS_REVOKED, KeyRings.CAN_CERTIFY,
+                    // basic data, primary uid in particular because it's expensive
-                        KeyRings.CREATION, KeyRings.EXPIRY,
+                    KeyRings.MASTER_KEY_ID, KeyRings.USER_ID, KeyRings.HAS_ANY_SECRET,
-                        KeyRings.ALGORITHM, KeyRings.FINGERPRINT,
+                    // complex knowledge about subkeys
-                        KeyRings.USER_ID, KeyRings.VERIFIED,
+                    KeyRings.IS_REVOKED, KeyRings.CAN_CERTIFY, KeyRings.HAS_ENCRYPT, KeyRings.HAS_SIGN,
-                        KeyRings.HAS_SECRET, KeyRings.PRIVKEY_DATA
+                    // stuff only the db knows and of course, ring data
+                    KeyRings.VERIFIED, secret ? KeyRings.PRIVKEY_DATA : KeyRings.PUBKEY_DATA
                 }, null, null, null);
         try {
             if (cursor != null && cursor.moveToFirst()) {
-                // check if a privkey is actually available
-                byte[] privkey = cursor.getBlob(11);
-                if(privkey == null) {
-                    throw new NotFoundException("Key found, but no secret key available!");
-                }
-
                 long masterKeyId = cursor.getLong(0);
-                int keySize = cursor.getInt(1);
+                String userId = cursor.getString(1);
-                boolean isRevoked = cursor.getInt(2) > 0;
+                boolean hasAnySecret = cursor.getInt(2) > 0;
-                boolean canCertify = cursor.getInt(3) > 0;
+                boolean isRevoked = cursor.getInt(3) > 0;
-                long creation = cursor.getLong(4);
+                boolean canCertify = cursor.getInt(4) > 0;
-                long expiry = cursor.getLong(5);
+                long hasEncryptId = cursor.getLong(5);
-                int algorithm = cursor.getInt(6);
+                long hasSignId = cursor.getLong(6);
-                byte[] fingerprint = cursor.getBlob(7);
+                int verified = cursor.getInt(7);
-                String userId = cursor.getString(8);
+                byte[] blob = cursor.getBlob(8);
-                int verified = cursor.getInt(9);
+                return secret
-                boolean hasSecret = cursor.getInt(10) > 0;
+                        ? new CachedSecretKeyRing(
-                return new CachedSecretKeyRing(
+                              masterKeyId, userId, hasAnySecret,
-                        masterKeyId, keySize, isRevoked, canCertify,
+                              isRevoked, canCertify, hasEncryptId, hasSignId,
-                        creation, expiry, algorithm, fingerprint,
+                              verified, blob)
-                        userId, verified, hasSecret, privkey
+                        : new CachedPublicKeyRing(
-                );
+                              masterKeyId, userId, hasAnySecret,
+                              isRevoked, canCertify, hasEncryptId, hasSignId,
+                              verified, blob);
             } else {
                 throw new NotFoundException("Key not found!");
             }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java

@@ -30,16 +30,10 @@ import android.widget.TextView;
 
 import com.beardedhen.androidbootstrap.BootstrapButton;
 
-import org.spongycastle.openpgp.PGPSecretKey;
-import org.spongycastle.openpgp.PGPSecretKeyRing;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
-import org.sufficientlysecure.keychain.pgp.CachedPublicKey;
 import org.sufficientlysecure.keychain.pgp.CachedPublicKeyRing;
-import org.sufficientlysecure.keychain.pgp.CachedSecretKey;
-import org.sufficientlysecure.keychain.pgp.CachedSecretKeyRing;
 import org.sufficientlysecure.keychain.pgp.PgpKeyHelper;
-import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
 import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
 import org.sufficientlysecure.keychain.util.Log;
@@ -155,7 +149,7 @@ public class EncryptAsymmetricFragment extends Fragment {
             try {
                 CachedPublicKeyRing keyring =
                         providerHelper.getCachedPublicKeyRing(preselectedSignatureKeyId);
-                if(keyring.hasSecret()) {
+                if(keyring.hasAnySecret()) {
                     setSignatureKeyId(keyring.getMasterKeyId());
                 }
             } catch (ProviderHelper.NotFoundException e) {