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

[Xuelei Fan]

Weijun Wang wrote:
> 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.
>
>   
Not too bad, I got the expected "-ext" updates.

BTW, I think there are many lines with more than 80 letters, which make 
the wide-diff hard to read.

> 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.
>
>   
I agree with the current approach.  Just keep the keytool stupid. ;-)

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