On Mon, 2 May 2022 15:08:17 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Updated spec in java.security
>
> 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.
Comment added.
> 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`.
Done.
> 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.
Done.
> 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.
Fixed this method. Changed to check the key size (via passing checkKey=true).
Added the suggested test to ensure that keytool will emit warning accordingly.
-------------
PR: https://git.openjdk.java.net/jdk/pull/8300
