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

Reviewed-by: mdoerr
Backport-of: 03f8c0fb9363dc1bb07bed1ae0359c029caa0130

Author: GoeLin

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 @@ 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;
 
@@ -428,7 +431,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);
@@ -685,7 +689,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());
 
@@ -742,18 +747,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) {

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

@@ -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;
@@ -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
@@ -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(
@@ -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.
      *
@@ -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);
     }
 }
 

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

@@ -691,7 +691,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);

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

@@ -199,16 +199,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":
@@ -226,36 +226,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 (!cachedCheckAlgorithm(curve)) {
-                    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 (!cachedCheckAlgorithm(curve)) {
+                        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) {
@@ -493,8 +495,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 + ", "
@@ -508,8 +510,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
@@ -519,6 +523,9 @@ public void permits(String algorithm, ConstraintsParameters cp)
                     continue;
                 }
                 for (Constraint constraint : list) {
+                    if (!checkKey && constraint instanceof KeySizeConstraint) {
+                        continue;
+                    }
                     constraint.permits(cp);
                 }
             }

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

@@ -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
@@ -178,7 +173,7 @@ public void setExtendedExceptionMsg(String file, String target) {
 
     @Override
     public String extendedExceptionMsg() {
-        return message;
+        return message == null ? "." : message;
     }
 
     @Override

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

@@ -295,7 +295,7 @@ private boolean checkConstraints(String algorithm,
             params.setExtendedExceptionMsg(JarFile.MANIFEST_NAME,
                 name + " entry");
             DisabledAlgorithmConstraints.jarConstraints()
-                   .permits(algorithm, params);
+                .permits(algorithm, params, false);
             return true;
         } catch (GeneralSecurityException e) {
             if (debug != null) {

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

@@ -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);