RFR: 8340327: A common framework to support public key algorithms with standard parameter sets [v10]

[Weijun Wang]

On Tue, 8 Oct 2024 19:11:45 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   null check as asserts, and better exception messages
>
> src/java.base/share/classes/sun/security/provider/NamedSignature.java line 121:
> 
>> 119:             return implSign(name, secKey, sk2, msg, appRandom);
>> 120:         } else {
>> 121:             throw new IllegalStateException("No private key");
> 
> `Signature.engineSign` is not specified to throw `IllegalStateException`, even though it seems like the more correct exception to throw. So this should probably be a `SignatureException`.
> 
> Same comment for `engineVerify`.

In fact, this should never happen since `engineVerify` should only be called when there is a public key (`Signature::verify` checks if `state == VERIFY`). But I'll use `SignatureException`.

> src/java.base/share/classes/sun/security/provider/NamedSignature.java line 196:
> 
>> 194:     /// This object will be passed into the [#implVerify] method along with the raw key.
>> 195:     ///
>> 196:     /// The default implementation returns `null`.
> 
> If the majority of implementations are supposed to implement this method, then it may be better to make it abstract (so that it won't be accidentally missed) and add a note that subclasses should return null if they don't have any additional checks.

I'll discuss my customers.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792455162
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1792452057