From 411b4cfeb2caa1d7d1c33129711bc1cd617778cf Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Sun, 4 May 2014 12:55:22 +0200 Subject: wrapped-key-ring: redesign underlying CachedKeyRing --- .../keychain/pgp/CachedKeyRing.java | 53 +++++++---- .../keychain/pgp/CachedPublicKeyRing.java | 80 ++++++++--------- .../keychain/pgp/CachedSecretKeyRing.java | 94 ++++++++----------- .../keychain/pgp/PgpSignEncrypt.java | 9 +- .../keychain/provider/ProviderHelper.java | 100 ++++++++------------- .../keychain/ui/EncryptAsymmetricFragment.java | 8 +- 6 files changed, 153 insertions(+), 191 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CachedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CachedKeyRing.java index 334f676c4..def673469 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CachedKeyRing.java +++ b/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, - byte[] fingerprint, String userId, int verified, boolean hasSecret) + protected CachedKeyRing(long masterKeyId, String userId, boolean hasAnySecret, + 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 long getMasterKeyId() { + return mMasterKeyId; } public String getPrimaryUserId() { return mUserId; } - public long getMasterKeyId() { - return mMasterKeyId; + public boolean hasAnySecret() { + return mHasAnySecret; } - public int getVerified() { - return mVerified; + public boolean isRevoked() { + return mIsRevoked; } public boolean canCertify() { return mCanCertify; } - public boolean hasSecret() { - return mHasSecret; + public long getEncryptId() { + return mHasEncryptId; + } + + public boolean hasEncrypt() { + return mHasEncryptId != 0; + } + + public long getSignId() { + return mHasSignId; + } + + public boolean hasSign() { + return mHasSignId != 0; + } + + public int getVerified() { + return mVerified; } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CachedPublicKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CachedPublicKeyRing.java index 8970d18ec..bbce42f86 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CachedPublicKeyRing.java +++ b/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, - boolean canCertify, long creation, long expiry, int algorithm, - byte[] fingerprint, String userId, int verified, boolean hasSecret, - byte[] pubkey) + public CachedPublicKeyRing(long masterKeyId, String userId, boolean hasAnySecret, + boolean isRevoked, boolean canCertify, long hasEncryptId, long hasSignId, + int verified, byte[] blob) { - 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 { - // only return master key if no other signing key is available - CachedPublicKey masterKey = null; - for (PGPPublicKey k : new IterableIterator(getRing().getPublicKeys())) { - CachedPublicKey key = new CachedPublicKey(this, k); - if (key.isRevoked() || key.canSign() || key.isExpired()) { - continue; + /** Getter that returns the subkey that should be used for signing. */ + CachedPublicKey getEncryptionSubKey() throws PgpGeneralException { + PGPPublicKey key = getRing().getPublicKey(getEncryptId()); + if(key != null) { + CachedPublicKey cKey = new CachedPublicKey(this, key); + if(!cKey.canEncrypt()) { + throw new PgpGeneralException("key error"); } - if (key.isMasterKey()) { - masterKey = key; - } else { - return key; - } - } - if(masterKey == null) { - // TODO proper exception - 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(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 cKey; } - return masterKey; + // TODO handle with proper exception + throw new PgpGeneralException("no encryption key available"); } public boolean verifySubkeyBinding(CachedPublicKey cachedSubkey) { @@ -189,4 +161,24 @@ public class CachedPublicKeyRing extends CachedKeyRing { return validPrimaryKeyBinding; } + public IterableIterator iterator() { + final Iterator it = getRing().getPublicKeys(); + return new IterableIterator(new Iterator() { + @Override + public boolean hasNext() { + return it.hasNext(); + } + + @Override + public CachedPublicKey next() { + return new CachedPublicKey(CachedPublicKeyRing.this, it.next()); + } + + @Override + public void remove() { + it.remove(); + } + }); + } + } \ No newline at end of file diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CachedSecretKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CachedSecretKeyRing.java index 590a02b95..398092aeb 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CachedSecretKeyRing.java +++ b/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, - boolean canCertify, long creation, long expiry, int algorithm, - byte[] fingerprint, String userId, int verified, boolean hasSecret, - byte[] blob) + public CachedSecretKeyRing(long masterKeyId, String userId, boolean hasAnySecret, + boolean isRevoked, boolean canCertify, long hasEncryptId, long hasSignId, + int verified, 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 iterator() { - return new IterableIterator(mRing.getSecretKeys()); + /** Getter that returns the subkey that should be used for signing. */ + 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(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(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 iterator() { + final Iterator it = mRing.getSecretKeys(); + return new IterableIterator(new Iterator() { + @Override + public boolean hasNext() { + return it.hasNext(); + } + + @Override + public CachedSecretKey next() { + return new CachedSecretKey(CachedSecretKeyRing.this, it.next()); + } + + @Override + public void remove() { + it.remove(); + } + }); + } + } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncrypt.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncrypt.java index 48cc5d6da..ccbbb3719 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncrypt.java +++ b/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(); - if (signingKey == null) { + try { + 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); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java index f2abd915e..d1a003ec1 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/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, - 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(); - } - } + return (CachedPublicKeyRing) getCachedKeyRing(queryUri, false); } 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, - KeyRings.IS_REVOKED, KeyRings.CAN_CERTIFY, - KeyRings.CREATION, KeyRings.EXPIRY, - KeyRings.ALGORITHM, KeyRings.FINGERPRINT, - KeyRings.USER_ID, KeyRings.VERIFIED, - KeyRings.HAS_SECRET, KeyRings.PRIVKEY_DATA + // we pick from cache: + // basic data, primary uid in particular because it's expensive + KeyRings.MASTER_KEY_ID, KeyRings.USER_ID, KeyRings.HAS_ANY_SECRET, + // complex knowledge about subkeys + KeyRings.IS_REVOKED, KeyRings.CAN_CERTIFY, KeyRings.HAS_ENCRYPT, KeyRings.HAS_SIGN, + // 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); - 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; - return new CachedSecretKeyRing( - masterKeyId, keySize, isRevoked, canCertify, - creation, expiry, algorithm, fingerprint, - userId, verified, hasSecret, privkey - ); + String userId = cursor.getString(1); + boolean hasAnySecret = cursor.getInt(2) > 0; + boolean isRevoked = cursor.getInt(3) > 0; + boolean canCertify = cursor.getInt(4) > 0; + long hasEncryptId = cursor.getLong(5); + long hasSignId = cursor.getLong(6); + int verified = cursor.getInt(7); + byte[] blob = cursor.getBlob(8); + return secret + ? new CachedSecretKeyRing( + masterKeyId, userId, hasAnySecret, + isRevoked, canCertify, hasEncryptId, hasSignId, + verified, blob) + : new CachedPublicKeyRing( + masterKeyId, userId, hasAnySecret, + isRevoked, canCertify, hasEncryptId, hasSignId, + verified, blob); } else { throw new NotFoundException("Key not found!"); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java index 51a229677..0b6449cda 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptAsymmetricFragment.java +++ b/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) { -- cgit v1.2.3