mirror of
https://github.com/pgpainless/pgpainless.git
synced 2024-12-23 11:27:57 +01:00
Add PublicKeyAlgorithmPolicy to reject weak public keys
BCs PGPPublicKey.getBitStrenght() appears to fail to recognize some elliptic curves. In such cases, bitStrength is reported as -1. I added BCUtil.getBitStrength(publicKey) to manually determine the bit strenght by OID. See https://github.com/bcgit/bc-java/issues/972 for an upstream bug report.
This commit is contained in:
parent
56ddd5e70f
commit
5bb4fd3687
5 changed files with 146 additions and 0 deletions
|
@ -17,10 +17,13 @@ package org.pgpainless.policy;
|
||||||
|
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
|
import java.util.HashMap;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
import java.util.Map;
|
||||||
|
|
||||||
import org.pgpainless.algorithm.CompressionAlgorithm;
|
import org.pgpainless.algorithm.CompressionAlgorithm;
|
||||||
import org.pgpainless.algorithm.HashAlgorithm;
|
import org.pgpainless.algorithm.HashAlgorithm;
|
||||||
|
import org.pgpainless.algorithm.PublicKeyAlgorithm;
|
||||||
import org.pgpainless.algorithm.SymmetricKeyAlgorithm;
|
import org.pgpainless.algorithm.SymmetricKeyAlgorithm;
|
||||||
import org.pgpainless.util.NotationRegistry;
|
import org.pgpainless.util.NotationRegistry;
|
||||||
|
|
||||||
|
@ -41,6 +44,8 @@ public final class Policy {
|
||||||
SymmetricKeyAlgorithmPolicy.defaultSymmetricKeyDecryptionAlgorithmPolicy();
|
SymmetricKeyAlgorithmPolicy.defaultSymmetricKeyDecryptionAlgorithmPolicy();
|
||||||
private CompressionAlgorithmPolicy compressionAlgorithmPolicy =
|
private CompressionAlgorithmPolicy compressionAlgorithmPolicy =
|
||||||
CompressionAlgorithmPolicy.defaultCompressionAlgorithmPolicy();
|
CompressionAlgorithmPolicy.defaultCompressionAlgorithmPolicy();
|
||||||
|
private PublicKeyAlgorithmPolicy publicKeyAlgorithmPolicy =
|
||||||
|
PublicKeyAlgorithmPolicy.defaultPublicKeyAlgorithmPolicy();
|
||||||
private final NotationRegistry notationRegistry = new NotationRegistry();
|
private final NotationRegistry notationRegistry = new NotationRegistry();
|
||||||
|
|
||||||
Policy() {
|
Policy() {
|
||||||
|
@ -156,6 +161,27 @@ public final class Policy {
|
||||||
this.compressionAlgorithmPolicy = policy;
|
this.compressionAlgorithmPolicy = policy;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Return the current public key algorithm policy.
|
||||||
|
*
|
||||||
|
* @return public key algorithm policy
|
||||||
|
*/
|
||||||
|
public PublicKeyAlgorithmPolicy getPublicKeyAlgorithmPolicy() {
|
||||||
|
return publicKeyAlgorithmPolicy;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Set a custom public key algorithm policy.
|
||||||
|
*
|
||||||
|
* @param publicKeyAlgorithmPolicy custom policy
|
||||||
|
*/
|
||||||
|
public void setPublicKeyAlgorithmPolicy(PublicKeyAlgorithmPolicy publicKeyAlgorithmPolicy) {
|
||||||
|
if (publicKeyAlgorithmPolicy == null) {
|
||||||
|
throw new NullPointerException("Public key algorithm policy cannot be null.");
|
||||||
|
}
|
||||||
|
this.publicKeyAlgorithmPolicy = publicKeyAlgorithmPolicy;
|
||||||
|
}
|
||||||
|
|
||||||
public static final class SymmetricKeyAlgorithmPolicy {
|
public static final class SymmetricKeyAlgorithmPolicy {
|
||||||
|
|
||||||
private final SymmetricKeyAlgorithm defaultSymmetricKeyAlgorithm;
|
private final SymmetricKeyAlgorithm defaultSymmetricKeyAlgorithm;
|
||||||
|
@ -344,6 +370,66 @@ public final class Policy {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public static final class PublicKeyAlgorithmPolicy {
|
||||||
|
|
||||||
|
private final Map<PublicKeyAlgorithm, Integer> algorithmStrengths = new HashMap<>();
|
||||||
|
|
||||||
|
public PublicKeyAlgorithmPolicy(Map<PublicKeyAlgorithm, Integer> minimalAlgorithmBitStrengths) {
|
||||||
|
this.algorithmStrengths.putAll(minimalAlgorithmBitStrengths);
|
||||||
|
}
|
||||||
|
|
||||||
|
public boolean isAcceptable(int algorithmId, int bitStrength) {
|
||||||
|
return isAcceptable(PublicKeyAlgorithm.fromId(algorithmId), bitStrength);
|
||||||
|
}
|
||||||
|
|
||||||
|
public boolean isAcceptable(PublicKeyAlgorithm algorithm, int bitStrength) {
|
||||||
|
if (!algorithmStrengths.containsKey(algorithm)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
int minStrength = algorithmStrengths.get(algorithm);
|
||||||
|
return bitStrength >= minStrength;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Return PGPainless' default public key algorithm policy.
|
||||||
|
* This policy is based upon recommendations made by the German Federal Office for Information Security (BSI).
|
||||||
|
*
|
||||||
|
* Basically this policy requires keys based on elliptic curves to have a bit strength of at least 250,
|
||||||
|
* and keys based on prime number factorization / discrete logarithm problems to have a strength of at least 2000 bits.
|
||||||
|
*
|
||||||
|
* @see <a href="https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/TechGuidelines/TG02102/BSI-TR-02102-1.pdf">
|
||||||
|
* BSI - Technical Guideline - Cryptographic Mechanisms: Recommendations and Key Lengths (2021-01)</a>
|
||||||
|
*
|
||||||
|
* @return default algorithm policy
|
||||||
|
*/
|
||||||
|
public static PublicKeyAlgorithmPolicy defaultPublicKeyAlgorithmPolicy() {
|
||||||
|
Map<PublicKeyAlgorithm, Integer> minimalBitStrengths = new HashMap<>();
|
||||||
|
// §5.4.1
|
||||||
|
minimalBitStrengths.put(PublicKeyAlgorithm.RSA_GENERAL, 2000);
|
||||||
|
minimalBitStrengths.put(PublicKeyAlgorithm.RSA_SIGN, 2000);
|
||||||
|
minimalBitStrengths.put(PublicKeyAlgorithm.RSA_ENCRYPT, 2000);
|
||||||
|
// TODO: ElGamal is not mentioned in the BSI document.
|
||||||
|
// We assume that the requirements are similar to other DH algorithms
|
||||||
|
minimalBitStrengths.put(PublicKeyAlgorithm.ELGAMAL_ENCRYPT, 2000);
|
||||||
|
minimalBitStrengths.put(PublicKeyAlgorithm.ELGAMAL_GENERAL, 2000);
|
||||||
|
// §5.4.2
|
||||||
|
minimalBitStrengths.put(PublicKeyAlgorithm.DSA, 2000);
|
||||||
|
// §5.4.3
|
||||||
|
minimalBitStrengths.put(PublicKeyAlgorithm.ECDSA, 250);
|
||||||
|
// TODO: EdDSA is not mentioned in the BSI document.
|
||||||
|
// We assume that the requirements are similar to other EC algorithms.
|
||||||
|
minimalBitStrengths.put(PublicKeyAlgorithm.EDDSA, 250);
|
||||||
|
// §7.2.1
|
||||||
|
minimalBitStrengths.put(PublicKeyAlgorithm.DIFFIE_HELLMAN, 2000);
|
||||||
|
// §7.2.2
|
||||||
|
minimalBitStrengths.put(PublicKeyAlgorithm.ECDH, 250);
|
||||||
|
minimalBitStrengths.put(PublicKeyAlgorithm.EC, 250);
|
||||||
|
|
||||||
|
return new PublicKeyAlgorithmPolicy(minimalBitStrengths);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Return the {@link NotationRegistry} of PGPainless.
|
* Return the {@link NotationRegistry} of PGPainless.
|
||||||
* The notation registry is used to decide, whether or not a Notation is known or not.
|
* The notation registry is used to decide, whether or not a Notation is known or not.
|
||||||
|
|
|
@ -40,6 +40,7 @@ import org.pgpainless.exception.SignatureValidationException;
|
||||||
import org.pgpainless.implementation.ImplementationFactory;
|
import org.pgpainless.implementation.ImplementationFactory;
|
||||||
import org.pgpainless.policy.Policy;
|
import org.pgpainless.policy.Policy;
|
||||||
import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil;
|
import org.pgpainless.signature.subpackets.SignatureSubpacketsUtil;
|
||||||
|
import org.pgpainless.util.BCUtil;
|
||||||
import org.pgpainless.util.NotationRegistry;
|
import org.pgpainless.util.NotationRegistry;
|
||||||
|
|
||||||
public abstract class SignatureValidator {
|
public abstract class SignatureValidator {
|
||||||
|
@ -265,6 +266,21 @@ public abstract class SignatureValidator {
|
||||||
signatureDoesNotHaveCriticalUnknownNotations(policy.getNotationRegistry()).verify(signature);
|
signatureDoesNotHaveCriticalUnknownNotations(policy.getNotationRegistry()).verify(signature);
|
||||||
signatureDoesNotHaveCriticalUnknownSubpackets().verify(signature);
|
signatureDoesNotHaveCriticalUnknownSubpackets().verify(signature);
|
||||||
signatureUsesAcceptableHashAlgorithm(policy).verify(signature);
|
signatureUsesAcceptableHashAlgorithm(policy).verify(signature);
|
||||||
|
signatureUsesAcceptablePublicKeyAlgorithm(policy, signingKey).verify(signature);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
private static SignatureValidator signatureUsesAcceptablePublicKeyAlgorithm(Policy policy, PGPPublicKey signingKey) {
|
||||||
|
return new SignatureValidator() {
|
||||||
|
@Override
|
||||||
|
public void verify(PGPSignature signature) throws SignatureValidationException {
|
||||||
|
PublicKeyAlgorithm algorithm = PublicKeyAlgorithm.fromId(signingKey.getAlgorithm());
|
||||||
|
int bitStrength = BCUtil.getBitStrenght(signingKey);
|
||||||
|
if (!policy.getPublicKeyAlgorithmPolicy().isAcceptable(algorithm, bitStrength)) {
|
||||||
|
throw new SignatureValidationException("Signature was made using unacceptable key. " +
|
||||||
|
algorithm + " (" + bitStrength + " bits) is not acceptable according to the public key algorithm policy.");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
|
@ -20,6 +20,9 @@ import java.io.IOException;
|
||||||
import java.io.InputStream;
|
import java.io.InputStream;
|
||||||
import javax.annotation.Nonnull;
|
import javax.annotation.Nonnull;
|
||||||
|
|
||||||
|
import org.bouncycastle.asn1.ASN1ObjectIdentifier;
|
||||||
|
import org.bouncycastle.bcpg.ECPublicBCPGKey;
|
||||||
|
import org.bouncycastle.openpgp.PGPPublicKey;
|
||||||
import org.bouncycastle.openpgp.PGPUtil;
|
import org.bouncycastle.openpgp.PGPUtil;
|
||||||
|
|
||||||
public class BCUtil {
|
public class BCUtil {
|
||||||
|
@ -34,4 +37,26 @@ public class BCUtil {
|
||||||
return PGPUtil.getDecoderStream(inputStream);
|
return PGPUtil.getDecoderStream(inputStream);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public static int getBitStrenght(PGPPublicKey key) {
|
||||||
|
int bitStrength = key.getBitStrength();
|
||||||
|
|
||||||
|
if (bitStrength == -1) {
|
||||||
|
// TODO: BC's PGPPublicKey.getBitStrength() does fail for some keys (EdDSA, X25519)
|
||||||
|
// Manually set the bit strength.
|
||||||
|
|
||||||
|
ASN1ObjectIdentifier oid = ((ECPublicBCPGKey) key.getPublicKeyPacket().getKey()).getCurveOID();
|
||||||
|
if (oid.getId().equals("1.3.6.1.4.1.11591.15.1")) {
|
||||||
|
// ed25519 is 256 bits
|
||||||
|
bitStrength = 256;
|
||||||
|
} else if (oid.getId().equals("1.3.6.1.4.1.3029.1.5.1")) {
|
||||||
|
// curvey25519 is 256 bits
|
||||||
|
bitStrength = 256;
|
||||||
|
} else {
|
||||||
|
throw new RuntimeException("Unknown curve: " + oid.getId());
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
||||||
|
return bitStrength;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -41,6 +41,7 @@ import org.pgpainless.key.generation.type.rsa.RsaLength;
|
||||||
import org.pgpainless.key.generation.type.xdh.XDHSpec;
|
import org.pgpainless.key.generation.type.xdh.XDHSpec;
|
||||||
import org.pgpainless.key.info.KeyInfo;
|
import org.pgpainless.key.info.KeyInfo;
|
||||||
import org.pgpainless.key.util.UserId;
|
import org.pgpainless.key.util.UserId;
|
||||||
|
import org.pgpainless.util.BCUtil;
|
||||||
import org.pgpainless.util.Passphrase;
|
import org.pgpainless.util.Passphrase;
|
||||||
|
|
||||||
public class BrainpoolKeyGenerationTest {
|
public class BrainpoolKeyGenerationTest {
|
||||||
|
@ -115,18 +116,22 @@ public class BrainpoolKeyGenerationTest {
|
||||||
PGPSecretKey ecdsaPrim = iterator.next();
|
PGPSecretKey ecdsaPrim = iterator.next();
|
||||||
KeyInfo ecdsaInfo = new KeyInfo(ecdsaPrim);
|
KeyInfo ecdsaInfo = new KeyInfo(ecdsaPrim);
|
||||||
assertEquals(EllipticCurve._BRAINPOOLP384R1.getName(), ecdsaInfo.getCurveName());
|
assertEquals(EllipticCurve._BRAINPOOLP384R1.getName(), ecdsaInfo.getCurveName());
|
||||||
|
assertEquals(384, BCUtil.getBitStrenght(ecdsaPrim.getPublicKey()));
|
||||||
|
|
||||||
PGPSecretKey eddsaSub = iterator.next();
|
PGPSecretKey eddsaSub = iterator.next();
|
||||||
KeyInfo eddsaInfo = new KeyInfo(eddsaSub);
|
KeyInfo eddsaInfo = new KeyInfo(eddsaSub);
|
||||||
assertEquals(EdDSACurve._Ed25519.getName(), eddsaInfo.getCurveName());
|
assertEquals(EdDSACurve._Ed25519.getName(), eddsaInfo.getCurveName());
|
||||||
|
assertEquals(256, BCUtil.getBitStrenght(eddsaSub.getPublicKey()));
|
||||||
|
|
||||||
PGPSecretKey xdhSub = iterator.next();
|
PGPSecretKey xdhSub = iterator.next();
|
||||||
KeyInfo xdhInfo = new KeyInfo(xdhSub);
|
KeyInfo xdhInfo = new KeyInfo(xdhSub);
|
||||||
assertEquals(XDHSpec._X25519.getCurveName(), xdhInfo.getCurveName());
|
assertEquals(XDHSpec._X25519.getCurveName(), xdhInfo.getCurveName());
|
||||||
|
assertEquals(256, BCUtil.getBitStrenght(xdhSub.getPublicKey()));
|
||||||
|
|
||||||
PGPSecretKey rsaSub = iterator.next();
|
PGPSecretKey rsaSub = iterator.next();
|
||||||
KeyInfo rsaInfo = new KeyInfo(rsaSub);
|
KeyInfo rsaInfo = new KeyInfo(rsaSub);
|
||||||
assertThrows(IllegalArgumentException.class, rsaInfo::getCurveName, "RSA is not a curve-based encryption system");
|
assertThrows(IllegalArgumentException.class, rsaInfo::getCurveName, "RSA is not a curve-based encryption system");
|
||||||
|
assertEquals(3072, BCUtil.getBitStrenght(rsaSub.getPublicKey()));
|
||||||
}
|
}
|
||||||
|
|
||||||
public PGPSecretKeyRing generateKey(KeySpec primaryKey, KeySpec subKey, String userId) throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, PGPException {
|
public PGPSecretKeyRing generateKey(KeySpec primaryKey, KeySpec subKey, String userId) throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, PGPException {
|
||||||
|
|
|
@ -25,6 +25,7 @@ import org.junit.jupiter.api.BeforeAll;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.pgpainless.algorithm.CompressionAlgorithm;
|
import org.pgpainless.algorithm.CompressionAlgorithm;
|
||||||
import org.pgpainless.algorithm.HashAlgorithm;
|
import org.pgpainless.algorithm.HashAlgorithm;
|
||||||
|
import org.pgpainless.algorithm.PublicKeyAlgorithm;
|
||||||
import org.pgpainless.algorithm.SymmetricKeyAlgorithm;
|
import org.pgpainless.algorithm.SymmetricKeyAlgorithm;
|
||||||
|
|
||||||
public class PolicyTest {
|
public class PolicyTest {
|
||||||
|
@ -48,6 +49,8 @@ public class PolicyTest {
|
||||||
|
|
||||||
policy.setRevocationSignatureHashAlgorithmPolicy(new Policy.HashAlgorithmPolicy(HashAlgorithm.SHA512,
|
policy.setRevocationSignatureHashAlgorithmPolicy(new Policy.HashAlgorithmPolicy(HashAlgorithm.SHA512,
|
||||||
Arrays.asList(HashAlgorithm.SHA512, HashAlgorithm.SHA384, HashAlgorithm.SHA256, HashAlgorithm.SHA224, HashAlgorithm.SHA1)));
|
Arrays.asList(HashAlgorithm.SHA512, HashAlgorithm.SHA384, HashAlgorithm.SHA256, HashAlgorithm.SHA224, HashAlgorithm.SHA1)));
|
||||||
|
|
||||||
|
policy.setPublicKeyAlgorithmPolicy(Policy.PublicKeyAlgorithmPolicy.defaultPublicKeyAlgorithmPolicy());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -130,6 +133,17 @@ public class PolicyTest {
|
||||||
assertEquals(HashAlgorithm.SHA512, policy.getRevocationSignatureHashAlgorithmPolicy().defaultHashAlgorithm());
|
assertEquals(HashAlgorithm.SHA512, policy.getRevocationSignatureHashAlgorithmPolicy().defaultHashAlgorithm());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testAcceptablePublicKeyAlgorithm() {
|
||||||
|
assertTrue(policy.getPublicKeyAlgorithmPolicy().isAcceptable(PublicKeyAlgorithm.ECDSA, 256));
|
||||||
|
assertTrue(policy.getPublicKeyAlgorithmPolicy().isAcceptable(PublicKeyAlgorithm.RSA_GENERAL, 3072));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testUnacceptablePublicKeyAlgorithm() {
|
||||||
|
assertFalse(policy.getPublicKeyAlgorithmPolicy().isAcceptable(PublicKeyAlgorithm.RSA_GENERAL, 1024));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testNotationRegistry() {
|
public void testNotationRegistry() {
|
||||||
assertFalse(policy.getNotationRegistry().isKnownNotation("notation@pgpainless.org"));
|
assertFalse(policy.getNotationRegistry().isKnownNotation("notation@pgpainless.org"));
|
||||||
|
|
Loading…
Reference in a new issue