From 70da21b46eaa894e323457243f4f1b55e79a44c4 Mon Sep 17 00:00:00 2001 From: Hai-May Chao Date: Tue, 22 Feb 2022 11:58:00 -0800 Subject: [PATCH] 8277474: jarsigner does not check if algorithm parameters are disabled --- .../util/DisabledAlgorithmConstraints.java | 2 +- .../sun/security/tools/jarsigner/Main.java | 40 +++++++-- .../tools/jarsigner/CheckAlgParams.java | 86 +++++++++++++++++++ 3 files changed, 119 insertions(+), 9 deletions(-) create mode 100644 test/jdk/sun/security/tools/jarsigner/CheckAlgParams.java diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index 03e9cf6a2c630..c0506d64b1444 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -202,7 +202,7 @@ public final void permits(String algorithm, AlgorithmParameters ap, } } - private void permits(AlgorithmParameters ap, ConstraintsParameters cp) + public void permits(AlgorithmParameters ap, ConstraintsParameters cp) throws CertPathValidatorException { switch (ap.getAlgorithm().toUpperCase(Locale.ENGLISH)) { diff --git a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java index 87ff579a7c15d..3ab6c7f1fb048 100644 --- a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java +++ b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java @@ -30,6 +30,7 @@ import java.net.URLClassLoader; import java.security.cert.CertPathValidatorException; import java.security.cert.PKIXBuilderParameters; +import java.security.spec.PSSParameterSpec; import java.util.*; import java.util.stream.Collectors; import java.util.zip.*; @@ -1021,6 +1022,8 @@ void verifyJar(String jarName) si.getDigestAlgorithmId(), si.getDigestEncryptionAlgorithmId(), si.getAuthenticatedAttributes() == null); + AlgorithmId encAlgId = si.getDigestEncryptionAlgorithmId(); + AlgorithmParameters sigAlgParams = encAlgId.getParameters(); PublicKey key = signer.getPublicKey(); PKCS7 tsToken = si.getTsToken(); if (tsToken != null) { @@ -1035,6 +1038,8 @@ void verifyJar(String jarName) tsSi.getDigestAlgorithmId(), tsSi.getDigestEncryptionAlgorithmId(), tsSi.getAuthenticatedAttributes() == null); + AlgorithmId tsEncAlgId = tsSi.getDigestEncryptionAlgorithmId(); + AlgorithmParameters tsSigAlgParams = tsEncAlgId.getParameters(); Calendar c = Calendar.getInstance( TimeZone.getTimeZone("UTC"), Locale.getDefault(Locale.Category.FORMAT)); @@ -1049,13 +1054,13 @@ void verifyJar(String jarName) history = String.format( rb.getString("history.with.ts"), signer.getSubjectX500Principal(), - verifyWithWeak(digestAlg, DIGEST_PRIMITIVE_SET, false, jcp), - verifyWithWeak(sigAlg, SIG_PRIMITIVE_SET, false, jcp), + verifyWithWeak(digestAlg, DIGEST_PRIMITIVE_SET, false, jcp, null), + verifyWithWeak(sigAlg, SIG_PRIMITIVE_SET, false, jcp, sigAlgParams), verifyWithWeak(key, jcp), c, tsSigner.getSubjectX500Principal(), - verifyWithWeak(tsDigestAlg, DIGEST_PRIMITIVE_SET, true, jcpts), - verifyWithWeak(tsSigAlg, SIG_PRIMITIVE_SET, true, jcpts), + verifyWithWeak(tsDigestAlg, DIGEST_PRIMITIVE_SET, true, jcpts, null), + verifyWithWeak(tsSigAlg, SIG_PRIMITIVE_SET, true, jcpts, tsSigAlgParams), verifyWithWeak(tsKey, jcpts)); } else { JarConstraintsParameters jcp = @@ -1063,8 +1068,8 @@ void verifyJar(String jarName) history = String.format( rb.getString("history.without.ts"), signer.getSubjectX500Principal(), - verifyWithWeak(digestAlg, DIGEST_PRIMITIVE_SET, false, jcp), - verifyWithWeak(sigAlg, SIG_PRIMITIVE_SET, false, jcp), + verifyWithWeak(digestAlg, DIGEST_PRIMITIVE_SET, false, jcp, null), + verifyWithWeak(sigAlg, SIG_PRIMITIVE_SET, false, jcp, sigAlgParams), verifyWithWeak(key, jcp)); } } catch (Exception e) { @@ -1393,7 +1398,7 @@ private void displayMessagesAndResult(boolean isSigning) { } private String verifyWithWeak(String alg, Set primitiveSet, - boolean tsa, JarConstraintsParameters jcp) { + boolean tsa, JarConstraintsParameters jcp, AlgorithmParameters algParams) { try { JAR_DISABLED_CHECK.permits(alg, jcp, false); @@ -1401,9 +1406,17 @@ private String verifyWithWeak(String alg, Set primitiveSet, disabledAlgFound = true; return String.format(rb.getString("with.disabled"), alg); } + if (algParams != null) { + try { + JAR_DISABLED_CHECK.permits(algParams, jcp); + } catch (CertPathValidatorException e) { + disabledAlgFound = true; + return String.format(rb.getString("with.disabled"), algParams); + } + } + try { LEGACY_CHECK.permits(alg, jcp, false); - return alg; } catch (CertPathValidatorException e) { if (primitiveSet == SIG_PRIMITIVE_SET) { legacyAlg |= 2; @@ -1419,6 +1432,17 @@ private String verifyWithWeak(String alg, Set primitiveSet, } return String.format(rb.getString("with.weak"), alg); } + if (algParams != null) { + try { + LEGACY_CHECK.permits(algParams, jcp); + return alg; + } catch (CertPathValidatorException e) { + legacyAlg |= 2; + legacySigAlg = alg; + return String.format(rb.getString("with.weak"), algParams); + } + } + return alg; } private String verifyWithWeak(PublicKey key, JarConstraintsParameters jcp) { diff --git a/test/jdk/sun/security/tools/jarsigner/CheckAlgParams.java b/test/jdk/sun/security/tools/jarsigner/CheckAlgParams.java new file mode 100644 index 0000000000000..d74895ef18db9 --- /dev/null +++ b/test/jdk/sun/security/tools/jarsigner/CheckAlgParams.java @@ -0,0 +1,86 @@ +/* + * Copyright (c) 2022, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8277474 + * @summary jarsigner -verify should check if the algorithm parameters of + * its signature algorithm use disabled or legacy algorithms + * @library /test/lib + * @modules java.base/sun.security.x509 + */ + +import jdk.test.lib.SecurityTools; +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.util.JarUtils; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +public class CheckAlgParams { + private static final String JAVA_SECURITY_FILE = "java.security"; + + public static void main(String[] args) throws Exception{ + + SecurityTools.keytool("-keystore ks -storepass changeit " + + "-genkeypair -keyalg RSASSA-PSS -alias ca -dname CN=CA " + + "-ext bc:c") + .shouldHaveExitValue(0); + + JarUtils.createJarFile(Path.of("a.jar"), Path.of("."), Path.of("ks")); + + SecurityTools.jarsigner("-keystore ks -storepass changeit " + + "-signedjar signeda.jar " + + "-verbose" + + " a.jar ca") + .shouldHaveExitValue(0); + + Files.writeString(Files.createFile(Paths.get(JAVA_SECURITY_FILE)), + "jdk.jar.disabledAlgorithms=SHA256\n" + + "jdk.security.legacyAlgorithms=\n"); + + SecurityTools.jarsigner("-verify signeda.jar " + + "-J-Djava.security.properties=" + + JAVA_SECURITY_FILE + + " -keystore ks -storepass changeit -verbose -debug") + .shouldMatch("Digest algorithm: SHA-256.*(disabled)") + .shouldMatch("Signature algorithm: PSSParameterSpec.*hashAlgorithm=SHA-256.*(disabled)") + .shouldContain("The jar will be treated as unsigned") + .shouldHaveExitValue(0); + + Files.deleteIfExists(Paths.get(JAVA_SECURITY_FILE)); + Files.writeString(Files.createFile(Paths.get(JAVA_SECURITY_FILE)), + "jdk.jar.disabledAlgorithms=\n" + + "jdk.security.legacyAlgorithms=SHA256\n"); + + SecurityTools.jarsigner("-verify signeda.jar " + + "-J-Djava.security.properties=" + + JAVA_SECURITY_FILE + + " -keystore ks -storepass changeit -verbose -debug") + .shouldMatch("Digest algorithm: SHA-256.*(weak)") + .shouldMatch("Signature algorithm: PSSParameterSpec.*hashAlgorithm=SHA-256.*(weak)") + .shouldNotContain("The jar will be treated as unsigned") + .shouldHaveExitValue(0); + } +}