Hi Max,
Here are some comments:
<src/jdk.crypto.mscapi/windows/classes/sun/security/mscapi/CSignature.java>
- line 39, add PSS here as well.
- line 97, CSignature ctr, better to initialize all fields
- line 109, has key algorithm been checked by JCA already? If not, it
should be checked here. Same goes for line 414, and 560
- with the class renaming, I think it's clearer to include "RSA" as part
of the subclass names, e.g. "MD2withRSA" instead of just "MD2"
- line 681: can be static, right? pkg-private?
<src/jdk.crypto.mscapi/windows/classes/sun/security/mscapi/CPublicKey.java>
- line 120: maybe a ProviderException instead of
IllegalArgumentException as the 'alg' is not from user but rather inside
the provider.
- line 127, add @Override
- line 140, must getPublicKeyBob be public? Maybe pkg private?
<src/jdk.crypto.mscapi/windows/classes/sun/security/mscapi/CKeyStore.java>
- line 119: why not just use RSAPrivateCrtKey instead of Key? The caller
has already did an instanceof check before this method.
<test/jdk/sun/security/mscapi/KeyAlgorithms.java>
- would be nice to have a String constant for "RSA".
Rest looks fine.
Thanks,
Valerie
On 11/15/2018 7:40 AM, Weijun Wang wrote:
Oops, my copy/paste sequence goes wrong.
On Nov 15, 2018, at 11:38 PM, Weijun Wang <weijun.w...@oracle.com> wrote:
Webrev updated at
https://cr.openjdk.java.net/~weijun/8213009/webrev.01/
More refactorings:
- getEncoded and getFormat of CKey removed, implemented in CPublicKey and
CPrivateKey.
- CPublicKey has child class CRSAPublicKey, CKeyPairGenerator has child class
RSA.
- CPublicKey and CPrivateKey now have a static of() method that can return a
child instance.
- CCipher renamed to CRSACipher. I realized there won't be CECCipher.
Thanks
Max
On Nov 7, 2018, at 12:13 AM, Weijun Wang <weijun.w...@oracle.com> wrote:
Webrev updated at
https://cr.openjdk.java.net/~weijun/8213009/webrev.00/
The subtask id is now used.
The previous refactoring has removed the "RSA" algorithm info from some keys.
This update adds them back.
Thanks
Max
On Oct 25, 2018, at 4:38 PM, Weijun Wang <weijun.w...@oracle.com> wrote:
Please review the change at
https://cr.openjdk.java.net/~weijun/8026953/webrev.00/
(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]
https://docs.microsoft.com/en-gb/windows/desktop/api/wincrypt/ns-wincrypt-_crypt_algorithm_identifier
