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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 26 additions & 11 deletions jdk/src/share/classes/sun/security/pkcs/SignerInfo.java
View file
Open in desktop
Expand Up @@ -84,10 +84,21 @@ 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 class AlgorithmInfo {
String field;
boolean checkKey;
private AlgorithmInfo(String f, boolean cK) {
field = f;
checkKey = cK;
}
String field() { return field; }
boolean checkKey() { return checkKey; }
}
private Map<AlgorithmId, AlgorithmInfo> algorithms = new HashMap<>();

public SignerInfo(X500Name issuerName,
BigInteger serial,
Expand Down Expand Up @@ -323,7 +334,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 @@ -382,7 +394,8 @@ SignerInfo verify(PKCS7 block, byte[] data)
new AlgorithmId(oid,
digestEncryptionAlgorithmId.getParameters());
algorithms.put(sigAlgId,
"SignerInfo digestEncryptionAlgorithm field");
new AlgorithmInfo(
"SignerInfo digestEncryptionAlgorithm field", true));
} catch (NoSuchAlgorithmException ignore) {
}

Expand Down Expand Up @@ -569,7 +582,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 @@ -626,18 +640,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 (Map.Entry<AlgorithmId,AlgorithmInfo> 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
31 changes: 3 additions & 28 deletions jdk/src/share/classes/sun/security/provider/certpath/AlgorithmChecker.java
View file
Open in desktop
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 @@ -285,13 +283,13 @@ public void check(Certificate cert,
// Check against local constraints if it is DisabledAlgorithmConstraints
if (constraints instanceof DisabledAlgorithmConstraints) {
((DisabledAlgorithmConstraints)constraints).permits(currSigAlg,
currSigAlgParams, cp);
currSigAlgParams, cp, true);
// DisabledAlgorithmsConstraints does not check primitives, so key
// additional key check.

} else {
// Perform the default constraints checking anyway.
certPathDefaultConstraints.permits(currSigAlg, currSigAlgParams, cp);
certPathDefaultConstraints.permits(currSigAlg, currSigAlgParams, cp, true);
// Call locally set constraints to check key with primitives.
if (!constraints.permits(primitives, currPubKey)) {
throw new CertPathValidatorException(
Expand Down Expand Up @@ -377,29 +375,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 @@ -414,7 +389,7 @@ static void check(PublicKey key, AlgorithmId algorithmId, String variant,

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

3 changes: 2 additions & 1 deletion jdk/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java
View file
Open in desktop
Expand Up @@ -689,7 +689,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
31 changes: 18 additions & 13 deletions jdk/src/share/classes/sun/security/tools/jarsigner/Main.java
View file
Open in desktop
Expand Up @@ -904,9 +904,14 @@ void verifyJar(String jarName)
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(),
Expand All @@ -915,9 +920,9 @@ void verifyJar(String jarName)
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);
Expand Down Expand Up @@ -1267,13 +1272,13 @@ private String verifyWithWeak(String alg, Set<CryptoPrimitive> primitiveSet,
boolean tsa, JarConstraintsParameters jcp) {

try {
DISABLED_CHECK.permits(alg, jcp);
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) {
Expand All @@ -1295,13 +1300,13 @@ private String verifyWithWeak(String alg, Set<CryptoPrimitive> primitiveSet,
private String verifyWithWeak(PublicKey key, JarConstraintsParameters jcp) {
int kLen = KeyUtil.getKeySize(key);
try {
DISABLED_CHECK.permits(key.getAlgorithm(), jcp);
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 {
Expand All @@ -1318,9 +1323,9 @@ private void checkWeakSign(String alg, Set<CryptoPrimitive> primitiveSet,
boolean tsa, JarConstraintsParameters jcp) {

try {
DISABLED_CHECK.permits(alg, jcp);
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;
Expand All @@ -1347,9 +1352,9 @@ private void checkWeakSign(String alg, Set<CryptoPrimitive> primitiveSet,

private void checkWeakSign(PrivateKey key, JarConstraintsParameters jcp) {
try {
DISABLED_CHECK.permits(key.getAlgorithm(), jcp);
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;
}
Expand Down
43 changes: 25 additions & 18 deletions jdk/src/share/classes/sun/security/util/DisabledAlgorithmConstraints.java
View file
Open in desktop
Expand Up @@ -192,16 +192,16 @@ 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);
}
}

private void permits(AlgorithmParameters ap, ConstraintsParameters cp)
throws CertPathValidatorException {
throws CertPathValidatorException {

switch (ap.getAlgorithm().toUpperCase(Locale.ENGLISH)) {
case "RSASSA-PSS":
Expand All @@ -219,36 +219,38 @@ 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 {
public final void permits(String algorithm, ConstraintsParameters cp,
boolean checkKey) 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(
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 @@ -479,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 @@ -494,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 @@ -505,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
11 changes: 3 additions & 8 deletions jdk/src/share/classes/sun/security/util/JarConstraintsParameters.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion jdk/src/share/classes/sun/security/util/ManifestEntryVerifier.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion jdk/src/share/classes/sun/security/util/SignatureFileVerifier.java
View file
Open in desktop
Expand Up @@ -360,7 +360,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