<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p>The CSR related to this change has been approved. <br>
</p>
<p>I made further edits to update the DNSName comment code to
reference RFC 5280 rather than the obsoleted RFC 2459. I also
updated the test case with a few extra tests per suggestion from
Chris and others. Moved the dataprovider into a good and bad data
set also.</p>
<p>A follow on bug has been logged to update all references of RFC
2459 to RFC 5280 (JDK-8214532)<br>
</p>
<pre class="moz-signature" cols="72">Regards,
Sean.</pre>
<div class="moz-cite-prefix">On 27/11/18 14:27, Seán Coffey wrote:<br>
</div>
<blockquote
cite="mid:868a73e0-0981-ffda-f070-7acc008f6baa@oracle.com"
type="cite">Thanks for the feedback Max. I'll ping Joe Darcy and
check if a CSR is required for this type of change. I'll revert
back to this list once I have an answer. <br>
<br>
keytool works well with the new change. I've modified the test as
per your suggestion : <br>
<br>
--- a/test/jdk/sun/security/tools/keytool/KeyToolTest.java <br>
+++ b/test/jdk/sun/security/tools/keytool/KeyToolTest.java <br>
@@ -1436,6 +1436,7 @@ <br>
testOK("", pre+"san3 -ext san=dns:me.org"); <br>
testOK("", pre+"san4 -ext san=ip:192.168.0.1"); <br>
testOK("", pre+"san5 -ext san=oid:1.2.3.4"); <br>
+ testOK("", pre+"san6 -ext san=dns:1abc.com"); //begin
with digit <br>
testOK("", pre+"san235 -ext
san=uri:http://me.org,dns:me.org,oid:1.2.3.4"); <br>
<br>
Regards, <br>
Sean. <br>
<br>
On 27/11/18 01:29, Weijun Wang wrote: <br>
<blockquote type="cite"> <br>
<blockquote type="cite">On Nov 26, 2018, at 11:15 AM, Weijun
Wang <a class="moz-txt-link-rfc2396E"
href="mailto:weijun.wang@oracle.com"><weijun.wang@oracle.com></a>
wrote: <br>
<br>
<br>
<br>
<blockquote type="cite">On Nov 24, 2018, at 12:45 AM, Seán
Coffey <a class="moz-txt-link-rfc2396E"
href="mailto:sean.coffey@oracle.com"><sean.coffey@oracle.com></a>
wrote: <br>
<br>
Thanks for your review Max. I've made the suggested edits. <br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ecoffeys/webrev.8213952.v3/webrev/">http://cr.openjdk.java.net/~coffeys/webrev.8213952.v3/webrev/</a>
<br>
</blockquote>
The change looks fine. Thanks. <br>
<br>
<blockquote type="cite">I've also submitted a CSR for this
change just to cover the behavioural aspect of the edit.
Will push if/once that's approved by CSR team. <br>
</blockquote>
The CSR focuses on keytool but this is actually a library
change. There is no change with "add/remove/modify command
line option" at all. <br>
<br>
I would suggest talking about the DNSName class instead of
keytool. I understand it's internal but we can describe this
change as a non-minimal change on behavior so it's still worth
a CSR. Or, you can consult Joe what the best way is. Maybe he
can spare you from filing a CSR at all. <br>
<br>
BTW, you did try out keytool after this code change and "-ext
dns=1abc.com" is working now, right? <br>
</blockquote>
You can add an extra line after line 1437 of
test/jdk/sun/security/tools/keytool/KeyToolTest.java. <br>
<br>
<blockquote type="cite">Thanks <br>
Max <br>
<br>
<blockquote type="cite">Regards, <br>
Sean. <br>
<br>
On 22/11/18 14:49, Weijun Wang wrote: <br>
<blockquote type="cite">* DNSName.java: <br>
<br>
91 if ((endIndex - startIndex) < 1) <br>
<br>
No need for inner parentheses. <br>
<br>
And, is there really a need to define
alphaDigitsAndHyphen? How about just (== '-' || inside
alphaDigits)? <br>
<br>
* DNSNameTest.java: <br>
<br>
Space cannot appear anywhere, please add a test case like
"a c.com". <br>
<br>
BTW, I assume you want to reuse this test for other
sub-tasks of JDK-8054380? I see the @summary is more
general. <br>
<br>
No other comments. <br>
<br>
Thanks <br>
Max <br>
<br>
<blockquote type="cite">On Nov 22, 2018, at 12:51 AM, Seán
Coffey <a class="moz-txt-link-rfc2396E"
href="mailto:sean.coffey@oracle.com"><sean.coffey@oracle.com></a>
wrote: <br>
<br>
p.s I've updated the testcase to test the IOException
message for presence of "DNSName". Webrev updated in
place. <br>
<br>
Regards, <br>
Sean. <br>
<br>
On 21/11/18 15:42, Seán Coffey wrote: <br>
<blockquote type="cite">Thanks for the comments Bernd.
Comments inline.. <br>
<br>
On 16/11/18 21:27, Bernd Eckenfels wrote: <br>
<blockquote type="cite">Hello Sean, <br>
<br>
I was only looking at the inspected DNSName class,
it still has some variations (but start now with
DNSNames which is good already): <br>
<br>
76 throw new IOException("DNSName must not be null
or empty"); <br>
78 throw new IOException("DNSNames or
NameConstraints with blank components are not
permitted"); <br>
80 throw new IOException("DNSNames or
NameConstraints may not begin or end with a ."); <br>
92 throw new IOException("DNSName SubjectAltNames
with empty components are not permitted"); <br>
96 throw new IOException("DNSName components must
begin with a letter or digit"); <br>
101 throw new IOException("DNSName components must
consist of letters, digits, and hyphens"); <br>
<br>
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 <br>
</blockquote>
Ok - I've cleaned up the exception messages on line
78, 80, 92. <br>
webrev updated in place : <a
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ecoffeys/webrev.8213952.v2/webrev/">http://cr.openjdk.java.net/~coffeys/webrev.8213952.v2/webrev/</a>
<br>
<br>
<br>
<blockquote type="cite">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) <br>
<br>
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. <br>
</blockquote>
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. <br>
<br>
e.g. <a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8207768">https://bugs.openjdk.java.net/browse/JDK-8207768</a>
<br>
<br>
regards, <br>
Sean. <br>
<blockquote type="cite">Gruss <br>
Bernd <br>
-- <br>
<a class="moz-txt-link-freetext"
href="http://bernd.eckenfels.net">http://bernd.eckenfels.net</a>
<br>
Von: Seán Coffey <a class="moz-txt-link-rfc2396E"
href="mailto:sean.coffey@oracle.com"><sean.coffey@oracle.com></a>
<br>
Gesendet: Freitag, November 16, 2018 5:15 PM <br>
An: Bernd Eckenfels; <a
class="moz-txt-link-abbreviated"
href="mailto:security-dev@openjdk.java.net">security-dev@openjdk.java.net</a>
<br>
Betreff: Re: RFR: 8213952: Relax DNSName restriction
as per RFC 1123 <br>
Thanks for the comments Bernd. comments inline.. <br>
<br>
On 16/11/18 12:40, Bernd Eckenfels wrote: <br>
<blockquote type="cite">You could also add (a..b,
false) and (.a, false), (a., false) to the
testcases. <br>
</blockquote>
good point. test updated. <br>
<blockquote type="cite">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? <br>
</blockquote>
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. <br>
<br>
new webrev : <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ecoffeys/webrev.8213952.v2/webrev/">http://cr.openjdk.java.net/~coffeys/webrev.8213952.v2/webrev/</a>
<br>
<br>
regards, <br>
Sean. <br>
<blockquote type="cite">Gruss <br>
Bernd <br>
-- <br>
<a class="moz-txt-link-freetext"
href="http://bernd.eckenfels.net">http://bernd.eckenfels.net</a>
<br>
Von: security-dev <a
class="moz-txt-link-rfc2396E"
href="mailto:security-dev-bounces@openjdk.java.net"><security-dev-bounces@openjdk.java.net></a>
im Auftrag von Seán Coffey <a
class="moz-txt-link-rfc2396E"
href="mailto:sean.coffey@oracle.com"><sean.coffey@oracle.com></a>
<br>
Gesendet: Freitag, November 16, 2018 12:25 PM <br>
An: OpenJDK Dev list <br>
Betreff: RFR: 8213952: Relax DNSName restriction
as per RFC 1123 <br>
Looking to make an adjustment to DNSName
constructor to help comply with <br>
RFC 1123 <br>
<br>
<a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8213952">https://bugs.openjdk.java.net/browse/JDK-8213952</a>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ecoffeys/webrev.8213952/webrev/">http://cr.openjdk.java.net/~coffeys/webrev.8213952/webrev/</a>
<br>
<br>
regards, <br>
Sean. <br>
<br>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>