RFR 8026953: Add support for MS Cryptography next generation (CNG) (step 1)

[Weijun Wang]

Hi Mike

Thanks for the feedback.

I understand the current SunMSCAPI implementation recognizes RSA keys only and it's certainly incorrect to put something like getModulus() and getPublicExponent() in a general CKey class. They will be fixed later. When I have more sub class names, I'll definitely use them. You can see I've moved some CSignature methods into CSignature$RSA. I just haven't done it everywhere.

We'll still need a base CKey for a CNG key, no matter what the underlying algorithm is. Since CNG uses the same NCRYPT_KEY_HANDLE type for different types of keys, we will do something similar on the Java side. Since the current CPublicKey and CPrivateKey are very light, it will be easy to move the content into algorithm-specific classes.

The main reason I want to take this first step is that after some study on CNG I make some progress and also see some blockers. For example, I am able to expose a EC CNG key stored in Windows-MY now and use it to sign/verify. However, I still don't know how to import external keys inside there (certmgr.exe can so it's possible). Until now, the most requested function is to use existing keys in signatures and I want to start working on it. The first thing I noticed then is that the current class names are unsuitable and I think a refactoring will make them look better.

Once I start working on the next step, I'll need to have different sub classes in CKey and CSignature. I even have an alternative plan to ditch CPublicKey and use the PublicKey classes in SunEC and SunRsaSign. This was actually already used in the RSASSA-PSS signature handling in SunMSCAPI we added into JDK 11 in the last minute.

As for KeyFactory, we do not have an urgent requirement to use external keys in a CNG Signature object or store them into Windows-MY. Also, we can use the one in SunRsaSign.

Thanks again.

--Max

> On Oct 26, 2018, at 1:25 AM, Michael StJohns <mstjohns at comcast.net> wrote:
> 
> Hi Max - 
> 
> 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.   
> 
> 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.  
> 
> And looking forward, I'm not sure how you actually implement the EC classes here using this model.
> 
> 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.
> 
> Are you missing a KeyFactory class as well?
> 
> Lastly, you may want to change the subclass/methods for CSignature (and probably other classes) to reflect the type of Signature you're returning:
> 
>>   if (algo.equals("NONEwithRSA")) {
>> 
>> -                        return new RSASignature.Raw();
>> +                        return new CSignature.Raw();
> 
> Instead:   "return new CSignature.RSARaw()"
> 
> And this definitely isn't going to work if you have even one other Cipher you'll be returning:
>>                      if (algo.equals("RSA") ||
>>                          algo.equals("RSA/ECB/PKCS1Padding")) {
>> 
>> -                        return new RSACipher();
>> +                        return new CCipher();
>> 
>>                      }
>> 
> 
> 
> Later, Mike
> 
> 
> 
> 
> 
> On 10/25/2018 4:38 AM, Weijun Wang 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
>