RFR: 8325448: Hybrid Public Key Encryption [v3]

[Weijun Wang]

On Fri, 28 Feb 2025 16:05:58 GMT, Kevin Driver <kdriver at openjdk.org> wrote:

>> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   example and KAT
>
> src/java.base/share/classes/com/sun/crypto/provider/DHKEM.java line 323:
> 
>> 321:                     }
>> 322:                     byte[] bytes = kdf.deriveData(extract.thenExpand(labeledInfo(
>> 323:                             suiteId, CANDIDATE, I2OSP(counter, 1), Nsk), Nsk));
> 
> I'm not through every class yet, but is more input validation needed on `Nsk`, which ultimately becomes the length in the `HKDFParameterSpec`? Later in this class I see that it is checked to not exceed `65536`, but an `IllegalArgumentException` may be thrown here if the value is < 0. I see that you're throwing `Exception` from this method, but I thought I'd mention it since you are doing `HKDFParameterSpec` initialization in-line with the `deriveData` call.

These are internally hardcoded numbers and should never be illegal. If it really goes wrong, there must be a coding error somewhere. I've added statements like `assert n < 65536` to ensure these. I didn't add an assertion like `assert n >= 0` because I think it's even more unlikely to happen.

> src/java.base/share/classes/com/sun/crypto/provider/DHKEM.java line 406:
> 
>> 404:     }
>> 405: 
>> 406:     public static byte[] I2OSP(int n, int w) {
> 
> A comment (non-javadoc) might be beneficial to explain why this method is doing what it is doing.

Sure. I can add some references into the RFC.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r1975905712
PR Review Comment: https://git.openjdk.org/jdk/pull/18411#discussion_r1975907356