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

[Weijun Wang]

Hi Andrew

Thanks for your quick review!

The update is published at http://hgrev.appspot.com/show?id=2087. This
is not a brand new fulldiff, but an incremental diff compared to
http://hgrev.appspot.com/show?id=3077. The main update is a beautified
-printcertreq command.

BTW, this is the first patch-over-patch patch I published on
http://hgrev.appspot.com. The support is still not perfect because I see
bad output in the wide diff links. Looks like there's something wrong
dealing with < > things in HTML. Will fix it tonight.

Xuelei Fan wrote:
> Max (Weijun) Wang wrote:
>>> I will look at KeyTool.java tomorrow, others looks fine for me by now.
>>
> A mini suggestion, would you please also add the "-ext" format into the
> usage() output? I did not find what the ext should looks like in the
> help message.

Changed, a little.

> 
> Do you have a test case with an empty "Subject" field? If the "Subject"
> field is empty, the SAN extension must be present, and marked as critical.

No, I haven't.

In fact, I provide no check on what extension combinations are legal.
There are quite a lot of them, say, when BC is true, then KU's some bit
should be true. The consistence checking is very complicated, and if
really necessary, might be implemented in a shared class instead of keytool.

> 
> Otherwise, looks fine to me.
> 

Thanks
Max

> Xuelei
>>>
>>>
>>> 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
>>>>
>>>>
>>>
>>>
>>
>