Hey Martin, Thanks for the thorough review, I agree with all of the suggestions and will be incorporating the changes into the next revision. Following up on one point about Section 7, I believe you may actually be thinking about another issue we had with the http-01 ACME challenge. The issue here was as described, if the ACME server was using HTTPS to validate a http-01 challenge and the expected domain name didn’t have a proper virtual host configuration many servers would redirect the request to a ‘default’ handler. This is called out in RFC 8555 Section 8.3. Thanks, Roland > On Sep 17, 2019, at 12:40 AM, Martin Thomson via Datatracker <noreply@xxxxxxxx> wrote: > > 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. > >
