Sorry - running behind on this thread.

In ECUtil.decodePoint(),

Since this code is open, I'm wondering if it might be useful to add the checks specified in NIST SP800-186 Appendix D.1.  or SP800-56Ar1 5.6.2.3 E.g.

D.1.2 Montgomery Curves
D.1.2.1 Partial Public Key Validation

Inputs:
1. Montgomery curve MA,B defined over the prime field GF(p)

2. Point Q=(u, v) 1712
Output: ACCEPT or REJECT Q as an affine point on MA,B.
Process:
1. If Q is the point at infinity ∅, output REJECT.
2. Verify that both u and v are integers in the interval [0, p−1]. Output REJECT if  verification fails. 3. Verify that (u, v) is a point on the MA,B by checking that (u, v) satisfies the defining equation v2 = u (u2 + A u + 1) where computations are carried out in GF(p). Output  REJECT if verification fails.
4. Otherwise output ACCEPT.

D.1.2.2 Full Public Key Validation
Inputs:
1. Montgomery curve MA,B defined over the prime field GF(p)
2. Point Q
Output: ACCEPT or REJECT Q as a point on MA,B of order n.
Process:
1. Perform partial public key validation on Q using the procedure of Appendix D.1.2.1.  Output REJECT if this procedure outputs REJECT.
2. Verify that n Q = ∅. Output REJECT if verification fails.
3. Otherwise output ACCEPT.

This mainly ensures that the X/Y provided is actually a point on the curve.   The threat to receiving a bad public key is more on the ECDH side, but this appears to be the code that would need to be modified so...

Later, Mike


On 2/20/2020 11:03 PM, Anthony Scarpino wrote:
I'm ok with this update

Tony

On 2/19/20 5:35 AM, Weijun Wang wrote:
New webrev at

   http://cr.openjdk.java.net/~weijun/8237218/webrev.04/

Only test change. For each signature, modify it a little and check if verify fails.

Thanks,
Max

On Feb 18, 2020, at 2:09 AM, Anthony Scarpino <anthony.scarp...@oracle.com> wrote:

The change looks fine.  I'm trusting that the existing Known Answer Tests are proving your verifySignedDigest() is correct.

You may want to comment in the code that your test depends on these method names.  I was going to suggest simplifying the all the verifySigned*() methods until I saw the test was dependent on it.

Tony


On 2/13/20 3:06 AM, Weijun Wang wrote:
Webrev updated at
    http://cr.openjdk.java.net/~weijun/8237218/webrev.03/
The test is modified. Instead of adding a hacked ECDSASignature I'm using JDI to detect if the Java impl or the native impl is used. Two method names in ECDSASignature are modified to ease method lookup in the test.
Thanks,
Max
On Feb 11, 2020, at 7:52 PM, Weijun Wang <weijun.w...@oracle.com> wrote:

Please take a review at

   http://cr.openjdk.java.net/~weijun/8237218/webrev.02/

A test is added that uses a patched ECDSASignature.java that exposes how the signature is verified.

BTW, I also updated ECDSASignature.java a little to accept non SunEC keys, so that I can do some interop testing. If you believe this is unnecessary I can revert the change.

Thanks,
Max





Reply via email to