8171319: keytool should print out warnings when reading or generating...

8171319: keytool should print out warnings when reading or generating cert/cert req using weak algorithms
Reviewed-by: coffeys

jdk/src/java.base/share/classes/sun/security/pkcs10/PKCS10.java

@@ -167,7 +167,8 @@ public class PKCS10 {
         // key and signature algorithm we found.
         //
         try {
-            sig = Signature.getInstance(id.getName());
+            sigAlg = id.getName();
+            sig = Signature.getInstance(sigAlg);
             sig.initVerify(subjectPublicKeyInfo);
             sig.update(data);
             if (!sig.verify(sigData))
@@ -218,6 +219,7 @@ public class PKCS10 {
         signature.update(certificateRequestInfo, 0,
                 certificateRequestInfo.length);
         sig = signature.sign();
+        sigAlg = signature.getAlgorithm();
 
         /*
          * Build guts of SIGNED macro
@@ -250,6 +252,11 @@ public class PKCS10 {
     public PublicKey getSubjectPublicKeyInfo()
         { return subjectPublicKeyInfo; }
 
+    /**
+     * Returns the signature algorithm.
+     */
+    public String getSigAlg() { return sigAlg; }
+
     /**
      * Returns the additional attributes requested.
      */
@@ -348,6 +355,7 @@ public class PKCS10 {
 
     private X500Name            subject;
     private PublicKey           subjectPublicKeyInfo;
+    private String              sigAlg;
     private PKCS10Attributes    attributeSet;
     private byte[]              encoded;        // signed
 }

jdk/src/java.base/share/classes/sun/security/provider/certpath/BasicChecker.java

@@ -51,7 +51,7 @@ import sun.security.util.Debug;
 
 /**
  * BasicChecker is a PKIXCertPathChecker that checks the basic information
- * on a PKIX certificate, namely the signature, timestamp, and subject/issuer
+ * on a PKIX certificate, namely the signature, validity, and subject/issuer
  * name chaining.
  *
  * @since       1.4
@@ -125,7 +125,7 @@ class BasicChecker extends PKIXCertPathChecker {
     }
 
     /**
-     * Performs the signature, timestamp, and subject/issuer name chaining
+     * Performs the signature, validity, and subject/issuer name chaining
      * checks on the certificate using its internal state. This method does
      * not remove any critical extensions from the Collection.
      *
@@ -141,7 +141,7 @@ class BasicChecker extends PKIXCertPathChecker {
         X509Certificate currCert = (X509Certificate)cert;
 
         if (!sigOnly) {
-            verifyTimestamp(currCert);
+            verifyValidity(currCert);
             verifyNameChaining(currCert);
         }
         verifySignature(currCert);
@@ -177,12 +177,12 @@ class BasicChecker extends PKIXCertPathChecker {
     }
 
     /**
-     * Internal method to verify the timestamp on a certificate
+     * Internal method to verify the validity on a certificate
      */
-    private void verifyTimestamp(X509Certificate cert)
+    private void verifyValidity(X509Certificate cert)
         throws CertPathValidatorException
     {
-        String msg = "timestamp";
+        String msg = "validity";
         if (debug != null)
             debug.println("---checking " + msg + ":" + date.toString() + "...");
 

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

@@ -360,8 +360,6 @@ public class Resources extends java.util.ListResourceBundle {
         {"Enter.alias.name.", "Enter alias name:  "},
         {".RETURN.if.same.as.for.otherAlias.",
                 "\t(RETURN if same as for <{0}>)"},
-        {".PATTERN.printX509Cert",
-                "Owner: {0}\nIssuer: {1}\nSerial number: {2}\nValid from: {3} until: {4}\nCertificate fingerprints:\n\t SHA1: {5}\n\t SHA256: {6}\nSignature algorithm name: {7}\nSubject Public Key Algorithm: {8} ({9,number,#})\nVersion: {10}"},
         {"What.is.your.first.and.last.name.",
                 "What is your first and last name?"},
         {"What.is.the.name.of.your.organizational.unit.",
@@ -428,16 +426,12 @@ public class Resources extends java.util.ListResourceBundle {
         {"Please.provide.keysize.for.secret.key.generation",
                 "Please provide -keysize for secret key generation"},
 
-        {"verified.by.s.in.s", "Verified by %s in %s"},
         {"warning.not.verified.make.sure.keystore.is.correct",
             "WARNING: not verified. Make sure -keystore is correct."},
 
         {"Extensions.", "Extensions: "},
         {".Empty.value.", "(Empty value)"},
         {"Extension.Request.", "Extension Request:"},
-        {"PKCS.10.Certificate.Request.Version.1.0.Subject.s.Public.Key.s.format.s.key.",
-                "PKCS #10 Certificate Request (Version 1.0)\n" +
-                "Subject: %s\nPublic Key: %s format %s key\n"},
         {"Unknown.keyUsage.type.", "Unknown keyUsage type: "},
         {"Unknown.extendedkeyUsage.type.", "Unknown extendedkeyUsage type: "},
         {"Unknown.AccessDescription.type.", "Unknown AccessDescription type: "},
@@ -446,7 +440,34 @@ public class Resources extends java.util.ListResourceBundle {
                  "This extension cannot be marked as critical. "},
         {"Odd.number.of.hex.digits.found.", "Odd number of hex digits found: "},
         {"Unknown.extension.type.", "Unknown extension type: "},
-        {"command.{0}.is.ambiguous.", "command {0} is ambiguous:"}
+        {"command.{0}.is.ambiguous.", "command {0} is ambiguous:"},
+
+        // 8171319: keytool should print out warnings when reading or
+        // generating cert/cert req using weak algorithms
+        {"the.certificate.request", "The certificate request"},
+        {"the.issuer", "The issuer"},
+        {"the.generated.certificate", "The generated certificate"},
+        {"the.generated.crl", "The generated CRL"},
+        {"the.generated.certificate.request", "The generated certificate request"},
+        {"the.certificate", "The certificate"},
+        {"the.crl", "The CRL"},
+        {"the.tsa.certificate", "The TSA certificate"},
+        {"the.input", "The input"},
+        {"reply", "Reply"},
+        {"one.in.many", "%s #%d of %d"},
+        {"alias.in.cacerts", "Issuer <%s> in cacerts"},
+        {"alias.in.keystore", "Issuer <%s>"},
+        {"with.weak", "%s (weak)"},
+        {"key.bit", "%d-bit %s key"},
+        {"key.bit.weak", "%d-bit %s key (weak)"},
+        {".PATTERN.printX509Cert.with.weak",
+                "Owner: {0}\nIssuer: {1}\nSerial number: {2}\nValid from: {3} until: {4}\nCertificate fingerprints:\n\t SHA1: {5}\n\t SHA256: {6}\nSignature algorithm name: {7}\nSubject Public Key Algorithm: {8}\nVersion: {9}"},
+        {"PKCS.10.with.weak",
+                "PKCS #10 Certificate Request (Version 1.0)\n" +
+                        "Subject: %s\nFormat: %s\nPublic Key: %s\nSignature algorithm: %s\n"},
+        {"verified.by.s.in.s.weak", "Verified by %s in %s with a %s"},
+        {"whose.sigalg.risk", "%s uses the %s signature algorithm which is considered a security risk."},
+        {"whose.key.risk", "%s uses a %s which is considered a security risk."},
     };
 
 

jdk/src/java.base/share/classes/sun/security/x509/X509CRLImpl.java

@@ -536,13 +536,18 @@ public class X509CRLImpl extends X509CRL implements DerEncoder {
      * @return value of this CRL in a printable form.
      */
     public String toString() {
+        return toStringWithAlgName("" + sigAlgId);
+    }
+
+    // Specifically created for keytool to append a (weak) label to sigAlg
+    public String toStringWithAlgName(String name) {
         StringBuilder sb = new StringBuilder();
         sb.append("X.509 CRL v")
             .append(version+1)
             .append('\n');
         if (sigAlgId != null)
             sb.append("Signature Algorithm: ")
-                .append(sigAlgId)
+                .append(name)
                 .append(", OID=")
                 .append(sigAlgId.getOID())
                 .append('\n');

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

@@ -43,6 +43,7 @@ import java.util.List;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 
+import jdk.test.lib.SecurityTools;
 import jdk.testlibrary.*;
 import jdk.testlibrary.JarUtils;
 import sun.security.pkcs.ContentInfo;
@@ -66,6 +67,7 @@ import sun.security.x509.X500Name;
  *          java.base/sun.security.util
  *          java.base/sun.security.tools.keytool
  * @library /lib/testlibrary
+ * @library /test/lib
  * @run main/timeout=600 TimestampCheck
  */
 public class TimestampCheck {
@@ -457,6 +459,18 @@ public class TimestampCheck {
         verify(file, "-J-Djava.security.debug=jar")
                 .shouldHaveExitValue(0)
                 .shouldMatch("SignatureException:.*disabled");
+
+        // For 8171319: keytool should print out warnings when reading or
+        //              generating cert/cert req using weak algorithms.
+        // Must call keytool the command, otherwise doPrintCert() might not
+        // be able to reset "jdk.certpath.disabledAlgorithms".
+        String sout = SecurityTools.keytool("-printcert -jarfile weak.jar")
+                .stderrShouldContain("The TSA certificate uses a 512-bit RSA key" +
+                        " which is considered a security risk.")
+                .getStdout();
+        if (sout.indexOf("weak", sout.indexOf("Timestamp:")) < 0) {
+            throw new RuntimeException("timestamp not weak: " + sout);
+        }
     }
 
     static void checkHalfWeak(String file) throws Throwable {