RFR: 8298127: HSS/LMS Signature Verification [v17]

[Ferenc Rakoczi]

On Wed, 24 May 2023 16:22:55 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Ferenc Rakoczi has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Removed dead code, accepted code style suggestions.
>
> src/java.base/share/classes/sun/security/provider/HSS.java line 58:
> 
>> 56: 
>> 57:     @Override
>> 58:     protected void engineInitSign(PrivateKey privateKey)
> 
> I think you should also override `engineInitSign(PrivateKey, SecureRandom)` even though the default impl. calls `engineInitSign(PrivateKey)`. It also assigns the `SecureRandom` to the protected `appRandom` field, which is probably best avoided.

Done.

> src/java.base/share/classes/sun/security/provider/HSS.java line 156:
> 
>> 154:             if ((inLen < (24 + m)) || (checkExactLength && (inLen != (24 + m))) ||
>> 155:                     !lmsParams.hasSameHash(lmotsParams)) {
>> 156:                 throw new InvalidKeyException("Wrong LMS public Key length");
> 
> s/Key/key/

Changed.

> src/java.base/share/classes/sun/security/provider/HSS.java line 664:
> 
>> 662:         protected PublicKey engineGeneratePublic(KeySpec keySpec)
>> 663:                 throws InvalidKeySpecException {
>> 664:             if (keySpec instanceof X509EncodedKeySpec) {
> 
> Can you also use `instanceof x` here and avoid the cast on line 666?

Done.

> src/java.base/share/classes/sun/security/provider/HSS.java line 700:
> 
>> 698:                     return keySpec.cast(new X509EncodedKeySpec(key.getEncoded()));
>> 699:                 }
>> 700:                 throw new InvalidKeySpecException("keySpec is not an X509 one");
> 
> Suggest saying name of class: "keySpec is not an X509EncodedKeySpec"

Said.

> src/java.base/share/classes/sun/security/provider/HSS.java line 787:
> 
>> 785: 
>> 786:         @java.io.Serial
>> 787:         protected Object writeReplace() throws java.io.ObjectStreamException {
> 
> Make this `private` instead of `protected`. Also, add an overridden private `readObject` method that simply throws an exception, ex: 
> 
> `throw new InvalidObjectException("HSS public keys are not directly deserializable");
> `

Privatized writeReplaced() and added readObject().

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1205575050
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1205574950
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1205575467
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1205575343
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1205575158