On Fri, 29 Apr 2022 19:42:27 GMT, Hai-May Chao <[email protected]> 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
