changeset 9659:e4925f09fe2e

8172529: Use PKIXValidator in jarsigner Reviewed-by: xuelei, mullan, alanb
author igerasim
date Sat, 20 Oct 2018 19:46:54 +0100
parents 8536436fb141
children 52425d2592c8
files src/share/classes/sun/security/tools/jarsigner/Main.java test/sun/security/tools/jarsigner/concise_jarsigner.sh
diffstat 2 files changed, 104 insertions(+), 69 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/classes/sun/security/tools/jarsigner/Main.java	Wed Jan 17 17:43:56 2018 -0800
+++ b/src/share/classes/sun/security/tools/jarsigner/Main.java	Sat Oct 20 19:46:54 2018 +0100
@@ -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.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.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 sun.misc.BASE64Encoder;
@@ -190,9 +192,7 @@
 
     private boolean seeWeak = false;
 
-    CertificateFactory certificateFactory;
-    CertPathValidator validator;
-    PKIXParameters pkixParameters;
+    PKIXBuilderParameters pkixParameters;
 
     public void run(String args[]) {
         try {
@@ -1801,19 +1801,10 @@
         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))) {
@@ -1871,9 +1862,6 @@
         }
 
         try {
-
-            certificateFactory = CertificateFactory.getInstance("X.509");
-            validator = CertPathValidator.getInstance("PKIX");
             Set<TrustAnchor> tas = new HashSet<>();
             try {
                 KeyStore caks = KeyStoreUtil.getCacertsKeyStore();
@@ -1949,7 +1937,7 @@
                 }
             } finally {
                 try {
-                    pkixParameters = new PKIXParameters(tas);
+                    pkixParameters = new PKIXBuilderParameters(tas, null);
                     pkixParameters.setRevocationEnabled(false);
                 } catch (InvalidAlgorithmParameterException ex) {
                     // Only if tas is empty
@@ -2116,17 +2104,8 @@
             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])) {
@@ -2185,18 +2164,40 @@
     }
 
     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;
         }
     }
 
--- a/test/sun/security/tools/jarsigner/concise_jarsigner.sh	Wed Jan 17 17:43:56 2018 -0800
+++ b/test/sun/security/tools/jarsigner/concise_jarsigner.sh	Sat Oct 20 19:46:54 2018 +0100
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2009, 2010, 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
@@ -44,12 +44,15 @@
     ;;
 esac
 
-KT="$TESTJAVA${FS}bin${FS}keytool -storepass changeit -keypass changeit -keystore js.jks"
-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
@@ -70,9 +73,9 @@
 
 # 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
@@ -84,15 +87,15 @@
 [ $? = 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
@@ -132,7 +135,7 @@
 [ $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
 # ==========================================================
 
@@ -150,31 +153,34 @@
 $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
@@ -191,20 +197,48 @@
 $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