From 1c1ae769ef78a9bd3c95f97b95215616ee9b15db Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 1 Jan 2015 21:13:07 +0100 Subject: [PATCH] small improvements regarding pin tests and logging --- .../keychain/pgp/PgpKeyOperationTest.java | 28 +++++++++++++++---- .../operations/results/OperationResult.java | 2 ++ .../keychain/pgp/PgpKeyOperation.java | 5 ++++ OpenKeychain/src/main/res/values/strings.xml | 4 ++- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java index 6b53a93ce..103e2dc88 100644 --- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java +++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java @@ -916,7 +916,7 @@ public class PgpKeyOperationTest { parcel.mNewUnlock = new ChangeUnlockParcel(""); // note that canonicalization here necessarily strips the empty notation packet UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, - passphrase, true, false); + passphrase); Assert.assertEquals("exactly three packets should have been modified (the secret keys)", 3, onlyB.size()); @@ -929,7 +929,7 @@ public class PgpKeyOperationTest { // modify keyring, change to non-empty passphrase String otherPassphrase = TestingUtils.genPassphrase(true); parcel.mNewUnlock = new ChangeUnlockParcel(otherPassphrase); - modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, "", true, false); + modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, ""); Assert.assertEquals("exactly three packets should have been modified (the secret keys)", 3, onlyB.size()); @@ -989,11 +989,15 @@ public class PgpKeyOperationTest { @Test public void testUnlockPin() throws Exception { + String pin = "5235125"; + // change passphrase to a pin type - parcel.mNewUnlock = new ChangeUnlockParcel(null, "52351"); + parcel.mNewUnlock = new ChangeUnlockParcel(null, pin); UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB); - Assert.assertEquals("exactly four packets should have been modified (the secret keys + notation packet)", + Assert.assertEquals("exactly three packets should have been added (the secret keys + notation packet)", + 3, onlyA.size()); + Assert.assertEquals("exactly four packets should have been added (the secret keys + notation packet)", 4, onlyB.size()); RawPacket dkSig = onlyB.get(1); @@ -1001,11 +1005,25 @@ public class PgpKeyOperationTest { PacketTags.SIGNATURE, dkSig.tag); // check that notation data contains pin - CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(modified.getEncoded(), false, 0); + CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing( + modified.getEncoded(), false, 0); Assert.assertEquals("secret key type should be 'pin' after this", SecretKeyType.PIN, secretRing.getSecretKey().getSecretKeyType()); + // need to sleep for a sec, so the timestamp changes for notation data + Thread.sleep(1000); + + { + parcel.mNewUnlock = new ChangeUnlockParcel("phrayse", null); + applyModificationWithChecks(parcel, modified, onlyA, onlyB, pin, true, false); + + Assert.assertEquals("exactly four packets should have been removed (the secret keys + notation packet)", + 4, onlyA.size()); + Assert.assertEquals("exactly three packets should have been added (no more notation packet)", + 3, onlyB.size()); + } + } private static UncachedKeyRing applyModificationWithChecks(SaveKeyringParcel parcel, diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java index 606dd49d5..426b0827e 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java @@ -461,6 +461,8 @@ public abstract class OperationResult implements Parcelable { MSG_MF_ERROR_SIG (LogLevel.ERROR, R.string.msg_mf_error_sig), MSG_MF_ERROR_SUBKEY_MISSING(LogLevel.ERROR, R.string.msg_mf_error_subkey_missing), MSG_MF_MASTER (LogLevel.DEBUG, R.string.msg_mf_master), + MSG_MF_NOTATION_PIN (LogLevel.DEBUG, R.string.msg_mf_notation_pin), + MSG_MF_NOTATION_EMPTY (LogLevel.DEBUG, R.string.msg_mf_notation_empty), MSG_MF_PASSPHRASE (LogLevel.INFO, R.string.msg_mf_passphrase), MSG_MF_PASSPHRASE_KEY (LogLevel.DEBUG, R.string.msg_mf_passphrase_key), MSG_MF_PASSPHRASE_EMPTY_RETRY (LogLevel.DEBUG, R.string.msg_mf_passphrase_empty_retry), 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 e39bde6b0..5ac5f7a9a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -921,6 +921,9 @@ public class PgpKeyOperation { // if there is any old packet with notation data if (hasNotationData(sKR)) { + + log.add(LogType.MSG_MF_NOTATION_EMPTY, indent); + // add packet with EMPTY notation data (updates old one, but will be stripped later) PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder( masterPrivateKey.getPublicKeyPacket().getAlgorithm(), HashAlgorithmTags.SHA512) @@ -945,6 +948,8 @@ public class PgpKeyOperation { if (newUnlock.mNewPin != null) { sKR = applyNewPassphrase(sKR, masterPublicKey, passphrase, newUnlock.mNewPin, log, indent); + log.add(LogType.MSG_MF_NOTATION_PIN, indent); + // add packet with "pin" notation data PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder( masterPrivateKey.getPublicKeyPacket().getAlgorithm(), HashAlgorithmTags.SHA512) diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 05ea2a99a..be409078a 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -803,7 +803,9 @@ "Internal PGP error!" "Signature exception!" "Modifying master certifications" - "Changing passphrase for keyring…" + "Adding empty notation packet" + "Adding PIN notation packet" + "Changing passphrase for keyring" "Re-encrypting subkey %s with new passphrase" "Setting new passphrase failed, trying again with empty old passphrase" "Passphrase for subkey could not be changed! (Does it have a different one from the other keys?)"