modify*Key: improve handling of passphrase modification (add tests, too)

2 files changed, 52 insertions, 2 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 222c47215..cdd2eacc0 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
@@ -698,7 +698,7 @@ public class PgpKeyOperation {
                     // Build key encrypter and decrypter based on passphrase
                     PBESecretKeyEncryptor keyEncryptor = new JcePBESecretKeyEncryptorBuilder(
                             PGPEncryptedData.CAST5, sha1Calc)
-                            .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build("".toCharArray());
+                            .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray());
 
                     sKey = new PGPSecretKey(keyPair.getPrivateKey(), pKey,
                             sha1Calc, false, keyEncryptor);
@@ -716,6 +716,8 @@ public class PgpKeyOperation {
             if (saveParcel.mNewPassphrase != null) {
                 progress(R.string.progress_modify_passphrase, 90);
                 log.add(LogLevel.INFO, LogType.MSG_MF_PASSPHRASE, indent);
+                indent += 1;
+
                 PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder().build()
                         .get(HashAlgorithmTags.SHA1);
                 PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
@@ -726,7 +728,51 @@ public class PgpKeyOperation {
                         .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
                                 saveParcel.mNewPassphrase.toCharArray());
 
-                sKR = PGPSecretKeyRing.copyWithNewPassword(sKR, keyDecryptor, keyEncryptorNew);
+                // noinspection unchecked
+                for (PGPSecretKey sKey : new IterableIterator<PGPSecretKey>(sKR.getSecretKeys())) {
+                    log.add(LogLevel.DEBUG, LogType.MSG_MF_PASSPHRASE_KEY, indent,
+                            PgpKeyHelper.convertKeyIdToHex(sKey.getKeyID()));
+
+                    boolean ok = false;
+
+                    try {
+                        // try to set new passphrase
+                        sKey = PGPSecretKey.copyWithNewPassword(sKey, keyDecryptor, keyEncryptorNew);
+                        ok = true;
+                    } catch (PGPException e) {
+
+                        // if this is the master key, error!
+                        if (sKey.getKeyID() == masterPublicKey.getKeyID()) {
+                            log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_PASSPHRASE_MASTER, indent+1);
+                            return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                        }
+
+                        // being in here means decrypt failed, likely due to a bad passphrase try
+                        // again with an empty passphrase, maybe we can salvage this
+                        try {
+                            log.add(LogLevel.DEBUG, LogType.MSG_MF_PASSPHRASE_EMPTY_RETRY, indent+1);
+                            PBESecretKeyDecryptor emptyDecryptor =
+                                new JcePBESecretKeyDecryptorBuilder().setProvider(
+                                    Constants.BOUNCY_CASTLE_PROVIDER_NAME).build("".toCharArray());
+                            sKey = PGPSecretKey.copyWithNewPassword(sKey, emptyDecryptor, keyEncryptorNew);
+                            ok = true;
+                        } catch (PGPException e2) {
+                            // non-fatal but not ok, handled below
+                        }
+                    }
+
+                    if (!ok) {
+                        // for a subkey, it's merely a warning
+                        log.add(LogLevel.WARN, LogType.MSG_MF_PASSPHRASE_FAIL, indent+1,
+                                PgpKeyHelper.convertKeyIdToHex(sKey.getKeyID()));
+                        continue;
+                    }
+
+                    sKR = PGPSecretKeyRing.insertSecretKey(sKR, sKey);
+
+                }
+
+                indent -= 1;
             }
 
         } catch (IOException 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 8e7dcd4aa..26fe63550 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
@@ -364,6 +364,7 @@ public class OperationResultParcel implements Parcelable {
         MSG_MF_ERROR_NOEXIST_PRIMARY (R.string.msg_mf_error_noexist_primary),
         MSG_MF_ERROR_NOEXIST_REVOKE (R.string.msg_mf_error_noexist_revoke),
         MSG_MF_ERROR_NULL_EXPIRY (R.string.msg_mf_error_null_expiry),
+        MSG_MF_ERROR_PASSPHRASE_MASTER(R.string.msg_mf_error_passphrase_master),
         MSG_MF_ERROR_PAST_EXPIRY(R.string.msg_mf_error_past_expiry),
         MSG_MF_ERROR_PGP (R.string.msg_mf_error_pgp),
         MSG_MF_ERROR_REVOKED_PRIMARY (R.string.msg_mf_error_revoked_primary),
@@ -371,6 +372,9 @@ public class OperationResultParcel implements Parcelable {
         MSG_MF_ERROR_SUBKEY_MISSING(R.string.msg_mf_error_subkey_missing),
         MSG_MF_MASTER (R.string.msg_mf_master),
         MSG_MF_PASSPHRASE (R.string.msg_mf_passphrase),
+        MSG_MF_PASSPHRASE_KEY (R.string.msg_mf_passphrase_key),
+        MSG_MF_PASSPHRASE_EMPTY_RETRY (R.string.msg_mf_passphrase_empty_retry),
+        MSG_MF_PASSPHRASE_FAIL (R.string.msg_mf_passphrase_fail),
         MSG_MF_PRIMARY_REPLACE_OLD (R.string.msg_mf_primary_replace_old),
         MSG_MF_PRIMARY_NEW (R.string.msg_mf_primary_new),
         MSG_MF_SUBKEY_CHANGE (R.string.msg_mf_subkey_change),