Re: RFR: 8285493: ECC calculation error

2022-04-27 Thread Anthony Scarpino
On Tue, 26 Apr 2022 21:02:49 GMT, Weijun Wang  wrote:

> Only numbers from the same modular fields can be involved in arithmetic 
> calculations. Add `assert` to guarantee this.
> 
> Also, found one broken case and rewrote it.

Changes look good

-

Marked as reviewed by ascarpino (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8409


Re: RFR: 8285493: ECC calculation error

2022-04-27 Thread Rajan Halade
On Tue, 26 Apr 2022 21:02:49 GMT, Weijun Wang  wrote:

> Only numbers from the same modular fields can be involved in arithmetic 
> calculations. Add `assert` to guarantee this.
> 
> Also, found one broken case and rewrote it.

Please update bug with applicable noreg label.

-

PR: https://git.openjdk.java.net/jdk/pull/8409


Re: RFR: 8285493: ECC calculation error

2022-04-27 Thread Xue-Lei Andrew Fan
On Wed, 27 Apr 2022 12:57:20 GMT, Weijun Wang  wrote:

>> src/jdk.crypto.ec/share/classes/sun/security/ec/ECDSAOperations.java line 
>> 261:
>> 
>>> 259: IntegerModuloP result = p1.asAffine().getX();
>>> 260: b2a(result, orderField, temp1);
>>> 261: return MessageDigest.isEqual(temp1, r);
>> 
>> I did not get the point of this update.  Is it the broken case you mentioned 
>> in the PR description?  What's the issue of the original code?
>
> Here, `result`'s modulus is `field`, and `ri`'s is `orderField`. Therefore 
> you cannot simply subtract one from the other. One new `assert` would fail.

Got it.  It looks like a safe update.

-

PR: https://git.openjdk.java.net/jdk/pull/8409


Re: RFR: 8285493: ECC calculation error

2022-04-27 Thread Xue-Lei Andrew Fan
On Tue, 26 Apr 2022 21:02:49 GMT, Weijun Wang  wrote:

> Only numbers from the same modular fields can be involved in arithmetic 
> calculations. Add `assert` to guarantee this.
> 
> Also, found one broken case and rewrote it.

Marked as reviewed by xuelei (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/8409


Re: RFR: 8285493: ECC calculation error

2022-04-27 Thread Weijun Wang
On Wed, 27 Apr 2022 06:28:27 GMT, Xue-Lei Andrew Fan  wrote:

>> Only numbers from the same modular fields can be involved in arithmetic 
>> calculations. Add `assert` to guarantee this.
>> 
>> Also, found one broken case and rewrote it.
>
> src/jdk.crypto.ec/share/classes/sun/security/ec/ECDSAOperations.java line 261:
> 
>> 259: IntegerModuloP result = p1.asAffine().getX();
>> 260: b2a(result, orderField, temp1);
>> 261: return MessageDigest.isEqual(temp1, r);
> 
> I did not get the point of this update.  Is it the broken case you mentioned 
> in the PR description?  What's the issue of the original code?

Here, `result`'s modulus is `field`, and `ri`'s is `orderField`. Therefore you 
cannot simply subtract one from the other. One new `assert` would fail.

-

PR: https://git.openjdk.java.net/jdk/pull/8409


Re: RFR: 8285493: ECC calculation error

2022-04-27 Thread Xue-Lei Andrew Fan
On Tue, 26 Apr 2022 21:02:49 GMT, Weijun Wang  wrote:

> Only numbers from the same modular fields can be involved in arithmetic 
> calculations. Add `assert` to guarantee this.
> 
> Also, found one broken case and rewrote it.

src/jdk.crypto.ec/share/classes/sun/security/ec/ECDSAOperations.java line 261:

> 259: IntegerModuloP result = p1.asAffine().getX();
> 260: b2a(result, orderField, temp1);
> 261: return MessageDigest.isEqual(temp1, r);

I did not get the point of this update.  Is it the broken case you mentioned in 
the PR description?  What's the issue of the original code?

src/jdk.crypto.ec/share/classes/sun/security/ec/ECDSAOperations.java line 261:

> 259: IntegerModuloP result = p1.asAffine().getX();
> 260: b2a(result, orderField, temp1);
> 261: return MessageDigest.isEqual(temp1, r);

I did not get the point of this update.  Is it the broken case you mentioned in 
the PR description?  What's the issue of the original code?

-

PR: https://git.openjdk.java.net/jdk/pull/8409