RFR 8213010: [cng] Supporting keys created with certmgr.exe

[Valerie Peng]

The updated webrev looks fine.

Would prefer to have importPublicKey and importECPublicKey methods 
merged as other methods do. But not a deal breaker as it does not affect 
functionality.

Also, would be nice to use a http URL for the EC public key blob format.

Thanks,
Valerie


On 12/12/2018 4:55 PM, Weijun Wang wrote:
> Hi Valerie,
>
> The updated webrev is at
>
>     https://cr.openjdk.java.net/~weijun/8213010/webrev.01/
>
> I haven't merged importPublicKey and importECPublicKey because the content is so different and we expect someday to remove the CAPI method at all.
>
> The interdiff.patch has some problem since I haven't really changed SunMSCAPI.java. Maybe a bug of the interdiff tool.
>
> Thanks
> Max
>
>> On Dec 12, 2018, at 9:18 AM, Weijun Wang <weijun.wang at oracle.com> wrote:
>>
>> Hi Valerie,
>>
>>> On Dec 12, 2018, at 6:21 AM, Valerie Peng <valerie.peng at oracle.com> wrote:
>>>
>>> Hi Max,
>>>
>>> <CSignature.java>
>>>
>>> - Comments (line 60-63) missed SHA224withECDSA?
>> Oops.
>>
>>> - Line 430: should be "ECPublicKey"
>> Oops again.
>>
>>> - Line 919, 922: is it really necessary to have two methods with algorithm name argument? It seems they are functionally the same but one calls CAPI vs CNG. Can they be merged?
>> Yes.
>>
>>> <CKey.java>
>>>
>>> - generateECBlob() method (line 110-148), is the extra 1 due to the sign bit added by the BigInteger.toByteArray()? I recall BigInteger adds an extra 00 byte sometimes to indicate the positive value, but why is there an extra 1? Are the bytes for x, y, bs longer than the allocated "keyLen" length?
>> You're right. I was thinking about the sign bit but got it wrong.
>>
>>> <security.cpp>
>>>
>>> - line 627, shouldn't 'C' be at buffer[1]? If not, what is at buffer[1]? len value is not checked, not sure how useful it is.
>> It's a Unicode algorithm name, so buffer[1] is zero. I didn't check len because 32 is always enough for an algorithm name and there are at least 3 bytes there.
>>
>> Thanks,
>> Max
>>
>>> Thanks,
>>>
>>> Valerie
>>>
>>> On 12/3/2018 7:14 AM, Weijun Wang wrote:
>>>> Please take a review at
>>>>
>>>> https://cr.openjdk.java.net/~weijun/8213010/webrev.00/
>>>>
>>>> A Windows keystore is now able to load EC keys and uses them in signing and verifying with SHA<n>withECDSA.
>>>>
>>>> Not supported:
>>>>
>>>> 1. No EC KeyPairGenerator yet.
>>>>
>>>> 2. Cannot store a EC key (from SunEC) into a Windows keystore. I still haven't figured out how to call NCryptImportKey, NCryptCreatePersistedKey and CertAddCertificateContextToStore together correctly to associate a EC private key to a cert and store them.
>>>>
>>>> 3. SHA<n>withECDSAinP1363Format not supported, but it's easy to add them.
>>>>
>>>> 4. NONEwithECDSA not supported.
>>>>
>>>> Currently I can only use certmgr.exe to import a pkcs12 file and then run a manual test with it. Therefore no automatic test is included. Like RSA support in SunMSCAPI, Signature::initSign only support native keys. Signature::initVerify supports both native and SunEC keys. That said, since we do not have EC KeyPairGenerator yet you won't meet a real native EC public key.
>>>>
>>>> Thanks
>>>> Max
>>>>