From 1b96919d84d3297a64a67d7bf2ee4b37f98a3d0f Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Mon, 9 Oct 2023 12:02:10 +0200 Subject: [PATCH] Allow generation of keys with empty key flags. Forbid certification of thirdparty certificates if CERTIFY_OTHERS flag is missing --- .../org/pgpainless/algorithm/KeyFlag.java | 2 +- .../pgpainless/exception/KeyException.java | 7 +++ .../key/certification/CertifyCertificate.java | 4 ++ .../key/generation/KeyRingBuilder.java | 9 ---- .../pgpainless/key/generation/KeySpec.java | 4 +- .../key/generation/KeySpecBuilder.java | 14 ++--- .../org/pgpainless/key/info/KeyRingInfo.java | 4 ++ ...GenerateKeyWithoutPrimaryKeyFlagsTest.java | 53 +++++++++++++++++++ 8 files changed, 76 insertions(+), 21 deletions(-) create mode 100644 pgpainless-core/src/test/java/org/pgpainless/key/generation/GenerateKeyWithoutPrimaryKeyFlagsTest.java diff --git a/pgpainless-core/src/main/java/org/pgpainless/algorithm/KeyFlag.java b/pgpainless-core/src/main/java/org/pgpainless/algorithm/KeyFlag.java index 92205fd9..149ef9d0 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/algorithm/KeyFlag.java +++ b/pgpainless-core/src/main/java/org/pgpainless/algorithm/KeyFlag.java @@ -18,7 +18,7 @@ import org.bouncycastle.bcpg.sig.KeyFlags; public enum KeyFlag { /** - * This key may be used to certify other keys. + * This key may be used to certify third-party keys. */ CERTIFY_OTHER (KeyFlags.CERTIFY_OTHER), diff --git a/pgpainless-core/src/main/java/org/pgpainless/exception/KeyException.java b/pgpainless-core/src/main/java/org/pgpainless/exception/KeyException.java index 66c5d2cf..65d27390 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/exception/KeyException.java +++ b/pgpainless-core/src/main/java/org/pgpainless/exception/KeyException.java @@ -67,6 +67,13 @@ public abstract class KeyException extends RuntimeException { } } + public static class UnacceptableThirdPartyCertificationKeyException extends KeyException { + + public UnacceptableThirdPartyCertificationKeyException(@Nonnull OpenPgpFingerprint fingerprint) { + super("Key " + fingerprint + " has no acceptable certification key.", fingerprint); + } + } + public static class UnacceptableSelfSignatureException extends KeyException { public UnacceptableSelfSignatureException(@Nonnull OpenPgpFingerprint fingerprint) { diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/certification/CertifyCertificate.java b/pgpainless-core/src/main/java/org/pgpainless/key/certification/CertifyCertificate.java index 9340d93d..5b6c6fe2 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/certification/CertifyCertificate.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/certification/CertifyCertificate.java @@ -280,6 +280,10 @@ public class CertifyCertificate { throw new KeyException.RevokedKeyException(fingerprint); } + if (!info.isUsableForThirdPartyCertification()) { + throw new KeyException.UnacceptableThirdPartyCertificationKeyException(fingerprint); + } + Date expirationDate = info.getExpirationDateForUse(KeyFlag.CERTIFY_OTHER); if (expirationDate != null && expirationDate.before(now)) { throw new KeyException.ExpiredKeyException(fingerprint, expirationDate); diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeyRingBuilder.java b/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeyRingBuilder.java index 208c17b6..84251c12 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeyRingBuilder.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeyRingBuilder.java @@ -19,7 +19,6 @@ import java.util.Map; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import org.bouncycastle.bcpg.sig.KeyFlags; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPKeyPair; import org.bouncycastle.openpgp.PGPKeyRingGenerator; @@ -128,19 +127,11 @@ public class KeyRingBuilder implements KeyRingBuilderInterface { } private void verifyMasterKeyCanCertify(KeySpec spec) { - if (!hasCertifyOthersFlag(spec)) { - throw new IllegalArgumentException("Certification Key MUST have KeyFlag CERTIFY_OTHER"); - } if (!keyIsCertificationCapable(spec)) { throw new IllegalArgumentException("Key algorithm " + spec.getKeyType().getName() + " is not capable of creating certifications."); } } - private boolean hasCertifyOthersFlag(KeySpec keySpec) { - KeyFlags keyFlags = keySpec.getSubpacketGenerator().getKeyFlagsSubpacket(); - return keyFlags != null && KeyFlag.hasKeyFlag(keyFlags.getFlags(), KeyFlag.CERTIFY_OTHER); - } - private boolean keyIsCertificationCapable(KeySpec keySpec) { return keySpec.getKeyType().canCertify(); } diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeySpec.java b/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeySpec.java index 63645edd..6ac08aa7 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeySpec.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeySpec.java @@ -56,7 +56,7 @@ public class KeySpec { return keyCreationDate; } - public static KeySpecBuilder getBuilder(KeyType type, KeyFlag flag, KeyFlag... flags) { - return new KeySpecBuilder(type, flag, flags); + public static KeySpecBuilder getBuilder(KeyType type, KeyFlag... flags) { + return new KeySpecBuilder(type, flags); } } diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeySpecBuilder.java b/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeySpecBuilder.java index 559dd3ce..1abd7dcd 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeySpecBuilder.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/generation/KeySpecBuilder.java @@ -21,7 +21,6 @@ import org.pgpainless.key.generation.type.KeyType; import org.pgpainless.signature.subpackets.SelfSignatureSubpackets; import org.pgpainless.signature.subpackets.SignatureSubpackets; import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil; -import org.pgpainless.util.CollectionUtils; public class KeySpecBuilder implements KeySpecBuilderInterface { @@ -34,17 +33,14 @@ public class KeySpecBuilder implements KeySpecBuilderInterface { private Set preferredSymmetricAlgorithms = algorithmSuite.getSymmetricKeyAlgorithms(); private Date keyCreationDate; - KeySpecBuilder(@Nonnull KeyType type, KeyFlag flag, KeyFlag... flags) { - if (flag == null) { - throw new IllegalArgumentException("Key MUST carry at least one key flag"); - } + KeySpecBuilder(@Nonnull KeyType type, KeyFlag... flags) { if (flags == null) { - throw new IllegalArgumentException("List of additional flags MUST NOT be null."); + this.keyFlags = new KeyFlag[0]; + } else { + SignatureSubpacketsUtil.assureKeyCanCarryFlags(type, flags); + this.keyFlags = flags; } - flags = CollectionUtils.concat(flag, flags); - SignatureSubpacketsUtil.assureKeyCanCarryFlags(type, flags); this.type = type; - this.keyFlags = flags; } @Override diff --git a/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java b/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java index 77277622..31a6dd86 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java +++ b/pgpainless-core/src/main/java/org/pgpainless/key/info/KeyRingInfo.java @@ -1170,6 +1170,10 @@ public class KeyRingInfo { return new KeyAccessor.SubKey(this, new SubkeyIdentifier(keys, keyId)).getPreferredCompressionAlgorithms(); } + public boolean isUsableForThirdPartyCertification() { + return isKeyValidlyBound(getKeyId()) && getKeyFlagsOf(getKeyId()).contains(KeyFlag.CERTIFY_OTHER); + } + /** * Returns true, if the certificate has at least one usable encryption subkey. * diff --git a/pgpainless-core/src/test/java/org/pgpainless/key/generation/GenerateKeyWithoutPrimaryKeyFlagsTest.java b/pgpainless-core/src/test/java/org/pgpainless/key/generation/GenerateKeyWithoutPrimaryKeyFlagsTest.java new file mode 100644 index 00000000..e2db311d --- /dev/null +++ b/pgpainless-core/src/test/java/org/pgpainless/key/generation/GenerateKeyWithoutPrimaryKeyFlagsTest.java @@ -0,0 +1,53 @@ +// SPDX-FileCopyrightText: 2023 Paul Schaub +// +// SPDX-License-Identifier: Apache-2.0 + +package org.pgpainless.key.generation; + +import org.bouncycastle.openpgp.PGPException; +import org.bouncycastle.openpgp.PGPPublicKeyRing; +import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.junit.jupiter.api.Test; +import org.pgpainless.PGPainless; +import org.pgpainless.algorithm.KeyFlag; +import org.pgpainless.exception.KeyException; +import org.pgpainless.key.TestKeys; +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 java.io.IOException; +import java.security.InvalidAlgorithmParameterException; +import java.security.NoSuchAlgorithmException; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class GenerateKeyWithoutPrimaryKeyFlagsTest { + + @Test + public void generateKeyWithoutCertifyKeyFlag_cannotCertifyThirdParties() throws PGPException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, IOException { + PGPSecretKeyRing secretKeys = PGPainless.buildKeyRing().setPrimaryKey(KeySpec.getBuilder(KeyType.EDDSA(EdDSACurve._Ed25519))) + .addSubkey(KeySpec.getBuilder(KeyType.EDDSA(EdDSACurve._Ed25519), KeyFlag.SIGN_DATA)) + .addSubkey(KeySpec.getBuilder(KeyType.XDH(XDHSpec._X25519), KeyFlag.ENCRYPT_STORAGE, KeyFlag.ENCRYPT_COMMS)) + .addUserId("Alice") + .build(); + + KeyRingInfo info = PGPainless.inspectKeyRing(secretKeys); + assertTrue(info.getValidUserIds().contains("Alice")); + + long primaryKeyId = info.getKeyId(); + assertTrue(info.getKeyFlagsOf("Alice").isEmpty()); + assertTrue(info.getKeyFlagsOf(primaryKeyId).isEmpty()); + assertFalse(info.isUsableForThirdPartyCertification()); + + // Key without CERTIFY_OTHER flag cannot be used to certify other keys + PGPPublicKeyRing thirdPartyCert = TestKeys.getCryptiePublicKeyRing(); + assertThrows(KeyException.UnacceptableThirdPartyCertificationKeyException.class, () -> + PGPainless.certify().certificate(thirdPartyCert) + .withKey(secretKeys, SecretKeyRingProtector.unprotectedKeys())); + } +}