> On Dec 13, 2018, at 10:07 AM, Valerie Peng <valerie.p...@oracle.com> wrote:
>
>
> 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.
I've updated it to
https://docs.microsoft.com/en-us/windows/desktop/api/bcrypt/ns-bcrypt-_bcrypt_ecckey_blob
in my local repo. Thanks for catching it.
--Max
>
> 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.w...@oracle.com> wrote:
>>>
>>> Hi Valerie,
>>>
>>>> On Dec 12, 2018, at 6:21 AM, Valerie Peng <valerie.p...@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
>>>>>
