RFR[15]: 8186143: keytool -ext option doesn’t accept wildcards for DNS subject alternatives names

2020-03-13 Thread Hai-May Chao
Hi,

I need a code review for -

Bug: https://bugs.openjdk.java.net/browse/JDK-8186143
Webrev: http://cr.openjdk.java.net/~weijun/8186143/webrev.00/

The keytool -ext option doesn’t accept wildcards for DNS subject alternatives 
names in certificates. Certificates with wildcarded domains are useful for 
allowing domain names under a common subdomain to share the same certificate.

The fix involves adding a new DNSName constructor with an additional boolean 
flag ‘allowWildcard’.

Thank you,
Hai-May



Re: RFR[15]: 8186143: keytool -ext option doesn’t accept wildcards for DNS subject alternatives names

2020-03-13 Thread Jamil Nimeh

Hello Hai-May,

The fix overall looks good.  One or two comments about the test:

 * 103: I think the comment might be more clear saying something like
   "partial wildcard disallowed" since it's not the "*" in and of
   itself that's the issue, it's that the next character following it
   isn't a domain separator (".").
 * A similar badSanNames test case (I think) that walks a different
   code path would be something like "a*.com".  Although the test on
   line 95 might walk the same codepath...If so then no need to add
   anything else.

--Jamil

On 3/13/2020 9:25 AM, Hai-May Chao wrote:

Hi,

I need a code review for -

Bug: https://bugs.openjdk.java.net/browse/JDK-8186143
Webrev: http://cr.openjdk.java.net/~weijun/8186143/webrev.00/

The keytool -ext option doesn’t accept wildcards for DNS subject alternatives 
names in certificates. Certificates with wildcarded domains are useful for 
allowing domain names under a common subdomain to share the same certificate.

The fix involves adding a new DNSName constructor with an additional boolean 
flag ‘allowWildcard’.

Thank you,
Hai-May



Re: RFR[15]: 8186143: keytool -ext option doesn’t accept wildcards for DNS subject alternatives names

2020-03-13 Thread Hai-May Chao
Hi Jamil,

Thanks for your review! Reply inline.

> On Mar 13, 2020, at 12:24 PM, Jamil Nimeh  wrote:
> 
> Hello Hai-May,
> 
> The fix overall looks good.  One or two comments about the test:
> 
> 103: I think the comment might be more clear saying something like "partial 
> wildcard disallowed" since it's not the "*" in and of itself that's the 
> issue, it's that the next character following it isn't a domain separator 
> (".”).

Agree. Will update the comment.

> A similar badSanNames test case (I think) that walks a different code path 
> would be something like "a*.com".  Although the test on line 95 might walk 
> the same codepath...If so then no need to add anything else.

I’ll add “a*.com” to badSanNames test case as it will detect the error for 
‘DNSName components must consist of letters, digits, and hyphens’.
Line 95 test case will give us a different error from “a*.com”. That is, 
‘DNSName with blank components is not permitted’.
The existing badNames test case does not have “a*.com”, and I will add it too. 

Thanks,
Hai-May

> --Jamil
> 
> On 3/13/2020 9:25 AM, Hai-May Chao wrote:
>> Hi,
>> 
>> I need a code review for -
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8186143 
>> 
>> Webrev: http://cr.openjdk.java.net/~weijun/8186143/webrev.00/ 
>> 
>> 
>> The keytool -ext option doesn’t accept wildcards for DNS subject 
>> alternatives names in certificates. Certificates with wildcarded domains are 
>> useful for allowing domain names under a common subdomain to share the same 
>> certificate.
>> 
>> The fix involves adding a new DNSName constructor with an additional boolean 
>> flag ‘allowWildcard’.
>> 
>> Thank you,
>> Hai-May
>>