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

[Sean Mullan]

On Thu, 28 Apr 2022 06:46:35 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:
> 
>   SecretKeyConstraintsParameters subclass created and property description updated

A few more comments.

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

> 1860:                     keysize = 168;
> 1861:                 } else if ("RC2".equalsIgnoreCase(keyAlgName)) {
> 1862:                     keysize = 128;

There are other algorithms that are also weak such as RC4, and maybe more in the future. Rather than special-casing each of them, I think instead you should call `keygen.init()` for DES and DESede only where you know that the keysize is always fixed.

src/java.base/share/conf/security/java.security line 641:

> 639: 
> 640: #
> 641: # Legacy cryptographic algorithms and key lengths

Nit: add period at end of sentence.

test/jdk/sun/security/tools/keytool/ReadJar.java line 26:

> 24: /**
> 25:  * @test
> 26:  * @bug 6890872 8168882 8257722 8255552

Here we are just updating the warning message, so I don't think the bugid needs to be added.

test/jdk/sun/security/tools/keytool/ReadJar.java line 162:

> 160:                 .shouldContain("Certificate #2:")
> 161:                 .shouldContain("Signer #2:")
> 162:                 .shouldNotMatch("The certificate #.* of signer #.*" + "uses the SHA1withRSA.*will be disabled")

You probably don't need to check for a non-occurrence here since the message has been changed and can no longer occur. I also think it doesn't need to list the bugid because it is not testing the main fix which is warnings on symmetric key algs.

-------------

Changes requested by mullan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8300