work on passphrase caching, make use of cached SecretKeyType data (WIP!)

This commit is contained in:
Vincent Breitmoser 2014-09-03 02:42:05 +02:00
parent e9b14585f5
commit 7bc424a8cb
5 changed files with 70 additions and 86 deletions

View File

@ -261,6 +261,8 @@ public class PgpDecryptVerify {
PGPPublicKeyEncryptedData encData = (PGPPublicKeyEncryptedData) obj; PGPPublicKeyEncryptedData encData = (PGPPublicKeyEncryptedData) obj;
long subKeyId = encData.getKeyID();
CanonicalizedSecretKeyRing secretKeyRing; CanonicalizedSecretKeyRing secretKeyRing;
try { try {
// get actual keyring object based on master key id // get actual keyring object based on master key id
@ -276,22 +278,20 @@ public class PgpDecryptVerify {
continue; continue;
} }
// get subkey which has been used for this encryption packet // get subkey which has been used for this encryption packet
secretEncryptionKey = secretKeyRing.getSecretKey(encData.getKeyID()); secretEncryptionKey = secretKeyRing.getSecretKey(subKeyId);
if (secretEncryptionKey == null) { if (secretEncryptionKey == null) {
// continue with the next packet in the while loop // continue with the next packet in the while loop
continue; continue;
} }
/* secret key exists in database! */
long masterKeyId = secretEncryptionKey.getRing().getMasterKeyId();
// allow only specific keys for decryption? // allow only specific keys for decryption?
if (mAllowedKeyIds != null) { 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, "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! // this key is in our db, but NOT allowed!
// continue with the next packet in the while loop // continue with the next packet in the while loop
continue; continue;
@ -306,12 +306,12 @@ public class PgpDecryptVerify {
// if no passphrase was explicitly set try to get it from the cache service // if no passphrase was explicitly set try to get it from the cache service
if (mPassphrase == null) { if (mPassphrase == null) {
// returns "" if key has no passphrase // returns "" if key has no passphrase
mPassphrase = mPassphraseCache.getCachedPassphrase(masterKeyId); mPassphrase = mPassphraseCache.getCachedPassphrase(subKeyId);
// if passphrase was not cached, return here // if passphrase was not cached, return here
// indicating that a passphrase is missing! // indicating that a passphrase is missing!
if (mPassphrase == null) { if (mPassphrase == null) {
result.setKeyIdPassphraseNeeded(masterKeyId); result.setKeyIdPassphraseNeeded(subKeyId);
result.setStatus(PgpDecryptVerifyResult.KEY_PASSHRASE_NEEDED); result.setStatus(PgpDecryptVerifyResult.KEY_PASSHRASE_NEEDED);
return result; return result;
} }

View File

@ -27,6 +27,7 @@ import org.sufficientlysecure.keychain.pgp.KeyRing;
import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings;
import org.sufficientlysecure.keychain.provider.KeychainContract.Keys; import org.sufficientlysecure.keychain.provider.KeychainContract.Keys;
import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException;
import org.sufficientlysecure.keychain.util.Log; import org.sufficientlysecure.keychain.util.Log;
/** This implementation of KeyRing provides a cached view of PublicKeyRing /** 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); return mProviderHelper.getContentResolver().query(keysUri, null, null, null, null);
} }
public SecretKeyType getSecretKeyType(long keyId) throws PgpGeneralException { public SecretKeyType getSecretKeyType(long keyId) throws NotFoundException {
try { Object data = mProviderHelper.getGenericData(Keys.buildKeysUri(mUri),
Object data = mProviderHelper.getGenericData(Keys.buildKeysUri(mUri), KeyRings.HAS_SECRET,
KeyRings.HAS_SECRET, ProviderHelper.FIELD_TYPE_INTEGER,
ProviderHelper.FIELD_TYPE_INTEGER, KeyRings.KEY_ID + " = " + Long.toString(keyId));
KeyRings.KEY_ID + " = " + Long.toString(keyId)); return SecretKeyType.fromNum(((Long) data).intValue());
return SecretKeyType.fromNum(((Long) data).intValue());
} catch(ProviderHelper.NotFoundException e) {
throw new PgpGeneralException(e);
}
} }
} }

View File

@ -141,7 +141,11 @@ public class ProviderHelper {
public static final int FIELD_TYPE_BLOB = 5; public static final int FIELD_TYPE_BLOB = 5;
public Object getGenericData(Uri uri, String column, int type) throws NotFoundException { 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) 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) { public CachedPublicKeyRing getCachedPublicKeyRing(Uri queryUri) {
return new CachedPublicKeyRing(this, queryUri); return new CachedPublicKeyRing(this, queryUri);
} }

View File

@ -41,10 +41,13 @@ import org.spongycastle.bcpg.S2K;
import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.Constants;
import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.R;
import org.sufficientlysecure.keychain.helper.Preferences; import org.sufficientlysecure.keychain.helper.Preferences;
import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey.SecretKeyType;
import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing;
import org.sufficientlysecure.keychain.provider.KeychainContract; import org.sufficientlysecure.keychain.provider.KeychainContract;
import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.provider.ProviderHelper;
import org.sufficientlysecure.keychain.provider.ProviderHelper.NotFoundException;
import org.sufficientlysecure.keychain.util.Log; import org.sufficientlysecure.keychain.util.Log;
import java.util.Date; import java.util.Date;
@ -180,12 +183,12 @@ public class PassphraseCacheService extends Service {
/** /**
* Internal implementation to get cached passphrase. * Internal implementation to get cached passphrase.
* *
* @param keyId * @param subKeyId
* @return * @return
*/ */
private String getCachedPassphraseImpl(long keyId) throws ProviderHelper.NotFoundException { private String getCachedPassphraseImpl(long subKeyId) throws ProviderHelper.NotFoundException {
// passphrase for symmetric encryption? // passphrase for symmetric encryption?
if (keyId == Constants.key.symmetric) { if (subKeyId == Constants.key.symmetric) {
Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for symmetric encryption"); Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for symmetric encryption");
String cachedPassphrase = mPassphraseCache.get(Constants.key.symmetric).getPassphrase(); String cachedPassphrase = mPassphraseCache.get(Constants.key.symmetric).getPassphrase();
if (cachedPassphrase == null) { if (cachedPassphrase == null) {
@ -195,32 +198,39 @@ public class PassphraseCacheService extends Service {
return cachedPassphrase; 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 // try to get master key id which is used as an identifier for cached passphrases
Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for masterKeyId " + keyId); Log.d(Constants.TAG, "PassphraseCacheService.getCachedPassphraseImpl() for masterKeyId " + subKeyId);
CanonicalizedSecretKeyRing key = new ProviderHelper(this).getCanonicalizedSecretKeyRing( // find a master key id for our key
KeychainContract.KeyRings.buildUnifiedKeyRingsFindBySubkeyUri(keyId)); 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 // get the type of key (from the database)
if (!key.hasPassphrase()) { SecretKeyType keyType = keyRing.getSecretKeyType(subKeyId);
Log.d(Constants.TAG, "Key has no passphrase! Caches and returns empty passphrase!");
switch (keyType) {
// TODO: HACK for yubikeys // TODO: HACK for yubikeys
if (key.getSecretKey().getSecretKey().getS2K().getType() == S2K.GNU_DUMMY_S2K case DIVERT_TO_CARD:
&& key.getSecretKey().getSecretKey().getS2K().getProtectionMode() == 2) {
// NFC!
return "123456"; return "123456";
} case PASSPHRASE_EMPTY:
try {
try { addCachedPassphrase(this, subKeyId, "", keyRing.getPrimaryUserIdWithFallback());
addCachedPassphrase(this, keyId, "", key.getPrimaryUserIdWithFallback()); } catch (PgpGeneralException e) {
} catch (PgpGeneralException e) { Log.d(Constants.TAG, "PgpGeneralException occured");
Log.d(Constants.TAG, "PgpGeneralException occured"); }
} return "";
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 // get cached passphrase
CachedPassphrase cachedPassphrase = mPassphraseCache.get(keyId); CachedPassphrase cachedPassphrase = mPassphraseCache.get(subKeyId);
if (cachedPassphrase == null) { if (cachedPassphrase == null) {
Log.d(Constants.TAG, "PassphraseCacheService: Passphrase not (yet) cached, returning 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 // 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 // set it again to reset the cache life cycle
Log.d(Constants.TAG, "PassphraseCacheService: Cache passphrase again when getting it!"); 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(); return cachedPassphrase.getPassphrase();
} }
@ -312,7 +322,6 @@ public class PassphraseCacheService extends Service {
long keyId = intent.getLongExtra(EXTRA_KEY_ID, -1); long keyId = intent.getLongExtra(EXTRA_KEY_ID, -1);
Messenger messenger = intent.getParcelableExtra(EXTRA_MESSENGER); Messenger messenger = intent.getParcelableExtra(EXTRA_MESSENGER);
Message msg = Message.obtain(); Message msg = Message.obtain();
try { try {
String passphrase = getCachedPassphraseImpl(keyId); String passphrase = getCachedPassphraseImpl(keyId);

View File

@ -47,6 +47,7 @@ import org.sufficientlysecure.keychain.compatibility.DialogFragmentWorkaround;
import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey;
import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings;
import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.provider.ProviderHelper;
import org.sufficientlysecure.keychain.service.PassphraseCacheService; import org.sufficientlysecure.keychain.service.PassphraseCacheService;
import org.sufficientlysecure.keychain.util.Log; 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, public static PassphraseDialogFragment newInstance(Context context, Messenger messenger,
long secretKeyId) throws PgpGeneralException { long secretKeyId) throws PgpGeneralException {
// check if secret key has a passphrase // do NOT check if the key even needs a passphrase. that's not our job here.
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);
}
}
PassphraseDialogFragment frag = new PassphraseDialogFragment(); PassphraseDialogFragment frag = new PassphraseDialogFragment();
Bundle args = new Bundle(); Bundle args = new Bundle();
args.putLong(ARG_SECRET_KEY_ID, secretKeyId); args.putLong(ARG_SECRET_KEY_ID, secretKeyId);
@ -141,7 +132,8 @@ public class PassphraseDialogFragment extends DialogFragment implements OnEditor
} else { } else {
try { try {
ProviderHelper helper = new ProviderHelper(activity); 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 // 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 // above can't be statically verified to have been set in all cases because
// the catch clause doesn't return. // the catch clause doesn't return.
@ -191,51 +183,28 @@ public class PassphraseDialogFragment extends DialogFragment implements OnEditor
return; return;
} }
CanonicalizedSecretKey unlockedSecretKey = null; try {
// make sure this unlocks
for (CanonicalizedSecretKey clickSecretKey : secretRing.secretKeyIterator()) { // TODO this is a very costly operation, we should not be doing this here!
try { secretRing.getSecretKey(secretKeyId).unlock(passphrase);
boolean unlocked = clickSecretKey.unlock(passphrase); } catch (PgpGeneralException e) {
if (unlocked) { Toast.makeText(activity, R.string.error_could_not_extract_private_key,
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,
Toast.LENGTH_SHORT).show(); Toast.LENGTH_SHORT).show();
sendMessageToHandler(MESSAGE_CANCEL); sendMessageToHandler(MESSAGE_CANCEL);
return; return; // ran out of keys to try
} }
long masterKeyId = secretRing.getMasterKeyId();
// cache the new passphrase // cache the new passphrase
Log.d(Constants.TAG, "Everything okay! Caching entered passphrase"); Log.d(Constants.TAG, "Everything okay! Caching entered passphrase");
try { try {
PassphraseCacheService.addCachedPassphrase(activity, masterKeyId, passphrase, PassphraseCacheService.addCachedPassphrase(activity, secretKeyId, passphrase,
secretRing.getPrimaryUserIdWithFallback()); secretRing.getPrimaryUserIdWithFallback());
} catch (PgpGeneralException e) { } catch (PgpGeneralException e) {
Log.e(Constants.TAG, "adding of a passphrase failed", 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 // also return passphrase back to activity
Bundle data = new Bundle(); Bundle data = new Bundle();
data.putString(MESSAGE_DATA_PASSPHRASE, passphrase); data.putString(MESSAGE_DATA_PASSPHRASE, passphrase);