[art] Artart last call review of draft-ietf-acme-tls-alpn-06

[Martin Thomson via Datatracker <[e-mail]>]

Reviewer: Martin Thomson
Review result: Ready with Nits

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.