8255552: Add DES/3DES/MD5 to jdk.security.legacyAlgorithms

Reviewed-by: mullan, weijun

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

@@ -63,6 +63,7 @@ import java.util.Base64;
 
 import sun.security.pkcs12.PKCS12KeyStore;
 import sun.security.provider.certpath.CertPathConstraintsParameters;
+import sun.security.util.ConstraintsParameters;
 import sun.security.util.ECKeySizeParameterSpec;
 import sun.security.util.KeyUtil;
 import sun.security.util.NamedCurve;
@@ -1869,6 +1870,11 @@ public final class Main {
             Object[] source = {keysize,
                                 secKey.getAlgorithm()};
             System.err.println(form.format(source));
+
+            SecretKeyConstraintsParameters skcp =
+                    new SecretKeyConstraintsParameters(secKey);
+            checkWeakConstraint(rb.getString("the.generated.secretkey"),
+                    secKey, skcp);
         }
 
         if (keyPass == null) {
@@ -2184,6 +2190,23 @@ public final class Main {
             } else {
                 out.println("SecretKeyEntry, ");
             }
+
+            try {
+                SecretKey secKey = (SecretKey) keyStore.getKey(alias, storePass);
+                SecretKeyConstraintsParameters skcp =
+                        new SecretKeyConstraintsParameters(secKey);
+                checkWeakConstraint(label, secKey, skcp);
+            } catch (UnrecoverableKeyException e) {
+                /*
+                 * UnrecoverableKeyException will be thrown for any secret key
+                 * entries that are protected by a different password than
+                 * storePass, and we will not be able to check the constraints
+                 * because we do not have the keyPass for this operation.
+                 * This may occurs for keystores such as JCEKS. Note that this
+                 * is not really a new issue as details about secret key entries
+                 * other than the fact they exist as entries are not listed.
+                 */
+            }
         } else if (keyStore.entryInstanceOf(alias, KeyStore.PrivateKeyEntry.class)) {
             if (verbose || rfc || debug) {
                 Object[] source = {"PrivateKeyEntry"};
@@ -2489,13 +2512,23 @@ public final class Main {
         }
 
         try {
+            keyStore.setEntry(newAlias, entry, pp);
             Certificate c = srckeystore.getCertificate(alias);
             if (c != null) {
                 CertPathConstraintsParameters cpcp =
                         buildCertPathConstraint((X509Certificate)c, null);
                 checkWeakConstraint("<" + newAlias + ">", c, cpcp);
+            } else {
+                try {
+                    Key key = keyStore.getKey(newAlias, newPass);
+                    SecretKeyConstraintsParameters skcp =
+                            new SecretKeyConstraintsParameters(key);
+                    checkWeakConstraint("<" + newAlias + ">", (SecretKey)key, skcp);
+                } catch (UnrecoverableKeyException e) {
+                    // skip
+                }
             }
-            keyStore.setEntry(newAlias, entry, pp);
+
             // Place the check so that only successful imports are blocked.
             // For example, we don't block a failed SecretEntry import.
             if (P12KEYSTORE.equalsIgnoreCase(storetype) && !isPasswordlessKeyStore) {
@@ -5009,6 +5042,38 @@ public final class Main {
         }
     }
 
+    private void checkWeakConstraint(String label, SecretKey secKey,
+            SecretKeyConstraintsParameters skcp) {
+        // Do not check disabled algorithms for symmetric key based algorithms for now
+        String secKeyAlg = secKey.getAlgorithm();
+        /*
+         * Skipping a secret key entry if its algorithm starts with "PBE".
+         * This is because keytool can only see it as a PBE key and "PBE" is
+         * an alias of "PBEwithMD5andDES" inside the SunJCE security provider,
+         * and its getAlgorithm() always returns "PBEwithMD5andDES". Thus, keytool
+         * won't be able to determine whether this secret key entry is protected
+         * by a weak algorithm or not.
+         */
+        if (secKeyAlg.startsWith("PBE"))
+            return;
+
+        try {
+            LEGACY_CHECK.permits(secKeyAlg, skcp, true);
+        } catch (CertPathValidatorException e) {
+            String eMessage = e.getMessage();
+            if (eMessage.contains("constraints check failed on keysize limits") &&
+                    e.getReason() == BasicReason.ALGORITHM_CONSTRAINED) {
+                weakWarnings.add(String.format(
+                        rb.getString("key.size.weak"), label,
+                        String.format(rb.getString("key.bit"),
+                        KeyUtil.getKeySize(secKey), secKeyAlg)));
+            } else {
+                weakWarnings.add(String.format(
+                        rb.getString("key.algorithm.weak"), label, secKeyAlg));
+            }
+        }
+    }
+
     private void checkWeakConstraint(String label, PKCS10 p10,
             CertPathConstraintsParameters cpcp) throws Exception {
         checkWeakConstraint(label, p10.getSigAlg(),
@@ -5206,6 +5271,38 @@ public final class Main {
         tinyHelp();
         return null;    // Useless, tinyHelp() already exits.
     }
+
+    private static class SecretKeyConstraintsParameters implements ConstraintsParameters {
+        private final Key key;
+        private SecretKeyConstraintsParameters(Key key) {
+            this.key = key;
+        }
+
+        @Override
+        public boolean anchorIsJdkCA() {
+            return false;
+        }
+
+        @Override
+        public Set<Key> getKeys() {
+            return Set.of(key);
+        }
+
+        @Override
+        public Date getDate() {
+            return null;
+        }
+
+        @Override
+        public String getVariant() {
+            return null;
+        }
+
+        @Override
+        public String extendedExceptionMsg() {
+            return null;
+        }
+    }
 }
 
 // This class is exactly the same as com.sun.tools.javac.util.Pair,

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

@@ -72,6 +72,8 @@ public class Resources extends java.util.ListResourceBundle {
                 "Generated {0} secret key"}, //-genseckey
         {"Generated.keysize.bit.keyAlgName.secret.key",
                 "Generated {0}-bit {1} secret key"}, //-genseckey
+        {"key.algorithm.weak", "%1$s uses the %2$s algorithm which is considered a security risk."},
+        {"key.size.weak", "%1$s uses a %2$s which is considered a security risk."},
         {"Imports.entries.from.a.JDK.1.1.x.style.identity.database",
                 "Imports entries from a JDK 1.1.x-style identity database"}, //-identitydb
         {"Imports.a.certificate.or.a.certificate.chain",
@@ -456,6 +458,7 @@ public class Resources extends java.util.ListResourceBundle {
         // generating cert/cert req using weak algorithms
         {"the.certificate.request", "The certificate request"},
         {"the.issuer", "The issuer"},
+        {"the.generated.secretkey", "The generated secret key"},
         {"the.generated.certificate", "The generated certificate"},
         {"the.generated.crl", "The generated CRL"},
         {"the.generated.certificate.request", "The generated certificate request"},
@@ -485,7 +488,7 @@ public class Resources extends java.util.ListResourceBundle {
         {"whose.sigalg.disabled", "%1$s uses the %2$s signature algorithm which is considered a security risk and is disabled."},
         {"whose.sigalg.usagesignedjar", "%1$s uses the %2$s signature algorithm which is considered a security risk and cannot be used to sign JARs after %3$s."},
         {"Unable.to.parse.denyAfter.string.in.exception.message", "Unable to parse denyAfter date string in exception message"},
-        {"whose.sigalg.weak", "%1$s uses the %2$s signature algorithm which is considered a security risk. This algorithm will be disabled in a future update."},
+        {"whose.sigalg.weak", "%1$s uses the %2$s signature algorithm which is considered a security risk."},
         {"whose.key.disabled", "%1$s uses a %2$s which is considered a security risk and is disabled."},
         {"whose.key.weak", "%1$s uses a %2$s which is considered a security risk. This key size will be disabled in a future update."},
         {"jks.storetype.warning", "The %1$s keystore uses a proprietary format. It is recommended to migrate to PKCS12 which is an industry standard format using \"keytool -importkeystore -srckeystore %2$s -destkeystore %2$s -deststoretype pkcs12\"."},