Last Call Review of draft-ietf-acme-tls-alpn-06
review-ietf-acme-tls-alpn-06-artart-lc-thomson-2019-09-17-00

Request Review of draft-ietf-acme-tls-alpn
Requested rev. no specific revision (document currently at 07)
Type Last Call Review
Team ART Area Review Team (artart)
Deadline 2019-09-25
Requested 2019-09-11
Authors Roland Shoemaker
Draft last updated 2019-09-17
Completed reviews Secdir Last Call review of -06 by Daniel Migault (diff)
Genart Last Call review of -06 by Linda Dunbar (diff)
Artart Last Call review of -06 by Martin Thomson (diff)
Assignment Reviewer Martin Thomson
State Completed
Review review-ietf-acme-tls-alpn-06-artart-lc-thomson-2019-09-17
Posted at https://mailarchive.ietf.org/arch/msg/art/n10MY5njUA0DHGTtICtk6YU5B4w
Reviewed rev. 06 (document currently at 07)
Review result Ready with Nits
Review completed: 2019-09-17

Review
review-ietf-acme-tls-alpn-06-artart-lc-thomson-2019-09-17

This is a clear description of a simple protocol that addresses a well-defined problem.  I didn't find any real issues, though I have some suggestions that might improve the document a little.  My view is that publication without these would still produce a useful RFC.

Suggestions:

The document should probably talk about minimum TLS versions and expected algorithms.  I think that it would be enough to say TLS 1.2 minimum with the mandatory cipher suites from that spec.  You might also want to require certificates with a PKCS#1v1.5 RSASSA key (as much as those are terrible), but to permit use and negotiation of other alternatives.  If your CA supports ECDSA, I see no reason not to prepare an ECDSA certificate on that basis, but that would be based on extra-textual knowledge, it's no guarantee of interoperability.

Section 4
I have two suggestions here for making the properties you rely on with the protocol clearer to readers.

You should explicitly say that the "acme-tls/1" protocol as negotiated here does not carry application data, it only requires that the server provide certificate-based authentication.  AND that the certificate-based authentication provided uses an alternative method than that described in RFC 5280, even though it uses X.509 certificates.  Both of these are already implicit in the text here, but I think that it would help to be very clear about these as they are a little surprising ways to use TLS.

You might also want to explicitly point out that though the protocol does not depend on the server providing a valid signature, or even that it successfully complete the TLS handshake, these are both required by the protocol and a validator MAY withhold authorization if the TLS handshake does not complete successfully.  (This is implied in Step 4, but not explained.)

Nits:

Section 1
The history lesson in the appendix is useful.  It helps to articulate why the design is this way.  However, I don't think that you need to spend a third of the introduction on a reference to that historical information.  I'd cut that paragraph entirely.

Section 3
https://www.quickanddirtytips.com/sites/default/files/styles/insert_large/public/images/937/use-utilize-pin.png?itok=Bt7A6RQq

Step 3: s/AMCE/ACME; this sentence is two with a comma-splice

Section 4
This is taste only, but I find the extra version decoration on these identifiers unnecessary.  Decorate v2 if you are unfortunate enough to need one.

Section 5

The parallel list construction in this sentence is a little awkward:

   "This means that if User A registers Host A and User B registers Host B the server should not allow a TLS request using a SNI value for Host A to be served by User B or Host B to be served by User A."

The first part of this sentence is fine, but the second part "or Host B to be served by User A" might be clearer if it said "or a TLS connection with a server_name extension identifying Host B to be answered by User A."  Or similar.

Section 6.2
Please don't use uppercase for an identifier when the actual protocol is identified using lowercase ASCII.  I know that RFC 7301 did this for HTTP/1.1, but that was a confusing mistake that doesn't need to be propagated.

Section 7
This is not an appendix.  You can make appendices using `<back>` in XML or by moving the section under `---back` in kramdown-rfc2629.

The implication of this text is to say that this is a replacement for an important part of ACME.  I would instead say that this is an iteration on an earlier design that used the TLS server_name extension to carry the challenge.  And that that earlier design was shown to have some serious issues in practice.

My understanding of the problem differs from what is described here though: the problem - as I recall - was that this used high-entropy, generated values for the server_name extension and an HTTP or absent ALPN identifier.  These names might not have as strong a binding to the user that controlled the validated portion of that name (the suffix) as desired.  Some servers would route these SNI values to a "default" handler.  The "default" handler could be under the control of any user; in fact, users could in some cases choose a name that would greatly improve their chances of having their certificate used as a default (e.g., aaaaa.example.host or zzzzz.example.host.  As formulated here, particularly with the User A/Host B phrasing, you are almost implying that the security property you rely on in the ALPN design doesn't hold.  As far as I'm aware, that security property does hold because you are including exactly the validated name in the SNI.