[security-dev 00577]: Re: Code review request: 6780416: New keytool commands/options: -gencert, -printcertreq, -ext

[Xuelei Fan]

> If you find the webrev too long, you might only review a part of it.

1. src/share/classes/sun/security/x509/IssuerAlternativeNameExtension.java

Adding a new constructor which allow mark this extension as critical.
The spec requires "Where present, conforming CAs SHOULD mark this
extension as non-critical. Do you really want to mark it critical freely 
as the request?

2. src/share/classes/sun/security/x509/CertificateExtensions.java
I have no reading the keytool class, so I don't know why you have to add
a getNameByOid(ObjectIdentifier) method here. The name of an oid could
be get from OIDMap by static. Or this name is not refer to that name in 
OIDMap?

3. src/share/classes/sun/security/x509/CertAndKeyGen.java
Why remove the SKID extension from getSelfCertificate()? Are you sure 
the remove has no impact on other models.

I will look at KeyTool.java tomorrow, others looks fine for me by now.

Xuelei

Max (Weijun) Wang wrote:
> Hi All
>
> Can you take a review of this RFE?
>
>   6780416: New keytool commands/options: -gencert, -printcertreq, -ext
>   bug: http://bugs.sun.com/view_bug.do?bug_id=6780416
>   webrev: http://hgrev.appspot.com/show?id=3077
>
> The spec of the 3 new commands/options is inside the evaluation 
> section of the bug report.
>
> The fix is mainly on KeyTool.java, with changes in Resources.java for 
> l10n strings. Some X.509 files are changed to provide new constructor, 
> new constants etc. A new class SubjectInfoAccessExtension.java is 
> created for the extension. The KeyToolTest.java regression test are 
> updated to cover the new commands/options.
>
> If you find the webrev too long, you might only review a part of it.
>
> Thanks
> Max
>
>