From 7dbb7cf1e1fd4de1098d4a2207a90ddc680598ed Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 9 Sep 2014 22:41:24 +0200 Subject: jury-rig broken sign mechanism Should improve situation on #811 --- .../keychain/pgp/PgpSignEncrypt.java | 34 +++++++++++++++------- .../keychain/service/KeychainIntentService.java | 21 ++++++++----- .../keychain/ui/EncryptFileActivity.java | 2 +- .../keychain/ui/EncryptTextActivity.java | 34 ++++++++++++++-------- 4 files changed, 61 insertions(+), 30 deletions(-) (limited to 'OpenKeychain/src') 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 2e4eafe41..4874b6eaa 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncrypt.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncrypt.java @@ -68,6 +68,7 @@ public class PgpSignEncrypt { private String mSymmetricPassphrase; private int mSymmetricEncryptionAlgorithm; private long mSignatureMasterKeyId; + private Long mSignatureSubKeyId; private int mSignatureHashAlgorithm; private String mSignaturePassphrase; private long mAdditionalEncryptId; @@ -101,6 +102,7 @@ public class PgpSignEncrypt { this.mSymmetricPassphrase = builder.mSymmetricPassphrase; this.mSymmetricEncryptionAlgorithm = builder.mSymmetricEncryptionAlgorithm; this.mSignatureMasterKeyId = builder.mSignatureMasterKeyId; + this.mSignatureSubKeyId = builder.mSignatureSubKeyId; this.mSignatureHashAlgorithm = builder.mSignatureHashAlgorithm; this.mSignaturePassphrase = builder.mSignaturePassphrase; this.mAdditionalEncryptId = builder.mAdditionalEncryptId; @@ -125,6 +127,7 @@ public class PgpSignEncrypt { private String mSymmetricPassphrase = null; private int mSymmetricEncryptionAlgorithm = 0; private long mSignatureMasterKeyId = Constants.key.none; + private Long mSignatureSubKeyId = null; private int mSignatureHashAlgorithm = 0; private String mSignaturePassphrase = null; private long mAdditionalEncryptId = Constants.key.none; @@ -179,6 +182,11 @@ public class PgpSignEncrypt { return this; } + public Builder setSignatureSubKeyId(long signatureSubKeyId) { + mSignatureSubKeyId = signatureSubKeyId; + return this; + } + public Builder setSignatureHashAlgorithm(int signatureHashAlgorithm) { mSignatureHashAlgorithm = signatureHashAlgorithm; return this; @@ -309,26 +317,32 @@ public class PgpSignEncrypt { /* Get keys for signature generation for later usage */ CanonicalizedSecretKey signingKey = null; if (enableSignature) { - CanonicalizedSecretKeyRing signingKeyRing; - try { - signingKeyRing = mProviderHelper.getCanonicalizedSecretKeyRing(mSignatureMasterKeyId); - } catch (ProviderHelper.NotFoundException e) { - throw new NoSigningKeyException(); + + // If we weren't handed a passphrase, throw early + if (mSignaturePassphrase == null) { + throw new NoPassphraseException(); } + try { - signingKey = signingKeyRing.getSigningSubKey(); - } catch (PgpGeneralException e) { + // fetch the indicated master key id (the one whose name we sign in) + CanonicalizedSecretKeyRing signingKeyRing = + mProviderHelper.getCanonicalizedSecretKeyRing(mSignatureMasterKeyId); + // fetch the specific subkey to sign with, or just use the master key if none specified + long signKeyId = mSignatureSubKeyId != null ? mSignatureSubKeyId : mSignatureMasterKeyId; + signingKey = signingKeyRing.getSecretKey(signKeyId); + // make sure it's a signing key alright! + } catch (ProviderHelper.NotFoundException e) { throw new NoSigningKeyException(); } - if (mSignaturePassphrase == null) { - throw new NoPassphraseException(); + if ( ! signingKey.canSign()) { + throw new NoSigningKeyException(); } updateProgress(R.string.progress_extracting_signature_key, 0, 100); try { - if (!signingKey.unlock(mSignaturePassphrase)) { + if ( ! signingKey.unlock(mSignaturePassphrase)) { throw new WrongPassphraseException(); } } catch (PgpGeneralException e) { 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 08f51d692..f085c8582 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java @@ -50,6 +50,7 @@ import org.sufficientlysecure.keychain.pgp.Progressable; import org.sufficientlysecure.keychain.pgp.UncachedKeyRing; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralMsgIdException; +import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing; import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRingData; import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; import org.sufficientlysecure.keychain.provider.KeychainDatabase; @@ -128,7 +129,7 @@ public class KeychainIntentService extends IntentService implements Progressable public static final String SELECTED_URI = "selected_uri"; // encrypt - public static final String ENCRYPT_SIGNATURE_KEY_ID = "secret_key_id"; + public static final String ENCRYPT_SIGNATURE_MASTER_ID = "secret_key_id"; public static final String ENCRYPT_USE_ASCII_ARMOR = "use_ascii_armor"; public static final String ENCRYPT_ENCRYPTION_KEYS_IDS = "encryption_keys_ids"; public static final String ENCRYPT_COMPRESSION_ID = "compression_id"; @@ -251,7 +252,7 @@ public class KeychainIntentService extends IntentService implements Progressable int source = data.get(SOURCE) != null ? data.getInt(SOURCE) : data.getInt(TARGET); Bundle resultData = new Bundle(); - long signatureKeyId = data.getLong(ENCRYPT_SIGNATURE_KEY_ID); + long sigMasterKeyId = data.getLong(ENCRYPT_SIGNATURE_MASTER_ID); String symmetricPassphrase = data.getString(ENCRYPT_SYMMETRIC_PASSPHRASE); boolean useAsciiArmor = data.getBoolean(ENCRYPT_USE_ASCII_ARMOR); @@ -280,14 +281,20 @@ public class KeychainIntentService extends IntentService implements Progressable .setOriginalFilename(originalFilename); try { - builder.setSignatureMasterKeyId(signatureKeyId) - .setSignaturePassphrase( - PassphraseCacheService.getCachedPassphrase(this, signatureKeyId)) + CachedPublicKeyRing signingRing = + new ProviderHelper(this).getCachedPublicKeyRing(sigMasterKeyId); + long sigSubKeyId = signingRing.getSignId(); + // It is assumed that the passphrase was cached prior to the service call. + String passphrase = PassphraseCacheService.getCachedPassphrase(this, sigSubKeyId); + builder.setSignatureMasterKeyId(sigMasterKeyId) + .setSignatureSubKeyId(sigSubKeyId) + .setSignaturePassphrase(passphrase) .setSignatureHashAlgorithm( Preferences.getPreferences(this).getDefaultHashAlgorithm()) - .setAdditionalEncryptId(signatureKeyId); - } catch (PassphraseCacheService.KeyNotFoundException e) { + .setAdditionalEncryptId(sigMasterKeyId); + } catch (PgpGeneralException e) { // encrypt-only + // TODO Just silently drop the requested signature? Shouldn't we throw here? } // this assumes that the bytes are cleartext (valid for current implementation!) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFileActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFileActivity.java index 31f4f5dfc..581145f8d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFileActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptFileActivity.java @@ -252,7 +252,7 @@ public class EncryptFileActivity extends DrawerActivity implements EncryptActivi } data.putString(KeychainIntentService.ENCRYPT_SYMMETRIC_PASSPHRASE, passphrase); } else { - data.putLong(KeychainIntentService.ENCRYPT_SIGNATURE_KEY_ID, mSigningKeyId); + data.putLong(KeychainIntentService.ENCRYPT_SIGNATURE_MASTER_ID, mSigningKeyId); data.putLongArray(KeychainIntentService.ENCRYPT_ENCRYPTION_KEYS_IDS, mEncryptionKeyIds); } return data; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptTextActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptTextActivity.java index 9bed6a5ae..290dc1ce6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptTextActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EncryptTextActivity.java @@ -36,6 +36,9 @@ import org.sufficientlysecure.keychain.compatibility.ClipboardReflection; import org.sufficientlysecure.keychain.helper.Preferences; import org.sufficientlysecure.keychain.helper.ShareHelper; import org.sufficientlysecure.keychain.pgp.KeyRing; +import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; +import org.sufficientlysecure.keychain.provider.CachedPublicKeyRing; +import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.service.KeychainIntentService; import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler; import org.sufficientlysecure.keychain.service.PassphraseCacheService; @@ -235,7 +238,7 @@ public class EncryptTextActivity extends DrawerActivity implements EncryptActivi } data.putString(KeychainIntentService.ENCRYPT_SYMMETRIC_PASSPHRASE, passphrase); } else { - data.putLong(KeychainIntentService.ENCRYPT_SIGNATURE_KEY_ID, mSigningKeyId); + data.putLong(KeychainIntentService.ENCRYPT_SIGNATURE_MASTER_ID, mSigningKeyId); data.putLongArray(KeychainIntentService.ENCRYPT_ENCRYPTION_KEYS_IDS, mEncryptionKeyIds); } return data; @@ -310,21 +313,28 @@ public class EncryptTextActivity extends DrawerActivity implements EncryptActivi } try { - if (mSigningKeyId != 0 && PassphraseCacheService.getCachedPassphrase(this, mSigningKeyId) == null) { - PassphraseDialogFragment.show(this, mSigningKeyId, - new Handler() { - @Override - public void handleMessage(Message message) { - if (message.what == PassphraseDialogFragment.MESSAGE_OKAY) { - // restart - startEncrypt(); + if (mSigningKeyId != 0) { + CachedPublicKeyRing signingRing = + new ProviderHelper(this).getCachedPublicKeyRing(mSigningKeyId); + long sigSubKeyId = signingRing.getSignId(); + if (PassphraseCacheService.getCachedPassphrase(this, sigSubKeyId) == null) { + PassphraseDialogFragment.show(this, sigSubKeyId, + new Handler() { + @Override + public void handleMessage(Message message) { + if (message.what == PassphraseDialogFragment.MESSAGE_OKAY) { + // restart + startEncrypt(); + } } } - } - ); + ); - return false; + return false; + } } + } catch (PgpGeneralException e) { + Log.e(Constants.TAG, "Key not found!", e); } catch (PassphraseCacheService.KeyNotFoundException e) { Log.e(Constants.TAG, "Key not found!", e); } -- cgit v1.2.3