RFR 8237218: Support NIST Curves verification in java implementation

[Michael StJohns]

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.scarpino at 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.wang at 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
>>>>>
>>>
>>
>