On Mon, 2 Mar 2026 21:15:56 GMT, Ben Perez <[email protected]> wrote:

>> In fact, `TestXDH` has tests on DER keys which shows this part has been 
>> working.
>> 
>> On the other hand, `TestXECOps` only tests on `encodedPointMultiply(byte[], 
>> byte[])`. Shall we also cover `encodedPointMultiply(byte[], BigInteger)` 
>> there to make sure it's safe to directly call these internal methods? (I 
>> admit that I've encouraged calling them in 
>> https://github.com/openjdk/jdk/pull/26032#issuecomment-3152384313).
>
> Good catch - I think the MSB zeroing functionality only needed to be added to 
> the `XDHPublicKeyImpl(XECParameters params, BigInteger u)` method.
> 
> As for `TestXECOps`, are you suggesting we add a similar test to the existing 
> `runDiffieHellmanTest` but use the `encodedPointMultiply(byte[], BigInteger)` 
> method instead? I don't think that method needs to zero out the MSB since 
> that information should be contained in the public key itself?

Yes, I'd suggest that test. The test will show if zeroing the MSB is needed but 
I guess yes. The `encodedPointMultiply` with bytes calls `decodeU` and it masks 
the MSB. I don't see `encodedPointMultiply` with `BigInteger` did it so I guess 
some code change is needed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29750#discussion_r2874915819

Reply via email to