From 9e715aabfe123bf8fabb5a5e0efdb69e6d7f5eea Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Sun, 21 Nov 2021 22:25:45 +0100 Subject: [PATCH] Test signature subpackets and fix bug for missing user-id sig --- .../secretkeyring/SecretKeyRingEditor.java | 66 ++++++-- .../SecretKeyRingEditorInterface.java | 5 + .../KeyGenerationSubpacketsTest.java | 145 ++++++++++++++++-- 3 files changed, 197 insertions(+), 19 deletions(-) diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.java b/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.java index 4cafe337..88d67440 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditor.java @@ -16,6 +16,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; +import java.util.Set; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -36,10 +37,14 @@ import org.bouncycastle.openpgp.operator.PBESecretKeyDecryptor; import org.bouncycastle.openpgp.operator.PBESecretKeyEncryptor; import org.bouncycastle.openpgp.operator.PGPContentSignerBuilder; import org.pgpainless.PGPainless; +import org.pgpainless.algorithm.AlgorithmSuite; +import org.pgpainless.algorithm.CompressionAlgorithm; +import org.pgpainless.algorithm.Feature; import org.pgpainless.algorithm.HashAlgorithm; import org.pgpainless.algorithm.KeyFlag; import org.pgpainless.algorithm.PublicKeyAlgorithm; import org.pgpainless.algorithm.SignatureType; +import org.pgpainless.algorithm.SymmetricKeyAlgorithm; import org.pgpainless.algorithm.negotiation.HashAlgorithmNegotiator; import org.pgpainless.implementation.ImplementationFactory; import org.pgpainless.key.OpenPgpFingerprint; @@ -101,9 +106,31 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { KeyRingInfo info = PGPainless.inspectKeyRing(secretKeyRing); List keyFlags = info.getKeyFlagsOf(info.getKeyId()); + Set hashAlgorithmPreferences; + Set symmetricKeyAlgorithmPreferences; + Set compressionAlgorithmPreferences; + try { + hashAlgorithmPreferences = info.getPreferredHashAlgorithms(); + symmetricKeyAlgorithmPreferences = info.getPreferredSymmetricKeyAlgorithms(); + compressionAlgorithmPreferences = info.getPreferredCompressionAlgorithms(); + } catch (IllegalStateException e) { + // missing user-id sig + AlgorithmSuite algorithmSuite = AlgorithmSuite.getDefaultAlgorithmSuite(); + hashAlgorithmPreferences = algorithmSuite.getHashAlgorithms(); + symmetricKeyAlgorithmPreferences = algorithmSuite.getSymmetricKeyAlgorithms(); + compressionAlgorithmPreferences = algorithmSuite.getCompressionAlgorithms(); + } + SelfSignatureBuilder builder = new SelfSignatureBuilder(primaryKey, protector); builder.setSignatureType(SignatureType.POSITIVE_CERTIFICATION); + + // Retain signature subpackets of previous signatures builder.getHashedSubpackets().setKeyFlags(keyFlags); + builder.getHashedSubpackets().setPreferredHashAlgorithms(hashAlgorithmPreferences); + builder.getHashedSubpackets().setPreferredSymmetricKeyAlgorithms(symmetricKeyAlgorithmPreferences); + builder.getHashedSubpackets().setPreferredCompressionAlgorithms(compressionAlgorithmPreferences); + builder.getHashedSubpackets().setFeatures(Feature.MODIFICATION_DETECTION); + builder.applyCallback(signatureSubpacketCallback); PGPSignature signature = builder.build(primaryKey.getPublicKey(), userId); @@ -121,9 +148,10 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { } @Override - public SecretKeyRingEditorInterface addSubKey(@Nonnull KeySpec keySpec, - @Nonnull Passphrase subKeyPassphrase, - SecretKeyRingProtector secretKeyRingProtector) + public SecretKeyRingEditorInterface addSubKey( + @Nonnull KeySpec keySpec, + @Nonnull Passphrase subKeyPassphrase, + SecretKeyRingProtector secretKeyRingProtector) throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, PGPException, IOException { PGPKeyPair keyPair = KeyRingBuilder.generateKeyPair(keySpec); @@ -146,12 +174,32 @@ public class SecretKeyRingEditor implements SecretKeyRingEditorInterface { } @Override - public SecretKeyRingEditorInterface addSubKey(PGPKeyPair subkey, - @Nullable SelfSignatureSubpackets.Callback bindingSignatureCallback, - SecretKeyRingProtector subkeyProtector, - SecretKeyRingProtector primaryKeyProtector, - KeyFlag keyFlag, - KeyFlag... additionalKeyFlags) + public SecretKeyRingEditorInterface addSubKey( + @Nonnull KeySpec keySpec, + @Nullable Passphrase subkeyPassphrase, + @Nullable SelfSignatureSubpackets.Callback subpacketsCallback, + SecretKeyRingProtector secretKeyRingProtector) + throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException { + PGPKeyPair keyPair = KeyRingBuilder.generateKeyPair(keySpec); + + SecretKeyRingProtector subKeyProtector = PasswordBasedSecretKeyRingProtector + .forKeyId(keyPair.getKeyID(), subkeyPassphrase); + + List keyFlags = KeyFlag.fromBitmask(keySpec.getSubpackets().getKeyFlags()); + KeyFlag firstFlag = keyFlags.remove(0); + KeyFlag[] otherFlags = keyFlags.toArray(new KeyFlag[0]); + + return addSubKey(keyPair, subpacketsCallback, subKeyProtector, secretKeyRingProtector, firstFlag, otherFlags); + } + + @Override + public SecretKeyRingEditorInterface addSubKey( + PGPKeyPair subkey, + @Nullable SelfSignatureSubpackets.Callback bindingSignatureCallback, + SecretKeyRingProtector subkeyProtector, + SecretKeyRingProtector primaryKeyProtector, + KeyFlag keyFlag, + KeyFlag... additionalKeyFlags) throws PGPException, IOException { KeyFlag[] flags = concat(keyFlag, additionalKeyFlags); PublicKeyAlgorithm subkeyAlgorithm = PublicKeyAlgorithm.fromId(subkey.getPublicKey().getAlgorithm()); diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.java b/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.java index f2a64190..67974ade 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/modification/secretkeyring/SecretKeyRingEditorInterface.java @@ -67,6 +67,11 @@ public interface SecretKeyRingEditorInterface { SecretKeyRingProtector secretKeyRingProtector) throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, PGPException, IOException; + SecretKeyRingEditorInterface addSubKey(@Nonnull KeySpec keySpec, + @Nullable Passphrase subkeyPassphrase, + @Nullable SelfSignatureSubpackets.Callback subpacketsCallback, + SecretKeyRingProtector secretKeyRingProtector) throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException; + SecretKeyRingEditorInterface addSubKey(PGPKeyPair subkey, @Nullable SelfSignatureSubpackets.Callback bindingSignatureCallback, SecretKeyRingProtector subkeyProtector, diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/generation/KeyGenerationSubpacketsTest.java b/pgpainless-core/src/test/java/org/pgpainless/key/generation/KeyGenerationSubpacketsTest.java index 4371b84c..5451a4dc 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/key/generation/KeyGenerationSubpacketsTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/key/generation/KeyGenerationSubpacketsTest.java @@ -4,29 +4,154 @@ package org.pgpainless.key.generation; -import org.bouncycastle.openpgp.PGPException; -import org.bouncycastle.openpgp.PGPSecretKeyRing; -import org.bouncycastle.openpgp.PGPSignature; -import org.junit.jupiter.api.Test; -import org.pgpainless.PGPainless; -import org.pgpainless.key.info.KeyRingInfo; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.IOException; import java.security.InvalidAlgorithmParameterException; import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; -import static org.junit.jupiter.api.Assertions.assertNotNull; +import org.bouncycastle.bcpg.sig.IssuerFingerprint; +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPPublicKey; +import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.bouncycastle.openpgp.PGPSignature; +import org.bouncycastle.openpgp.PGPSignatureSubpacketVector; +import org.junit.jupiter.api.Test; +import org.pgpainless.PGPainless; +import org.pgpainless.algorithm.HashAlgorithm; +import org.pgpainless.algorithm.KeyFlag; +import org.pgpainless.key.generation.type.KeyType; +import org.pgpainless.key.generation.type.eddsa.EdDSACurve; +import org.pgpainless.key.generation.type.xdh.XDHSpec; +import org.pgpainless.key.info.KeyRingInfo; +import org.pgpainless.key.protection.SecretKeyRingProtector; +import org.pgpainless.signature.subpackets.SelfSignatureSubpackets; public class KeyGenerationSubpacketsTest { @Test - public void verifyDefaultSubpackets() + public void verifyDefaultSubpacketsForUserIdSignatures() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException { PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing().modernKeyRing("Alice", null); - KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); PGPSignature userIdSig = info.getLatestUserIdCertification("Alice"); assertNotNull(userIdSig); - assertNotNull(userIdSig.getHashedSubPackets().getIssuerFingerprint()); + int keyFlags = userIdSig.getHashedSubPackets().getKeyFlags(); + int[] preferredHashAlgorithms = userIdSig.getHashedSubPackets().getPreferredHashAlgorithms(); + int[] preferredSymmetricAlgorithms = userIdSig.getHashedSubPackets().getPreferredSymmetricAlgorithms(); + int[] preferredCompressionAlgorithms = userIdSig.getHashedSubPackets().getPreferredCompressionAlgorithms(); + assureSignatureHasDefaultSubpackets(userIdSig, secretKeys, KeyFlag.CERTIFY_OTHER); + assertTrue(userIdSig.getHashedSubPackets().isPrimaryUserID()); + assertEquals("Alice", info.getPrimaryUserId()); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .addUserId("Bob", + new SelfSignatureSubpackets.Callback() { + @Override + public void modifyHashedSubpackets(SelfSignatureSubpackets hashedSubpackets) { + hashedSubpackets.setPrimaryUserId(); + } + }, + SecretKeyRingProtector.unprotectedKeys()) + .addUserId("Alice", SecretKeyRingProtector.unprotectedKeys()) + .done(); + + info = PGPainless.inspectKeyRing(secretKeys); + + userIdSig = info.getLatestUserIdCertification("Alice"); + assertNotNull(userIdSig); + assureSignatureHasDefaultSubpackets(userIdSig, secretKeys, KeyFlag.CERTIFY_OTHER); + assertFalse(userIdSig.getHashedSubPackets().isPrimaryUserID()); + assertEquals(keyFlags, userIdSig.getHashedSubPackets().getKeyFlags()); + assertArrayEquals(preferredHashAlgorithms, userIdSig.getHashedSubPackets().getPreferredHashAlgorithms()); + assertArrayEquals(preferredSymmetricAlgorithms, userIdSig.getHashedSubPackets().getPreferredSymmetricAlgorithms()); + assertArrayEquals(preferredCompressionAlgorithms, userIdSig.getHashedSubPackets().getPreferredCompressionAlgorithms()); + + userIdSig = info.getLatestUserIdCertification("Bob"); + assertNotNull(userIdSig); + assureSignatureHasDefaultSubpackets(userIdSig, secretKeys, KeyFlag.CERTIFY_OTHER); + assertTrue(userIdSig.getHashedSubPackets().isPrimaryUserID()); + assertArrayEquals(preferredHashAlgorithms, userIdSig.getHashedSubPackets().getPreferredHashAlgorithms()); + assertArrayEquals(preferredSymmetricAlgorithms, userIdSig.getHashedSubPackets().getPreferredSymmetricAlgorithms()); + assertArrayEquals(preferredCompressionAlgorithms, userIdSig.getHashedSubPackets().getPreferredCompressionAlgorithms()); + + assertEquals("Bob", info.getPrimaryUserId()); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .addUserId("Alice", new SelfSignatureSubpackets.Callback() { + @Override + public void modifyHashedSubpackets(SelfSignatureSubpackets hashedSubpackets) { + hashedSubpackets.setPrimaryUserId(); + hashedSubpackets.setPreferredHashAlgorithms(HashAlgorithm.SHA1); + } + }, SecretKeyRingProtector.unprotectedKeys()) + .done(); + info = PGPainless.inspectKeyRing(secretKeys); + assertEquals("Alice", info.getPrimaryUserId()); + assertEquals(Collections.singleton(HashAlgorithm.SHA1), info.getPreferredHashAlgorithms("Alice")); + } + + @Test + public void verifyDefaultSubpacketsForSubkeyBindingSignatures() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException { + PGPSecretKeyRing secretKeys = PGPainless.generateKeyRing().modernKeyRing("Alice", null); + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); + List keysBefore = info.getPublicKeys(); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .addSubKey(KeySpec.getBuilder(KeyType.EDDSA(EdDSACurve._Ed25519), KeyFlag.SIGN_DATA).build(), null, SecretKeyRingProtector.unprotectedKeys()) + .done(); + + + info = PGPainless.inspectKeyRing(secretKeys); + List keysAfter = new ArrayList<>(info.getPublicKeys()); + keysAfter.removeAll(keysBefore); + assertEquals(1, keysAfter.size()); + PGPPublicKey newSigningKey = keysAfter.get(0); + + PGPSignature bindingSig = info.getCurrentSubkeyBindingSignature(newSigningKey.getKeyID()); + assertNotNull(bindingSig); + assureSignatureHasDefaultSubpackets(bindingSig, secretKeys, KeyFlag.SIGN_DATA); + assertNotNull(bindingSig.getHashedSubPackets().getEmbeddedSignatures().get(0)); + + secretKeys = PGPainless.modifyKeyRing(secretKeys) + .addSubKey(KeySpec.getBuilder(KeyType.XDH(XDHSpec._X25519), KeyFlag.ENCRYPT_COMMS).build(), null, + new SelfSignatureSubpackets.Callback() { + @Override + public void modifyHashedSubpackets(SelfSignatureSubpackets hashedSubpackets) { + hashedSubpackets.setIssuerFingerprint((IssuerFingerprint) null); + } + }, SecretKeyRingProtector.unprotectedKeys()) + .done(); + + info = PGPainless.inspectKeyRing(secretKeys); + keysAfter = new ArrayList<>(info.getPublicKeys()); + keysAfter.removeAll(keysBefore); + keysAfter.remove(newSigningKey); + assertEquals(1, keysAfter.size()); + PGPPublicKey newEncryptionKey = keysAfter.get(0); + bindingSig = info.getCurrentSubkeyBindingSignature(newEncryptionKey.getKeyID()); + assertNotNull(bindingSig); + assertNull(bindingSig.getHashedSubPackets().getIssuerFingerprint()); + assertEquals(KeyFlag.toBitmask(KeyFlag.ENCRYPT_COMMS), bindingSig.getHashedSubPackets().getKeyFlags()); + } + + private void assureSignatureHasDefaultSubpackets(PGPSignature signature, PGPSecretKeyRing secretKeys, KeyFlag... keyFlags) { + PGPSignatureSubpacketVector hashedSubpackets = signature.getHashedSubPackets(); + assertNotNull(hashedSubpackets.getIssuerFingerprint()); + assertEquals(secretKeys.getPublicKey().getKeyID(), hashedSubpackets.getIssuerKeyID()); + assertArrayEquals( + secretKeys.getPublicKey().getFingerprint(), + hashedSubpackets.getIssuerFingerprint().getFingerprint()); + assertEquals(hashedSubpackets.getKeyFlags(), KeyFlag.toBitmask(keyFlags)); } }