RFR: 8260693: Provide the support for specifying a signer in keytool -genkeypair [v3]

[Hai-May Chao]

On Thu, 1 Apr 2021 21:27:52 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Hai-May Chao has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Updated with review comments
>
> src/java.base/share/classes/sun/security/tools/keytool/CertAndKeyGen.java line 104:
> 
>> 102:      *          key will be chosen after the first keypair is generated.
>> 103:      * @param providerName name of the provider
>> 104:      * @param signerPrivateKey signer's private key
> 
> Add an "(optional)" or "can be null" to the last 2 `@param`s.

Done.

> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 1163:
> 
>> 1161:             }
>> 1162:             doGenKeyPair(alias, dname, keyAlgName, keysize, groupName, sigAlgName,
>> 1163:                     signerAlias);
> 
> Maybe we can just keep using the old argument list. `signerAlias` is a global variable and it's visible everywhere.

I'd prefer to have it. It is the same as other global arguments being passed to this method.

> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 1927:
> 
>> 1925:         CertAndKeyGen keypair;
>> 1926:         if (signerAlias != null) {
>> 1927:             signerFlag = true;
> 
> Is the `signerFlag` really useful? We can just check `signerAlias != null`.

Sure, use `signerAlias` instead.

> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 1978:
> 
>> 1976:             } else {
>> 1977:                 certImpl = new X509CertImpl(signerCert.getEncoded());
>> 1978:             }
> 
> The exact same 7 lines above also appears in the code change above. Is it possible to combine them?

Here is mainly to locate the signer’s SKID, and moved it up to the first occurrence of these lines of code to combine them.

> src/java.base/share/classes/sun/security/tools/keytool/Resources.java line 310:
> 
>> 308:                 "Generating {0} bit {1} key pair and self-signed certificate ({2}) with a validity of {3} days\n\tfor: {4}"},
>> 309:         {"Generating.keysize.bit.keyAlgName.key.pair.and.a.certificate.sigAlgName.issued.by.an.entry.signerAlias.specified.by.the.signer.option.with.a.validity.of.validality.days.for",
>> 310:                 "Generating {0} bit {1} key pair and a certificate ({2}) issued by an entry <{3}> specified by the -signer option with a validity of {4} days\n\tfor: {5}"},
> 
> A little too long? Can we remove the "specified by the -signer option" words or even the whole "issued by an entry <{3}> specified by the -signer option"?

Removed the "specified by the -signer option”, and keep the "issued by an entry <{3}>” that I agreed it’d be good to print out the signer’s alias in response to your previous review comment.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3281