aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java71
-rw-r--r--OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java6
-rw-r--r--OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java2
3 files changed, 75 insertions, 4 deletions
diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java
index 4fae49694..e8232a934 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringCanonicalizeTest.java
@@ -26,8 +26,10 @@ import org.junit.Before;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.shadows.ShadowLog;
import org.spongycastle.bcpg.BCPGInputStream;
+import org.spongycastle.bcpg.HashAlgorithmTags;
import org.spongycastle.bcpg.Packet;
import org.spongycastle.bcpg.PacketTags;
+import org.spongycastle.bcpg.SymmetricKeyAlgorithmTags;
import org.spongycastle.bcpg.UserIDPacket;
import org.spongycastle.bcpg.sig.KeyFlags;
import org.spongycastle.jce.provider.BouncyCastleProvider;
@@ -40,16 +42,22 @@ import org.spongycastle.openpgp.PGPSignatureGenerator;
import org.spongycastle.openpgp.PGPSignatureSubpacketGenerator;
import org.spongycastle.openpgp.PGPUtil;
import org.spongycastle.openpgp.operator.PBESecretKeyDecryptor;
+import org.spongycastle.openpgp.operator.PBESecretKeyEncryptor;
import org.spongycastle.openpgp.operator.PGPContentSignerBuilder;
+import org.spongycastle.openpgp.operator.PGPDigestCalculator;
import org.spongycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator;
import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentSignerBuilder;
+import org.spongycastle.openpgp.operator.jcajce.JcaPGPDigestCalculatorProviderBuilder;
import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder;
+import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyEncryptorBuilder;
import org.spongycastle.util.Strings;
import org.sufficientlysecure.keychain.Constants;
import org.sufficientlysecure.keychain.service.results.OperationResult;
import org.sufficientlysecure.keychain.service.results.EditKeyResult;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
+import org.sufficientlysecure.keychain.service.results.OperationResult.LogType;
+import org.sufficientlysecure.keychain.service.results.OperationResult.OperationLog;
import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket;
@@ -465,6 +473,69 @@ public class UncachedKeyringCanonicalizeTest {
}
+ @Test
+ public void testDuplicateSubkey() throws Exception {
+
+ { // duplicate subkey
+
+ // get subkey packets
+ Iterator<RawPacket> it = KeyringTestingHelper.parseKeyring(ring.getEncoded());
+ RawPacket subKey = KeyringTestingHelper.getNth(it, 5);
+ RawPacket subSig = it.next();
+
+ // inject at a second position
+ UncachedKeyRing modified = ring;
+ modified = KeyringTestingHelper.injectPacket(modified, subKey.buf, 7);
+ modified = KeyringTestingHelper.injectPacket(modified, subSig.buf, 8);
+
+ // canonicalize, and check if we lose the bad signature
+ OperationLog log = new OperationLog();
+ CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
+ Assert.assertNull("canonicalization with duplicate subkey should fail", canonicalized);
+ Assert.assertTrue("log should contain dup_key event", log.containsType(LogType.MSG_KC_ERROR_DUP_KEY));
+ }
+
+ { // duplicate subkey, which is the same as the master key
+
+ // We actually encountered one of these in the wild:
+ // https://www.sparkasse-holstein.de/firmenkunden/electronic_banking/secure-e-mail/pdf/Spk_Holstein_PGP_Domain-Zertifikat.asc
+
+ CanonicalizedSecretKeyRing canonicalized = (CanonicalizedSecretKeyRing) ring.canonicalize(log, 0);
+
+ CanonicalizedSecretKey masterSecretKey = canonicalized.getSecretKey();
+ masterSecretKey.unlock("");
+ PGPPublicKey masterPublicKey = masterSecretKey.getPublicKey();
+ PGPSignature cert = PgpKeyOperation.generateSubkeyBindingSignature(
+ masterPublicKey, masterSecretKey.getPrivateKey(), masterSecretKey.getPrivateKey(),
+ masterPublicKey, masterSecretKey.getKeyUsage(), 0);
+ PGPPublicKey subPubKey = PGPPublicKey.addSubkeyBindingCertification(masterPublicKey, cert);
+
+ PGPSecretKey sKey;
+ {
+ // Build key encrypter and decrypter based on passphrase
+ PGPDigestCalculator encryptorHashCalc = new JcaPGPDigestCalculatorProviderBuilder()
+ .build().get(HashAlgorithmTags.SHA256);
+ PBESecretKeyEncryptor keyEncryptor = new JcePBESecretKeyEncryptorBuilder(
+ SymmetricKeyAlgorithmTags.AES_256, encryptorHashCalc, 10)
+ .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME).build("".toCharArray());
+
+ // NOTE: only SHA1 is supported for key checksum calculations.
+ PGPDigestCalculator sha1Calc = new JcaPGPDigestCalculatorProviderBuilder()
+ .build().get(HashAlgorithmTags.SHA1);
+ sKey = new PGPSecretKey(masterSecretKey.getPrivateKey(), subPubKey, sha1Calc, false, keyEncryptor);
+ }
+
+ UncachedKeyRing modified = KeyringTestingHelper.injectPacket(ring, sKey.getEncoded(), 5);
+
+ // canonicalize, and check if we lose the bad signature
+ OperationLog log = new OperationLog();
+ CanonicalizedKeyRing result = modified.canonicalize(log, 0);
+ Assert.assertNull("canonicalization with duplicate subkey (from master) should fail", result);
+ Assert.assertTrue("log should contain dup_key event", log.containsType(LogType.MSG_KC_ERROR_DUP_KEY));
+ }
+
+ }
+
private static final int[] sigtypes_direct = new int[] {
PGPSignature.KEY_REVOCATION,
PGPSignature.DIRECT_KEY,
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java
index bf28465f5..4106ab73d 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedSecretKey.java
@@ -329,9 +329,9 @@ public class CanonicalizedSecretKey extends CanonicalizedPublicKey {
return new UncachedSecretKey(mSecretKey);
}
- // HACK
- public PGPSecretKey getSecretKey() {
- return mSecretKey;
+ // HACK, for TESTING ONLY!!
+ PGPPrivateKey getPrivateKey () {
+ return mPrivateKey;
}
}
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 0a8774029..c4070e39a 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
@@ -1118,7 +1118,7 @@ public class PgpKeyOperation {
pKey, flags, expiry);
}
- private static PGPSignature generateSubkeyBindingSignature(
+ static PGPSignature generateSubkeyBindingSignature(
PGPPublicKey masterPublicKey, PGPPrivateKey masterPrivateKey,
PGPPrivateKey subPrivateKey, PGPPublicKey pKey, int flags, long expiry)
throws IOException, PGPException, SignatureException {