From 7bc424a8cb1c3d4d4c77778f27bd18fe61da0736 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 3 Sep 2014 02:42:05 +0200 Subject: work on passphrase caching, make use of cached SecretKeyType data (WIP!) --- .../keychain/pgp/PgpDecryptVerify.java | 18 +++---- .../keychain/provider/CachedPublicKeyRing.java | 17 +++---- .../keychain/provider/ProviderHelper.java | 11 ++++- .../keychain/service/PassphraseCacheService.java | 55 +++++++++++++--------- .../ui/dialog/PassphraseDialogFragment.java | 55 +++++----------------- 5 files changed, 70 insertions(+), 86 deletions(-) (limited to 'OpenKeychain/src/main/java/org') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java index 11e986562..d7eb30f16 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java @@ -261,6 +261,8 @@ public class PgpDecryptVerify { PGPPublicKeyEncryptedData encData = (PGPPublicKeyEncryptedData) obj; + long subKeyId = encData.getKeyID(); + CanonicalizedSecretKeyRing secretKeyRing; try { // get actual keyring object based on master key id @@ -276,22 +278,20 @@ public class PgpDecryptVerify { continue; } // get subkey which has been used for this encryption packet - secretEncryptionKey = secretKeyRing.getSecretKey(encData.getKeyID()); + secretEncryptionKey = secretKeyRing.getSecretKey(subKeyId); if (secretEncryptionKey == null) { // continue with the next packet in the while loop continue; } - /* secret key exists in database! */ - long masterKeyId = secretEncryptionKey.getRing().getMasterKeyId(); - // allow only specific keys for decryption? if (mAllowedKeyIds != null) { - Log.d(Constants.TAG, "encData.getKeyID(): " + encData.getKeyID()); + Log.d(Constants.TAG, "encData.getKeyID(): " + subKeyId); Log.d(Constants.TAG, "mAllowedKeyIds: " + mAllowedKeyIds); - Log.d(Constants.TAG, "masterKeyId: " + masterKeyId); + Log.d(Constants.TAG, "masterKeyId: " + + secretEncryptionKey.getRing().getMasterKeyId()); - if (!mAllowedKeyIds.contains(masterKeyId)) { + if (!mAllowedKeyIds.contains(subKeyId)) { // this key is in our db, but NOT allowed! // continue with the next packet in the while loop continue; @@ -306,12 +306,12 @@ public class PgpDecryptVerify { // if no passphrase was explicitly set try to get it from the cache service if (mPassphrase == null) { // returns "" if key has no passphrase - mPassphrase = mPassphraseCache.getCachedPassphrase(masterKeyId); + mPassphrase = mPassphraseCache.getCachedPassphrase(subKeyId); // if passphrase was not cached, return here // indicating that a passphrase is missing! if (mPassphrase == null) { - result.setKeyIdPassphraseNeeded(masterKeyId); + result.setKeyIdPassphraseNeeded(subKeyId); result.setStatus(PgpDecryptVerifyResult.KEY_PASSHRASE_NEEDED); return result; } 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 18b02156d..cfb4a915e 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/CachedPublicKeyRing.java @@ -27,6 +27,7 @@ import org.sufficientlysecure.keychain.pgp.KeyRing; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; import org.sufficientlysecure.keychain.provider.KeychainContract.Keys; +import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException; import org.sufficientlysecure.keychain.util.Log; /** This implementation of KeyRing provides a cached view of PublicKeyRing @@ -226,16 +227,12 @@ public class CachedPublicKeyRing extends KeyRing { return mProviderHelper.getContentResolver().query(keysUri, null, null, null, null); } - public SecretKeyType getSecretKeyType(long keyId) throws PgpGeneralException { - try { - Object data = mProviderHelper.getGenericData(Keys.buildKeysUri(mUri), - KeyRings.HAS_SECRET, - ProviderHelper.FIELD_TYPE_INTEGER, - KeyRings.KEY_ID + " = " + Long.toString(keyId)); - return SecretKeyType.fromNum(((Long) data).intValue()); - } catch(ProviderHelper.NotFoundException e) { - throw new PgpGeneralException(e); - } + public SecretKeyType getSecretKeyType(long keyId) throws NotFoundException { + Object data = mProviderHelper.getGenericData(Keys.buildKeysUri(mUri), + KeyRings.HAS_SECRET, + ProviderHelper.FIELD_TYPE_INTEGER, + KeyRings.KEY_ID + " = " + Long.toString(keyId)); + return SecretKeyType.fromNum(((Long) data).intValue()); } } 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 bfa95280e..186a0fc88 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java @@ -141,7 +141,11 @@ public class ProviderHelper { public static final int FIELD_TYPE_BLOB = 5; public Object getGenericData(Uri uri, String column, int type) throws NotFoundException { - return getGenericData(uri, new String[]{column}, new int[]{type}, null).get(column); + Object result = getGenericData(uri, new String[]{column}, new int[]{type}, null).get(column); + if (result == null) { + throw new NotFoundException(); + } + return result; } public Object getGenericData(Uri uri, String column, int type, String selection) @@ -229,6 +233,11 @@ public class ProviderHelper { } + public long getMasterKeyId(long subKeyId) throws NotFoundException { + return (Long) getGenericData(KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(subKeyId), + KeyRings.MASTER_KEY_ID, FIELD_TYPE_INTEGER); + } + public CachedPublicKeyRing getCachedPublicKeyRing(Uri queryUri) { return new CachedPublicKeyRing(this, queryUri); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java index 1b357bd65..fe5c88f62 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/PassphraseCacheService.java @@ -41,10 +41,13 @@ import org.spongycastle.bcpg.S2K; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.helper.Preferences; +import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey.SecretKeyType; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; +import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing; import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.ProviderHelper; +import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException; import org.sufficientlysecure.keychain.util.Log; import java.util.Date; @@ -180,12 +183,12 @@ public class PassphraseCacheService extends Service { /** * Internal implementation to get cached passphrase. * - * @param keyId + * @param subKeyId * @return */ - private String getCachedPassphraseImpl(long keyId) throws ProviderHelper.NotFoundException { + private String getCachedPassphraseImpl(long subKeyId) throws ProviderHelper.NotFoundException { // passphrase for symmetric encryption? - if (keyId == Constants.key.symmetric) { + if (subKeyId == Constants.key.symmetric) { Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for symmetric encryption"); String cachedPassphrase = mPassphraseCache.get(Constants.key.symmetric).getPassphrase(); if (cachedPassphrase == null) { @@ -195,32 +198,39 @@ public class PassphraseCacheService extends Service { return cachedPassphrase; } + // on "none" key, just do nothing + if(subKeyId == Constants.key.none) { + return null; + } + // try to get master key id which is used as an identifier for cached passphrases - Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for masterKeyId " + keyId); - CanonicalizedSecretKeyRing key = new ProviderHelper(this).getCanonicalizedSecretKeyRing( - KeychainContract.KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(keyId)); + Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for masterKeyId " + subKeyId); + // find a master key id for our key + long masterKeyId = new ProviderHelper(this).getMasterKeyId(subKeyId); + CachedPublicKeyRing keyRing = new ProviderHelper(this).getCachedPublicKeyRing(masterKeyId); - // no passphrase needed? just add empty string and return it, then - if (!key.hasPassphrase()) { - Log.d(Constants.TAG, "Key has no passphrase! Caches and returns empty passphrase!"); + // get the type of key (from the database) + SecretKeyType keyType = keyRing.getSecretKeyType(subKeyId); + switch (keyType) { // TODO: HACK for yubikeys - if (key.getSecretKey().getSecretKey().getS2K().getType() == S2K.GNU_DUMMY_S2K - && key.getSecretKey().getSecretKey().getS2K().getProtectionMode() == 2) { - // NFC! + case DIVERT_TO_CARD: return "123456"; - } - - try { - addCachedPassphrase(this, keyId, "", key.getPrimaryUserIdWithFallback()); - } catch (PgpGeneralException e) { - Log.d(Constants.TAG, "PgpGeneralException occured"); - } - return ""; + case PASSPHRASE_EMPTY: + try { + addCachedPassphrase(this, subKeyId, "", keyRing.getPrimaryUserIdWithFallback()); + } catch (PgpGeneralException e) { + Log.d(Constants.TAG, "PgpGeneralException occured"); + } + return ""; + case UNAVAILABLE: + throw new NotFoundException("secret key for this subkey is not available"); + case GNU_DUMMY: + throw new NotFoundException("secret key for stripped subkey is not available"); } // get cached passphrase - CachedPassphrase cachedPassphrase = mPassphraseCache.get(keyId); + CachedPassphrase cachedPassphrase = mPassphraseCache.get(subKeyId); if (cachedPassphrase == null) { Log.d(Constants.TAG, "PassphraseCacheService: Passphrase not (yet) cached, returning null"); // not really an error, just means the passphrase is not cached but not empty either @@ -229,7 +239,7 @@ public class PassphraseCacheService extends Service { // set it again to reset the cache life cycle Log.d(Constants.TAG, "PassphraseCacheService: Cache passphrase again when getting it!"); - addCachedPassphrase(this, keyId, cachedPassphrase.getPassphrase(), cachedPassphrase.getPrimaryUserID()); + addCachedPassphrase(this, subKeyId, cachedPassphrase.getPassphrase(), cachedPassphrase.getPrimaryUserID()); return cachedPassphrase.getPassphrase(); } @@ -312,7 +322,6 @@ public class PassphraseCacheService extends Service { long keyId = intent.getLongExtra(EXTRA_KEY_ID, -1); Messenger messenger = intent.getParcelableExtra(EXTRA_MESSENGER); - Message msg = Message.obtain(); try { String passphrase = getCachedPassphraseImpl(keyId); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java index ef2659cf8..f19309b51 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/dialog/PassphraseDialogFragment.java @@ -47,6 +47,7 @@ import org.sufficientlysecure.keychain.compatibility.DialogFragmentWorkaround; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; +import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.service.PassphraseCacheService; import org.sufficientlysecure.keychain.util.Log; @@ -98,17 +99,7 @@ public class PassphraseDialogFragment extends DialogFragment implements OnEditor */ public static PassphraseDialogFragment newInstance(Context context, Messenger messenger, long secretKeyId) throws PgpGeneralException { - // check if secret key has a passphrase - if (!(secretKeyId == Constants.key.symmetric || secretKeyId == Constants.key.none)) { - try { - if (!new ProviderHelper(context).getCanonicalizedSecretKeyRing(secretKeyId).hasPassphrase()) { - throw new PgpGeneralException("No passphrase! No passphrase dialog needed!"); - } - } catch (ProviderHelper.NotFoundException e) { - throw new PgpGeneralException("Error: Key not found!", e); - } - } - + // do NOT check if the key even needs a passphrase. that's not our job here. PassphraseDialogFragment frag = new PassphraseDialogFragment(); Bundle args = new Bundle(); args.putLong(ARG_SECRET_KEY_ID, secretKeyId); @@ -141,7 +132,8 @@ public class PassphraseDialogFragment extends DialogFragment implements OnEditor } else { try { ProviderHelper helper = new ProviderHelper(activity); - secretRing = helper.getCanonicalizedSecretKeyRing(secretKeyId); + secretRing = helper.getCanonicalizedSecretKeyRing( + KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(secretKeyId)); // yes the inner try/catch block is necessary, otherwise the final variable // above can't be statically verified to have been set in all cases because // the catch clause doesn't return. @@ -191,51 +183,28 @@ public class PassphraseDialogFragment extends DialogFragment implements OnEditor return; } - CanonicalizedSecretKey unlockedSecretKey = null; - - for (CanonicalizedSecretKey clickSecretKey : secretRing.secretKeyIterator()) { - try { - boolean unlocked = clickSecretKey.unlock(passphrase); - if (unlocked) { - unlockedSecretKey = clickSecretKey; - break; - } - } catch (PgpGeneralException e) { - Toast.makeText(activity, R.string.error_could_not_extract_private_key, - Toast.LENGTH_SHORT).show(); - - sendMessageToHandler(MESSAGE_CANCEL); - return; // ran out of keys to try - } - } - - // Means we got an exception every time - if (unlockedSecretKey == null) { - Toast.makeText(activity, R.string.wrong_passphrase, + try { + // make sure this unlocks + // TODO this is a very costly operation, we should not be doing this here! + secretRing.getSecretKey(secretKeyId).unlock(passphrase); + } catch (PgpGeneralException e) { + Toast.makeText(activity, R.string.error_could_not_extract_private_key, Toast.LENGTH_SHORT).show(); sendMessageToHandler(MESSAGE_CANCEL); - return; + return; // ran out of keys to try } - long masterKeyId = secretRing.getMasterKeyId(); - // cache the new passphrase Log.d(Constants.TAG, "Everything okay! Caching entered passphrase"); try { - PassphraseCacheService.addCachedPassphrase(activity, masterKeyId, passphrase, + PassphraseCacheService.addCachedPassphrase(activity, secretKeyId, passphrase, secretRing.getPrimaryUserIdWithFallback()); } catch (PgpGeneralException e) { Log.e(Constants.TAG, "adding of a passphrase failed", e); } - if (unlockedSecretKey.getKeyId() != masterKeyId) { - PassphraseCacheService.addCachedPassphrase( - activity, unlockedSecretKey.getKeyId(), passphrase, - unlockedSecretKey.getPrimaryUserIdWithFallback()); - } - // also return passphrase back to activity Bundle data = new Bundle(); data.putString(MESSAGE_DATA_PASSPHRASE, passphrase); -- cgit v1.2.3