diff options
Diffstat (limited to 'OpenKeychain')
4 files changed, 158 insertions, 72 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 e08c96dad..bbdbfffd2 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -47,6 +47,9 @@ import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyEncryptorBuilder; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralMsgIdException; +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.util.IterableIterator; import org.sufficientlysecure.keychain.util.Primes; @@ -98,12 +101,6 @@ public class PgpKeyOperation { } } - void updateProgress(int current, int total) { - if (mProgress != null) { - mProgress.setProgress(current, total); - } - } - /** Creates new secret key. */ private PGPSecretKey createKey(int algorithmChoice, int keySize, String passphrase, boolean isMasterKey) throws PgpGeneralMsgIdException { @@ -182,7 +179,9 @@ public class PgpKeyOperation { } /** This method introduces a list of modifications specified by a SaveKeyringParcel to a - * PGPSecretKeyRing. + * WrappedSecretKeyRing. + * + * This method relies on WrappedSecretKeyRing's canonicalization property! * * Note that PGPPublicKeyRings can not be directly modified. Instead, the corresponding * PGPSecretKeyRing must be modified and consequently consolidated with its public counterpart. @@ -191,8 +190,7 @@ public class PgpKeyOperation { * */ public UncachedKeyRing modifySecretKeyRing(WrappedSecretKeyRing wsKR, SaveKeyringParcel saveParcel, - String passphrase) - throws PgpGeneralMsgIdException, PGPException, SignatureException, IOException { + String passphrase, OperationLog log, int indent) { /* * 1. Unlock private key @@ -205,6 +203,8 @@ public class PgpKeyOperation { * 6. If requested, change passphrase */ + log.add(LogLevel.START, LogType.MSG_MR, indent); + indent += 1; updateProgress(R.string.progress_building_key, 0, 100); // We work on bouncycastle object level here @@ -213,10 +213,16 @@ public class PgpKeyOperation { PGPSecretKey masterSecretKey = sKR.getSecretKey(); // 1. Unlock private key + log.add(LogLevel.DEBUG, LogType.MSG_MR_UNLOCK, indent); PGPPrivateKey masterPrivateKey; { - PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider( - Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray()); - masterPrivateKey = masterSecretKey.extractPrivateKey(keyDecryptor); + 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_MR_UNLOCK_ERROR, indent+1); + return null; + } } if (!Arrays.equals(saveParcel.mFingerprint, sKR.getPublicKey().getFingerprint())) { return null; @@ -224,12 +230,14 @@ public class PgpKeyOperation { updateProgress(R.string.progress_certifying_master_key, 20, 100); - { // work on master secret key + // work on master secret key + try { PGPPublicKey modifiedPublicKey = masterPublicKey; // 2a. Add certificates for new user ids for (String userId : saveParcel.addUserIds) { + log.add(LogLevel.INFO, LogType.MSG_MR_UID_ADD, indent); PGPSignature cert = generateUserIdSignature(masterPrivateKey, masterPublicKey, userId, false); modifiedPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert); @@ -237,6 +245,7 @@ public class PgpKeyOperation { // 2b. Add revocations for revoked user ids for (String userId : saveParcel.revokeUserIds) { + log.add(LogLevel.INFO, LogType.MSG_MR_UID_REVOKE, indent); PGPSignature cert = generateRevocationSignature(masterPrivateKey, masterPublicKey, userId); modifiedPublicKey = PGPPublicKey.addCertification(masterPublicKey, userId, cert); @@ -244,6 +253,7 @@ public class PgpKeyOperation { // 3. If primary user id changed, generate new certificates for both old and new if (saveParcel.changePrimaryUserId != null) { + log.add(LogLevel.INFO, LogType.MSG_MR_UID_PRIMARY, indent); // todo } @@ -254,70 +264,105 @@ public class PgpKeyOperation { sKR = PGPSecretKeyRing.insertSecretKey(sKR, masterSecretKey); } - } - // 4a. For each subkey change, generate new subkey binding certificate - for (SaveKeyringParcel.SubkeyChange change : saveParcel.changeSubKeys) { - PGPSecretKey sKey = sKR.getSecretKey(change.mKeyId); - if (sKey == null) { - return null; - } - PGPPublicKey pKey = sKey.getPublicKey(); + for (SaveKeyringParcel.SubkeyChange change : saveParcel.changeSubKeys) { + log.add(LogLevel.INFO, LogType.MSG_MR_SUBKEY_CHANGE, + new String[]{PgpKeyHelper.convertKeyIdToHex(change.mKeyId)}, indent); + PGPSecretKey sKey = sKR.getSecretKey(change.mKeyId); + if (sKey == null) { + log.add(LogLevel.ERROR, LogType.MSG_MR_SUBKEY_MISSING, + new String[]{PgpKeyHelper.convertKeyIdToHex(change.mKeyId)}, indent + 1); + return null; + } + PGPPublicKey pKey = sKey.getPublicKey(); - // generate and add new signature - PGPSignature sig = generateSubkeyBindingSignature(masterPublicKey, masterPrivateKey, - sKey, pKey, change.mFlags, change.mExpiry, passphrase); - pKey = PGPPublicKey.addCertification(pKey, sig); - sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey)); - } + if (change.mExpiry != null && new Date(change.mExpiry).before(new Date())) { + log.add(LogLevel.ERROR, LogType.MSG_MR_SUBKEY_PAST_EXPIRY, + new String[]{PgpKeyHelper.convertKeyIdToHex(change.mKeyId)}, indent + 1); + return null; + } - // 4b. For each subkey change, generate new subkey binding certificate - for (long revocation : saveParcel.revokeSubKeys) { - PGPSecretKey sKey = sKR.getSecretKey(revocation); - if (sKey == null) { - return null; + // generate and add new signature + PGPSignature sig = generateSubkeyBindingSignature(masterPublicKey, masterPrivateKey, + sKey, pKey, change.mFlags, change.mExpiry, passphrase); + pKey = PGPPublicKey.addCertification(pKey, sig); + sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey)); } - PGPPublicKey pKey = sKey.getPublicKey(); - // generate and add new signature - PGPSignature sig = generateRevocationSignature(masterPublicKey, masterPrivateKey, pKey); - - pKey = PGPPublicKey.addCertification(pKey, sig); - sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey)); - } - - // 5. Generate and add new subkeys - for (SaveKeyringParcel.SubkeyAdd add : saveParcel.addSubKeys) { - try { - PGPSecretKey sKey = createKey(add.mAlgorithm, add.mKeysize, passphrase, false); + // 4b. For each subkey revocation, generate new subkey revocation certificate + for (long revocation : saveParcel.revokeSubKeys) { + log.add(LogLevel.INFO, LogType.MSG_MR_SUBKEY_REVOKE, + new String[] { PgpKeyHelper.convertKeyIdToHex(revocation) }, indent); + PGPSecretKey sKey = sKR.getSecretKey(revocation); + if (sKey == null) { + log.add(LogLevel.ERROR, LogType.MSG_MR_SUBKEY_MISSING, + new String[] { PgpKeyHelper.convertKeyIdToHex(revocation) }, indent+1); + return null; + } PGPPublicKey pKey = sKey.getPublicKey(); - PGPSignature cert = generateSubkeyBindingSignature(masterPublicKey, masterPrivateKey, - sKey, pKey, add.mFlags, add.mExpiry, passphrase); - pKey = PGPPublicKey.addCertification(pKey, cert); - sKey = PGPSecretKey.replacePublicKey(sKey, pKey); + // generate and add new signature + PGPSignature sig = generateRevocationSignature(masterPublicKey, masterPrivateKey, pKey); + + pKey = PGPPublicKey.addCertification(pKey, sig); sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey)); - } catch (PgpGeneralMsgIdException e) { - return null; } - } - // 6. If requested, change passphrase - if (saveParcel.newPassphrase != null) { - PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder().build() - .get(HashAlgorithmTags.SHA1); - PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider( - Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray()); - // Build key encryptor based on new passphrase - PBESecretKeyEncryptor keyEncryptorNew = new JcePBESecretKeyEncryptorBuilder( - PGPEncryptedData.CAST5, sha1Calc) - .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build( - saveParcel.newPassphrase.toCharArray()); + // 5. Generate and add new subkeys + for (SaveKeyringParcel.SubkeyAdd add : saveParcel.addSubKeys) { + try { + + if (add.mExpiry != null && new Date(add.mExpiry).before(new Date())) { + log.add(LogLevel.ERROR, LogType.MSG_MR_SUBKEY_PAST_EXPIRY, indent +1); + return null; + } + + log.add(LogLevel.INFO, LogType.MSG_MR_SUBKEY_NEW, indent); + PGPSecretKey sKey = createKey(add.mAlgorithm, add.mKeysize, passphrase, false); + log.add(LogLevel.DEBUG, LogType.MSG_MR_SUBKEY_NEW_ID, + new String[] { PgpKeyHelper.convertKeyIdToHex(sKey.getKeyID()) }, indent+1); + + PGPPublicKey pKey = sKey.getPublicKey(); + PGPSignature cert = generateSubkeyBindingSignature(masterPublicKey, masterPrivateKey, + sKey, pKey, add.mFlags, add.mExpiry, passphrase); + pKey = PGPPublicKey.addCertification(pKey, cert); + sKey = PGPSecretKey.replacePublicKey(sKey, pKey); + sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey)); + } catch (PgpGeneralMsgIdException e) { + return null; + } + } + + // 6. If requested, change passphrase + if (saveParcel.newPassphrase != null) { + log.add(LogLevel.INFO, LogType.MSG_MR_PASSPHRASE, indent); + PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder().build() + .get(HashAlgorithmTags.SHA1); + PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider( + Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray()); + // Build key encryptor based on new passphrase + PBESecretKeyEncryptor keyEncryptorNew = new JcePBESecretKeyEncryptorBuilder( + PGPEncryptedData.CAST5, sha1Calc) + .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build( + saveParcel.newPassphrase.toCharArray()); + + sKR = PGPSecretKeyRing.copyWithNewPassword(sKR, keyDecryptor, keyEncryptorNew); + } - sKR = PGPSecretKeyRing.copyWithNewPassword(sKR, keyDecryptor, keyEncryptorNew); + // This one must only be thrown by + } catch (IOException e) { + log.add(LogLevel.ERROR, LogType.MSG_MR_ERROR_ENCODE, indent+1); + return null; + } catch (PGPException e) { + log.add(LogLevel.ERROR, LogType.MSG_MR_ERROR_PGP, indent+1); + return null; + } catch (SignatureException e) { + log.add(LogLevel.ERROR, LogType.MSG_MR_ERROR_SIG, indent+1); + return null; } + log.add(LogLevel.OK, LogType.MSG_MR_SUCCESS, indent); return new UncachedKeyRing(sKR); } @@ -377,7 +422,7 @@ public class PgpKeyOperation { private static PGPSignature generateSubkeyBindingSignature( PGPPublicKey masterPublicKey, PGPPrivateKey masterPrivateKey, PGPSecretKey sKey, PGPPublicKey pKey, int flags, Long expiry, String passphrase) - throws PgpGeneralMsgIdException, IOException, PGPException, SignatureException { + throws IOException, PGPException, SignatureException { // date for signing Date todayDate = new Date(); @@ -414,13 +459,10 @@ public class PgpKeyOperation { if (expiry != null) { Calendar creationDate = Calendar.getInstance(TimeZone.getTimeZone("UTC")); creationDate.setTime(pKey.getCreationTime()); - // note that the below, (a/c) - (b/c) is *not* the same as (a - b) /c - // here we purposefully ignore partial days in each date - long type has - // no fractional part! - long numDays = (expiry / 86400000) - - (creationDate.getTimeInMillis() / 86400000); - if (numDays <= 0) { - throw new PgpGeneralMsgIdException(R.string.error_expiry_must_come_after_creation); + + // (Just making sure there's no programming error here, this MUST have been checked above!) + if (new Date(expiry).before(todayDate)) { + throw new RuntimeException("Bad subkey creation date, this is a bug!"); } hashedPacketsGen.setKeyExpirationTime(false, expiry - creationDate.getTimeInMillis()); } else { 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 d3f46a7a4..25a9387f4 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java @@ -52,6 +52,7 @@ import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralMsgIdException; import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; import org.sufficientlysecure.keychain.provider.KeychainDatabase; import org.sufficientlysecure.keychain.provider.ProviderHelper; +import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog; import org.sufficientlysecure.keychain.util.InputData; import org.sufficientlysecure.keychain.util.Log; import org.sufficientlysecure.keychain.util.ProgressScaler; @@ -486,7 +487,9 @@ public class KeychainIntentService extends IntentService String passphrase = data.getString(SAVE_KEYRING_PASSPHRASE); WrappedSecretKeyRing secRing = providerHelper.getWrappedSecretKeyRing(masterKeyId); - UncachedKeyRing ring = keyOperations.modifySecretKeyRing(secRing, saveParcel, passphrase); + OperationLog log = new OperationLog(); + UncachedKeyRing ring = keyOperations.modifySecretKeyRing(secRing, saveParcel, + passphrase, log, 0); setProgress(R.string.progress_saving_key_ring, 90, 100); providerHelper.saveSecretKeyRing(ring); } catch (ProviderHelper.NotFoundException e) { 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 0760aadf8..73f552c92 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -215,6 +215,24 @@ public class OperationResultParcel implements Parcelable { MSG_KC_UID_NO_CERT (R.string.msg_kc_uid_no_cert), MSG_KC_UID_REVOKE_DUP (R.string.msg_kc_uid_revoke_dup), MSG_KC_UID_REVOKE_OLD (R.string.msg_kc_uid_revoke_old), + + MSG_MR (R.string.msg_mr), + MSG_MR_ERROR_ENCODE (R.string.msg_mr_error_encode), + MSG_MR_ERROR_PGP (R.string.msg_mr_error_pgp), + MSG_MR_ERROR_SIG (R.string.msg_mr_error_sig), + MSG_MR_PASSPHRASE (R.string.msg_mr_passphrase), + MSG_MR_SUBKEY_CHANGE (R.string.msg_mr_subkey_change), + MSG_MR_SUBKEY_MISSING (R.string.msg_mr_subkey_missing), + MSG_MR_SUBKEY_NEW_ID (R.string.msg_mr_subkey_new_id), + MSG_MR_SUBKEY_NEW (R.string.msg_mr_subkey_new), + MSG_MR_SUBKEY_PAST_EXPIRY (R.string.msg_mr_subkey_past_expiry), + MSG_MR_SUBKEY_REVOKE (R.string.msg_mr_subkey_revoke), + MSG_MR_SUCCESS (R.string.msg_mr_success), + MSG_MR_UID_ADD (R.string.msg_mr_uid_add), + MSG_MR_UID_PRIMARY (R.string.msg_mr_uid_primary), + MSG_MR_UID_REVOKE (R.string.msg_mr_uid_revoke), + MSG_MR_UNLOCK_ERROR (R.string.msg_mr_unlock_error), + MSG_MR_UNLOCK (R.string.msg_mr_unlock), ; private final int mMsgId; @@ -264,6 +282,10 @@ public class OperationResultParcel implements Parcelable { add(new OperationResultParcel.LogEntryParcel(level, type, parameters, indent)); } + public void add(LogLevel level, LogType type, int indent) { + add(new OperationResultParcel.LogEntryParcel(level, type, null, indent)); + } + public boolean containsWarnings() { for(LogEntryParcel entry : new IterableIterator<LogEntryParcel>(iterator())) { if (entry.mLevel == LogLevel.WARN || entry.mLevel == LogLevel.ERROR) { diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index ed06f983b..bf3333458 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -600,6 +600,25 @@ <string name="msg_kc_uid_revoke_old">Removing outdated revocation certificate for user id "%s"</string> <string name="msg_kc_uid_no_cert">No valid self-certificate found for user id %s, removing from ring</string> + <!-- modifySecretKeyRing --> + <string name="msg_mr">Modifying keyring %s</string> + <string name="msg_mr_error_encode">Encoding exception!</string> + <string name="msg_mr_error_pgp">PGP internal exception!</string> + <string name="msg_mr_error_sig">Signature exception!</string> + <string name="msg_mr_passphrase">Changing passphrase</string> + <string name="msg_mr_subkey_change">Modifying subkey %s</string> + <string name="msg_mr_subkey_missing">Tried to operate on missing subkey %s!</string> + <string name="msg_mr_subkey_new">Generating new %1$s bit %2$s subkey</string> + <string name="msg_mr_subkey_new_id">New subkey id: %s</string> + <string name="msg_mr_subkey_past_expiry">Expiry date cannot be in the past!</string> + <string name="msg_mr_subkey_revoke">Revoking subkey %s</string> + <string name="msg_mr_success">Keyring successfully modified</string> + <string name="msg_mr_uid_add">Adding user id %s</string> + <string name="msg_mr_uid_primary">Changing primary uid to %s</string> + <string name="msg_mr_uid_revoke">Revoking user id %s</string> + <string name="msg_mr_unlock_error">Error unlocking keyring!</string> + <string name="msg_mr_unlock">Unlocking keyring</string> + <!-- unsorted --> <string name="section_certifier_id">Certifier</string> <string name="section_cert">Certificate Details</string> |