RFR: 8255552: Add DES/3DES/MD5 to jdk.security.legacyAlgorithms [v5]

[Sean Mullan]

On Fri, 29 Apr 2022 19:42:27 GMT, Hai-May Chao <hchao at 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