code review request: 7109096: keytool -genkeypair needn't call -selfcert

[Xuelei Fan]

Did you get any failure report about the CR?

I asked the question because I concern about the format of the encoded
public key. I'm not sure whether it is always be X.509 or not. If it is
not of X.509, we properly cannot calculate the KID properly, and then
would be in the risk to chain the AKID, SKID incorrectly. Or if it is
not of X.509, we may get another exception during parse public key.

If we got complains about the issue, I would suggest you check the
encoded format:

 179     public PublicKey getPublicKeyAnyway() {
 +           if ((publicKey instanceof X509Key) ||
 +               publicKey.getFormat().equalsIgnoreCases("X.509")) {
 180             return publicKey;
 +           }
 +
 +           return null;
 181     }


If we cannot get the expected public key as expected, I think we proper
have to resign it in order to get the public key from signed
certificate. Fortunately, our PKCS11 implementation do returns "x.509"
format for RSA, DSA, DH and EC public key.


And a minor comment:

178  // Used by KeyTool, where publicKey is not a X509Key on Solaris.

I think is is not because of Solaris platform, but because of the PKCS11
provider, so that publicKey is not an instance of X509Key. How about:

     /**
      * Always returns the public key of the generated key pair. Used
      * by KeyTool only.
      *
      * The publicKey is not necessarily to be an instance of
      * X509Key in some JCA/JCE providers, for example SunPKCS11.
      */

BTW, I just noticed that KeyTool does not use AKID in self-signed
certificated. It's fine, but personally, I prefer to use both AKID and
SKID in all certificates.

Xuelei

On 11/8/2011 4:29 PM, Weijun Wang wrote:
> webrev updated at
> 
>    http://cr.openjdk.java.net/~weijun/7109096/webrev.01/
> 
> This time JPRT tests jdk_security3 passes on all platforms.
> 
> Thanks
> Max
> 
> 
> On 11/08/2011 03:18 PM, Weijun Wang wrote:
>> I only run tests on my Linux before posting the webrev. Then, in the
>> pre-push JPRT run, it fails on all Solaris!
>>
>> Turns out that CertAndKeyGen has
>>
>> public X509Key getPublicKey()
>> {
>> if (!(publicKey instanceof X509Key)) {
>> return null;
>> }
>> return (X509Key)publicKey;
>> }
>>
>> So the public key, which I guess is a P11RSAPublicKey, is now null. I'll
>> try to find a workaround.
>>
>> Thanks
>> Max
>>
>>
>> On 11/08/2011 11:19 AM, Xuelei Fan wrote:
>>> Looks fine in general. Please make sure all regression tests are passed.
>>>
>>> Thanks,
>>> Xuelei
>>>
>>> On 11/7/2011 7:34 PM, Weijun Wang wrote:
>>>> Description:
>>>>
>>>> keytool uses CertAndKeyGen to generate a basic self-signed certificate
>>>> with no extensions. When -ext option was introduced, -genkeypair was
>>>> implemented as original -genkeypair plus -selfcert, and extensions info
>>>> was added in the -selfcert step.
>>>>
>>>> This means the keystore object is modified twice in this single
>>>> operation. In the case of PKCS11 or MSCAPI, it is actually written to
>>>> the token twice. If a token can only be written once, the action will
>>>> fail.
>>>>
>>>> Webrev:
>>>>
>>>> http://cr.openjdk.java.net/~weijun/7109096/webrev.00/
>>>>
>>>> No new regression test (noreg-cleanup).
>>>>
>>>> Note: NetBeans consolidates the multiple import lines in CertAndKeyGen
>>>> into one. I'm not against that.
>>>>
>>>> Thanks
>>>> Max
>>>