On Wed, 18 Sep 2024 21:49:14 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/HKDFKeyDerivation.java
>> line 92:
>>
>>> 90: }
>>> 91: this.hmacAlgName = hmacAlgName;
>>> 92: this.hmacLen = hmacLen;
>>
>> Instead of doing a binary search whenever an HKDFKeyDerivation object is
>> constructed, it is better to organize the algorithm and output length into
>> an enum, this way, supporting new Hmac algorithms would require adding new
>> enum value. This should be sufficient since these arguments are internally
>> supplied. For example,
>>
>> public enum SupportedHmac {
>> SHA256("HmacSHA256", 32),
>> SHA384("HmacSHA384", 48),
>> SHA512("HmacSHA512", 64);
>>
>> public final String algo;
>> public final int outLen;
>> private SupportedHmac(String algo, int outLen) {
>> this.algo = algo;
>> this.outLen = outLen;
>> }
>> };
>
> Then the constructor can specify enum as the argument, e.g.
>
> private HKDFKeyDerivation(SupportedHmac h,
> KDFParameters kdfParameters)
> throws InvalidAlgorithmParameterException {
> super(kdfParameters);
> if (kdfParameters != null) {
> throw new InvalidAlgorithmParameterException(
> h.algo + " does not support parameters");
> }
> this.hmacAlgName = h.algo;
> this.hmacLen = h.outLen;
> }
Addressed in
https://github.com/openjdk/jdk/pull/20301/commits/f786a38179651a5bca8c4884eeb52d2cef0adc78.
>> src/java.base/share/classes/com/sun/crypto/provider/HKDFKeyDerivation.java
>> line 396:
>>
>>> 394: public HKDFSHA256(KDFParameters kdfParameters)
>>> 395: throws InvalidAlgorithmParameterException {
>>> 396: super("HmacSHA256", SHA256_HMAC_SIZE, kdfParameters);
>>
>> Using the enum, this line would be:
>> ` super(SupportedHmac.SHA256, kdfParameters);`
>
> Similar changes to the other child classes below.
Addressed in
https://github.com/openjdk/jdk/pull/20301/commits/f786a38179651a5bca8c4884eeb52d2cef0adc78.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1769194465
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1769194017
