RFR 8213009: Refactoring existing SunMSCAPI classes

[Valerie Peng]

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