RFR: 8213952: Relax DNSName restriction as per RFC 1123

[Seán Coffey]

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/3b607ef8/attachment.htm>