cthrace/open-keychain
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

better handling of divert-to-card keys for subkey modifications

Browse Source
This commit is contained in:
Vincent Breitmoser
2015-07-08 01:41:55 +02:00
parent ac2c8f994e
commit 642f83c1f4
5 changed files with 81 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
View File

@@ -499,6 +499,7 @@ public abstract class OperationResult implements Parcelable {
MSG_MF_ERROR_RESTRICTED(LogLevel.ERROR, R.string.msg_mf_error_restricted),
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_SUB_STRIPPED(LogLevel.ERROR, R.string.msg_mf_error_sub_stripped),
MSG_MF_ERROR_SUBKEY_MISSING(LogLevel.ERROR, R.string.msg_mf_error_subkey_missing),
MSG_MF_ERROR_CONFLICTING_NFC_COMMANDS(LogLevel.ERROR, R.string.msg_mf_error_conflicting_nfc_commands),
MSG_MF_ERROR_DUPLICATE_KEYTOCARD_FOR_SLOT(LogLevel.ERROR, R.string.msg_mf_error_duplicate_keytocard_for_slot),

75
OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
View File

@@ -897,18 +897,35 @@ public class PgpKeyOperation {
pKey = PGPPublicKey.removeCertification(pKey, sig);
}
PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder()
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
cryptoInput.getPassphrase().getCharArray());
PGPPrivateKey subPrivateKey = sKey.extractPrivateKey(keyDecryptor);
PGPSignature sig = generateSubkeyBindingSignature(
getSignatureGenerator(masterSecretKey, cryptoInput),
cryptoInput.getSignatureTime(),
masterPublicKey, masterPrivateKey, subPrivateKey, pKey, flags, expiry);
PGPPrivateKey subPrivateKey;
if (!isDivertToCard(sKey)) {
PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder()
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
cryptoInput.getPassphrase().getCharArray());
subPrivateKey = sKey.extractPrivateKey(keyDecryptor);
// super special case: subkey is allowed to sign, but isn't available
if (subPrivateKey == null) {
log.add(LogType.MSG_MF_ERROR_SUB_STRIPPED,
indent + 1, KeyFormattingUtils.convertKeyIdToHex(change.mKeyId));
return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null);
}
} else {
subPrivateKey = null;
}
try {
PGPSignature sig = generateSubkeyBindingSignature(
getSignatureGenerator(masterSecretKey, cryptoInput),
cryptoInput.getSignatureTime(), masterPublicKey, masterPrivateKey,
getSignatureGenerator(sKey, cryptoInput), subPrivateKey,
pKey, flags, expiry);
// generate and add new signature
pKey = PGPPublicKey.addCertification(pKey, sig);
sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey));
} catch (NfcInteractionNeeded e) {
nfcSignOps.addHash(e.hashToSign, e.hashAlgo);
}
// generate and add new signature
pKey = PGPPublicKey.addCertification(pKey, sig);
sKR = PGPSecretKeyRing.insertSecretKey(sKR, PGPSecretKey.replacePublicKey(sKey, pKey));
}
subProgressPop();
@@ -987,7 +1004,8 @@ public class PgpKeyOperation {
PGPSignature cert = generateSubkeyBindingSignature(
getSignatureGenerator(masterSecretKey, cryptoInput),
cryptoInput.getSignatureTime(),
masterPublicKey, masterPrivateKey, keyPair.getPrivateKey(), pKey,
masterPublicKey, masterPrivateKey,
getSignatureGenerator(pKey, cryptoInput, false), keyPair.getPrivateKey(), pKey,
add.mFlags, add.mExpiry);
pKey = PGPPublicKey.addSubkeyBindingCertification(pKey, cert);
} catch (NfcInteractionNeeded e) {
@@ -1002,7 +1020,8 @@ public class PgpKeyOperation {
PgpConstants.SECRET_KEY_ENCRYPTOR_SYMMETRIC_ALGO, encryptorHashCalc,
PgpConstants.SECRET_KEY_ENCRYPTOR_S2K_COUNT)
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(
cryptoInput.getPassphrase().getCharArray());
cryptoInput.hasPassphrase()
? cryptoInput.getPassphrase().getCharArray() : new char[]{} );
PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder()
.build().get(PgpConstants.SECRET_KEY_SIGNATURE_CHECKSUM_HASH_ALGO);
@@ -1402,22 +1421,27 @@ public class PgpKeyOperation {
static PGPSignatureGenerator getSignatureGenerator(
PGPSecretKey secretKey, CryptoInputParcel cryptoInput) {
PGPContentSignerBuilder builder;
S2K s2k = secretKey.getS2K();
if (s2k != null && s2k.getType() == S2K.GNU_DUMMY_S2K
&& s2k.getProtectionMode() == S2K.GNU_PROTECTION_MODE_DIVERT_TO_CARD) {
boolean isDivertToCard = s2k != null && s2k.getType() == S2K.GNU_DUMMY_S2K
&& s2k.getProtectionMode() == S2K.GNU_PROTECTION_MODE_DIVERT_TO_CARD;
return getSignatureGenerator(secretKey.getPublicKey(), cryptoInput, isDivertToCard);
}
static PGPSignatureGenerator getSignatureGenerator(
PGPPublicKey pKey, CryptoInputParcel cryptoInput, boolean divertToCard) {
PGPContentSignerBuilder builder;
if (divertToCard) {
// use synchronous "NFC based" SignerBuilder
builder = new NfcSyncPGPContentSignerBuilder(
secretKey.getPublicKey().getAlgorithm(),
PgpConstants.SECRET_KEY_SIGNATURE_HASH_ALGO,
secretKey.getKeyID(), cryptoInput.getCryptoData())
pKey.getAlgorithm(), PgpConstants.SECRET_KEY_SIGNATURE_HASH_ALGO,
pKey.getKeyID(), cryptoInput.getCryptoData())
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
} else {
// content signer based on signing key algorithm and chosen hash algorithm
builder = new JcaPGPContentSignerBuilder(
secretKey.getPublicKey().getAlgorithm(),
PgpConstants.SECRET_KEY_SIGNATURE_HASH_ALGO)
pKey.getAlgorithm(), PgpConstants.SECRET_KEY_SIGNATURE_HASH_ALGO)
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
}
@@ -1524,7 +1548,8 @@ public class PgpKeyOperation {
static PGPSignature generateSubkeyBindingSignature(
PGPSignatureGenerator sGen, Date creationTime,
PGPPublicKey masterPublicKey, PGPPrivateKey masterPrivateKey,
PGPPrivateKey subPrivateKey, PGPPublicKey pKey, int flags, long expiry)
PGPSignatureGenerator subSigGen, PGPPrivateKey subPrivateKey, PGPPublicKey pKey,
int flags, long expiry)
throws IOException, PGPException, SignatureException {
PGPSignatureSubpacketGenerator unhashedPacketsGen = new PGPSignatureSubpacketGenerator();
@@ -1534,10 +1559,6 @@ public class PgpKeyOperation {
// cross-certify signing keys
PGPSignatureSubpacketGenerator subHashedPacketsGen = new PGPSignatureSubpacketGenerator();
subHashedPacketsGen.setSignatureCreationTime(false, creationTime);
PGPContentSignerBuilder signerBuilder = new JcaPGPContentSignerBuilder(
pKey.getAlgorithm(), PgpConstants.SECRET_KEY_SIGNATURE_HASH_ALGO)
.setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME);
PGPSignatureGenerator subSigGen = new PGPSignatureGenerator(signerBuilder);
subSigGen.init(PGPSignature.PRIMARYKEY_BINDING, subPrivateKey);
subSigGen.setHashedSubpackets(subHashedPacketsGen.generate());
PGPSignature certification = subSigGen.generateCertification(masterPublicKey, pKey);

1
OpenKeychain/src/main/res/values/strings.xml
View File

@@ -991,6 +991,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_pgp">"Internal OpenPGP error!"</string>
<string name="msg_mf_error_sig">"Signature exception!"</string>
<string name="msg_mf_error_sub_stripped">"Cannot modify stripped subkey %s!"</string>
<string name="msg_mf_error_subkey_missing">"Tried to operate on missing subkey %s!"</string>
<string name="msg_mf_error_conflicting_nfc_commands">"Cannot move key to smart card in same operation that creates an on-card signature."</string>
<string name="msg_mf_error_duplicate_keytocard_for_slot">"Smart card supports only one slot per key type."</string>

34
OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
View File

@@ -94,11 +94,11 @@ public class PgpKeyOperationTest {
SaveKeyringParcel parcel = new SaveKeyringParcel();
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.RSA, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
Algorithm.DSA, 1024, null, KeyFlags.CERTIFY_OTHER, 0L));
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.DSA, 1024, null, KeyFlags.SIGN_DATA, 0L));
Algorithm.RSA, 2048, null, KeyFlags.SIGN_DATA, 0L));
parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
Algorithm.RSA, 2048, null, KeyFlags.ENCRYPT_COMMS, 0L));
Algorithm.RSA, 1024, null, KeyFlags.ENCRYPT_COMMS, 0L));
parcel.mAddUserIds.add("twi");
parcel.mAddUserIds.add("pink");
@@ -821,6 +821,15 @@ public class PgpKeyOperationTest {
Assert.assertEquals("new packet should have GNU_DUMMY protection mode stripped",
S2K.GNU_PROTECTION_MODE_NO_PRIVATE_KEY, ((SecretKeyPacket) p).getS2K().getProtectionMode());
}
{ // trying to edit a subkey with signing capability should fail
parcel.reset();
parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true));
assertModifyFailure("subkey modification for signing-enabled but stripped subkey should fail",
modified, parcel, LogType.MSG_MF_ERROR_SUB_STRIPPED);
}
}
@Test
@@ -829,7 +838,7 @@ public class PgpKeyOperationTest {
UncachedKeyRing modified;
{ // keytocard should fail with BAD_NFC_SIZE when presented with the RSA-1024 key
long keyId = KeyringTestingHelper.getSubkeyId(ring, 0);
long keyId = KeyringTestingHelper.getSubkeyId(ring, 2);
parcel.reset();
parcel.mChangeSubKeys.add(new SubkeyChange(keyId, false, true));
@@ -838,7 +847,7 @@ public class PgpKeyOperationTest {
}
{ // keytocard should fail with BAD_NFC_ALGO when presented with the DSA-1024 key
long keyId = KeyringTestingHelper.getSubkeyId(ring, 1);
long keyId = KeyringTestingHelper.getSubkeyId(ring, 0);
parcel.reset();
parcel.mChangeSubKeys.add(new SubkeyChange(keyId, false, true));
@@ -846,9 +855,10 @@ public class PgpKeyOperationTest {
parcel, cryptoInput, LogType.MSG_MF_ERROR_BAD_NFC_ALGO);
}
long keyId = KeyringTestingHelper.getSubkeyId(ring, 1);
{ // keytocard should return a pending NFC_MOVE_KEY_TO_CARD result when presented with the RSA-2048
// key, and then make key divert-to-card when it gets a serial in the cryptoInputParcel.
long keyId = KeyringTestingHelper.getSubkeyId(ring, 2);
parcel.reset();
parcel.mChangeSubKeys.add(new SubkeyChange(keyId, false, true));
@@ -880,7 +890,19 @@ public class PgpKeyOperationTest {
S2K.GNU_PROTECTION_MODE_DIVERT_TO_CARD, ((SecretKeyPacket) p).getS2K().getProtectionMode());
Assert.assertArrayEquals("new packet should have correct serial number as iv",
serial, ((SecretKeyPacket) p).getIV());
}
{ // editing a signing subkey requires a primary key binding sig -> pendinginput
parcel.reset();
parcel.mChangeSubKeys.add(new SubkeyChange(keyId, true));
CanonicalizedSecretKeyRing secretRing =
new CanonicalizedSecretKeyRing(modified.getEncoded(), false, 0);
PgpKeyOperation op = new PgpKeyOperation(null);
PgpEditKeyResult result = op.modifySecretKeyRing(secretRing, cryptoInput, parcel);
Assert.assertTrue("keytocard operation should be pending", result.isPending());
Assert.assertEquals("required input should be RequiredInputType.NFC_SIGN",
RequiredInputType.NFC_SIGN, result.getRequiredInputParcel().mType);
}
}

5
OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java
View File

@@ -558,8 +558,9 @@ public class UncachedKeyringCanonicalizeTest {
PGPSignature cert = PgpKeyOperation.generateSubkeyBindingSignature(
PgpKeyOperation.getSignatureGenerator(masterSecretKey.getSecretKey(), cryptoInput),
cryptoInput.getSignatureTime(),
masterPublicKey, masterSecretKey.getPrivateKey(), masterSecretKey.getPrivateKey(),
masterPublicKey, masterSecretKey.getKeyUsage(), 0);
masterPublicKey, masterSecretKey.getPrivateKey(),
PgpKeyOperation.getSignatureGenerator(masterSecretKey.getSecretKey(), null),
masterSecretKey.getPrivateKey(), masterPublicKey, masterSecretKey.getKeyUsage(), 0);
PGPPublicKey subPubKey = PGPPublicKey.addSubkeyBindingCertification(masterPublicKey, cert);
PGPSecretKey sKey;