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 
> <nore...@ietf.org> 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.
> 
> 

_______________________________________________
Acme mailing list
Acme@ietf.org
https://www.ietf.org/mailman/listinfo/acme

Reply via email to