feat: 🎸 js implment for issue 246

[oleggrib]

issue 246: java PR#248, validate point against curve

✅ Closes: #246

@jot2re , can you check what is wrong with code :
https://github.com/TokenScript/attestation/blob/246-js-constanttime/src/main/javascript/crypto/src/libs/AttestationCrypto.ts#L389-L393

sometimes it gets symetric points, but maybe because of missing normalization it breaks. Maybe you can help to solve it.

or help me to understand reason of point.multiplyDA(curve.n).isInfinity()

[github-actions]

Coverage report for src/main/javascript/crypto/

St.❔	Category	Percentage	Covered / Total
🟡	Statements	72.4%	1894/2616
🔴	Branches	45.32% (+1.24% 🔼)	247/545
🟡	Functions	79.41%	351/442
🟡	Lines	72.59%	1856/2557

Show files with reduced coverage 🔻

St.❔	File	Statements	Branches	Functions	Lines
🟢	libs/utils.ts	86.7%	63.89% (-1.39% 🔻)	94.12%	87.94%
🟡	... / AttestationCrypto.ts	66.42% (-2.15% 🔻)	37.04% (-2.96% 🔻)	90.91%	66.67% (-2.27% 🔻)
🟡	... / FullProofOfExponent.ts	67.86% (-2.6% 🔻)	53.33% (+53.33% 🔼)	78.57% (+1.65% 🔼)	67.86% (-2.6% 🔻)

Test suite run success

38 tests passing in 2 suites.

Report generated by 🧪jest coverage report action from 6829c34

[jot2re]

@oleggrib that is a very good question! Normalisation should not have any influence here (normalisation only has something to do with internal representation of a point on a curve). Basically when the check fails it mathematically means that the point does not have the correct order. In non-math terms it means that the point is not safe to use.
This could indicate a bug somewhere either in computation of points or decoding of points.
Does it fail when it uses data made with attestation.jar, or does the failure only happen when it decoded something constructed by authenticator.jar?

[jot2re]

I had a quick check on the Authenticator code and I could not find anything wrong with your code. The risk is that one of the libraries used might contain a bug.

[jot2re]

Looking a bit closer it might be an issue with the code in Point.ts. It looks like you normally represent the point at infinity as either x=null or y=null. But in the implementation of multiplyDA it looks like you use x=0 and y=0 to represent the point at infinity through the command newZero.

[oleggrib]

Does it fail when it uses data made with attestation.jar, or does the failure only happen when it decoded something constructed by authenticator.jar?

it fails for example in next tests:
MagicLink reader › Decode Magic Link from Java Build
MagicLink reader › Encode/Decode Magic Link from JS

so in both cases it failed.

how it fails:
https://github.com/TokenScript/attestation/blob/246-js-constanttime/src/main/javascript/crypto/src/libs/AttestationCrypto.ts#L390
->
https://github.com/TokenScript/attestation/blob/246-js-constanttime/src/main/javascript/crypto/src/libs/Point.ts#L114
->
https://github.com/TokenScript/attestation/blob/8468dac0d749ae5e55e61eed5d89db84e2b7ce6e/src/main/javascript/crypto/src/libs/Point.ts#L102 (by logic it can be a this.newZero(), but our point have positive Y2, so Y1 != -Y2)
-> then code try to invert point where X1 - X2 == 0 , but Y1 == -(Y2 - module) and fails

maybe it can help you to understand how to solve the issue. @jot2re

[jot2re]

@oleggrib Okay, it took me awhile to figure out the problem because I thought it was something completely different. It turned out to be super simple. The check you mentioned simply needed to be done with a positive value. But another issue was that the isInfinity() function was not completely sound.
I also noticed that a similar issue could occur in the future, so I implemented a small fix for this.
Can you please check commit 9060a92 and validate that the changes I have made are the correct way to do it in JS?

[oleggrib]

thank you, @jot2re , it looks and works great, I thought it something simple, so when error happened it was already correct result, but wrong check happened.

So we can merge this PR , close issue and remove branches. Can you do it, @jot2re if it looks good for you?

[jot2re]

I checked and everything looks good. There were a couple of checks missing which I added. Everything runs locally on my machine.