Re: [Acme] Second AD Review: draft-ietf-acme-tls-alpn

Hi Roland!

> -----Original Message-----
> From: Roland Shoemaker [mailto:roland@letsencrypt.org]
> Sent: Tuesday, July 30, 2019 3:08 PM
> To: Roman Danyliw <rdd@cert.org>
> Cc: acme@ietf.org
> Subject: Re: [Acme] Second AD Review: draft-ietf-acme-tls-alpn
> 
> Hey Roman,
> 
> I’ve address most of the comments below and have a draft of the changes
> here: https://github.com/rolandshoemaker/acme-tls-alpn/compare/in-
> proc?w=1
> 
> There are a few comments I’m not sure I agree with which I’ve responded to
> inline below, if this all looks good to you I’ll push up a new numbered draft.
> 
> Thanks!
> 
> > On Jun 21, 2019, at 4:57 AM, Roman Danyliw <rdd@cert.org> wrote:
> >
> > Hi!
> >
> > I conducted as second AD review of draft-ietf-acme-tls-apln per the AD
> hand-off.  I have the following feedback/questions:
> >
> > ** Please address the issues from AD Review #1 and update the text as
> discussed on the ML (specifically about Section 3 and Section 6):
> > https://mailarchive.ietf.org/arch/msg/acme/LQ-
> _rdrH5xVSxW64T7w3LONZ1RM
>
> >
> > ** Section 3.  (My ASN.1 foo is lacking but ...)  Per the ASN.1 format of
> acmeIdentifier, where is id-pe defined?  The descriptive text mentions an
> "extnValue" in the "id-pe-acmeIdentifier extension" where is that defined?
> 
> Both id-pe and extnValue are defined in RFC 5280. I’ve added some
> clarifying text to the draft.

Got it.  The text works for me.

> >
> > ** Section 3 and Section 3.1.  Per:
> >
> > Section 3: Once the TLS handshake has been completed the connection
> MUST be immediately closed and no further data should be exchanged.
> > Vs.
> > Section 3.1: Once the handshake is completed the client MUST NOT
> exchange any further data with the server and MUST immediately close the
> connection.
> >
> > Why does Section 3 and 3.1 provide slightly different normative language
> about closing the TLS connections and not exchanging data.  I don't think we
> need both.

The updated text works for me.  Thanks.

> > ** Section 4.  The Security Considerations of RFC8555 hold too.

Thanks for adding the reference.

> > Below is additional editorial feedback:
> >
> > ** Section 3.  The list of fields, type and token, doesn't follow from the
> introductory sentence.  Provide some transition and introduction on the
> presence of those fields.
> >
> > ** Section 3.  Cite the base64url alphabet.

Thanks.

> > ** Section 3. The purpose of the two HTTP blob isn't made clear; they
> aren't referenced in the text; and don't have a figure number.
> 
> This follows the challenge definition format in 8555, I agree the GET blob
> doesn’t really make sense and have removed it, but I think the POST is
> appropriate and is referenced in both the preceding and following text.

Concur that this text uses a similar style to RFC8555.  FWIW, I found this format confusing.  I can agree to disagree on this editorial point.

> > ** Section 3.  Specify that that the format is acmeIdentifier ASN.1 as:
> >   [X680]     ITU-T, "Information technology -- Abstract Syntax Notation
> >              One (ASN.1): Specification of basic notation",
> >              ITU-T Recommendation X.680, 2015.
> >
> > ** Section 3.  Cite ASN.1 DER encoding as:
> >    [X690]     ITU-T, "Information Technology -- ASN.1 encoding rules:
> >              Specification of Basic Encoding Rules (BER), Canonical
> >              Encoding Rules (CER) and Distinguished Encoding Rules
> >              (DER)", ITU-T Recommendation X.690, 2015.
> >
> > ** Section 3.  Cite "SNI extension" (RFC6066) on first use

Thanks for these changes

> > ** Section 3.  Step 4.  Per "Verify that the ServerHello", consider re-writing
> this sentence so it doesn't use "contains" five times.

This whole numbers section is now clearer.  Thanks.

> > ** Section 3.  Step 4.  Typo (missing period).
> > s/Note that as ACME doesn't support Unicode identifiers all dNSNames
> MUST be encoded using the [RFC3492] rules./Note that as ACME doesn't
> support Unicode identifiers.  All dNSNames MUST be encoded using the
> [RFC3492] rules.
> 
> I don’t think splitting this sentence makes sense, both sections rely on each
> other.

No problem.

> >
> > ** Section 7.  Typo.  s/specication/specification/

Thanks.

Roman

> > Roman
> >
> > _______________________________________________
> > Acme mailing list
> > Acme@ietf.org
> > https://www.ietf.org/mailman/listinfo/acme