8172529: Use PKIXValidator in jarsigner

Reviewed-by: xuelei, mullan, alanb

src/share/classes/sun/security/tools/jarsigner/Main.java

@@ -26,6 +26,8 @@
 package sun.security.tools.jarsigner;
 
 import java.io.*;
+import java.security.cert.CertPathValidatorException;
+import java.security.cert.PKIXBuilderParameters;
 import java.util.*;
 import java.util.zip.*;
 import java.util.jar.*;
@@ -46,11 +48,9 @@ import java.net.SocketTimeoutException;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.security.cert.CertPath;
-import java.security.cert.CertPathValidator;
 import java.security.cert.CertificateExpiredException;
 import java.security.cert.CertificateFactory;
 import java.security.cert.CertificateNotYetValidException;
-import java.security.cert.PKIXParameters;
 import java.security.cert.TrustAnchor;
 import java.util.Map.Entry;
 import sun.security.pkcs.PKCS7;
@@ -58,6 +58,8 @@ import sun.security.pkcs.SignerInfo;
 import sun.security.timestamp.TimestampToken;
 import sun.security.tools.KeyStoreUtil;
 import sun.security.tools.PathList;
+import sun.security.validator.Validator;
+import sun.security.validator.ValidatorException;
 import sun.security.x509.*;
 import sun.security.util.*;
 import java.util.Base64;
@@ -190,9 +192,7 @@ public class Main {
 
     private boolean seeWeak = false;
 
-    CertificateFactory certificateFactory;
-    CertPathValidator validator;
-    PKIXParameters pkixParameters;
+    PKIXBuilderParameters pkixParameters;
 
     public void run(String args[]) {
         try {
@@ -1791,19 +1791,10 @@ public class Main {
         try {
             validateCertChain(certs);
         } catch (Exception e) {
-            if (debug) {
-                e.printStackTrace();
-            }
-            if (e.getCause() != null &&
-                    (e.getCause() instanceof CertificateExpiredException ||
-                     e.getCause() instanceof CertificateNotYetValidException)) {
-                // No more warning, we alreay have hasExpiredCert or notYetValidCert
-            } else {
-                chainNotValidated = true;
-                chainNotValidatedReason = e;
-                s.append(tab + rb.getString(".CertPath.not.validated.") +
-                        e.getLocalizedMessage() + "]\n");   // TODO
-            }
+            chainNotValidated = true;
+            chainNotValidatedReason = e;
+            s.append(tab).append(rb.getString(".CertPath.not.validated."))
+                    .append(e.getLocalizedMessage()).append("]\n"); // TODO
         }
         if (certs.size() == 1
                 && KeyStoreUtil.isSelfSigned((X509Certificate)certs.get(0))) {
@@ -1861,9 +1852,6 @@ public class Main {
         }
 
         try {
-
-            certificateFactory = CertificateFactory.getInstance("X.509");
-            validator = CertPathValidator.getInstance("PKIX");
             Set<TrustAnchor> tas = new HashSet<>();
             try {
                 KeyStore caks = KeyStoreUtil.getCacertsKeyStore();
@@ -1939,7 +1927,7 @@ public class Main {
                 }
             } finally {
                 try {
-                    pkixParameters = new PKIXParameters(tas);
+                    pkixParameters = new PKIXBuilderParameters(tas, null);
                     pkixParameters.setRevocationEnabled(false);
                 } catch (InvalidAlgorithmParameterException ex) {
                     // Only if tas is empty
@@ -2106,17 +2094,8 @@ public class Main {
             try {
                 validateCertChain(Arrays.asList(certChain));
             } catch (Exception e) {
-                if (debug) {
-                    e.printStackTrace();
-                }
-                if (e.getCause() != null &&
-                        (e.getCause() instanceof CertificateExpiredException ||
-                        e.getCause() instanceof CertificateNotYetValidException)) {
-                    // No more warning, we already have hasExpiredCert or notYetValidCert
-                } else {
-                    chainNotValidated = true;
-                    chainNotValidatedReason = e;
-                }
+                chainNotValidated = true;
+                chainNotValidatedReason = e;
             }
 
             if (KeyStoreUtil.isSelfSigned(certChain[0])) {
@@ -2175,18 +2154,40 @@ public class Main {
     }
 
     void validateCertChain(List<? extends Certificate> certs) throws Exception {
-        int cpLen = 0;
-        out: for (; cpLen<certs.size(); cpLen++) {
-            for (TrustAnchor ta: pkixParameters.getTrustAnchors()) {
-                if (ta.getTrustedCert().equals(certs.get(cpLen))) {
-                    break out;
+        try {
+            Validator.getInstance(Validator.TYPE_PKIX,
+                    Validator.VAR_CODE_SIGNING,
+                    pkixParameters)
+                    .validate(certs.toArray(new X509Certificate[certs.size()]));
+        } catch (Exception e) {
+            if (debug) {
+                e.printStackTrace();
+            }
+            if (e instanceof ValidatorException) {
+                // Throw cause if it's CertPathValidatorException,
+                if (e.getCause() != null &&
+                        e.getCause() instanceof CertPathValidatorException) {
+                    e = (Exception) e.getCause();
+                    Throwable t = e.getCause();
+                    if ((t instanceof CertificateExpiredException &&
+                                hasExpiredCert) ||
+                            (t instanceof CertificateNotYetValidException &&
+                                    notYetValidCert)) {
+                        // we already have hasExpiredCert and notYetValidCert
+                        return;
+                    }
+                }
+                if (e instanceof ValidatorException) {
+                    ValidatorException ve = (ValidatorException)e;
+                    if (ve.getErrorType() == ValidatorException.T_EE_EXTENSIONS &&
+                            (badKeyUsage || badExtendedKeyUsage || badNetscapeCertType)) {
+                        // We already have badKeyUsage, badExtendedKeyUsage
+                        // and badNetscapeCertType
+                        return;
+                    }
                 }
             }
-        }
-        if (cpLen > 0) {
-            CertPath cp = certificateFactory.generateCertPath(
-                    (cpLen == certs.size())? certs: certs.subList(0, cpLen));
-            validator.validate(cp, pkixParameters);
+            throw e;
         }
     }
 

test/sun/security/tools/jarsigner/concise_jarsigner.sh

 #
-# Copyright (c) 2009, 2012, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2009, 2014, 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
@@ -22,7 +22,7 @@
 #
 
 # @test
-# @bug 6802846
+# @bug 6802846 8172529
 # @summary jarsigner needs enhanced cert validation(options)
 #
 # @run shell/timeout=240 concise_jarsigner.sh
@@ -47,12 +47,15 @@ esac
 # Choose 1024-bit RSA to make sure it runs fine and fast on all platforms. In
 # fact, every keyalg/keysize combination is OK for this test.
 
-KT="$TESTJAVA${FS}bin${FS}keytool -storepass changeit -keypass changeit -keystore js.jks -keyalg rsa -keysize 1024"
-JAR=$TESTJAVA${FS}bin${FS}jar
-JARSIGNER=$TESTJAVA${FS}bin${FS}jarsigner
-JAVAC=$TESTJAVA${FS}bin${FS}javac
+TESTTOOLVMOPTS="$TESTTOOLVMOPTS -J-Duser.language=en -J-Duser.country=US"
 
-rm js.jks
+KS=js.ks
+KT="$TESTJAVA${FS}bin${FS}keytool ${TESTTOOLVMOPTS} -storepass changeit -keypass changeit -keystore $KS -keyalg rsa -keysize 1024"
+JAR="$TESTJAVA${FS}bin${FS}jar ${TESTTOOLVMOPTS}"
+JARSIGNER="$TESTJAVA${FS}bin${FS}jarsigner ${TESTTOOLVMOPTS} -debug"
+JAVAC="$TESTJAVA${FS}bin${FS}javac ${TESTTOOLVMOPTS} ${TESTJAVACOPTS}"
+
+rm $KS
 
 echo class A1 {} > A1.java
 echo class A2 {} > A2.java
@@ -73,9 +76,9 @@ $KT -genkeypair -alias a2 -dname CN=a2 -validity 366
 
 # a.jar includes 8 unsigned, 2 signed by a1 and a2, 2 signed by a3
 $JAR cvf a.jar A1.class A2.class
-$JARSIGNER -keystore js.jks -storepass changeit a.jar a1
+$JARSIGNER -keystore $KS -storepass changeit a.jar a1
 $JAR uvf a.jar A3.class A4.class
-$JARSIGNER -keystore js.jks -storepass changeit a.jar a2
+$JARSIGNER -keystore $KS -storepass changeit a.jar a2
 $JAR uvf a.jar A5.class A6.class
 
 # Verify OK
@@ -87,15 +90,15 @@ $JARSIGNER -verify a.jar -strict
 [ $? = 20 ] || exit $LINENO
 
 # 16(hasUnsignedEntry)
-$JARSIGNER -verify a.jar -strict -keystore js.jks
+$JARSIGNER -verify a.jar -strict -keystore $KS -storepass changeit
 [ $? = 16 ] || exit $LINENO
 
 # 16(hasUnsignedEntry)+32(notSignedByAlias)
-$JARSIGNER -verify a.jar a1 -strict -keystore js.jks
+$JARSIGNER -verify a.jar a1 -strict -keystore $KS -storepass changeit
 [ $? = 48 ] || exit $LINENO
 
 # 16(hasUnsignedEntry)
-$JARSIGNER -verify a.jar a1 a2 -strict -keystore js.jks
+$JARSIGNER -verify a.jar a1 a2 -strict -keystore $KS -storepass changeit
 [ $? = 16 ] || exit $LINENO
 
 # 12 entries all together
@@ -135,7 +138,7 @@ LINES=`$JARSIGNER -verify a.jar -verbose:summary -certs | grep "more)" | wc -l`
 [ $LINES = 4 ] || exit $LINENO
 
 # ==========================================================
-# Second part: exit code 2, 4, 8
+# Second part: exit code 2, 4, 8.
 # 16 and 32 already covered in the first part
 # ==========================================================
 
@@ -153,31 +156,34 @@ $KT -certreq -alias goodku | $KT -gencert -alias ca -ext KU=dig -validity 365 |
 $KT -genkeypair -alias goodeku -dname CN=goodeku
 $KT -certreq -alias goodeku | $KT -gencert -alias ca -ext EKU=codesign -validity 365 | $KT -import -alias goodeku
 
-$JARSIGNER -strict -keystore js.jks -storepass changeit a.jar expired
+$JARSIGNER -strict -keystore $KS -storepass changeit a.jar expired
 [ $? = 4 ] || exit $LINENO
 
-$JARSIGNER -strict -keystore js.jks -storepass changeit a.jar notyetvalid
+$JARSIGNER -strict -keystore $KS -storepass changeit a.jar notyetvalid
 [ $? = 4 ] || exit $LINENO
 
-$JARSIGNER -strict -keystore js.jks -storepass changeit a.jar badku
+$JARSIGNER -strict -keystore $KS -storepass changeit a.jar badku
 [ $? = 8 ] || exit $LINENO
 
-$JARSIGNER -strict -keystore js.jks -storepass changeit a.jar badeku
+$JARSIGNER -strict -keystore $KS -storepass changeit a.jar badeku
 [ $? = 8 ] || exit $LINENO
 
-$JARSIGNER -strict -keystore js.jks -storepass changeit a.jar goodku
+$JARSIGNER -strict -keystore $KS -storepass changeit a.jar goodku
 [ $? = 0 ] || exit $LINENO
 
-$JARSIGNER -strict -keystore js.jks -storepass changeit a.jar goodeku
+$JARSIGNER -strict -keystore $KS -storepass changeit a.jar goodeku
 [ $? = 0 ] || exit $LINENO
 
-# badchain signed by ca, but ca is removed later
+# badchain signed by ca1, but ca1 is removed later
 $KT -genkeypair -alias badchain -dname CN=badchain -validity 365
-$KT -certreq -alias badchain | $KT -gencert -alias ca -validity 365 | \
+$KT -genkeypair -alias ca1 -dname CN=ca1 -ext bc -validity 365
+$KT -certreq -alias badchain | $KT -gencert -alias ca1 -validity 365 | \
         $KT -importcert -alias badchain
-$KT -delete -alias ca
+# save ca1.cert for easy replay
+$KT -exportcert -file ca1.cert -alias ca1
+$KT -delete -alias ca1
 
-$JARSIGNER -strict -keystore js.jks -storepass changeit a.jar badchain
+$JARSIGNER -strict -keystore $KS -storepass changeit a.jar badchain
 [ $? = 4 ] || exit $LINENO
 
 $JARSIGNER -verify a.jar
@@ -194,20 +200,48 @@ $KT -certreq -alias altchain | $KT -gencert -alias ca2 -validity 365 -rfc > cert
 $KT -exportcert -alias ca2 -rfc >> certchain
 
 # Self-signed cert does not work
-$JARSIGNER -strict -keystore js.jks -storepass changeit a.jar altchain
+$JARSIGNER -strict -keystore $KS -storepass changeit a.jar altchain
 [ $? = 4 ] || exit $LINENO
 
 # -certchain works
 $JARSIGNER -strict -keystore $KS -storepass changeit -certchain certchain a.jar altchain
 [ $? = 0 ] || exit $LINENO
 
-# but if ca2 is removed, -certchain does not work
+# if ca2 is removed, -certchain still work because altchain is a self-signed entry and
+# it is trusted by jarsigner
+# save ca2.cert for easy replay
+$KT -exportcert -file ca2.cert -alias ca2
 $KT -delete -alias ca2
-$JARSIGNER -strict -keystore js.jks -storepass changeit -certchain certchain a.jar altchain
+$JARSIGNER -strict -keystore $KS -storepass changeit -certchain certchain a.jar altchain
+[ $? = 0 ] || exit $LINENO
+
+# if cert is imported, -certchain won't work because this certificate entry is not trusted
+$KT -importcert -file certchain -alias altchain -noprompt
+$JARSIGNER -strict -keystore $KS -storepass changeit -certchain certchain a.jar altchain
 [ $? = 4 ] || exit $LINENO
 
 $JARSIGNER -verify a.jar
 [ $? = 0 ] || exit $LINENO
 
+# ==========================================================
+# 8172529
+# ==========================================================
+
+$KT -genkeypair -alias ee -dname CN=ee
+$KT -genkeypair -alias caone -dname CN=caone
+$KT -genkeypair -alias catwo -dname CN=catwo
+
+$KT -certreq -alias ee | $KT -gencert -alias catwo -rfc > ee.cert
+$KT -certreq -alias catwo | $KT -gencert -alias caone -sigalg MD5withRSA -rfc > catwo.cert
+
+# This certchain contains a cross-signed weak catwo.cert
+cat ee.cert catwo.cert | $KT -importcert -alias ee
+
+$JAR cvf a.jar A1.class
+$JARSIGNER -strict -keystore $KS -storepass changeit a.jar ee
+[ $? = 0 ] || exit $LINENO
+$JARSIGNER -strict -keystore $KS -storepass changeit -verify a.jar
+[ $? = 0 ] || exit $LINENO
+
 echo OK
 exit 0