On Fri, 27 Feb 2026 13:17:29 GMT, Weijun Wang <[email protected]> wrote:

>> src/java.base/share/classes/sun/security/ec/XDHPublicKeyImpl.java line 94:
>> 
>>> 92:         this.u = (params == XECParameters.X448) ?
>>> 93:             new BigInteger(1, u_arr) :
>>> 94:             new BigInteger(1, u_arr).clearBit(255);
>> 
>> Aren't lines 86-89 already doing it?
>
> 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 calli 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?

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

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

Reply via email to