RFR: 8213952: Relax DNSName restriction as per RFC 1123

[Seán Coffey]

p.s I've updated the testcase to test the IOException message for 
presence of "DNSName". Webrev updated in place.

Regards,
Sean.

On 21/11/18 15:42, Seán Coffey wrote:
>
> Thanks for the comments Bernd. Comments inline..
>
> On 16/11/18 21:27, Bernd Eckenfels wrote:
>> Hello Sean,
>>
>> I was only looking at the inspected DNSName class, it still has some 
>> variations (but start now with DNSNames which is good already):
>>
>>   76 throw new IOException("DNSName must not be null or empty");
>>   78 throw new IOException("*DNSNames or NameConstraints* with blank 
>> components are not permitted");
>>   80 throw new IOException("*DNSNames or NameConstraints* may not 
>> begin or end with a .");
>>   92 throw new IOException("*DNSName SubjectAltNames* with empty 
>> components are not permitted");
>>  96 throw new IOException("DNSName components must begin with a 
>> letter or digit");
>> 101 throw new IOException("DNSName components must consist of 
>> letters, digits, and hyphens");
>>
>> I did not check if those exceptions are catched and rethrown with 
>> something like „while validating the SubjectAltName Extension <num> 
>> of certificate <subject>...“, in my experience it helps if you do not 
>> have to find and review the actual certificates (in this case it 
>> might not be a problem if SAN is only in the end entity). You can 
>> probably remove „or NameConstraints“ and „SubjectAltNames“ from lines 
>> 78-92 (this is good if DNsNa
> Ok - I've cleaned up the exception messages on line 78, 80, 92.
> webrev updated in place : 
> http://cr.openjdk.java.net/~coffeys/webrev.8213952.v2/webrev/
>
>
>> me applies to SAN or NameConstrained context and the validation logic 
>> does not know — so it’s not only more unified but also less missleading)
>>
>> BTW: probably not inthe scope of this fix but a subtype for 
>> validation errors which have getters for context/location and maybe 
>> error code and getValue() would allow callers to print nicer 
>> validation reports without relying on the message or Stacktraces.
>
> That's a nice idea and one that should be followed up in separate 
> enhancement. We could lean on the new `jdk.includeInExceptions` 
> Security property which other component areas have started to use lately.
>
> e.g. https://bugs.openjdk.java.net/browse/JDK-8207768
>
> regards,
> Sean.
>>
>> Gruss
>> Bernd
>> -- 
>> http://bernd.eckenfels.net
>> ------------------------------------------------------------------------
>> *Von:* Seán Coffey <sean.coffey at oracle.com>
>> *Gesendet:* Freitag, November 16, 2018 5:15 PM
>> *An:* Bernd Eckenfels; security-dev at openjdk.java.net
>> *Betreff:* Re: RFR: 8213952: Relax DNSName restriction as per RFC 1123
>>
>> Thanks for the comments Bernd. comments inline..
>>
>> On 16/11/18 12:40, Bernd Eckenfels wrote:
>>> You could also add (a..b, false) and (.a, false), (a., false) to the 
>>> testcases.
>> good point. test updated.
>>>
>>> I noticed that there are different types of Exception messages (DNS 
>>> name, DNSName, DNS Name or name constrained, DNS name and SAN), 
>>> would be good if all of them have the same keyword?
>> I cleaned up some other references to DNSName in the 
>> sun.security.x509 package. I'm not sure what classes you were 
>> referencing the above examples from.
>>
>> new webrev : 
>> http://cr.openjdk.java.net/~coffeys/webrev.8213952.v2/webrev/
>>
>> regards,
>> Sean.
>>>
>>> Gruss
>>> Bernd
>>> -- 
>>> http://bernd.eckenfels.net
>>> ------------------------------------------------------------------------
>>> *Von:* security-dev <security-dev-bounces at openjdk.java.net> im 
>>> Auftrag von Seán Coffey <sean.coffey at oracle.com>
>>> *Gesendet:* Freitag, November 16, 2018 12:25 PM
>>> *An:* OpenJDK Dev list
>>> *Betreff:* RFR: 8213952: Relax DNSName restriction as per RFC 1123
>>> Looking to make an adjustment to DNSName constructor to help comply 
>>> with
>>> RFC 1123
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8213952
>>> http://cr.openjdk.java.net/~coffeys/webrev.8213952/webrev/
>>>
>>> regards,
>>> Sean.
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20181121/dafcfc5c/attachment.htm>