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.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
