stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
Reviewed-by: weijun
This commit is contained in:
Sean Mullan 2021-11-19 14:36:07 +00:00
parent 936f7ff49e
commit 03f8c0fb93
9 changed files with 90 additions and 89 deletions
src
java.base/share/classes/sun/security
pkcs
SignerInfo.java
provider/certpath
AlgorithmChecker.javaDistributionPointFetcher.java
util
DisabledAlgorithmConstraints.javaJarConstraintsParameters.javaManifestEntryVerifier.javaSignatureFileVerifier.java
jdk.jartool/share/classes/sun/security/tools/jarsigner
Main.java
test/jdk/sun/security/tools/jarsigner
TimestampCheck.java

28
src/java.base/share/classes/sun/security/pkcs/SignerInfo.java

@ -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,
@ -350,7 +352,8 @@ public class SignerInfo implements DerEncoder {
}
String digestAlgName = digestAlgorithmId.getName();
algorithms.put(digestAlgorithmId, "SignerInfo digestAlgorithm field");
algorithms.put(digestAlgorithmId,
new AlgorithmInfo("SignerInfo digestAlgorithm field", false));
byte[] dataSigned;
@ -421,7 +424,8 @@ public class SignerInfo implements DerEncoder {
new AlgorithmId(ObjectIdentifier.of(oid),
digestEncryptionAlgorithmId.getParameters());
algorithms.put(sigAlgId,
"SignerInfo digestEncryptionAlgorithm field");
new AlgorithmInfo(
"SignerInfo digestEncryptionAlgorithm field", true));
}
X509Certificate cert = getCertificate(block);
@ -677,7 +681,8 @@ public class SignerInfo implements DerEncoder {
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());
@ -734,18 +739,19 @@ public class SignerInfo implements DerEncoder {
*/
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) {

31
src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java

@ -38,7 +38,6 @@ import java.security.KeyFactory;
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;
@ -57,7 +56,6 @@ import sun.security.util.DisabledAlgorithmConstraints;
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
@ -226,13 +224,13 @@ public final class AlgorithmChecker extends PKIXCertPathChecker {
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,
@ -362,29 +360,6 @@ public final class AlgorithmChecker extends PKIXCertPathChecker {
}
}
/**
* 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.
*
@ -399,7 +374,7 @@ public final class AlgorithmChecker extends PKIXCertPathChecker {
DisabledAlgorithmConstraints.certPathConstraints().permits(
algorithmId.getName(), algorithmId.getParameters(),
new CertPathConstraintsParameters(key, variant, anchor, null));
new CertPathConstraintsParameters(key, variant, anchor, null), true);
}
}

3
src/java.base/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java

@ -653,7 +653,8 @@ public class DistributionPointFetcher {
// 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);

43
src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java

@ -192,9 +192,9 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
}
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);
}
@ -219,13 +219,13 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
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) {
@ -233,22 +233,22 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
}
}
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) {
@ -481,8 +481,8 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
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 + ", "
@ -496,8 +496,10 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
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
@ -507,6 +509,9 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
continue;
}
for (Constraint constraint : list) {
if (!checkKey && constraint instanceof KeySizeConstraint) {
continue;
}
constraint.permits(cp);
}
}

11
src/java.base/share/classes/sun/security/util/JarConstraintsParameters.java

@ -98,16 +98,11 @@ public class JarConstraintsParameters implements ConstraintsParameters {
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
@ -178,7 +173,7 @@ public class JarConstraintsParameters implements ConstraintsParameters {
@Override
public String extendedExceptionMsg() {
return message;
return message == null ? "." : message;
}
@Override

4
src/java.base/share/classes/sun/security/util/ManifestEntryVerifier.java

@ -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
@ -217,7 +217,7 @@ public class ManifestEntryVerifier {
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);

2
src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java

@ -383,7 +383,7 @@ public class SignatureFileVerifier {
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);

39
src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java

@ -1018,9 +1018,14 @@ public class Main {
Calendar c = Calendar.getInstance(
TimeZone.getTimeZone("UTC"),
Locale.getDefault(Locale.Category.FORMAT));
c.setTime(tsTokenInfo.getDate());
Date tsDate = tsTokenInfo.getDate();
c.setTime(tsDate);
JarConstraintsParameters jcp =
new JarConstraintsParameters(chain, si.getTimestamp());
new JarConstraintsParameters(chain, tsDate);
JarConstraintsParameters jcpts =
new JarConstraintsParameters(
tsSi.getCertificateChain(tsToken),
tsDate);
history = String.format(
rb.getString("history.with.ts"),
signer.getSubjectX500Principal(),
@ -1029,9 +1034,9 @@ public class Main {
verifyWithWeak(key, jcp),
c,
tsSigner.getSubjectX500Principal(),
verifyWithWeak(tsDigestAlg, DIGEST_PRIMITIVE_SET, true, jcp),
verifyWithWeak(tsSigAlg, SIG_PRIMITIVE_SET, true, jcp),
verifyWithWeak(tsKey, jcp));
verifyWithWeak(tsDigestAlg, DIGEST_PRIMITIVE_SET, true, jcpts),
verifyWithWeak(tsSigAlg, SIG_PRIMITIVE_SET, true, jcpts),
verifyWithWeak(tsKey, jcpts));
} else {
JarConstraintsParameters jcp =
new JarConstraintsParameters(chain, null);
@ -1371,13 +1376,13 @@ public class Main {
boolean tsa, JarConstraintsParameters jcp) {
try {
JAR_DISABLED_CHECK.permits(alg, jcp);
JAR_DISABLED_CHECK.permits(alg, jcp, false);
} catch (CertPathValidatorException e) {
disabledAlgFound = true;
return String.format(rb.getString("with.disabled"), alg);
}
try {
LEGACY_CHECK.permits(alg, jcp);
LEGACY_CHECK.permits(alg, jcp, false);
return alg;
} catch (CertPathValidatorException e) {
if (primitiveSet == SIG_PRIMITIVE_SET) {
@ -1399,13 +1404,13 @@ public class Main {
private String verifyWithWeak(PublicKey key, JarConstraintsParameters jcp) {
int kLen = KeyUtil.getKeySize(key);
try {
JAR_DISABLED_CHECK.permits(key.getAlgorithm(), jcp);
JAR_DISABLED_CHECK.permits(key.getAlgorithm(), jcp, true);
} catch (CertPathValidatorException e) {
disabledAlgFound = true;
return String.format(rb.getString("key.bit.disabled"), kLen);
}
try {
LEGACY_CHECK.permits(key.getAlgorithm(), jcp);
LEGACY_CHECK.permits(key.getAlgorithm(), jcp, true);
if (kLen >= 0) {
return String.format(rb.getString("key.bit"), kLen);
} else {
@ -1422,9 +1427,9 @@ public class Main {
boolean tsa, JarConstraintsParameters jcp) {
try {
JAR_DISABLED_CHECK.permits(alg, jcp);
JAR_DISABLED_CHECK.permits(alg, jcp, false);
try {
LEGACY_CHECK.permits(alg, jcp);
LEGACY_CHECK.permits(alg, jcp, false);
} catch (CertPathValidatorException e) {
if (primitiveSet == SIG_PRIMITIVE_SET) {
legacyAlg |= 2;
@ -1451,9 +1456,9 @@ public class Main {
private void checkWeakSign(PrivateKey key, JarConstraintsParameters jcp) {
try {
JAR_DISABLED_CHECK.permits(key.getAlgorithm(), jcp);
JAR_DISABLED_CHECK.permits(key.getAlgorithm(), jcp, true);
try {
LEGACY_CHECK.permits(key.getAlgorithm(), jcp);
LEGACY_CHECK.permits(key.getAlgorithm(), jcp, true);
} catch (CertPathValidatorException e) {
legacyAlg |= 8;
}
@ -1465,12 +1470,12 @@ public class Main {
private static String checkWeakKey(PublicKey key, CertPathConstraintsParameters cpcp) {
int kLen = KeyUtil.getKeySize(key);
try {
CERTPATH_DISABLED_CHECK.permits(key.getAlgorithm(), cpcp);
CERTPATH_DISABLED_CHECK.permits(key.getAlgorithm(), cpcp, true);
} catch (CertPathValidatorException e) {
return String.format(rb.getString("key.bit.disabled"), kLen);
}
try {
LEGACY_CHECK.permits(key.getAlgorithm(), cpcp);
LEGACY_CHECK.permits(key.getAlgorithm(), cpcp, true);
if (kLen >= 0) {
return String.format(rb.getString("key.bit"), kLen);
} else {
@ -1483,12 +1488,12 @@ public class Main {
private static String checkWeakAlg(String alg, CertPathConstraintsParameters cpcp) {
try {
CERTPATH_DISABLED_CHECK.permits(alg, cpcp);
CERTPATH_DISABLED_CHECK.permits(alg, cpcp, false);
} catch (CertPathValidatorException e) {
return String.format(rb.getString("with.disabled"), alg);
}
try {
LEGACY_CHECK.permits(alg, cpcp);
LEGACY_CHECK.permits(alg, cpcp, false);
return alg;
} catch (CertPathValidatorException e) {
return String.format(rb.getString("with.weak"), alg);

18
test/jdk/sun/security/tools/jarsigner/TimestampCheck.java

@ -55,7 +55,7 @@ import sun.security.timestamp.TimestampToken;
/*
* @test
* @bug 6543842 6543440 6939248 8009636 8024302 8163304 8169911 8180289 8172404
* 8247960 8242068 8269039
* 8247960 8242068 8269039 8275887
* @summary checking response of timestamp
* @modules java.base/sun.security.pkcs
* java.base/sun.security.timestamp
@ -340,6 +340,7 @@ public class TimestampCheck {
verify("tsdisabled2.jar", "-verbose")
.shouldHaveExitValue(16)
.shouldContain("treated as unsigned")
.shouldNotMatch("Signature.*(disabled)")
.shouldMatch("Timestamp.*512.*(disabled)");
// Algorithm used in signing is disabled
@ -356,6 +357,8 @@ public class TimestampCheck {
// sign with RSAkeysize < 1024
signVerbose("normal", "sign1.jar", "sign2.jar", "disabledkeysize")
.shouldContain("Algorithm constraints check failed on keysize")
.shouldNotContain("option is considered a security " +
"risk and is disabled")
.shouldHaveExitValue(4);
checkMultiple("sign2.jar");
@ -419,6 +422,17 @@ public class TimestampCheck {
// sign with RSAkeysize < 2048
signVerbose("normal", "sign1.jar", "sign2.jar", "weakkeysize")
.shouldNotContain("Algorithm constraints check failed on keysize")
.shouldNotContain("The SHA-256 algorithm specified " +
"for the -digestalg option is considered a " +
"security risk")
.shouldNotContain("The SHA256withRSA algorithm " +
"specified for the -sigalg option is considered " +
"a security risk")
.shouldNotContain("The SHA-256 algorithm specified " +
"for the -tsadigestalg option is considered a " +
"security risk")
.shouldContain("The RSA signing key has a keysize " +
"of 1024 which is considered a security risk")
.shouldHaveExitValue(0);
checkMultipleWeak("sign2.jar");
@ -673,7 +687,7 @@ public class TimestampCheck {
.shouldMatch("Timestamp signature algorithm: .*key.*(disabled)");
verify(file, "-J-Djava.security.debug=jar")
.shouldHaveExitValue(16)
.shouldMatch("SignatureException:.*keysize");
.shouldMatch("SignatureException:.*MD5");
// For 8171319: keytool should print out warnings when reading or
// generating cert/cert req using disabled algorithms.