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

From: Roman Danyliw <[email-masked]>
Date: Fri, 21 Jun 2019 11:57:56 +0000

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?

** 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.

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

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.

** 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.  

** 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

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

** 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./

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

Roman