On Fri, 28 Apr 2023 13:52:30 GMT, Weijun Wang <[email protected]> wrote:
>> Ferenc Rakoczi has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review comments addressed
>
> src/java.base/share/classes/sun/security/provider/HSS.java line 86:
>
>> 84: lmsPubKey = sig.pubList[i];
>> 85: }
>> 86: return result & lmsVerify(lmsPubKey, sig.siglist[sig.Nspk],
>> messageStream.toByteArray());
>
> You should probably reset `messageStream` so that it can be called in another
> `update/verify` sequence. This is also worth a test.
Added.
> src/java.base/share/classes/sun/security/provider/HSS.java line 173:
>
>> 171: int m = lmParams.m;
>> 172: if ((inLen < (24 + m)) || (checkExactLength && (inLen !=
>> (24 + m))) ||
>> 173:
>> !LMOTSParams.of(otsType).hashAlgName.equals(lmParams.hashAlgStr)) {
>
> This algorithm name comparison is not sufficient. You are using "SHA-256" for
> both M32 and M24 types. Either add a comparison on `LMParams.m` and
> `LMOTSParams.n`, or use "SHA-256/192" as the hash algorithm name.
Changed.
> src/java.base/share/classes/sun/security/provider/HSS.java line 213:
>
>> 211:
>> 212: static class LMSUtils {
>> 213: public final static int LMS_RESERVED = 0;
>
> Is the `LMS_RESERVED` and `LMOTS_RESERVED` constants used anywhere?
No, but they are defined in the standard so it seemed to be a good idea to
document that this way.
> src/java.base/share/classes/sun/security/util/SecurityProviderConstants.java
> line 274:
>
>> 272: store("DSA", KnownOIDs.DSA, KnownOIDs.OIW_DSA.value());
>> 273:
>> 274: store("HSS/LMS", KnownOIDs.HSSLMS, KnownOIDs.HSSLMS.value());
>
> This is only necessary if an non-OID alias is needed. For example, if we want
> to use "LMS" as an alias of "HSS/LMS".
Removed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1183024118
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1183024398
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1183024209
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1183023989
