> On Oct 30, 2018, at 2:22 AM, Michael StJohns <mstjo...@comcast.net> wrote:
>
> On 10/25/2018 11:09 PM, Weijun Wang wrote:
>> 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.
> OK.
>>
>> 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.
> This is where I think you need to fix the structure:
>
> abstract class CKey
>
> public class CRSAPublicKey extends CKey implements RSAKey, RSAPublicKey,
> PublicKey
> public class CRSAExtractablePrivateKey extends CKey implements RSAKey,
> RSAPrivateKey, PrivateKey[,RSAMultiPrimePrivateCrt | RSAPrivateCrtKey]
> public class CRSAPrivateKey extends CKey implements RSAKey, PrivateKey
>
> public class CECPublicKey extends CKey implements ECKey, ECPublicKey,
> PublicKey
> public class CECExtractablePrivateKey extends CKey implements ECKey,
> PrivateKey
> public class CECPrivateKey extends CKey implements ECKey, ECPrivateKey,
> ECPrivateKey
Very likely.
I'll get the signature signing/verification working first, and then will have
time to expose the key info with these classes.
>
> Note the two different versions of the private keys to match up with the key
> handling bits as well as some additional interfaces that may be needed to be
> added to support the underlying provider's requirements for the RSA keys.
>
> Also, I'm looking ahead a little bit and thinking about how the JCA would use
> the windows PCP (Platform Crypto Provider) which uses the TPM to enforce
> hardware security. It would be useful if you didn't have to re-write
> everything just because of a different underlying Windows provider. (There's
> a PCP development kit that's got some useful sample code that might help a
> little bit with refactoring the JCA MSCAPI provider even for the existing
> code). E.g. eventually supporting an MSCAPI-PCP provider shouldn't require
> all new code.
Great. I really need more sample code to study.
>
>>
>> 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.
> AFAICT, you're not going to be able to use any external key without importing
> it or running it through a key factory. The main class you're going to be
> using is NCryptImportKey (or alternately BCryptImportKeyPair).
I tried NCryptCreatePersistedKey and NCryptImportKey but there are always some
flags I cannot get right.
>
>>
>> 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.
>
> So you just use software classes in another provider for
> encrypting/verifying? To be honest this sounds messy and may come back to
> bite you down the road.
In JDK 11 that was a workaround because I don't have time to get the native
verifying part correctly. I don't mean that's a better solution.
>
>>
>> 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.
> Hmm... one of the more common things is to move around .p12 files with your
> certs and keys. They can be imported by the Windows tools - it would be
> *really* nice if you can do the same thing with the Java provider.
That's how I am testing now. Creating p12 files in Java and import them using
certmgr.exe and then see if I can signing from inside Windows.
In Java, either there will be a factory class or the CKeyStore::setKey method
will need to understand soft keys.
Thanks
Max
>
>>
>> Thanks again.
>>
>> --Max
>>
>>> On Oct 26, 2018, at 1:25 AM, Michael StJohns <mstjo...@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
>
>