From b00b70c2400d28070d26630614a010bc52237827 Mon Sep 17 00:00:00 2001
From: Hai-May Chao <hchao@openjdk.org>
Date: Thu, 2 Feb 2023 21:17:08 +0000
Subject: [PATCH] 8286907: keytool should warn about weak PBE algorithms

Reviewed-by: mullan, weijun
---
 .../sun/security/tools/keytool/Main.java      | 17 +++++++++++++-
 .../tools/keytool/WeakSecretKeyTest.java      | 23 +++++++++++++++++--
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/src/java.base/share/classes/sun/security/tools/keytool/Main.java b/src/java.base/share/classes/sun/security/tools/keytool/Main.java
index 95f0c99fc2f..edb0b41950a 100644
--- a/src/java.base/share/classes/sun/security/tools/keytool/Main.java
+++ b/src/java.base/share/classes/sun/security/tools/keytool/Main.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2023, 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
@@ -1837,6 +1837,11 @@ public final class Main {
                 useDefaultPBEAlgorithm = false;
             }
 
+            SecretKeyConstraintsParameters skcp =
+                    new SecretKeyConstraintsParameters(secKey);
+            checkWeakConstraint(rb.getString("the.generated.secretkey"),
+                    keyAlgName, skcp);
+
             if (verbose) {
                 MessageFormat form = new MessageFormat(rb.getString(
                     "Generated.keyAlgName.secret.key"));
@@ -5068,6 +5073,16 @@ public final class Main {
         }
     }
 
+    private void checkWeakConstraint(String label, String keyAlg,
+            SecretKeyConstraintsParameters skcp) {
+        try {
+            LEGACY_CHECK.permits(keyAlg, skcp, false);
+        } catch (CertPathValidatorException e) {
+            weakWarnings.add(String.format(
+                    rb.getString("key.algorithm.weak"), label, keyAlg));
+        }
+    }
+
     private void checkWeak(String label, CRL crl, Key key) {
         if (crl instanceof X509CRLImpl impl) {
             checkWeak(label, impl.getSigAlgName(), key);
diff --git a/test/jdk/sun/security/tools/keytool/WeakSecretKeyTest.java b/test/jdk/sun/security/tools/keytool/WeakSecretKeyTest.java
index 9cf917f6682..5eaba11f9eb 100644
--- a/test/jdk/sun/security/tools/keytool/WeakSecretKeyTest.java
+++ b/test/jdk/sun/security/tools/keytool/WeakSecretKeyTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, 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 8255552 8286090
+ * @bug 8255552 8286090 8286907
  * @summary Test keytool commands associated with secret key entries which use weak algorithms
  * @library /test/lib
  */
@@ -108,5 +108,24 @@ public class WeakSecretKeyTest {
                 .shouldContain("Warning")
                 .shouldMatch("The generated secret key uses a 128-bit AES key.*considered a security risk")
                 .shouldHaveExitValue(0);
+
+        SecurityTools.keytool("-keystore ks.p12 -storepass changeit " +
+                "-genseckey -keyalg PBEWithMD5AndDES -alias pbekey1")
+                .shouldContain("Warning")
+                .shouldMatch("The generated secret key uses the PBEWithMD5AndDES algorithm.*considered a security risk")
+                .shouldHaveExitValue(0);
+
+        SecurityTools.keytool("-keystore ks.p12 -storepass changeit " +
+                "-genseckey -keyalg PBEWithSHA1AndDESede -alias pbekey2")
+                .shouldContain("Warning")
+                .shouldMatch("The generated secret key uses the PBEWithSHA1AndDESede algorithm.*considered a security risk")
+                .shouldHaveExitValue(0);
+
+        SecurityTools.setResponse("changeit", "changeit");
+        SecurityTools.keytool("-keystore ks.p12 -storepass changeit " +
+                "-importpass -keyalg PBEWithMD5AndDES -alias newentry")
+                .shouldContain("Warning")
+                .shouldMatch("The generated secret key uses the PBEWithMD5AndDES algorithm.*considered a security risk")
+                .shouldHaveExitValue(0);
     }
 }