8257722: Improve "keytool -printcert -jarfile" output

Reviewed-by: weijun

Author: Hai-May Chao

src/java.base/share/classes/sun/security/tools/keytool/Main.java

@@ -2847,6 +2847,23 @@ private static String oneInMany(String label, int i, int num) {
         }
     }
 
+    private static String oneInManys(String label, int certNo, int certCnt, int signerNo,
+        int signerCnt) {
+        if (certCnt == 1 && signerCnt == 1) {
+            return label;
+        }
+        if (certCnt > 1 && signerCnt == 1) {
+            return String.format(rb.getString("one.in.many1"), label, certNo);
+        }
+        if (certCnt == 1 && signerCnt > 1) {
+            return String.format(rb.getString("one.in.many2"), label, signerNo);
+        }
+        if (certCnt > 1 && signerCnt > 1) {
+            return String.format(rb.getString("one.in.many3"), label, certNo, signerNo);
+        }
+        return label;
+    }
+
     private void doPrintCert(final PrintStream out) throws Exception {
         if (jarfile != null) {
             // reset "jdk.certpath.disabledAlgorithms" security property
@@ -2855,7 +2872,7 @@ private void doPrintCert(final PrintStream out) throws Exception {
 
             JarFile jf = new JarFile(jarfile, true);
             Enumeration<JarEntry> entries = jf.entries();
-            Set<CodeSigner> ss = new HashSet<>();
+            LinkedHashSet<CodeSigner> ss = new LinkedHashSet<>();
             byte[] buffer = new byte[8192];
             int pos = 0;
             while (entries.hasMoreElements()) {
@@ -2872,48 +2889,59 @@ private void doPrintCert(final PrintStream out) throws Exception {
                     for (CodeSigner signer: signers) {
                         if (!ss.contains(signer)) {
                             ss.add(signer);
-                            out.printf(rb.getString("Signer.d."), ++pos);
-                            out.println();
-                            out.println();
-                            out.println(rb.getString("Signature."));
-                            out.println();
-
-                            List<? extends Certificate> certs
-                                    = signer.getSignerCertPath().getCertificates();
-                            int cc = 0;
-                            for (Certificate cert: certs) {
-                                X509Certificate x = (X509Certificate)cert;
-                                if (rfc) {
-                                    out.println(rb.getString("Certificate.owner.") + x.getSubjectX500Principal() + "\n");
-                                    dumpCert(x, out);
-                                } else {
-                                    printX509Cert(x, out);
-                                }
-                                out.println();
-                                checkWeak(oneInMany(rb.getString("the.certificate"), cc++, certs.size()), x);
-                            }
-                            Timestamp ts = signer.getTimestamp();
-                            if (ts != null) {
-                                out.println(rb.getString("Timestamp."));
-                                out.println();
-                                certs = ts.getSignerCertPath().getCertificates();
-                                cc = 0;
-                                for (Certificate cert: certs) {
-                                    X509Certificate x = (X509Certificate)cert;
-                                    if (rfc) {
-                                        out.println(rb.getString("Certificate.owner.") + x.getSubjectX500Principal() + "\n");
-                                        dumpCert(x, out);
-                                    } else {
-                                        printX509Cert(x, out);
-                                    }
-                                    out.println();
-                                    checkWeak(oneInMany(rb.getString("the.tsa.certificate"), cc++, certs.size()), x);
-                                }
-                            }
                         }
                     }
                 }
             }
+
+            for (CodeSigner signer: ss) {
+                out.printf(rb.getString("Signer.d."), ++pos);
+                out.println();
+                out.println();
+
+                List<? extends Certificate> certs
+                        = signer.getSignerCertPath().getCertificates();
+                int cc = 0;
+                for (Certificate cert: certs) {
+                    out.printf(rb.getString("Certificate.d."), ++cc);
+                    out.println();
+                    X509Certificate x = (X509Certificate)cert;
+                    if (rfc) {
+                        out.println(rb.getString("Certificate.owner.") + x.getSubjectX500Principal() + "\n");
+                        dumpCert(x, out);
+                    } else {
+                        printX509Cert(x, out);
+                    }
+                    out.println();
+                    checkWeak(oneInManys(rb.getString(
+                            "the.certificate"), cc,
+                            certs.size(), pos,
+                            ss.size()), x);
+                }
+                Timestamp ts = signer.getTimestamp();
+                if (ts != null) {
+                    out.println(rb.getString("Timestamp."));
+                    out.println();
+                    certs = ts.getSignerCertPath().getCertificates();
+                    cc = 0;
+                    for (Certificate cert: certs) {
+                        out.printf(rb.getString("Certificate.d."), ++cc);
+                        out.println();
+                        X509Certificate x = (X509Certificate)cert;
+                        if (rfc) {
+                            out.println(rb.getString("Certificate.owner.") + x.getSubjectX500Principal() + "\n");
+                            dumpCert(x, out);
+                        } else {
+                            printX509Cert(x, out);
+                        }
+                        out.println();
+                        checkWeak(oneInManys(rb.getString(
+                                "the.tsa.certificate"), cc,
+                                certs.size(), pos,
+                                ss.size()), x);
+                    }
+                }
+            }
             jf.close();
             if (ss.isEmpty()) {
                 out.println(rb.getString("Not.a.signed.jar.file"));

src/java.base/share/classes/sun/security/tools/keytool/Resources.java

@@ -397,8 +397,8 @@ public class Resources extends java.util.ListResourceBundle {
         {".WARNING.WARNING.WARNING.",
             "*****************  WARNING WARNING WARNING  *****************"},
         {"Signer.d.", "Signer #%d:"},
+        {"Certificate.d.", "Certificate #%d:"},
         {"Timestamp.", "Timestamp:"},
-        {"Signature.", "Signature:"},
         {"Certificate.owner.", "Certificate owner: "},
         {"Not.a.signed.jar.file", "Not a signed jar file"},
         {"No.certificate.from.the.SSL.server",
@@ -465,6 +465,9 @@ public class Resources extends java.util.ListResourceBundle {
         {"the.input", "The input"},
         {"reply", "Reply"},
         {"one.in.many", "%1$s #%2$d of %3$d"},
+        {"one.in.many1", "%1$s #%2$d"},
+        {"one.in.many2", "%1$s of signer #%2$d"},
+        {"one.in.many3", "%1$s #%2$d of signer #%3$d"},
         {"alias.in.cacerts", "Issuer <%s> in cacerts"},
         {"alias.in.keystore", "Issuer <%s>"},
         {"with.weak", "%s (weak)"},

test/jdk/sun/security/tools/jarsigner/TimestampCheck.java

@@ -722,51 +722,6 @@ static void checkMultiple(String file) throws Exception {
                 .shouldMatch("Signature algorithm: .*key.*(disabled)");
     }
 
-    static void checkWeak(String file) throws Exception {
-        verify(file)
-                .shouldHaveExitValue(0)
-                .shouldNotContain("treated as unsigned");
-        verify(file, "-verbose")
-                .shouldHaveExitValue(0)
-                .shouldNotContain("treated as unsigned")
-                .shouldMatch("Digest algorithm: .*(weak)")
-                .shouldMatch("Signature algorithm: .*(weak)")
-                .shouldMatch("Timestamp digest algorithm: .*(weak)")
-                .shouldNotMatch("Timestamp signature algorithm: .*(weak).*(weak)")
-                .shouldMatch("Timestamp signature algorithm: .*key.*(weak)");
-        verify(file, "-J-Djava.security.debug=jar")
-                .shouldHaveExitValue(0)
-                .shouldNotMatch("SignatureException:.*disabled");
-
-        // keytool should print out warnings when reading or
-        // generating cert/cert req using legacy algorithms.
-        String sout = SecurityTools.keytool("-printcert -jarfile " + file)
-                .stderrShouldContain("The TSA certificate uses a 1024-bit RSA key" +
-                        " which is considered a security risk." +
-                        " This key size will be disabled in a future update.")
-                .getStdout();
-        if (sout.indexOf("weak", sout.indexOf("Timestamp:")) < 0) {
-            throw new RuntimeException("timestamp not weak: " + sout);
-        }
-    }
-
-    static void checkHalfWeak(String file) throws Exception {
-        verify(file)
-                .shouldHaveExitValue(0)
-                .shouldNotContain("treated as unsigned");
-        verify(file, "-verbose")
-                .shouldHaveExitValue(0)
-                .shouldNotContain("treated as unsigned")
-                .shouldMatch("Digest algorithm: .*(weak)")
-                .shouldNotMatch("Signature algorithm: .*(weak)")
-                .shouldNotMatch("Signature algorithm: .*(disabled)")
-                .shouldNotMatch("Timestamp digest algorithm: .*(weak)")
-                .shouldNotMatch("Timestamp signature algorithm: .*(weak).*(weak)")
-                .shouldNotMatch("Timestamp signature algorithm: .*(disabled).*(disabled)")
-                .shouldNotMatch("Timestamp signature algorithm: .*key.*(weak)")
-                .shouldNotMatch("Timestamp signature algorithm: .*key.*(disabled)");
-    }
-
     static void checkMultipleWeak(String file) throws Exception {
         verify(file)
                 .shouldHaveExitValue(0)

test/jdk/sun/security/tools/keytool/ReadJar.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2021, 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
@@ -23,7 +23,7 @@
 
 /**
  * @test
- * @bug 6890872 8168882
+ * @bug 6890872 8168882 8257722
  * @summary keytool -printcert to recognize signed jar files
  * @library /test/lib
  * @build jdk.test.lib.SecurityTools
@@ -42,11 +42,24 @@
 import jdk.test.lib.SecurityTools;
 import jdk.test.lib.process.OutputAnalyzer;
 import jdk.test.lib.util.JarUtils;
+import java.nio.file.Path;
 
 public class ReadJar {
 
+    static OutputAnalyzer kt(String cmd, String ks) throws Exception {
+        return SecurityTools.keytool("-storepass changeit " + cmd +
+                " -keystore " + ks);
+    }
+
+    static void gencert(String owner, String cmd) throws Exception {
+        kt("-certreq -alias " + owner + " -file tmp.req", "ks");
+        kt("-gencert -infile tmp.req -outfile tmp.cert " + cmd, "ks");
+        kt("-importcert -alias " + owner + " -file tmp.cert", "ks");
+    }
+
     public static void main(String[] args) throws Throwable {
         testWithMD5();
+        testCertOutput();
     }
 
     // make sure that -printcert option works
@@ -91,4 +104,87 @@ private static void printCert(String jar) throws Throwable {
         out.shouldHaveExitValue(0);
         out.shouldNotContain("Not a signed jar file");
     }
+
+    private static void testCertOutput() throws Throwable {
+        kt("-genkeypair -keyalg rsa -alias e0 -dname CN=E0 " +
+                "-keysize 512", "ks");
+        JarUtils.createJarFile(Path.of("a0.jar"), Path.of("."), Path.of("ks"));
+        // sign a0.jar file
+        SecurityTools.jarsigner("-keystore ks -storepass changeit " +
+                " a0.jar e0")
+                .shouldHaveExitValue(0);
+
+        SecurityTools.keytool("-printcert -jarfile a0.jar")
+                .shouldNotContain("Signature:")
+                .shouldContain("Signer #1:")
+                .shouldContain("Certificate #1:")
+                .shouldNotContain("Certificate #2:")
+                .shouldNotContain("Signer #2:")
+                .shouldMatch("The certificate uses a 512-bit RSA key.*is disabled")
+                .shouldHaveExitValue(0);
+
+        kt("-genkeypair -keyalg rsa -alias ca1 -dname CN=CA1 -ext bc:c " +
+                "-keysize 512", "ks");
+        kt("-genkeypair -keyalg rsa -alias e1 -dname CN=E1", "ks");
+        gencert("e1", "-alias ca1 -ext san=dns:e1");
+
+        JarUtils.createJarFile(Path.of("a1.jar"), Path.of("."), Path.of("ks"));
+        // sign a1.jar file
+        SecurityTools.jarsigner("-keystore ks -storepass changeit " +
+                " a1.jar e1")
+                .shouldHaveExitValue(0);
+
+        SecurityTools.keytool("-printcert -jarfile a1.jar")
+                .shouldNotContain("Signature:")
+                .shouldContain("Signer #1:")
+                .shouldContain("Certificate #1:")
+                .shouldContain("Certificate #2:")
+                .shouldNotContain("Signer #2:")
+                .shouldMatch("The certificate #2 uses a 512-bit RSA key.*is disabled")
+                .shouldHaveExitValue(0);
+
+        kt("-genkeypair -keyalg rsa -alias ca2 -dname CN=CA2 -ext bc:c " +
+                "-sigalg SHA1withRSA", "ks");
+        kt("-genkeypair -keyalg rsa -alias e2 -dname CN=E2", "ks");
+        gencert("e2", "-alias ca2 -ext san=dns:e2");
+
+        // sign a1.jar file again with different signer
+        SecurityTools.jarsigner("-keystore ks -storepass changeit " +
+                " a1.jar e2")
+                .shouldHaveExitValue(0);
+
+        SecurityTools.keytool("-printcert -jarfile a1.jar")
+                .shouldNotContain("Signature:")
+                .shouldContain("Signer #1:")
+                .shouldContain("Certificate #1:")
+                .shouldContain("Certificate #2:")
+                .shouldContain("Signer #2:")
+                .shouldMatch("The certificate #.* of signer #.*" + "uses the SHA1withRSA.*will be disabled")
+                .shouldMatch("The certificate #.* of signer #.*" + "uses a 512-bit RSA key.*is disabled")
+                .shouldHaveExitValue(0);
+
+        kt("-genkeypair -keyalg rsa -alias e3 -dname CN=E3",
+                "ks");
+        JarUtils.createJarFile(Path.of("a2.jar"), Path.of("."), Path.of("ks"));
+        // sign a2.jar file
+        SecurityTools.jarsigner("-keystore ks -storepass changeit " +
+                " a2.jar e3")
+                .shouldHaveExitValue(0);
+
+        kt("-genkeypair -keyalg rsa -alias e4 -dname CN=E4 " +
+                "-keysize 1024", "ks");
+        // sign a2.jar file again with different signer
+        SecurityTools.jarsigner("-keystore ks -storepass changeit " +
+                " a2.jar e4")
+                .shouldHaveExitValue(0);
+
+        SecurityTools.keytool("-printcert -jarfile a2.jar")
+                .shouldNotContain("Signature:")
+                .shouldContain("Signer #1:")
+                .shouldContain("Certificate #1:")
+                .shouldNotContain("Certificate #2:")
+                .shouldContain("Signer #2:")
+                .shouldMatch("The certificate of signer #.*" + "uses a 1024-bit RSA key.*will be disabled")
+                .shouldHaveExitValue(0);
+    }
 }