Merge branch 'development' of github.com:open-keychain/open-keychain into development

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpEncryptDecryptTest.java

@@ -68,7 +68,7 @@ public class PgpEncryptDecryptTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ELGAMAL, 1024, null, KeyFlags.ENCRYPT_COMMS, 0L));
             parcel.mAddUserIds.add("bloom");
-            parcel.mNewPassphrase = mKeyPhrase1;
+            parcel.mNewUnlock = mKeyPhrase1;
 
             EditKeyResult result = op.createSecretKeyRing(parcel);
             Assert.assertTrue("initial test key creation must succeed", result.success());
@@ -86,7 +86,7 @@ public class PgpEncryptDecryptTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ELGAMAL, 1024, null, KeyFlags.ENCRYPT_COMMS, 0L));
             parcel.mAddUserIds.add("belle");
-            parcel.mNewPassphrase = mKeyPhrase2;
+            parcel.mNewUnlock = mKeyPhrase2;
 
             EditKeyResult result = op.createSecretKeyRing(parcel);
             Assert.assertTrue("initial test key creation must succeed", result.success());

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java

@@ -91,7 +91,7 @@ public class PgpKeyOperationTest {
 
         parcel.mAddUserIds.add("twi");
         parcel.mAddUserIds.add("pink");
-        parcel.mNewPassphrase = passphrase;
+        parcel.mNewUnlock = passphrase;
         PgpKeyOperation op = new PgpKeyOperation(null);
 
         EditKeyResult result = op.createSecretKeyRing(parcel);
@@ -127,7 +127,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.RSA, new Random().nextInt(256)+255, null, KeyFlags.CERTIFY_OTHER, 0L));
             parcel.mAddUserIds.add("shy");
-            parcel.mNewPassphrase = passphrase;
+            parcel.mNewUnlock = passphrase;
 
             assertFailure("creating ring with < 512 bytes keysize should fail", parcel,
                     LogType.MSG_CR_ERROR_KEYSIZE_512);
@@ -138,7 +138,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.ELGAMAL, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
             parcel.mAddUserIds.add("shy");
-            parcel.mNewPassphrase = passphrase;
+            parcel.mNewUnlock = passphrase;
 
             assertFailure("creating ring with ElGamal master key should fail", parcel,
                     LogType.MSG_CR_ERROR_FLAGS_ELGAMAL);
@@ -149,7 +149,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.RSA, 1024, null, KeyFlags.CERTIFY_OTHER, null));
             parcel.mAddUserIds.add("lotus");
-            parcel.mNewPassphrase = passphrase;
+            parcel.mNewUnlock = passphrase;
 
             assertFailure("creating master key with null expiry should fail", parcel,
                     LogType.MSG_CR_ERROR_NULL_EXPIRY);
@@ -160,7 +160,7 @@ public class PgpKeyOperationTest {
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.RSA, 1024, null, KeyFlags.SIGN_DATA, 0L));
             parcel.mAddUserIds.add("shy");
-            parcel.mNewPassphrase = passphrase;
+            parcel.mNewUnlock = passphrase;
 
             assertFailure("creating ring with non-certifying master key should fail", parcel,
                     LogType.MSG_CR_ERROR_NO_CERTIFY);
@@ -170,7 +170,7 @@ public class PgpKeyOperationTest {
             parcel.reset();
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Algorithm.RSA, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
-            parcel.mNewPassphrase = passphrase;
+            parcel.mNewUnlock = passphrase;
 
             assertFailure("creating ring without user ids should fail", parcel,
                     LogType.MSG_CR_ERROR_NO_USER_ID);
@@ -179,7 +179,7 @@ public class PgpKeyOperationTest {
         {
             parcel.reset();
             parcel.mAddUserIds.add("shy");
-            parcel.mNewPassphrase = passphrase;
+            parcel.mNewUnlock = passphrase;
 
             assertFailure("creating ring with no master key should fail", parcel,
                     LogType.MSG_CR_ERROR_NO_MASTER);
@@ -910,7 +910,7 @@ public class PgpKeyOperationTest {
     public void testPassphraseChange() throws Exception {
 
         // change passphrase to empty
-        parcel.mNewPassphrase = "";
+        parcel.mNewUnlock = "";
         UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB);
 
         Assert.assertEquals("exactly three packets should have been modified (the secret keys)",
@@ -923,7 +923,7 @@ public class PgpKeyOperationTest {
 
         // modify keyring, change to non-empty passphrase
         String otherPassphrase = TestingUtils.genPassphrase(true);
-        parcel.mNewPassphrase = otherPassphrase;
+        parcel.mNewUnlock = otherPassphrase;
         modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, "");
 
         Assert.assertEquals("exactly three packets should have been modified (the secret keys)",
@@ -948,7 +948,7 @@ public class PgpKeyOperationTest {
                 PacketTags.SECRET_SUBKEY, sKeyNoPassphrase.tag);
 
         String otherPassphrase2 = TestingUtils.genPassphrase(true);
-        parcel.mNewPassphrase = otherPassphrase2;
+        parcel.mNewUnlock = otherPassphrase2;
         {
             // if we replace a secret key with one without passphrase
             modified = KeyringTestingHelper.removePacket(modified, sKeyNoPassphrase.position);

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java

@@ -104,7 +104,7 @@ public class UncachedKeyringCanonicalizeTest {
         parcel.mAddUserIds.add("twi");
         parcel.mAddUserIds.add("pink");
         // passphrase is tested in PgpKeyOperationTest, just use empty here
-        parcel.mNewPassphrase = "";
+        parcel.mNewUnlock = "";
         PgpKeyOperation op = new PgpKeyOperation(null);
 
         EditKeyResult result = op.createSecretKeyRing(parcel);

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringMergeTest.java

@@ -96,7 +96,7 @@ public class UncachedKeyringMergeTest {
             parcel.mAddUserIds.add("twi");
             parcel.mAddUserIds.add("pink");
             // passphrase is tested in PgpKeyOperationTest, just use empty here
-            parcel.mNewPassphrase = "";
+            parcel.mNewUnlock = "";
             PgpKeyOperation op = new PgpKeyOperation(null);
 
             OperationResult.OperationLog log = new OperationResult.OperationLog();
@@ -112,7 +112,7 @@ public class UncachedKeyringMergeTest {
 
             parcel.mAddUserIds.add("shy");
             // passphrase is tested in PgpKeyOperationTest, just use empty here
-            parcel.mNewPassphrase = "";
+            parcel.mNewUnlock = "";
             PgpKeyOperation op = new PgpKeyOperation(null);
 
             OperationResult.OperationLog log = new OperationResult.OperationLog();

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java

@@ -57,7 +57,7 @@ public class UncachedKeyringTest {
         parcel.mAddUserIds.add("twi");
         parcel.mAddUserIds.add("pink");
         // passphrase is tested in PgpKeyOperationTest, just use empty here
-        parcel.mNewPassphrase = "";
+        parcel.mNewUnlock = "";
         PgpKeyOperation op = new PgpKeyOperation(null);
 
         EditKeyResult result = op.createSecretKeyRing(parcel);

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java

@@ -52,6 +52,7 @@ import org.sufficientlysecure.keychain.operations.results.OperationResult.Operat
 import org.sufficientlysecure.keychain.operations.results.EditKeyResult;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Curve;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd;
 import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils;
@@ -871,11 +872,63 @@ public class PgpKeyOperation {
             }
 
             // 6. If requested, change passphrase
-            if (saveParcel.mNewPassphrase != null) {
+            if (saveParcel.mNewUnlock != null) {
                 progress(R.string.progress_modify_passphrase, 90);
                 log.add(LogType.MSG_MF_PASSPHRASE, indent);
                 indent += 1;
 
+                sKR = applyNewUnlock(sKR, masterPublicKey, passphrase, saveParcel.mNewUnlock, log, indent);
+                if (sKR == null) {
+                    // The error has been logged above, just return a bad state
+                    return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                }
+
+                indent -= 1;
+            }
+
+        } catch (IOException e) {
+            Log.e(Constants.TAG, "encountered IOException while modifying key", e);
+            log.add(LogType.MSG_MF_ERROR_ENCODE, indent+1);
+            return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+        } catch (PGPException e) {
+            Log.e(Constants.TAG, "encountered pgp error while modifying key", e);
+            log.add(LogType.MSG_MF_ERROR_PGP, indent+1);
+            return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+        } catch (SignatureException e) {
+            Log.e(Constants.TAG, "encountered SignatureException while modifying key", e);
+            log.add(LogType.MSG_MF_ERROR_SIG, indent+1);
+            return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+        }
+
+        progress(R.string.progress_done, 100);
+        log.add(LogType.MSG_MF_SUCCESS, indent);
+        return new EditKeyResult(OperationResult.RESULT_OK, log, new UncachedKeyRing(sKR));
+
+    }
+
+    private static PGPSecretKeyRing applyNewUnlock(
+            PGPSecretKeyRing sKR,
+            PGPPublicKey masterPublicKey,
+            String passphrase,
+            ChangeUnlockParcel newUnlock,
+            OperationLog log, int indent) throws PGPException {
+
+        if (newUnlock.mNewPassphrase != null) {
+            return applyNewPassphrase(sKR, masterPublicKey, passphrase, newUnlock.mNewPassphrase, log, indent);
+        }
+
+        throw new UnsupportedOperationException("PIN passphrases not yet implemented!");
+
+    }
+
+
+    private static PGPSecretKeyRing applyNewPassphrase(
+            PGPSecretKeyRing sKR,
+            PGPPublicKey masterPublicKey,
+            String passphrase,
+            String newPassphrase,
+            OperationLog log, int indent) throws PGPException {
+
         PGPDigestCalculator encryptorHashCalc = new JcaPGPDigestCalculatorProviderBuilder().build()
                 .get(SECRET_KEY_ENCRYPTOR_HASH_ALGO);
         PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
@@ -884,7 +937,7 @@ public class PgpKeyOperation {
         PBESecretKeyEncryptor keyEncryptorNew = new JcePBESecretKeyEncryptorBuilder(
                 SECRET_KEY_ENCRYPTOR_SYMMETRIC_ALGO, encryptorHashCalc, SECRET_KEY_ENCRYPTOR_S2K_COUNT)
                 .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
-                                saveParcel.mNewPassphrase.toCharArray());
+                        newPassphrase.toCharArray());
 
         // noinspection unchecked
         for (PGPSecretKey sKey : new IterableIterator<PGPSecretKey>(sKR.getSecretKeys())) {
@@ -902,7 +955,7 @@ public class PgpKeyOperation {
                 // if this is the master key, error!
                 if (sKey.getKeyID() == masterPublicKey.getKeyID()) {
                     log.add(LogType.MSG_MF_ERROR_PASSPHRASE_MASTER, indent+1);
-                            return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                    return null;
                 }
 
                 // being in here means decrypt failed, likely due to a bad passphrase try
@@ -930,26 +983,7 @@ public class PgpKeyOperation {
 
         }
 
-                indent -= 1;
+        return sKR;
-            }
-
-        } catch (IOException e) {
-            Log.e(Constants.TAG, "encountered IOException while modifying key", e);
-            log.add(LogType.MSG_MF_ERROR_ENCODE, indent+1);
-            return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
-        } catch (PGPException e) {
-            Log.e(Constants.TAG, "encountered pgp error while modifying key", e);
-            log.add(LogType.MSG_MF_ERROR_PGP, indent+1);
-            return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
-        } catch (SignatureException e) {
-            Log.e(Constants.TAG, "encountered SignatureException while modifying key", e);
-            log.add(LogType.MSG_MF_ERROR_SIG, indent+1);
-            return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
-        }
-
-        progress(R.string.progress_done, 100);
-        log.add(LogType.MSG_MF_SUCCESS, indent);
-        return new EditKeyResult(OperationResult.RESULT_OK, log, new UncachedKeyRing(sKR));
 
     }
 

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/KeychainIntentService.java

@@ -399,9 +399,14 @@ public class KeychainIntentService extends IntentService implements Progressable
                 }
 
                 // cache new passphrase
-                if (saveParcel.mNewPassphrase != null) {
+                if (saveParcel.mNewUnlock != null) {
-                    PassphraseCacheService.addCachedPassphrase(this, ring.getMasterKeyId(), ring.getMasterKeyId(),
+                    PassphraseCacheService.addCachedPassphrase(this,
-                            saveParcel.mNewPassphrase, ring.getPublicKey().getPrimaryUserIdWithFallback());
+                            ring.getMasterKeyId(),
+                            ring.getMasterKeyId(),
+                            saveParcel.mNewUnlock.mNewPassphrase != null
+                                    ? saveParcel.mNewUnlock.mNewPassphrase
+                                    : saveParcel.mNewUnlock.mNewPin,
+                            ring.getPublicKey().getPrimaryUserIdWithFallback());
                 }
 
                 setProgress(R.string.progress_done, 100, 100);

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java

@@ -46,7 +46,7 @@ public class SaveKeyringParcel implements Parcelable {
     // the key fingerprint, for safety. MUST be null for a new key.
     public byte[] mFingerprint;
 
-    public String mNewPassphrase;
+    public ChangeUnlockParcel mNewUnlock;
 
     public ArrayList<String> mAddUserIds;
     public ArrayList<SubkeyAdd> mAddSubKeys;
@@ -69,7 +69,7 @@ public class SaveKeyringParcel implements Parcelable {
     }
 
     public void reset() {
-        mNewPassphrase = null;
+        mNewUnlock = null;
         mAddUserIds = new ArrayList<String>();
         mAddSubKeys = new ArrayList<SubkeyAdd>();
         mChangePrimaryUserId = null;
@@ -159,7 +159,7 @@ public class SaveKeyringParcel implements Parcelable {
         mMasterKeyId = source.readInt() != 0 ? source.readLong() : null;
         mFingerprint = source.createByteArray();
 
-        mNewPassphrase = source.readString();
+        mNewUnlock = source.readParcelable(getClass().getClassLoader());
 
         mAddUserIds = source.createStringArrayList();
         mAddSubKeys = (ArrayList<SubkeyAdd>) source.readSerializable();
@@ -180,7 +180,8 @@ public class SaveKeyringParcel implements Parcelable {
         }
         destination.writeByteArray(mFingerprint);
 
-        destination.writeString(mNewPassphrase);
+        // yes, null values are ok for parcelables
+        destination.writeParcelable(mNewUnlock, 0);
 
         destination.writeStringList(mAddUserIds);
         destination.writeSerializable(mAddSubKeys);
@@ -211,7 +212,7 @@ public class SaveKeyringParcel implements Parcelable {
     @Override
     public String toString() {
         String out = "mMasterKeyId: " + mMasterKeyId + "\n";
-        out += "mNewPassphrase: " + mNewPassphrase + "\n";
+        out += "mNewUnlock: " + mNewUnlock + "\n";
         out += "mAddUserIds: " + mAddUserIds + "\n";
         out += "mAddSubKeys: " + mAddSubKeys + "\n";
         out += "mChangeSubKeys: " + mChangeSubKeys + "\n";
@@ -238,4 +239,64 @@ public class SaveKeyringParcel implements Parcelable {
         // BRAINPOOL_P256, BRAINPOOL_P384, BRAINPOOL_P512
     }
 
+    /** This subclass contains information on how the passphrase should be changed.
+     *
+     * If no changes are to be made, this class should NOT be used!
+     *
+     * At this point, there must be *exactly one* non-null value here, which specifies the type
+     * of unlocking mechanism to use.
+     *
+     */
+    public static class ChangeUnlockParcel implements Parcelable {
+
+        // The new passphrase to use
+        public final String mNewPassphrase;
+        // A new pin to use. Must only contain [0-9]+
+        public final String mNewPin;
+
+        public ChangeUnlockParcel(String newPassphrase, String newPin) {
+            if (newPassphrase == null && newPin == null) {
+                throw new RuntimeException("Cannot set both passphrase and pin. THIS IS A BUG!");
+            }
+            if (newPin != null && !newPin.matches("[0-9]+")) {
+                throw new RuntimeException("Pin must be numeric digits only. THIS IS A BUG!");
+            }
+            mNewPassphrase = newPassphrase;
+            mNewPin = newPin;
+        }
+
+        public ChangeUnlockParcel(Parcel source) {
+            mNewPassphrase = source.readString();
+            mNewPin = source.readString();
+        }
+
+        @Override
+        public void writeToParcel(Parcel destination, int flags) {
+            destination.writeString(mNewPassphrase);
+            destination.writeString(mNewPin);
+        }
+
+        @Override
+        public int describeContents() {
+            return 0;
+        }
+
+        public static final Creator<ChangeUnlockParcel> CREATOR = new Creator<ChangeUnlockParcel>() {
+            public ChangeUnlockParcel createFromParcel(final Parcel source) {
+                return new ChangeUnlockParcel(source);
+            }
+
+            public ChangeUnlockParcel[] newArray(final int size) {
+                return new ChangeUnlockParcel[size];
+            }
+        };
+
+        public String toString() {
+            return mNewPassphrase != null
+                    ? ("passphrase (" + mNewPassphrase + ")")
+                    : ("pin (" + mNewPin + ")");
+        }
+
+    }
+
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java

@@ -34,6 +34,7 @@ import android.widget.TextView;
 import org.spongycastle.bcpg.sig.KeyFlags;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.R;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
 import org.sufficientlysecure.keychain.util.Preferences;
 import org.sufficientlysecure.keychain.pgp.KeyRing;
 import org.sufficientlysecure.keychain.provider.KeychainContract;
@@ -165,7 +166,9 @@ public class CreateKeyFinalFragment extends Fragment {
             String userId = KeyRing.createUserId(mName, mEmail, null);
             mSaveKeyringParcel.mAddUserIds.add(userId);
             mSaveKeyringParcel.mChangePrimaryUserId = userId;
-            mSaveKeyringParcel.mNewPassphrase = mPassphrase;
+            mSaveKeyringParcel.mNewUnlock = mPassphrase != null
+                    ? new ChangeUnlockParcel(mPassphrase, null)
+                    : null;
         }
     }
 

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java

@@ -53,6 +53,7 @@ import org.sufficientlysecure.keychain.service.KeychainIntentService;
 import org.sufficientlysecure.keychain.service.KeychainIntentServiceHandler;
 import org.sufficientlysecure.keychain.service.PassphraseCacheService;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.ChangeUnlockParcel;
 import org.sufficientlysecure.keychain.ui.adapter.SubkeysAdapter;
 import org.sufficientlysecure.keychain.ui.adapter.SubkeysAddedAdapter;
 import org.sufficientlysecure.keychain.ui.adapter.UserIdsAdapter;
@@ -189,7 +190,9 @@ public class EditKeyFragment extends LoaderFragment implements
     private void loadSaveKeyringParcel(SaveKeyringParcel saveKeyringParcel) {
         mSaveKeyringParcel = saveKeyringParcel;
         mPrimaryUserId = saveKeyringParcel.mChangePrimaryUserId;
-        mCurrentPassphrase = saveKeyringParcel.mNewPassphrase;
+        if (saveKeyringParcel.mNewUnlock != null) {
+            mCurrentPassphrase = saveKeyringParcel.mNewUnlock.mNewPassphrase;
+        }
 
         mUserIdsAddedAdapter = new UserIdsAddedAdapter(getActivity(), mSaveKeyringParcel.mAddUserIds, true);
         mUserIdsAddedList.setAdapter(mUserIdsAddedAdapter);
@@ -387,8 +390,10 @@ public class EditKeyFragment extends LoaderFragment implements
                     Bundle data = message.getData();
 
                     // cache new returned passphrase!
-                    mSaveKeyringParcel.mNewPassphrase = data
+                    mSaveKeyringParcel.mNewUnlock = new ChangeUnlockParcel(
-                            .getString(SetPassphraseDialogFragment.MESSAGE_NEW_PASSPHRASE);
+                        data.getString(SetPassphraseDialogFragment.MESSAGE_NEW_PASSPHRASE),
+                        null
+                    );
                 }
             }
         };