Skip to content

Commit

Permalink
8266225: jarsigner is using incorrect security property to show weakn…
Browse files Browse the repository at this point in the history
…ess of certs

Reviewed-by: weijun, mullan
  • Loading branch information
Hai-May Chao committed May 11, 2021
1 parent 0a12605 commit 995e956
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 8 deletions.
Expand Up @@ -97,10 +97,14 @@ public class Main {
private static final long SIX_MONTHS = 180*24*60*60*1000L; //milliseconds
private static final long ONE_YEAR = 366*24*60*60*1000L;

private static final DisabledAlgorithmConstraints DISABLED_CHECK =
private static final DisabledAlgorithmConstraints JAR_DISABLED_CHECK =
new DisabledAlgorithmConstraints(
DisabledAlgorithmConstraints.PROPERTY_JAR_DISABLED_ALGS);

private static final DisabledAlgorithmConstraints CERTPATH_DISABLED_CHECK =
new DisabledAlgorithmConstraints(
DisabledAlgorithmConstraints.PROPERTY_CERTPATH_DISABLED_ALGS);

private static final DisabledAlgorithmConstraints LEGACY_CHECK =
new DisabledAlgorithmConstraints(
DisabledAlgorithmConstraints.PROPERTY_SECURITY_LEGACY_ALGS);
Expand Down Expand Up @@ -1321,7 +1325,7 @@ private void displayMessagesAndResult(boolean isSigning) {
}

private String verifyWithWeak(String alg, Set<CryptoPrimitive> primitiveSet, boolean tsa) {
if (DISABLED_CHECK.permits(primitiveSet, alg, null)) {
if (JAR_DISABLED_CHECK.permits(primitiveSet, alg, null)) {
if (LEGACY_CHECK.permits(primitiveSet, alg, null)) {
return alg;
} else {
Expand All @@ -1347,7 +1351,7 @@ private String verifyWithWeak(String alg, Set<CryptoPrimitive> primitiveSet, boo

private String verifyWithWeak(PublicKey key) {
int kLen = KeyUtil.getKeySize(key);
if (DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, key)) {
if (JAR_DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, key)) {
if (LEGACY_CHECK.permits(SIG_PRIMITIVE_SET, key)) {
if (kLen >= 0) {
return String.format(rb.getString("key.bit"), kLen);
Expand All @@ -1366,7 +1370,7 @@ private String verifyWithWeak(PublicKey key) {
}

private void checkWeakSign(String alg, Set<CryptoPrimitive> primitiveSet, boolean tsa) {
if (DISABLED_CHECK.permits(primitiveSet, alg, null)) {
if (JAR_DISABLED_CHECK.permits(primitiveSet, alg, null)) {
if (!LEGACY_CHECK.permits(primitiveSet, alg, null)) {
if (primitiveSet == SIG_PRIMITIVE_SET) {
legacyAlg |= 2;
Expand All @@ -1392,7 +1396,7 @@ private void checkWeakSign(String alg, Set<CryptoPrimitive> primitiveSet, boolea
}

private void checkWeakSign(PrivateKey key) {
if (DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, key)) {
if (JAR_DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, key)) {
if (!LEGACY_CHECK.permits(SIG_PRIMITIVE_SET, key)) {
legacyAlg |= 8;
}
Expand All @@ -1403,7 +1407,7 @@ private void checkWeakSign(PrivateKey key) {

private static String checkWeakKey(PublicKey key) {
int kLen = KeyUtil.getKeySize(key);
if (DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, key)) {
if (CERTPATH_DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, key)) {
if (LEGACY_CHECK.permits(SIG_PRIMITIVE_SET, key)) {
if (kLen >= 0) {
return String.format(rb.getString("key.bit"), kLen);
Expand All @@ -1419,7 +1423,7 @@ private static String checkWeakKey(PublicKey key) {
}

private static String checkWeakAlg(String alg) {
if (DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, alg, null)) {
if (CERTPATH_DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, alg, null)) {
if (LEGACY_CHECK.permits(SIG_PRIMITIVE_SET, alg, null)) {
return alg;
} else {
Expand Down
55 changes: 54 additions & 1 deletion test/jdk/sun/security/tools/jarsigner/CheckSignerCertChain.java
Expand Up @@ -23,7 +23,7 @@

/*
* @test
* @bug 8259401
* @bug 8259401 8266225
* @summary Check certificates in signer's cert chain to see if warning emitted
* @library /test/lib
*/
Expand All @@ -32,10 +32,14 @@
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 CheckSignerCertChain {

private static final String JAVA_SECURITY_FILE = "java.security";

static OutputAnalyzer kt(String cmd, String ks) throws Exception {
return SecurityTools.keytool("-storepass changeit " + cmd +
" -keystore " + ks);
Expand Down Expand Up @@ -88,5 +92,54 @@ public static void main(String[] args) throws Exception {
// key, but not for its SHA1withRSA algorithm.
.shouldContain("Signature algorithm: SHA1withRSA, 1024-bit key (weak)")
.shouldHaveExitValue(0);

/*
* Generate a non-self-signed certificate using MD5withRSA as its signature
* algorithm to sign a JAR file.
*/
kt("-genkeypair -keyalg rsa -alias cacert -dname CN=CACERT -ext bc:c ", "ks");
kt("-genkeypair -keyalg rsa -alias ee -dname CN=EE -ext bc:c ", "ks");
gencert("ee", "-alias cacert -ext san=dns:ee -sigalg MD5withRSA");

Files.writeString(Files.createFile(Paths.get(JAVA_SECURITY_FILE)),
"jdk.certpath.disabledAlgorithms=\n" +
"jdk.jar.disabledAlgorithms=MD5\n");

SecurityTools.jarsigner("-keystore ks -storepass changeit " +
"-signedjar signeda.jar " +
"-verbose " +
"-J-Djava.security.properties=" +
JAVA_SECURITY_FILE +
" a.jar ee")
.shouldNotContain("Signature algorithm: MD5withRSA (disabled), 2048-bit key")
.shouldContain("Signature algorithm: SHA256withRSA, 2048-bit key")
.shouldNotContain("Invalid certificate chain: Algorithm constraints check failed on signature algorithm: MD5withRSA")
.shouldHaveExitValue(0);

Files.deleteIfExists(Paths.get(JAVA_SECURITY_FILE));
Files.writeString(Files.createFile(Paths.get(JAVA_SECURITY_FILE)),
"jdk.certpath.disabledAlgorithms=MD5\n" +
"jdk.jar.disabledAlgorithms=\n");

SecurityTools.jarsigner("-keystore ks -storepass changeit " +
"-signedjar signeda.jar " +
"-verbose " +
"-J-Djava.security.properties=" +
JAVA_SECURITY_FILE +
" a.jar ee")
.shouldContain("Signature algorithm: MD5withRSA (disabled), 2048-bit key")
.shouldContain("Signature algorithm: SHA256withRSA, 2048-bit key")
.shouldContain("Invalid certificate chain: Algorithm constraints check failed on signature algorithm: MD5withRSA")
.shouldHaveExitValue(0);

kt("-exportcert -alias cacert -rfc -file cacert", "ks");
kt("-importcert -noprompt -file cacert", "caks1");

SecurityTools.jarsigner("-verify -certs signeda.jar " +
"-keystore caks1 -storepass changeit -verbose -debug")
.shouldContain("Signature algorithm: MD5withRSA (disabled), 2048-bit key")
.shouldContain("Signature algorithm: SHA256withRSA, 2048-bit key")
.shouldContain("Invalid certificate chain: Algorithm constraints check failed on signature algorithm: MD5withRSA")
.shouldHaveExitValue(0);
}
}

1 comment on commit 995e956

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.