Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8275887: jarsigner prints invalid digest/signature algorithm warnings if keysize is weak/disabled #6296

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions src/java.base/share/classes/sun/security/pkcs/SignerInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,12 @@ public class SignerInfo implements DerEncoder {
/**
* A map containing the algorithms in this SignerInfo. This is used to
* avoid checking algorithms to see if they are disabled more than once.
* The key is the AlgorithmId of the algorithm, and the value is the name of
* the field or attribute.
* The key is the AlgorithmId of the algorithm, and the value is a record
* containing the name of the field or attribute and whether the key
* should also be checked (ex: if it is a signature algorithm).
*/
private Map<AlgorithmId, String> algorithms = new HashMap<>();
private record AlgorithmInfo(String field, boolean checkKey) {}
private Map<AlgorithmId, AlgorithmInfo> algorithms = new HashMap<>();

public SignerInfo(X500Name issuerName,
BigInteger serial,
Expand Down Expand Up @@ -350,7 +352,8 @@ SignerInfo verify(PKCS7 block, byte[] data)
}

String digestAlgName = digestAlgorithmId.getName();
algorithms.put(digestAlgorithmId, "SignerInfo digestAlgorithm field");
algorithms.put(digestAlgorithmId,
new AlgorithmInfo("SignerInfo digestAlgorithm field", false));

byte[] dataSigned;

Expand Down Expand Up @@ -421,7 +424,8 @@ SignerInfo verify(PKCS7 block, byte[] data)
new AlgorithmId(ObjectIdentifier.of(oid),
digestEncryptionAlgorithmId.getParameters());
algorithms.put(sigAlgId,
"SignerInfo digestEncryptionAlgorithm field");
new AlgorithmInfo(
"SignerInfo digestEncryptionAlgorithm field", true));
}

X509Certificate cert = getCertificate(block);
Expand Down Expand Up @@ -677,7 +681,8 @@ private void verifyTimestamp(TimestampToken token)
throws NoSuchAlgorithmException, SignatureException {

AlgorithmId digestAlgId = token.getHashAlgorithm();
algorithms.put(digestAlgId, "TimestampToken digestAlgorithm field");
algorithms.put(digestAlgId,
new AlgorithmInfo("TimestampToken digestAlgorithm field", false));

MessageDigest md = MessageDigest.getInstance(digestAlgId.getName());

Expand Down Expand Up @@ -734,18 +739,19 @@ public String toString() {
*/
public static Set<String> verifyAlgorithms(SignerInfo[] infos,
JarConstraintsParameters params, String name) throws SignatureException {
Map<AlgorithmId, String> algorithms = new HashMap<>();
Map<AlgorithmId, AlgorithmInfo> algorithms = new HashMap<>();
for (SignerInfo info : infos) {
algorithms.putAll(info.algorithms);
}

Set<String> enabledAlgorithms = new HashSet<>();
try {
for (Map.Entry<AlgorithmId, String> algorithm : algorithms.entrySet()) {
params.setExtendedExceptionMsg(name, algorithm.getValue());
AlgorithmId algId = algorithm.getKey();
for (var algEntry : algorithms.entrySet()) {
AlgorithmInfo info = algEntry.getValue();
params.setExtendedExceptionMsg(name, info.field());
AlgorithmId algId = algEntry.getKey();
JAR_DISABLED_CHECK.permits(algId.getName(),
algId.getParameters(), params);
algId.getParameters(), params, info.checkKey());
enabledAlgorithms.add(algId.getName());
}
} catch (CertPathValidatorException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import java.security.AlgorithmParameters;
import java.security.GeneralSecurityException;
import java.security.cert.Certificate;
import java.security.cert.X509CRL;
import java.security.cert.X509Certificate;
import java.security.cert.PKIXCertPathChecker;
import java.security.cert.TrustAnchor;
Expand All @@ -57,7 +56,6 @@
import sun.security.validator.Validator;
import sun.security.x509.AlgorithmId;
import sun.security.x509.X509CertImpl;
import sun.security.x509.X509CRLImpl;

/**
* A {@code PKIXCertPathChecker} implementation to check whether a
Expand Down Expand Up @@ -226,13 +224,13 @@ public void check(Certificate cert,
CertPathConstraintsParameters cp =
new CertPathConstraintsParameters(trustedPubKey, variant,
anchor, date);
dac.permits(trustedPubKey.getAlgorithm(), cp);
dac.permits(trustedPubKey.getAlgorithm(), cp, true);
}
// Check the signature algorithm and parameters against constraints
CertPathConstraintsParameters cp =
new CertPathConstraintsParameters(x509Cert, variant,
anchor, date);
dac.permits(currSigAlg, currSigAlgParams, cp);
dac.permits(currSigAlg, currSigAlgParams, cp, true);
} else {
if (prevPubKey != null) {
if (!constraints.permits(SIGNATURE_PRIMITIVE_SET,
Expand Down Expand Up @@ -362,29 +360,6 @@ void trySetTrustAnchor(TrustAnchor anchor) {
}
}

/**
* Check the signature algorithm with the specified public key.
*
* @param key the public key to verify the CRL signature
* @param crl the target CRL
* @param variant the Validator variant of the operation. A null value
* passed will set it to Validator.GENERIC.
* @param anchor the trust anchor selected to validate the CRL issuer
*/
static void check(PublicKey key, X509CRL crl, String variant,
TrustAnchor anchor) throws CertPathValidatorException {

X509CRLImpl x509CRLImpl = null;
try {
x509CRLImpl = X509CRLImpl.toImpl(crl);
} catch (CRLException ce) {
throw new CertPathValidatorException(ce);
}

AlgorithmId algorithmId = x509CRLImpl.getSigAlgId();
check(key, algorithmId, variant, anchor);
}

/**
* Check the signature algorithm with the specified public key.
*
Expand All @@ -399,7 +374,7 @@ static void check(PublicKey key, AlgorithmId algorithmId, String variant,

DisabledAlgorithmConstraints.certPathConstraints().permits(
algorithmId.getName(), algorithmId.getParameters(),
new CertPathConstraintsParameters(key, variant, anchor, null));
new CertPathConstraintsParameters(key, variant, anchor, null), true);
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,8 @@ static boolean verifyCRL(X509CertImpl certImpl, DistributionPoint point,

// check the crl signature algorithm
try {
AlgorithmChecker.check(prevKey, crl, variant, anchor);
AlgorithmChecker.check(prevKey, crlImpl.getSigAlgId(),
variant, anchor);
} catch (CertPathValidatorException cpve) {
if (debug != null) {
debug.println("CRL signature algorithm check failed: " + cpve);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@ public final boolean permits(Set<CryptoPrimitive> primitives,
}

public final void permits(String algorithm, AlgorithmParameters ap,
ConstraintsParameters cp) throws CertPathValidatorException {

permits(algorithm, cp);
ConstraintsParameters cp, boolean checkKey)
throws CertPathValidatorException {
permits(algorithm, cp, checkKey);
if (ap != null) {
permits(ap, cp);
}
Expand All @@ -219,36 +219,36 @@ private void permitsPSSParams(AlgorithmParameters ap,
PSSParameterSpec pssParams =
ap.getParameterSpec(PSSParameterSpec.class);
String digestAlg = pssParams.getDigestAlgorithm();
permits(digestAlg, cp);
permits(digestAlg, cp, false);
AlgorithmParameterSpec mgfParams = pssParams.getMGFParameters();
if (mgfParams instanceof MGF1ParameterSpec) {
String mgfDigestAlg =
((MGF1ParameterSpec)mgfParams).getDigestAlgorithm();
if (!mgfDigestAlg.equalsIgnoreCase(digestAlg)) {
permits(mgfDigestAlg, cp);
permits(mgfDigestAlg, cp, false);
}
}
} catch (InvalidParameterSpecException ipse) {
// ignore
}
}

public final void permits(String algorithm, ConstraintsParameters cp)
throws CertPathValidatorException {

// Check if named curves in the key are disabled.
for (Key key : cp.getKeys()) {
for (String curve : getNamedCurveFromKey(key)) {
if (!checkAlgorithm(disabledAlgorithms, curve, decomposer)) {
throw new CertPathValidatorException(
public final void permits(String algorithm, ConstraintsParameters cp,
boolean checkKey) throws CertPathValidatorException {
if (checkKey) {
// Check if named curves in the key are disabled.
for (Key key : cp.getKeys()) {
for (String curve : getNamedCurveFromKey(key)) {
if (!checkAlgorithm(disabledAlgorithms, curve, decomposer)) {
throw new CertPathValidatorException(
"Algorithm constraints check failed on disabled " +
"algorithm: " + curve,
null, null, -1, BasicReason.ALGORITHM_CONSTRAINED);
}
}
}
}

algorithmConstraints.permits(algorithm, cp);
algorithmConstraints.permits(algorithm, cp, checkKey);
}

private static List<String> getNamedCurveFromKey(Key key) {
Expand Down Expand Up @@ -481,8 +481,8 @@ public boolean permits(String algorithm, AlgorithmParameters aps) {
return true;
}

public void permits(String algorithm, ConstraintsParameters cp)
throws CertPathValidatorException {
public void permits(String algorithm, ConstraintsParameters cp,
boolean checkKey) throws CertPathValidatorException {

if (debug != null) {
debug.println("Constraints.permits(): " + algorithm + ", "
Expand All @@ -496,8 +496,10 @@ public void permits(String algorithm, ConstraintsParameters cp)
algorithms.add(algorithm);
}

for (Key key : cp.getKeys()) {
algorithms.add(key.getAlgorithm());
if (checkKey) {
for (Key key : cp.getKeys()) {
algorithms.add(key.getAlgorithm());
}
}

// Check all applicable constraints
Expand All @@ -507,6 +509,9 @@ public void permits(String algorithm, ConstraintsParameters cp)
continue;
}
for (Constraint constraint : list) {
if (!checkKey && constraint instanceof KeySizeConstraint) {
continue;
}
constraint.permits(cp);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,11 @@ public JarConstraintsParameters(CodeSigner[] signers) {
this.timestamp = latestTimestamp;
}

public JarConstraintsParameters(List<X509Certificate> chain, Timestamp timestamp) {
public JarConstraintsParameters(List<X509Certificate> chain, Date timestamp) {
this.keys = new HashSet<>();
this.certsIssuedByAnchor = new HashSet<>();
addToCertsAndKeys(chain);
if (timestamp != null) {
addToCertsAndKeys(timestamp.getSignerCertPath());
this.timestamp = timestamp.getTimestamp();
} else {
this.timestamp = null;
}
this.timestamp = timestamp;
}

// extract last certificate and signer's public key from chain
Expand Down Expand Up @@ -178,7 +173,7 @@ public void setExtendedExceptionMsg(String file, String target) {

@Override
public String extendedExceptionMsg() {
return message;
return message == null ? "." : message;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -217,7 +217,7 @@ public CodeSigner[] verify(Hashtable<String, CodeSigner[]> verifiedSigners,
params.setExtendedExceptionMsg(JarFile.MANIFEST_NAME,
name + " entry");
DisabledAlgorithmConstraints.jarConstraints()
.permits(digest.getAlgorithm(), params);
.permits(digest.getAlgorithm(), params, false);
} catch (GeneralSecurityException e) {
if (debug != null) {
debug.println("Digest algorithm is restricted: " + e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ private boolean permittedCheck(String key, String algorithm) {
try {
params.setExtendedExceptionMsg(name + ".SF", key + " attribute");
DisabledAlgorithmConstraints
.jarConstraints().permits(algorithm, params);
.jarConstraints().permits(algorithm, params, false);
} catch (GeneralSecurityException e) {
permittedAlgs.put(algorithm, Boolean.FALSE);
permittedAlgs.put(key.toUpperCase(), Boolean.FALSE);
Expand Down
Loading