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.

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

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.

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

I don’t think splitting this sentence makes sense, both sections rely on each other.

> 
> ** Section 7.  Typo.  s/specication/specification/
> 
> Roman
> 
> _______________________________________________
> Acme mailing list
> Acme@ietf.org
> https://www.ietf.org/mailman/listinfo/acme