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

[Ferenc Rakoczi]

On Fri, 12 May 2023 22:11:07 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Ferenc Rakoczi has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Removed Length from HSSPublicKey, changed the handling of X509 encoded keys in the factory, did some more beautification.
>
> src/java.base/share/classes/sun/security/provider/HSS.java line 38:
> 
>> 36: 
>> 37: /*
>> 38:  * This class implements the Hierarchical Signature System using the
> 
> Add the short name "(HSS)" after the long name.

Added.

> src/java.base/share/classes/sun/security/provider/HSS.java line 96:
> 
>> 94:         HSSSignature sig = new HSSSignature(signature, pubKey);
>> 95:         LMSPublicKey lmsPubKey = pubKey.lmsPublicKey;
>> 96:         boolean result = true;
> 
> Please put all 3 lines above into the `try` block so we make sure `messageStream.reset()` is always called.

Done.

> src/java.base/share/classes/sun/security/provider/SHA2.java line 51:
> 
>> 49: 
>> 50:     private static final int ITERATION = 64;
>> 51:     private static final int BLOCKSIZE = 64;
> 
> I'm not sure if it's worth defining this. A lot of other numbers (like 56 and 120) are hardcoded and it's not easy to modify this number to implement a different algorithm.

This is not about modifiability but about readability. It is easier to understand the code if the constants have names. Especially when there exist constants with the same value that serve different purposes.

> src/java.base/share/classes/sun/security/provider/SunEntries.java line 221:
> 
>> 219:         addWithAlias(p, "KeyFactory", "DSA",
>> 220:                 "sun.security.provider.DSAKeyFactory", attrs);
>> 221:         addWithAlias(p, "KeyFactory", "HSS/LMS", "sun.security.provider.HSS$KeyFactoryImpl", attrs);
> 
> This line can be shorter. Overall, it will be nice to keep lines (including comment lines) shorter than 80 chars. If the code does look really ugly, better no more than 120 chars.

I think in the 21st century (when 80 column punch cards and 80-character wide screens are long gone) the 80 character line length is quite  an unreasonable requirement for code (decreases readability in most cases). Still, I made the lines shorter.

> src/java.base/share/classes/sun/security/util/RawKeySpec.java line 32:
> 
>> 30: /**
>> 31:  * This is a KeySpec that is used to specify a key by its byte array implementation.
>> 32:  * It is intended to be used in testing algorithms where the algorithm specification describes the key in this form.
> 
> This comment line can be shorter.

Split the line.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1193578661
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1193578727
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1193578474
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1193578546
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1193578593