PeterCxy/open-keychain
1
0
Fork 0
Code Issues Pull requests Packages Projects Releases 1 Wiki Activity

get rid of code path for pin and notation data handling

Browse source
This commit is contained in:
Vincent Breitmoser 2016-03-23 17:49:26 +01:00
parent a8603d6974
commit c917262957
9 changed files with 75 additions and 157 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
View file

@ -566,8 +566,6 @@ public abstract class OperationResult implements Parcelable {
MSG_MF_ERROR_BAD_SECURITY_TOKEN_SIZE(LogLevel.ERROR, R.string.edit_key_error_bad_security_token_size),
MSG_MF_ERROR_BAD_SECURITY_TOKEN_STRIPPED(LogLevel.ERROR, R.string.edit_key_error_bad_security_token_stripped),
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_PIN (LogLevel.INFO, R.string.msg_mf_pin),
MSG_MF_ADMIN_PIN (LogLevel.INFO, R.string.msg_mf_admin_pin),

113
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
View file

@ -18,6 +18,23 @@
package org.sufficientlysecure.keychain.pgp;
import java.io.IOException;
import java.math.BigInteger;
import java.nio.ByteBuffer;
import java.security.InvalidAlgorithmParameterException;
import java.security.KeyPairGenerator;
import java.security.NoSuchAlgorithmException;
import java.security.NoSuchProviderException;
import java.security.SecureRandom;
import java.security.SignatureException;
import java.security.spec.ECGenParameterSpec;
import java.util.Arrays;
import java.util.Date;
import java.util.Iterator;
import java.util.Stack;
import java.util.concurrent.atomic.AtomicBoolean;
import org.bouncycastle.bcpg.PublicKeyAlgorithmTags;
import org.bouncycastle.bcpg.S2K;
import org.bouncycastle.bcpg.sig.Features;
@ -57,13 +74,12 @@ import org.sufficientlysecure.keychain.operations.results.OperationResult.Operat
import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult;
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.service.input.CryptoInputParcel;
import org.sufficientlysecure.keychain.service.input.RequiredInputParcel;
import org.sufficientlysecure.keychain.service.input.RequiredInputParcel.NfcSignOperationsBuilder;
import org.sufficientlysecure.keychain.service.input.RequiredInputParcel.NfcKeyToCardOperationsBuilder;
import org.sufficientlysecure.keychain.service.input.RequiredInputParcel.NfcSignOperationsBuilder;
import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils;
import org.sufficientlysecure.keychain.util.IterableIterator;
import org.sufficientlysecure.keychain.util.Log;
@ -71,22 +87,6 @@ import org.sufficientlysecure.keychain.util.Passphrase;
import org.sufficientlysecure.keychain.util.Primes;
import org.sufficientlysecure.keychain.util.ProgressScaler;
import java.io.IOException;
import java.math.BigInteger;
import java.nio.ByteBuffer;
import java.security.InvalidAlgorithmParameterException;
import java.security.KeyPairGenerator;
import java.security.NoSuchAlgorithmException;
import java.security.NoSuchProviderException;
import java.security.SecureRandom;
import java.security.SignatureException;
import java.security.spec.ECGenParameterSpec;
import java.util.Arrays;
import java.util.Date;
import java.util.Iterator;
import java.util.Stack;
import java.util.concurrent.atomic.AtomicBoolean;
/**
* This class is the single place where ALL operations that actually modify a PGP public or secret
* key take place.
@ -1058,8 +1058,8 @@ public class PgpKeyOperation {
log.add(LogType.MSG_MF_PASSPHRASE, indent);
indent += 1;
sKR = applyNewUnlock(sKR, masterPublicKey, masterPrivateKey,
cryptoInput.getPassphrase(), saveParcel.mNewUnlock, log, indent);
sKR = applyNewPassphrase(sKR, masterPublicKey, cryptoInput.getPassphrase(),
saveParcel.mNewUnlock.mNewPassphrase, log, indent);
if (sKR == null) {
// The error has been logged above, just return a bad state
return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
@ -1191,76 +1191,6 @@ public class PgpKeyOperation {
}
private static PGPSecretKeyRing applyNewUnlock(
PGPSecretKeyRing sKR,
PGPPublicKey masterPublicKey,
PGPPrivateKey masterPrivateKey,
Passphrase passphrase,
ChangeUnlockParcel newUnlock,
OperationLog log, int indent) throws PGPException {
if (newUnlock.mNewPassphrase != null) {
sKR = applyNewPassphrase(sKR, masterPublicKey, passphrase, newUnlock.mNewPassphrase, log, indent);
// 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(),
PgpSecurityConstants.SECRET_KEY_BINDING_SIGNATURE_HASH_ALGO)
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
{ // set subpackets
PGPSignatureSubpacketGenerator hashedPacketsGen = new PGPSignatureSubpacketGenerator();
hashedPacketsGen.setExportable(false, false);
sGen.setHashedSubpackets(hashedPacketsGen.generate());
}
sGen.init(PGPSignature.DIRECT_KEY, masterPrivateKey);
PGPSignature emptySig = sGen.generateCertification(masterPublicKey);
masterPublicKey = PGPPublicKey.addCertification(masterPublicKey, emptySig);
sKR = PGPSecretKeyRing.insertSecretKey(sKR,
PGPSecretKey.replacePublicKey(sKR.getSecretKey(), masterPublicKey));
}
return sKR;
}
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(),
PgpSecurityConstants.SECRET_KEY_BINDING_SIGNATURE_HASH_ALGO)
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
{ // set subpackets
PGPSignatureSubpacketGenerator hashedPacketsGen = new PGPSignatureSubpacketGenerator();
hashedPacketsGen.setExportable(false, false);
hashedPacketsGen.setNotationData(false, true, "unlock.pin@sufficientlysecure.org", "1");
sGen.setHashedSubpackets(hashedPacketsGen.generate());
}
sGen.init(PGPSignature.DIRECT_KEY, masterPrivateKey);
PGPSignature emptySig = sGen.generateCertification(masterPublicKey);
masterPublicKey = PGPPublicKey.addCertification(masterPublicKey, emptySig);
sKR = PGPSecretKeyRing.insertSecretKey(sKR,
PGPSecretKey.replacePublicKey(sKR.getSecretKey(), masterPublicKey));
return sKR;
}
throw new UnsupportedOperationException("PIN passphrases not yet implemented!");
}
/** This method returns true iff the provided keyring has a local direct key signature
* with notation data.
*/
@ -1294,8 +1224,7 @@ public class PgpKeyOperation {
PgpSecurityConstants.SECRET_KEY_ENCRYPTOR_S2K_COUNT)
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(newPassphrase.getCharArray());
// noinspection unchecked
for (PGPSecretKey sKey : new IterableIterator<PGPSecretKey>(sKR.getSecretKeys())) {
for (PGPSecretKey sKey : new IterableIterator<>(sKR.getSecretKeys())) {
log.add(LogType.MSG_MF_PASSPHRASE_KEY, indent,
KeyFormattingUtils.convertKeyIdToHex(sKey.getKeyID()));

42
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
View file

@ -35,6 +35,8 @@ import java.util.Set;
import java.util.TimeZone;
import java.util.TreeSet;
import android.support.annotation.VisibleForTesting;
import org.bouncycastle.bcpg.ArmoredOutputStream;
import org.bouncycastle.bcpg.PublicKeyAlgorithmTags;
import org.bouncycastle.bcpg.SignatureSubpacketTags;
@ -42,15 +44,22 @@ import org.bouncycastle.bcpg.UserAttributeSubpacketTags;
import org.bouncycastle.bcpg.sig.KeyFlags;
import org.bouncycastle.openpgp.PGPKeyRing;
import org.bouncycastle.openpgp.PGPObjectFactory;
import org.bouncycastle.openpgp.PGPPrivateKey;
import org.bouncycastle.openpgp.PGPPublicKey;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPSecretKey;
import org.bouncycastle.openpgp.PGPSecretKeyRing;
import org.bouncycastle.openpgp.PGPSignature;
import org.bouncycastle.openpgp.PGPSignatureGenerator;
import org.bouncycastle.openpgp.PGPSignatureList;
import org.bouncycastle.openpgp.PGPSignatureSubpacketGenerator;
import org.bouncycastle.openpgp.PGPUserAttributeSubpacketVector;
import org.bouncycastle.openpgp.PGPUtil;
import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor;
import org.bouncycastle.openpgp.operator.PGPContentSignerBuilder;
import org.bouncycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator;
import org.bouncycastle.openpgp.operator.jcajce.JcaPGPContentSignerBuilder;
import org.bouncycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder;
import org.sufficientlysecure.keychain.Constants;
import org.sufficientlysecure.keychain.operations.results.OperationResult.LogType;
import org.sufficientlysecure.keychain.operations.results.OperationResult.OperationLog;
@ -1310,4 +1319,37 @@ public class UncachedKeyRing {
|| algorithm == PGPPublicKey.ECDH;
}
// ONLY TO BE USED FOR TESTING!!
@VisibleForTesting
public static UncachedKeyRing forTestingOnlyAddDummyLocalSignature(
UncachedKeyRing uncachedKeyRing, String passphrase) throws Exception {
PGPSecretKeyRing sKR = (PGPSecretKeyRing) uncachedKeyRing.mRing;
PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider(
Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray());
PGPPrivateKey masterPrivateKey = sKR.getSecretKey().extractPrivateKey(keyDecryptor);
PGPPublicKey masterPublicKey = uncachedKeyRing.mRing.getPublicKey();
// add packet with "pin" notation data
PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
masterPrivateKey.getPublicKeyPacket().getAlgorithm(),
PgpSecurityConstants.SECRET_KEY_BINDING_SIGNATURE_HASH_ALGO)
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
PGPSignatureGenerator sGen = new PGPSignatureGenerator(signerBuilder);
{ // set subpackets
PGPSignatureSubpacketGenerator hashedPacketsGen = new PGPSignatureSubpacketGenerator();
hashedPacketsGen.setExportable(false, false);
hashedPacketsGen.setNotationData(false, true, "dummynotationdata", "some data");
sGen.setHashedSubpackets(hashedPacketsGen.generate());
}
sGen.init(PGPSignature.DIRECT_KEY, masterPrivateKey);
PGPSignature emptySig = sGen.generateCertification(masterPublicKey);
masterPublicKey = PGPPublicKey.addCertification(masterPublicKey, emptySig);
sKR = PGPSecretKeyRing.insertSecretKey(sKR,
PGPSecretKey.replacePublicKey(sKR.getSecretKey(), masterPublicKey));
return new UncachedKeyRing(sKR);
}
}

16
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/SaveKeyringParcel.java
View file

@ -356,29 +356,21 @@ public class SaveKeyringParcel implements Parcelable {
// The new passphrase to use
public final Passphrase mNewPassphrase;
// A new pin to use. Must only contain [0-9]+
public final Passphrase mNewPin;
public ChangeUnlockParcel(Passphrase newPassphrase) {
this(newPassphrase, null);
}
public ChangeUnlockParcel(Passphrase newPassphrase, Passphrase newPin) {
if (newPassphrase == null && newPin == null) {
throw new RuntimeException("Cannot set both passphrase and pin. THIS IS A BUG!");
if (newPassphrase == null) {
throw new AssertionError("newPassphrase must be non-null. THIS IS A BUG!");
}
mNewPassphrase = newPassphrase;
mNewPin = newPin;
}
public ChangeUnlockParcel(Parcel source) {
mNewPassphrase = source.readParcelable(Passphrase.class.getClassLoader());
mNewPin = source.readParcelable(Passphrase.class.getClassLoader());
}
@Override
public void writeToParcel(Parcel destination, int flags) {
destination.writeParcelable(mNewPassphrase, flags);
destination.writeParcelable(mNewPin, flags);
}
@Override
@ -397,9 +389,7 @@ public class SaveKeyringParcel implements Parcelable {
};
public String toString() {
return mNewPassphrase != null
? ("passphrase (" + mNewPassphrase + ")")
: ("pin (" + mNewPin + ")");
return "passphrase (" + mNewPassphrase + ")";
}
}

4
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyFinalFragment.java
View file

@ -278,7 +278,7 @@ public class CreateKeyFinalFragment extends Fragment {
2048, null, KeyFlags.AUTHENTICATION, 0L));
// use empty passphrase
saveKeyringParcel.mNewUnlock = new ChangeUnlockParcel(new Passphrase(), null);
saveKeyringParcel.mNewUnlock = new ChangeUnlockParcel(new Passphrase());
} else {
saveKeyringParcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(Algorithm.RSA,
4096, null, KeyFlags.CERTIFY_OTHER, 0L));
@ -288,7 +288,7 @@ public class CreateKeyFinalFragment extends Fragment {
4096, null, KeyFlags.ENCRYPT_COMMS | KeyFlags.ENCRYPT_STORAGE, 0L));
saveKeyringParcel.mNewUnlock = createKeyActivity.mPassphrase != null
? new ChangeUnlockParcel(createKeyActivity.mPassphrase, null)
? new ChangeUnlockParcel(createKeyActivity.mPassphrase)
: null;
}
String userId = KeyRing.createUserId(

3
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/EditKeyFragment.java
View file

@ -339,8 +339,7 @@ public class EditKeyFragment extends QueueingCryptoOperationFragment<SaveKeyring
// cache new returned passphrase!
mSaveKeyringParcel.mNewUnlock = new ChangeUnlockParcel(
(Passphrase) data.getParcelable(SetPassphraseDialogFragment.MESSAGE_NEW_PASSPHRASE),
null
(Passphrase) data.getParcelable(SetPassphraseDialogFragment.MESSAGE_NEW_PASSPHRASE)
);
}
}

3
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java
View file

@ -469,8 +469,7 @@ public class ViewKeyActivity extends BaseSecurityTokenNfcActivity implements
// use new passphrase!
mSaveKeyringParcel.mNewUnlock = new SaveKeyringParcel.ChangeUnlockParcel(
(Passphrase) data.getParcelable(SetPassphraseDialogFragment.MESSAGE_NEW_PASSPHRASE),
null
(Passphrase) data.getParcelable(SetPassphraseDialogFragment.MESSAGE_NEW_PASSPHRASE)
);
mEditOpHelper.cryptoOperation();

7
OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java
View file

@ -31,6 +31,8 @@ import android.content.ContentResolver;
import android.content.ContentValues;
import android.net.Uri;
import org.bouncycastle.bcpg.sig.KeyFlags;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
@ -40,8 +42,6 @@ import org.robolectric.RobolectricGradleTestRunner;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config;
import org.robolectric.shadows.ShadowLog;
import org.bouncycastle.bcpg.sig.KeyFlags;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.sufficientlysecure.keychain.WorkaroundBuildConfig;
import org.sufficientlysecure.keychain.operations.results.DecryptVerifyResult;
import org.sufficientlysecure.keychain.operations.results.ExportResult;
@ -124,13 +124,14 @@ public class ExportTest {
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.ECDH, 0, SaveKeyringParcel.Curve.NIST_P256, KeyFlags.ENCRYPT_COMMS, 0L));
parcel.mAddUserIds.add("snails");
parcel.mNewUnlock = new ChangeUnlockParcel(null, new Passphrase("1234"));
parcel.mNewUnlock = new ChangeUnlockParcel(new Passphrase("1234"));
PgpEditKeyResult result = op.createSecretKeyRing(parcel);
assertTrue("initial test key creation must succeed", result.success());
Assert.assertNotNull("initial test key creation must succeed", result.getRing());
mStaticRing2 = result.getRing();
mStaticRing2 = UncachedKeyRing.forTestingOnlyAddDummyLocalSignature(mStaticRing2, "1234");
}
}

42
OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
View file

@ -1232,47 +1232,7 @@ public class PgpKeyOperationTest {
}
@Test
public void testUnlockPin() throws Exception {
Passphrase pin = new Passphrase("5235125");
// change passphrase to a pin type
parcel.mNewUnlock = new ChangeUnlockParcel(null, pin);
UncachedKeyRing modified = applyModificationWithChecks(parcel, ring, onlyA, onlyB);
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);
Assert.assertEquals("second modified packet should be notation data",
PacketTags.SIGNATURE, dkSig.tag);
// check that notation data contains pin
CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(
modified.getEncoded(), false, 0);
Assert.assertEquals("secret key type should be 'pin' after this",
SecretKeyType.PIN,
secretRing.getSecretKey().getSecretKeyTypeSuperExpensive());
// need to sleep for a sec, so the timestamp changes for notation data
Thread.sleep(1000);
{
parcel.mNewUnlock = new ChangeUnlockParcel(new Passphrase("phrayse"), null);
applyModificationWithChecks(parcel, modified, onlyA, onlyB, new CryptoInputParcel(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());
}
}
@Test
public void testRestricted () throws Exception {
public void testRestricted() throws Exception {
CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);