<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Max - <br>
<br>
For the same reason I was pushing back on Adam's EC provider I
think I need to push back here. You're recasting an RSAPublicKey
to just a PublicKey and making it difficult to move key material
in and out of the MSCAPI proivider. Same thing with the private
key. <br>
<br>
For example, in the CPublicKey class you still have "getModulus()"
and "getPublicExponent()", but to get at them you'd have to use
CPublicKey rather than PublicKey. <br>
<br>
And looking forward, I'm not sure how you actually implement the
EC classes here using this model.<br>
<br>
I'd suggest not making the change this way and overloading the
existing classes, but instead adding the appropriate provider
classes for new key types as you implement support for them. E.g.
Keep CRSAKey, CRSAPublicKey and CRSAPrivateKey as distinct
classes, add CECKey, CECPublicKey and CECPrivateKey when you get
there.<br>
<br>
Are you missing a KeyFactory class as well?<br>
<br>
Lastly, you may want to change the subclass/methods for CSignature
(and probably other classes) to reflect the type of Signature
you're returning:<br>
<br>
<blockquote type="cite">
<pre> if (algo.equals("NONEwithRSA")) {
<span class="removed">- return new RSASignature.Raw();</span>
<span class="new">+ return new CSignature.Raw();</span></pre>
</blockquote>
<br>
Instead: "return new CSignature.RSARaw()"<br>
<br>
And this definitely isn't going to work if you have even one other
Cipher you'll be returning:<br>
<blockquote type="cite">
<pre> if (algo.equals("RSA") ||
algo.equals("RSA/ECB/PKCS1Padding")) {
<span class="removed">- return new RSACipher();</span>
<span class="new">+ return new CCipher();</span>
}</pre>
</blockquote>
<br>
<br>
Later, Mike<br>
<br>
<br>
<br>
<br>
<br>
On 10/25/2018 4:38 AM, Weijun Wang wrote:<br>
</div>
<blockquote type="cite"
cite="mid:75632D2A-5E1A-42D9-9434-E20736E20B3C@oracle.com">
<pre wrap="">Please review the change at
<a class="moz-txt-link-freetext" href="https://cr.openjdk.java.net/~weijun/8026953/webrev.00/">https://cr.openjdk.java.net/~weijun/8026953/webrev.00/</a>
(I will use a sub-task id for this change but currently JBS is down).
The major change is renaming classes. Since we are going to support algorithms other than RSA, I've renamed the classes like RSAPrivateKey -> CPrivateKey. Classes that have the same name as JCA classes (like Key, KeyStore) are also renamed (to CKey, CKeyStore) so it's easy to tell them apart.
Others are not about renaming but they are also related to supporting other algorithms, and there is no behavior change. They include:
- CKey (plus its child classes CPublicKey and CPrivateKey) has a new field "algorithm". This field is used by CKeyStore::generateRSAKeyAndCertificateChain and its value is obtained from the public key algorithm in a cert [1].
- Child class named "RSA" of CKeyPairGenerator.
- Child class named "RSA" of CSignature. I also moved some RSA-related methods into this child class as overridden methods.
- CKeyStore::setPrivateKey's key parameter has a new type Key, but it still only accepts RSAPrivateCrtKey now.
Noreg-cleanup.
Thanks
Max
[1] <a class="moz-txt-link-freetext" href="https://docs.microsoft.com/en-gb/windows/desktop/api/wincrypt/ns-wincrypt-_crypt_algorithm_identifier">https://docs.microsoft.com/en-gb/windows/desktop/api/wincrypt/ns-wincrypt-_crypt_algorithm_identifier</a></pre>
</blockquote>
<p><br>
</p>
</body>
</html>