Re: RFR: 8285493: ECC calculation error
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
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
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
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
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
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
RFR: 8285493: ECC calculation error
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. - Commit messages: - ECC calculation error fix Changes: https://git.openjdk.java.net/jdk/pull/8409/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8409=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285493 Stats: 16 lines in 2 files changed: 3 ins; 4 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/8409.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8409/head:pull/8409 PR: https://git.openjdk.java.net/jdk/pull/8409
