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