On Fri, 29 Apr 2022 19:42:27 GMT, Hai-May Chao <hc...@openjdk.org> wrote:
>> Please review these changes to add DES/3DES/MD5 to >> `jdk.security.legacyAlgorithms` security property, and to add the legacy >> algorithm constraint checking to `keytool` commands that are associated with >> secret key entries stored in the keystore. These `keytool` commands are >> -genseckey, -importpass, -list, and -importkeystore. As a result, `keytool` >> will be able to generate warnings when it detects that the secret key based >> algorithms and PBE based Mac and cipher algorithms are weak. Also removes >> the "This algorithm will be disabled in a future update.” from the existing >> warnings for the asymmetric keys/certificates. >> Will also file a CSR. > > Hai-May Chao has updated the pull request incrementally with one additional > commit since the last revision: > > Updated spec in java.security Changes requested by mullan (Reviewer). src/java.base/share/classes/sun/security/tools/keytool/Main.java line 2196: > 2194: > 2195: try { > 2196: SecretKey secKey = (SecretKey) keyStore.getKey(alias, > storePass); This means any secret key entries that are protected by a different password than `storePass` will throw an `UnrecoverableKeyException` and we will not be able to check the constraints. For PKCS12 this is not an issue since `storePass` and `keyPass` have to be the same. But for other keystores like JCEKS it might be a problem. However, I note this is not really a new issue as details about secret key entries other than the fact they exist as entries are not listed, presumably because we may not have the right password. I would recommend adding a comment explaining this. For a future RFE, it might be useful to enhance `keytool -list -alias` to have a `-keypass` option so that additional information for entries protected by a different password than `storePass` could be listed, such as their algorithm and key size. src/java.base/share/classes/sun/security/tools/keytool/Main.java line 5039: > 5037: private void checkWeakConstraint(String label, String algName, > 5038: ConstraintsParameters cp) { > 5039: // Do not check disabled algorithms for symmetric key based > algorithms for now If this method is intended only to be called for symmetric keys, you should change the type of `cp` to `SecretKeyConstraintsParameters`. src/java.base/share/classes/sun/security/tools/keytool/Main.java line 5248: > 5246: private static class SecretKeyConstraintsParameters implements > ConstraintsParameters { > 5247: private final Key key; > 5248: public SecretKeyConstraintsParameters(Key key) { This ctor could just be private. src/java.base/share/classes/sun/security/tools/keytool/Main.java line 5259: > 5257: @Override > 5258: public Set<Key> getKeys() { > 5259: return null; You should return `Set.of(key)` here. This allows the constraints to also check the key size, if that is also restricted. I would suggest adding a test where `jdk.security.legacyAlgorithms` includes a constraint for "AES < 256" to see if `keytool` warns about it when generating an AES key of 128 bits. ------------- PR: https://git.openjdk.java.net/jdk/pull/8300