On Wed, 24 May 2023 16:22:55 GMT, Sean Mullan <mul...@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
