On Wed, 4 May 2022 05:55:08 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:
> 
>   Skip alg constraint check for PBE secret key entry

Changes requested by mullan (Reviewer).

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

> 2206:                  * is not really a new issue as details about secret 
> key entries
> 2207:                  * other than the fact they exist as entries are not 
> listed ,
> 2208:                  * presumably because we may not have the right 
> password.

I would leave out this last sentence as that was more of an editorial comment 
by me. In the first sentence, I would add at the end "... and we will not be 
able to check the constraints because we do not have the keyPass for this 
operation."

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

> 5284:         @Override
> 5285:         public Set<Key> getKeys() {
> 5286:             return (key == null) ? Set.of() : Set.of(key);

key should never be null, so you don't need to check for this.

test/jdk/sun/security/tools/keytool/WeakSecretKeyTest.java line 92:

> 90:                 .shouldContain("Warning")
> 91:                 .shouldMatch("The generated secret key uses a 128-bit AES 
> key.*considered a security risk")
> 92:                 .shouldHaveExitValue(0);

Nice - thanks for adding this test.

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

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

Reply via email to