From 53c39c775c4b848c85c535c1f52905ee0762ae82 Mon Sep 17 00:00:00 2001 From: igerasim Date: Wed, 17 Jan 2018 17:45:24 -0800 Subject: [PATCH] 8172529: Use PKIXValidator in jarsigner Reviewed-by: xuelei, mullan, alanb --- .../sun/security/tools/jarsigner/Main.java | 87 ++++++++++--------- .../tools/jarsigner/concise_jarsigner.sh | 86 ++++++++++++------ 2 files changed, 104 insertions(+), 69 deletions(-) diff --git a/src/share/classes/sun/security/tools/jarsigner/Main.java b/src/share/classes/sun/security/tools/jarsigner/Main.java index 63496b160..303a04878 100644 --- a/src/share/classes/sun/security/tools/jarsigner/Main.java +++ b/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 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 certs) throws Exception { - int cpLen = 0; - out: for (; cpLen 0) { - CertPath cp = certificateFactory.generateCertPath( - (cpLen == certs.size())? certs: certs.subList(0, cpLen)); - validator.validate(cp, pkixParameters); + throw e; } } diff --git a/test/sun/security/tools/jarsigner/concise_jarsigner.sh b/test/sun/security/tools/jarsigner/concise_jarsigner.sh index 1052b966e..b9ec9e832 100644 --- a/test/sun/security/tools/jarsigner/concise_jarsigner.sh +++ b/test/sun/security/tools/jarsigner/concise_jarsigner.sh @@ -1,5 +1,5 @@ # -# 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 -- GitLab