From de698b89552a02a445f808cf97d3ce94d35a2777 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Sun, 29 Jun 2014 22:34:53 +0200 Subject: add create key capabilities to SaveKeyringParcel --- .../keychain/pgp/PgpKeyOperation.java | 76 +++++++++++++++++++--- .../keychain/pgp/WrappedSecretKeyRing.java | 26 -------- .../keychain/service/KeychainIntentService.java | 44 ++++++++----- .../keychain/service/OperationResultParcel.java | 5 ++ .../keychain/service/SaveKeyringParcel.java | 25 ++++--- OpenKeychain/src/main/res/values/strings.xml | 2 + .../src/test/java/tests/PgpDecryptVerifyTest.java | 1 - 7 files changed, 118 insertions(+), 61 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java index 1b59e7cc0..bde79eee0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -38,6 +38,7 @@ import org.spongycastle.openpgp.operator.PBESecretKeyDecryptor; import org.spongycastle.openpgp.operator.PBESecretKeyEncryptor; import org.spongycastle.openpgp.operator.PGPContentSignerBuilder; import org.spongycastle.openpgp.operator.PGPDigestCalculator; +import org.spongycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator; import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentSignerBuilder; import org.spongycastle.openpgp.operator.jcajce.JcaPGPDigestCalculatorProviderBuilder; import org.spongycastle.openpgp.operator.jcajce.JcaPGPKeyPair; @@ -50,6 +51,8 @@ import org.sufficientlysecure.keychain.service.OperationResultParcel.LogLevel; import org.sufficientlysecure.keychain.service.OperationResultParcel.LogType; import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog; import org.sufficientlysecure.keychain.service.SaveKeyringParcel; +import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd; +import org.sufficientlysecure.keychain.util.Log; import org.sufficientlysecure.keychain.util.Primes; import java.io.IOException; @@ -175,6 +178,40 @@ public class PgpKeyOperation { } } + public UncachedKeyRing createSecretKeyRing(SaveKeyringParcel saveParcel, OperationLog log, + int indent) { + + try { + + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_KEYID, indent); + indent += 1; + updateProgress(R.string.progress_building_key, 0, 100); + + if (saveParcel.addSubKeys == null || saveParcel.addSubKeys.isEmpty()) { + log.add(LogLevel.ERROR, LogType.MSG_CR_ERROR_NO_MASTER, indent); + return null; + } + + SubkeyAdd add = saveParcel.addSubKeys.remove(0); + PGPSecretKey masterSecretKey = createKey(add.mAlgorithm, add.mKeysize, + saveParcel.newPassphrase, true); + PGPSecretKeyRing sKR = new PGPSecretKeyRing( + masterSecretKey.getEncoded(), new JcaKeyFingerprintCalculator()); + + return internal(sKR, masterSecretKey, saveParcel, saveParcel.newPassphrase, log, indent); + + } catch (PGPException e) { + Log.e(Constants.TAG, "pgp error encoding key", e); + return null; + } catch (IOException e) { + Log.e(Constants.TAG, "io error encoding key", e); + return null; + } catch (PgpGeneralMsgIdException e) { + return null; + } + + } + /** This method introduces a list of modifications specified by a SaveKeyringParcel to a * WrappedSecretKeyRing. * @@ -204,28 +241,49 @@ public class PgpKeyOperation { indent += 1; updateProgress(R.string.progress_building_key, 0, 100); + // Make sure this is called with a proper SaveKeyringParcel + if (saveParcel.mMasterKeyId == null || saveParcel.mMasterKeyId != wsKR.getMasterKeyId()) { + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_KEYID, indent); + return null; + } + // We work on bouncycastle object level here PGPSecretKeyRing sKR = wsKR.getRing(); - PGPPublicKey masterPublicKey = sKR.getPublicKey(); PGPSecretKey masterSecretKey = sKR.getSecretKey(); + // Make sure the fingerprint matches + if (saveParcel.mFingerprint == null + || !Arrays.equals(saveParcel.mFingerprint, + masterSecretKey.getPublicKey().getFingerprint())) { + log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_FINGERPRINT, indent); + return null; + } + + return internal(sKR, masterSecretKey, saveParcel, passphrase, log, indent); + + } + + private UncachedKeyRing internal(PGPSecretKeyRing sKR, PGPSecretKey masterSecretKey, + SaveKeyringParcel saveParcel, String passphrase, + OperationLog log, int indent) { + + updateProgress(R.string.progress_certifying_master_key, 20, 100); + + PGPPublicKey masterPublicKey = masterSecretKey.getPublicKey(); + // 1. Unlock private key log.add(LogLevel.DEBUG, LogType.MSG_MF_UNLOCK, indent); - PGPPrivateKey masterPrivateKey; { + PGPPrivateKey masterPrivateKey; + { try { PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider( Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray()); masterPrivateKey = masterSecretKey.extractPrivateKey(keyDecryptor); } catch (PGPException e) { - log.add(LogLevel.ERROR, LogType.MSG_MF_UNLOCK_ERROR, indent+1); + log.add(LogLevel.ERROR, LogType.MSG_MF_UNLOCK_ERROR, indent + 1); return null; } } - if (!Arrays.equals(saveParcel.mFingerprint, sKR.getPublicKey().getFingerprint())) { - return null; - } - - updateProgress(R.string.progress_certifying_master_key, 20, 100); // work on master secret key try { @@ -262,7 +320,7 @@ public class PgpKeyOperation { } - // 4a. For each subkey change, generate new subkey binding certificate + // 4a. For each subkey change, generate new subkey binding certificate for (SaveKeyringParcel.SubkeyChange change : saveParcel.changeSubKeys) { log.add(LogLevel.INFO, LogType.MSG_MF_SUBKEY_CHANGE, indent, PgpKeyHelper.convertKeyIdToHex(change.mKeyId)); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKeyRing.java index d7148f710..c737b7c46 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKeyRing.java @@ -8,16 +8,13 @@ import org.spongycastle.openpgp.PGPPublicKey; import org.spongycastle.openpgp.PGPSecretKey; import org.spongycastle.openpgp.PGPSecretKeyRing; 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 org.sufficientlysecure.keychain.util.Log; import java.io.IOException; -import java.security.NoSuchProviderException; import java.util.Iterator; public class WrappedSecretKeyRing extends WrappedKeyRing { @@ -91,29 +88,6 @@ public class WrappedSecretKeyRing extends WrappedKeyRing { } } - public UncachedKeyRing changeSecretKeyPassphrase(String oldPassphrase, - String newPassphrase) - throws IOException, PGPException, NoSuchProviderException { - - if (oldPassphrase == null) { - oldPassphrase = ""; - } - if (newPassphrase == null) { - newPassphrase = ""; - } - - PGPSecretKeyRing newKeyRing = PGPSecretKeyRing.copyWithNewPassword( - mRing, - new JcePBESecretKeyDecryptorBuilder(new JcaPGPDigestCalculatorProviderBuilder() - .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build()).setProvider( - Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(oldPassphrase.toCharArray()), - new JcePBESecretKeyEncryptorBuilder(mRing.getSecretKey() - .getKeyEncryptionAlgorithm()).build(newPassphrase.toCharArray())); - - return new UncachedKeyRing(newKeyRing); - - } - public IterableIterator secretKeyIterator() { final Iterator it = mRing.getSecretKeys(); return new IterableIterator(new Iterator() { 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 e1514b16f..4fbdfab76 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java @@ -209,6 +209,10 @@ public class KeychainIntentService extends IntentService mMessenger = (Messenger) extras.get(EXTRA_MESSENGER); Bundle data = extras.getBundle(EXTRA_DATA); + if (data == null) { + Log.e(Constants.TAG, "data extra is null!"); + return; + } OtherHelper.logDebugBundle(data, "EXTRA_DATA"); @@ -320,33 +324,40 @@ public class KeychainIntentService extends IntentService try { /* Input */ SaveKeyringParcel saveParcel = data.getParcelable(SAVE_KEYRING_PARCEL); - long masterKeyId = saveParcel.mMasterKeyId; + if (saveParcel == null) { + Log.e(Constants.TAG, "bug: missing save_keyring_parcel in data!"); + return; + } /* Operation */ ProviderHelper providerHelper = new ProviderHelper(this); PgpKeyOperation keyOperations = new PgpKeyOperation(new ProgressScaler(this, 10, 50, 100)); try { - String passphrase = data.getString(SAVE_KEYRING_PASSPHRASE); - WrappedSecretKeyRing secRing = providerHelper.getWrappedSecretKeyRing(masterKeyId); - OperationLog log = new OperationLog(); - UncachedKeyRing ring = keyOperations.modifySecretKeyRing(secRing, saveParcel, - passphrase, log, 0); - providerHelper.saveSecretKeyRing(ring, new ProgressScaler(this, 60, 95, 100)); + UncachedKeyRing ring; + if (saveParcel.mMasterKeyId != null) { + String passphrase = data.getString(SAVE_KEYRING_PASSPHRASE); + WrappedSecretKeyRing secRing = + providerHelper.getWrappedSecretKeyRing(saveParcel.mMasterKeyId); + + ring = keyOperations.modifySecretKeyRing(secRing, saveParcel, + passphrase, log, 0); + } else { + ring = keyOperations.createSecretKeyRing(saveParcel, log, 0); + } + + providerHelper.saveSecretKeyRing(ring, new ProgressScaler(this, 10, 95, 100)); + + if (saveParcel.newPassphrase != null) { + PassphraseCacheService.addCachedPassphrase(this, ring.getMasterKeyId(), + saveParcel.newPassphrase); + } } catch (ProviderHelper.NotFoundException e) { - // UncachedKeyRing ring = keyOperations.(saveParcel); //new Keyring - // save the pair - setProgress(R.string.progress_saving_key_ring, 95, 100); - // providerHelper.saveSecretKeyRing(ring); sendErrorToHandler(e); } setProgress(R.string.progress_done, 100, 100); - if (saveParcel.newPassphrase != null) { - PassphraseCacheService.addCachedPassphrase(this, masterKeyId, saveParcel.newPassphrase); - } - /* Output */ sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY); } catch (Exception e) { @@ -437,7 +448,7 @@ public class KeychainIntentService extends IntentService new FileOutputStream(outputFile)); if (mIsCanceled) { - boolean isDeleted = new File(outputFile).delete(); + new File(outputFile).delete(); } sendMessageToHandler(KeychainIntentServiceHandler.MESSAGE_OKAY, resultData); @@ -593,6 +604,7 @@ public class KeychainIntentService extends IntentService return; } Message msg = Message.obtain(); + assert msg != null; msg.arg1 = arg1; if (arg2 != null) { msg.arg2 = arg2; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java index 7f91ab490..535fa08cf 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -233,9 +233,14 @@ public class OperationResultParcel implements Parcelable { MSG_MG_NEW_SUBKEY (R.string.msg_mg_new_subkey), MSG_MG_FOUND_NEW (R.string.msg_mg_found_new), + // secret key create + MSG_CR_ERROR_NO_MASTER (R.string.msg_mr), + // secret key modify MSG_MF (R.string.msg_mr), MSG_MF_ERROR_ENCODE (R.string.msg_mf_error_encode), + MSG_MF_ERROR_FINGERPRINT (R.string.msg_mf_error_fingerprint), + MSG_MF_ERROR_KEYID (R.string.msg_mf_error_keyid), MSG_MF_ERROR_PGP (R.string.msg_mf_error_pgp), MSG_MF_ERROR_SIG (R.string.msg_mf_error_sig), MSG_MF_PASSPHRASE (R.string.msg_mf_passphrase), diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java index 020b808b9..1ad19cdd0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java @@ -22,10 +22,10 @@ import java.util.ArrayList; */ public class SaveKeyringParcel implements Parcelable { - // the master key id to be edited - public final long mMasterKeyId; - // the key fingerprint, for safety - public final byte[] mFingerprint; + // the master key id to be edited. if this is null, a new one will be created + public Long mMasterKeyId; + // the key fingerprint, for safety. MUST be null for a new key. + public byte[] mFingerprint; public String newPassphrase; @@ -38,9 +38,7 @@ public class SaveKeyringParcel implements Parcelable { public ArrayList revokeUserIds; public ArrayList revokeSubKeys; - public SaveKeyringParcel(long masterKeyId, byte[] fingerprint) { - mMasterKeyId = masterKeyId; - mFingerprint = fingerprint; + public SaveKeyringParcel() { addUserIds = new ArrayList(); addSubKeys = new ArrayList(); changeSubKeys = new ArrayList(); @@ -48,6 +46,12 @@ public class SaveKeyringParcel implements Parcelable { revokeSubKeys = new ArrayList(); } + public SaveKeyringParcel(long masterKeyId, byte[] fingerprint) { + this(); + mMasterKeyId = masterKeyId; + mFingerprint = fingerprint; + } + // performance gain for using Parcelable here would probably be negligible, // use Serializable instead. public static class SubkeyAdd implements Serializable { @@ -75,7 +79,7 @@ public class SaveKeyringParcel implements Parcelable { } public SaveKeyringParcel(Parcel source) { - mMasterKeyId = source.readLong(); + mMasterKeyId = source.readInt() != 0 ? source.readLong() : null; mFingerprint = source.createByteArray(); addUserIds = source.createStringArrayList(); @@ -90,7 +94,10 @@ public class SaveKeyringParcel implements Parcelable { @Override public void writeToParcel(Parcel destination, int flags) { - destination.writeLong(mMasterKeyId); + destination.writeInt(mMasterKeyId == null ? 0 : 1); + if(mMasterKeyId != null) { + destination.writeLong(mMasterKeyId); + } destination.writeByteArray(mFingerprint); destination.writeStringList(addUserIds); diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 39b105664..d886fe241 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -626,6 +626,8 @@ Modifying keyring %s Encoding exception! + Actual key fingerprint does not match expected! + No keyid. This is a programming error, please file a bug report! PGP internal exception! Signature exception! Changing passphrase diff --git a/OpenKeychain/src/test/java/tests/PgpDecryptVerifyTest.java b/OpenKeychain/src/test/java/tests/PgpDecryptVerifyTest.java index 346a1f9df..d759bce05 100644 --- a/OpenKeychain/src/test/java/tests/PgpDecryptVerifyTest.java +++ b/OpenKeychain/src/test/java/tests/PgpDecryptVerifyTest.java @@ -22,7 +22,6 @@ public class PgpDecryptVerifyTest { Assert.assertEquals(expectedSignatureResult, status); } - @Test public void testVerifyFailure() throws Exception { -- cgit v1.2.3