On Tue, 2 May 2023 21:43:19 GMT, Ferenc Rakoczi <d...@openjdk.org> wrote:

>> Implement support for Leighton-Micali Signatures (LMS) as described in RFC 
>> 8554. LMS is an approved software signing algorithm for CNSA 2.0, with 
>> SHA-256/192 parameters recommended.
>
> Ferenc Rakoczi has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   adding key translation, finally block, removing 24-byte LMOTS parameters

src/java.base/share/classes/sun/security/provider/HSS.java line 37:

> 35: import java.util.Arrays;
> 36: import java.util.Objects;
> 37: 

Can you add some comments describing this class?

src/java.base/share/classes/sun/security/provider/HSS.java line 38:

> 36: import java.util.Objects;
> 37: 
> 38: public class HSS extends SignatureSpi {

Make the class `final`?

src/java.base/share/classes/sun/security/provider/HSS.java line 43:

> 41: 
> 42:     @Deprecated
> 43:     protected void engineSetParameter(String param, Object value) {

Technically, this API is not defined to throw UOE. I think throwing IPE is 
better and would be compliant with the specification. (I am aware that there 
are other JDK SignatureSpi implementations that already throw UOE for this 
method, but they should be fixed).

src/java.base/share/classes/sun/security/provider/HSS.java line 48:

> 46: 
> 47:     @Deprecated
> 48:     protected AlgorithmParameters engineGetParameter(String param) {

Same comment as above.

src/java.base/share/classes/sun/security/provider/HSS.java line 56:

> 54:     }
> 55: 
> 56:     protected byte[] engineSign() throws SignatureException {

This API is not defined to throw UOE. Suggest throwing `SignatureException` 
instead with a message "Signing is not supported".

Though, technically this method should never be called, as `Signature.sign()` 
will throw a `SignatureException` because it has not been initialized for 
signing.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1186440423
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1186448496
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1186443155
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1186443576
PR Review Comment: https://git.openjdk.org/jdk/pull/13691#discussion_r1186445307

Reply via email to