fix unit tests (for real)

This commit is contained in:
Vincent Breitmoser 2015-03-20 18:45:00 +01:00
parent e00ce86de9
commit 3e51da3afa
7 changed files with 39 additions and 20 deletions

View File

@ -75,7 +75,6 @@ public class PgpKeyOperationTest {
static UncachedKeyRing staticRing; static UncachedKeyRing staticRing;
final static Passphrase passphrase = TestingUtils.genPassphrase(); final static Passphrase passphrase = TestingUtils.genPassphrase();
final static CryptoInputParcel cryptoInput = new CryptoInputParcel(new Date(), passphrase);
UncachedKeyRing ring; UncachedKeyRing ring;
PgpKeyOperation op; PgpKeyOperation op;
@ -83,6 +82,8 @@ public class PgpKeyOperationTest {
ArrayList<RawPacket> onlyA = new ArrayList<RawPacket>(); ArrayList<RawPacket> onlyA = new ArrayList<RawPacket>();
ArrayList<RawPacket> onlyB = new ArrayList<RawPacket>(); ArrayList<RawPacket> onlyB = new ArrayList<RawPacket>();
static CryptoInputParcel cryptoInput;
@BeforeClass @BeforeClass
public static void setUpOnce() throws Exception { public static void setUpOnce() throws Exception {
Security.insertProviderAt(new BouncyCastleProvider(), 1); Security.insertProviderAt(new BouncyCastleProvider(), 1);
@ -110,6 +111,9 @@ public class PgpKeyOperationTest {
// we sleep here for a second, to make sure all new certificates have different timestamps // we sleep here for a second, to make sure all new certificates have different timestamps
Thread.sleep(1000); Thread.sleep(1000);
cryptoInput = new CryptoInputParcel(new Date(), passphrase);
} }
@Before public void setUp() throws Exception { @Before public void setUp() throws Exception {
@ -302,6 +306,7 @@ public class PgpKeyOperationTest {
if (badphrase.equals(passphrase)) { if (badphrase.equals(passphrase)) {
badphrase = new Passphrase("a"); badphrase = new Passphrase("a");
} }
parcel.mAddUserIds.add("allure");
assertModifyFailure("keyring modification with bad passphrase should fail", assertModifyFailure("keyring modification with bad passphrase should fail",
ring, parcel, new CryptoInputParcel(badphrase), LogType.MSG_MF_UNLOCK_ERROR); ring, parcel, new CryptoInputParcel(badphrase), LogType.MSG_MF_UNLOCK_ERROR);
@ -659,7 +664,8 @@ public class PgpKeyOperationTest {
parcel.reset(); parcel.reset();
parcel.mRevokeSubKeys.add(keyId); parcel.mRevokeSubKeys.add(keyId);
modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB); modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB,
new CryptoInputParcel(new Date(), passphrase));
Assert.assertEquals("no extra packets in original", 0, onlyA.size()); Assert.assertEquals("no extra packets in original", 0, onlyA.size());
Assert.assertEquals("exactly one extra packet in modified", 1, onlyB.size()); Assert.assertEquals("exactly one extra packet in modified", 1, onlyB.size());
@ -779,7 +785,7 @@ public class PgpKeyOperationTest {
{ // we should be able to change the stripped/divert status of subkeys without passphrase { // we should be able to change the stripped/divert status of subkeys without passphrase
parcel.reset(); parcel.reset();
parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, null)); parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true, null));
modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, null); modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, new CryptoInputParcel());
Assert.assertEquals("one extra packet in modified", 1, onlyB.size()); Assert.assertEquals("one extra packet in modified", 1, onlyB.size());
Packet p = new BCPGInputStream(new ByteArrayInputStream(onlyB.get(0).buf)).readPacket(); Packet p = new BCPGInputStream(new ByteArrayInputStream(onlyB.get(0).buf)).readPacket();
Assert.assertEquals("new packet should have GNU_DUMMY S2K type", Assert.assertEquals("new packet should have GNU_DUMMY S2K type",
@ -795,7 +801,7 @@ public class PgpKeyOperationTest {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
}; };
parcel.mChangeSubKeys.add(new SubkeyChange(keyId, false, serial)); parcel.mChangeSubKeys.add(new SubkeyChange(keyId, false, serial));
modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, null); modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB, new CryptoInputParcel());
Assert.assertEquals("one extra packet in modified", 1, onlyB.size()); Assert.assertEquals("one extra packet in modified", 1, onlyB.size());
Packet p = new BCPGInputStream(new ByteArrayInputStream(onlyB.get(0).buf)).readPacket(); Packet p = new BCPGInputStream(new ByteArrayInputStream(onlyB.get(0).buf)).readPacket();
Assert.assertEquals("new packet should have GNU_DUMMY S2K type", Assert.assertEquals("new packet should have GNU_DUMMY S2K type",
@ -972,12 +978,12 @@ public class PgpKeyOperationTest {
Assert.assertEquals("signature type must be positive certification", Assert.assertEquals("signature type must be positive certification",
PGPSignature.POSITIVE_CERTIFICATION, ((SignaturePacket) p).getSignatureType()); PGPSignature.POSITIVE_CERTIFICATION, ((SignaturePacket) p).getSignatureType());
// make sure packets can be distinguished by timestamp
Thread.sleep(1000); Thread.sleep(1000);
// applying the same modification AGAIN should not add more certifications but drop those // applying the same modification AGAIN should not add more certifications but drop those
// as duplicates // as duplicates
modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, cryptoInput, true, false); modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB,
new CryptoInputParcel(new Date(), passphrase), true, false);
Assert.assertEquals("duplicate modification: one extra packet in original", 1, onlyA.size()); Assert.assertEquals("duplicate modification: one extra packet in original", 1, onlyA.size());
Assert.assertEquals("duplicate modification: one extra packet in modified", 1, onlyB.size()); Assert.assertEquals("duplicate modification: one extra packet in modified", 1, onlyB.size());
@ -1055,7 +1061,8 @@ public class PgpKeyOperationTest {
Passphrase otherPassphrase = TestingUtils.genPassphrase(true); Passphrase otherPassphrase = TestingUtils.genPassphrase(true);
CryptoInputParcel otherCryptoInput = new CryptoInputParcel(otherPassphrase); CryptoInputParcel otherCryptoInput = new CryptoInputParcel(otherPassphrase);
parcel.mNewUnlock = new ChangeUnlockParcel(otherPassphrase); parcel.mNewUnlock = new ChangeUnlockParcel(otherPassphrase);
modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB, new CryptoInputParcel(new Date())); modified = applyModificationWithChecks(parcel, modified, onlyA, onlyB,
new CryptoInputParcel(new Date(), new Passphrase()));
Assert.assertEquals("exactly three packets should have been modified (the secret keys)", Assert.assertEquals("exactly three packets should have been modified (the secret keys)",
3, onlyB.size()); 3, onlyB.size());

View File

@ -190,11 +190,11 @@ public class UncachedKeyringMergeTest {
parcel.reset(); parcel.reset();
parcel.mAddUserIds.add("flim"); parcel.mAddUserIds.add("flim");
modifiedA = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing(); modifiedA = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
parcel.reset(); parcel.reset();
parcel.mAddUserIds.add("flam"); parcel.mAddUserIds.add("flam");
modifiedB = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing(); modifiedB = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
} }
{ // merge A into base { // merge A into base
@ -231,8 +231,8 @@ public class UncachedKeyringMergeTest {
parcel.reset(); parcel.reset();
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd( parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.RSA, 1024, null, KeyFlags.SIGN_DATA, 0L)); Algorithm.RSA, 1024, null, KeyFlags.SIGN_DATA, 0L));
modifiedA = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing(); modifiedA = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
modifiedB = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing(); modifiedB = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
subKeyIdA = KeyringTestingHelper.getSubkeyId(modifiedA, 2); subKeyIdA = KeyringTestingHelper.getSubkeyId(modifiedA, 2);
subKeyIdB = KeyringTestingHelper.getSubkeyId(modifiedB, 2); subKeyIdB = KeyringTestingHelper.getSubkeyId(modifiedB, 2);
@ -273,7 +273,7 @@ public class UncachedKeyringMergeTest {
parcel.mRevokeSubKeys.add(KeyringTestingHelper.getSubkeyId(ringA, 1)); parcel.mRevokeSubKeys.add(KeyringTestingHelper.getSubkeyId(ringA, 1));
CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing( CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(
ringA.getEncoded(), false, 0); ringA.getEncoded(), false, 0);
modified = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing(); modified = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
} }
{ {
@ -372,7 +372,7 @@ public class UncachedKeyringMergeTest {
CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing( CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(
ringA.getEncoded(), false, 0); ringA.getEncoded(), false, 0);
modified = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(), parcel).getRing(); modified = op.modifySecretKeyRing(secretRing, new CryptoInputParcel(new Passphrase()), parcel).getRing();
} }
{ {

View File

@ -530,7 +530,6 @@ public abstract class OperationResult implements Parcelable {
MSG_MF_ERROR_REVOKED_PRIMARY (LogLevel.ERROR, R.string.msg_mf_error_revoked_primary), MSG_MF_ERROR_REVOKED_PRIMARY (LogLevel.ERROR, R.string.msg_mf_error_revoked_primary),
MSG_MF_ERROR_SIG (LogLevel.ERROR, R.string.msg_mf_error_sig), 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_ERROR_SUBKEY_MISSING(LogLevel.ERROR, R.string.msg_mf_error_subkey_missing),
MSG_MF_INPUT_REQUIRED (LogLevel.OK, R.string.msg_mf_input_required),
MSG_MF_MASTER (LogLevel.DEBUG, R.string.msg_mf_master), 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_PIN (LogLevel.DEBUG, R.string.msg_mf_notation_pin),
MSG_MF_NOTATION_EMPTY (LogLevel.DEBUG, R.string.msg_mf_notation_empty), MSG_MF_NOTATION_EMPTY (LogLevel.DEBUG, R.string.msg_mf_notation_empty),
@ -540,6 +539,9 @@ public abstract class OperationResult implements Parcelable {
MSG_MF_PASSPHRASE_FAIL (LogLevel.WARN, R.string.msg_mf_passphrase_fail), MSG_MF_PASSPHRASE_FAIL (LogLevel.WARN, R.string.msg_mf_passphrase_fail),
MSG_MF_PRIMARY_REPLACE_OLD (LogLevel.DEBUG, R.string.msg_mf_primary_replace_old), MSG_MF_PRIMARY_REPLACE_OLD (LogLevel.DEBUG, R.string.msg_mf_primary_replace_old),
MSG_MF_PRIMARY_NEW (LogLevel.DEBUG, R.string.msg_mf_primary_new), MSG_MF_PRIMARY_NEW (LogLevel.DEBUG, R.string.msg_mf_primary_new),
MSG_MF_RESTRICTED_MODE (LogLevel.INFO, R.string.msg_mf_restricted_mode),
MSG_MF_REQUIRE_DIVERT (LogLevel.OK, R.string.msg_mf_require_divert),
MSG_MF_REQUIRE_PASSPHRASE (LogLevel.OK, R.string.msg_mf_require_passphrase),
MSG_MF_SUBKEY_CHANGE (LogLevel.INFO, R.string.msg_mf_subkey_change), MSG_MF_SUBKEY_CHANGE (LogLevel.INFO, R.string.msg_mf_subkey_change),
MSG_MF_SUBKEY_NEW_ID (LogLevel.DEBUG, R.string.msg_mf_subkey_new_id), MSG_MF_SUBKEY_NEW_ID (LogLevel.DEBUG, R.string.msg_mf_subkey_new_id),
MSG_MF_SUBKEY_NEW (LogLevel.INFO, R.string.msg_mf_subkey_new), MSG_MF_SUBKEY_NEW (LogLevel.INFO, R.string.msg_mf_subkey_new),

View File

@ -395,12 +395,14 @@ public class PgpKeyOperation {
return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
} }
if (saveParcel.isRestrictedOnly()) { if (isDummy(masterSecretKey) || saveParcel.isRestrictedOnly()) {
log.add(LogType.MSG_MF_RESTRICTED_MODE, indent);
return internalRestricted(sKR, saveParcel, log); return internalRestricted(sKR, saveParcel, log);
} }
// Do we require a passphrase? If so, pass it along // Do we require a passphrase? If so, pass it along
if (!isDivertToCard(masterSecretKey) && !cryptoInput.hasPassphrase()) { if (!isDivertToCard(masterSecretKey) && !cryptoInput.hasPassphrase()) {
log.add(LogType.MSG_MF_REQUIRE_PASSPHRASE, indent);
return new PgpEditKeyResult(log, RequiredInputParcel.createRequiredPassphrase( return new PgpEditKeyResult(log, RequiredInputParcel.createRequiredPassphrase(
masterSecretKey.getKeyID(), cryptoInput.getSignatureTime())); masterSecretKey.getKeyID(), cryptoInput.getSignatureTime()));
} }
@ -971,7 +973,7 @@ public class PgpKeyOperation {
progress(R.string.progress_done, 100); progress(R.string.progress_done, 100);
if (!nfcSignOps.isEmpty()) { if (!nfcSignOps.isEmpty()) {
log.add(LogType.MSG_MF_INPUT_REQUIRED, indent); log.add(LogType.MSG_MF_REQUIRE_DIVERT, indent);
return new PgpEditKeyResult(log, nfcSignOps.build()); return new PgpEditKeyResult(log, nfcSignOps.build());
} }
@ -1459,6 +1461,12 @@ public class PgpKeyOperation {
return flags; return flags;
} }
private static boolean isDummy(PGPSecretKey secretKey) {
S2K s2k = secretKey.getS2K();
return s2k.getType() == S2K.GNU_DUMMY_S2K
&& s2k.getProtectionMode() == S2K.GNU_PROTECTION_MODE_NO_PRIVATE_KEY;
}
private static boolean isDivertToCard(PGPSecretKey secretKey) { private static boolean isDivertToCard(PGPSecretKey secretKey) {
S2K s2k = secretKey.getS2K(); S2K s2k = secretKey.getS2K();
return s2k.getType() == S2K.GNU_DUMMY_S2K return s2k.getType() == S2K.GNU_DUMMY_S2K

View File

@ -85,7 +85,7 @@ public class SaveKeyringParcel implements Parcelable {
/** Returns true iff this parcel does not contain any operations which require a passphrase. */ /** Returns true iff this parcel does not contain any operations which require a passphrase. */
public boolean isRestrictedOnly() { public boolean isRestrictedOnly() {
if (mNewUnlock != null || !mAddUserIds.isEmpty() || !mAddUserAttribute.isEmpty() if (mNewUnlock != null || !mAddUserIds.isEmpty() || !mAddUserAttribute.isEmpty()
|| !mAddSubKeys.isEmpty() || mChangePrimaryUserId != null || !mRevokeSubKeys .isEmpty() || !mAddSubKeys.isEmpty() || mChangePrimaryUserId != null || !mRevokeUserIds.isEmpty()
|| !mRevokeSubKeys.isEmpty()) { || !mRevokeSubKeys.isEmpty()) {
return false; return false;
} }

View File

@ -36,7 +36,7 @@ public class CryptoInputParcel implements Parcelable {
public CryptoInputParcel(Passphrase passphrase) { public CryptoInputParcel(Passphrase passphrase) {
mSignatureTime = new Date(); mSignatureTime = new Date();
mPassphrase = null; mPassphrase = passphrase;
} }
public CryptoInputParcel(Date signatureTime) { public CryptoInputParcel(Date signatureTime) {

View File

@ -913,7 +913,7 @@
<string name="msg_mf_error_passphrase_master">"Fatal error decrypting master key! This is likely a programming error, please file a bug report!"</string> <string name="msg_mf_error_passphrase_master">"Fatal error decrypting master key! This is likely a programming error, please file a bug report!"</string>
<string name="msg_mf_error_pgp">"Internal OpenPGP error!"</string> <string name="msg_mf_error_pgp">"Internal OpenPGP error!"</string>
<string name="msg_mf_error_sig">"Signature exception!"</string> <string name="msg_mf_error_sig">"Signature exception!"</string>
<string name="msg_mf_input_required">"Diverting to card/nfc for crypto operations"</string> <string name="msg_mf_error_subkey_missing">"Tried to operate on missing subkey %s!"</string>
<string name="msg_mf_master">"Modifying master certifications"</string> <string name="msg_mf_master">"Modifying master certifications"</string>
<string name="msg_mf_notation_empty">"Adding empty notation packet"</string> <string name="msg_mf_notation_empty">"Adding empty notation packet"</string>
<string name="msg_mf_notation_pin">"Adding PIN notation packet"</string> <string name="msg_mf_notation_pin">"Adding PIN notation packet"</string>
@ -923,8 +923,10 @@
<string name="msg_mf_passphrase_fail">"Passphrase for subkey could not be changed! (Does it have a different one from the other keys?)"</string> <string name="msg_mf_passphrase_fail">"Passphrase for subkey could not be changed! (Does it have a different one from the other keys?)"</string>
<string name="msg_mf_primary_replace_old">"Replacing certificate of previous primary user ID"</string> <string name="msg_mf_primary_replace_old">"Replacing certificate of previous primary user ID"</string>
<string name="msg_mf_primary_new">"Generating new certificate for new primary user ID"</string> <string name="msg_mf_primary_new">"Generating new certificate for new primary user ID"</string>
<string name="msg_mf_restricted_mode">"Changing to restricted operation mode"</string>
<string name="msg_mf_subkey_change">"Modifying subkey %s"</string> <string name="msg_mf_subkey_change">"Modifying subkey %s"</string>
<string name="msg_mf_error_subkey_missing">"Tried to operate on missing subkey %s!"</string> <string name="msg_mf_require_divert">"Diverting to card/nfc for crypto operations"</string>
<string name="msg_mf_require_passphrase">"Passphrase required for operations"</string>
<string name="msg_mf_subkey_new">"Adding new subkey of type %s"</string> <string name="msg_mf_subkey_new">"Adding new subkey of type %s"</string>
<string name="msg_mf_subkey_new_id">"New subkey ID: %s"</string> <string name="msg_mf_subkey_new_id">"New subkey ID: %s"</string>
<string name="msg_mf_error_past_expiry">"Expiry date cannot be in the past!"</string> <string name="msg_mf_error_past_expiry">"Expiry date cannot be in the past!"</string>