On Tue, 9 Apr 2024 02:01:36 GMT, Anthony Scarpino <ascarp...@openjdk.org> wrote:
>> Volodymyr Paprotski has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> remove use of jdk.crypto.ec
>
> src/java.base/share/classes/sun/security/ec/ECOperations.java line 308:
>
>> 306:
>> 307: /*
>> 308: * public Point addition. Used by ECDSAOperations
>
> Was the old description not applicable anymore? It would be nice to improve
> on the existing description that shortening it.
Forgot to go back and fix the comment. Fixed..
As for the 'meaning'. Notice the signature of the function changed (i.e. no
longer a 'mixed point', but two ProjectivePoints. This is a good idea
regardless of Montgomery, but it affects montgomery particularly badly (need to
compute zInv for 'no reason'. )
For sake of completeness. Apart from constructor, the 'API' for ECOperations
(i.e. as used by ECDHE, ECDSAOperations and KeyGeneration) are these three
functions (everything else is used internally by this class)
public void setSum(MutablePoint p, MutablePoint p2)
public MutablePoint multiply(AffinePoint affineP, byte[] s)
public MutablePoint multiply(ECPoint ecPoint, byte[] s)
> src/java.base/share/classes/sun/security/ec/ECOperations.java line 321:
>
>> 319: ECOperations ops = this;
>> 320: if (this.montgomeryOps != null) {
>> 321: assert p.getField() instanceof
>> IntegerMontgomeryFieldModuloP;
>
> This should throw a ProviderException, I believe this would throw an
> AssertionException
Missed this comment. No longer applicable (this.montgomeryOps got refactored
away)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1578144125
PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1578161140
