<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <p>whoops: <br>
    </p>
    <p>latest webrev :
      <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~coffeys/webrev.8213952.v4/webrev/">http://cr.openjdk.java.net/~coffeys/webrev.8213952.v4/webrev/</a><br>
    </p>
    <pre class="moz-signature" cols="72">Regards,
Sean.</pre>
    <div class="moz-cite-prefix">On 03/12/18 17:10, Seán Coffey wrote:<br>
    </div>
    <blockquote
      cite="mid:296eee95-58c7-d658-90e5-379d3091df8c@oracle.com"
      type="cite">
      <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
      <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 moz-do-not-send="true"
              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 moz-do-not-send="true"
                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 moz-do-not-send="true" 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 moz-do-not-send="true"
                    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 moz-do-not-send="true"
                      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 moz-do-not-send="true"
                      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 moz-do-not-send="true"
                        class="moz-txt-link-freetext"
                        href="http://bernd.eckenfels.net">http://bernd.eckenfels.net</a>
                      <br>
                      Von: Seán Coffey <a moz-do-not-send="true"
                        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 moz-do-not-send="true"
                        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 moz-do-not-send="true"
                        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 moz-do-not-send="true"
                          class="moz-txt-link-freetext"
                          href="http://bernd.eckenfels.net">http://bernd.eckenfels.net</a>
                        <br>
                        Von: security-dev <a moz-do-not-send="true"
                          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
                          moz-do-not-send="true"
                          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 moz-do-not-send="true"
                          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 moz-do-not-send="true"
                          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>
    </blockquote>
    <br>
  </body>
</html>