Reviewer: Dale Worley Review result: Ready with Issues I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at <https://wiki.tools.ietf.org/area/gen/wiki/GenArtfaq>. Document: draft-ietf-acme-acme-11 Reviewer: Dale R. Worley Review Date: 2018-04-18 IETF LC End Date: 2018-04-18 IESG Telechat date: 2018-05-10 Summary: This draft is on the right track but has open issues, described in the review. This draft is much better than the version (-08) that I previously was the assigned Gen-ART reviewer for. The overall work and exposition are quite solid, though there are some open technical issues that need to be resolved. * Technical issues What is the versioning and extensibility system? -- It seems that the natural approach is that structures returned by fetches of Acme resources may contain elements that are not defined in this document, and a client should ignore them if it doesn't understand them. This allows plenty of flexibility for extending the Acme protocol; in particular, by adding further resources to the directory object. The handling of "terms of service agreement" seems to be insufficient, in that none of the information passed around says *what* agreement the client has agreed to. The client only sends "termsOfServiceAgreed:true" in a request, leaving what has been agreed to unspecified -- and unauditable. One possibility is to add an argument for the client to provide the URL from which it fetched the ToS (so the server knows what agreement the client is referring to) and the hash of the ToS document (so the client must attest to what the agreement text was). 6.6.1. Subproblems What error type should be used in a problem document when there are subproblem documents? It seems that in this situation, the intention is that the top-level problem document is not intended to carry error information itself, so you want to define a "subproblems" error type, to use when there is no natural overall error type. * Editorial issues 7.1. Resources The position of "newNonce" in the diagram is strange. Does it have a different relationship to the directory resource than newAccount, etc.? Similarly for the "finalize" and "cert" URLs of an order object. -- Reading further in the draft suggests that the graphical difference indicates that that these values are optional in the respective objects, although the text doesn't identify that. 7.1.2. Account Objects contact (optional, array of string): An array of URLs that the server can use to contact the client for issues related to this account. For example, the server may wish to notify the client about server-initiated revocation or certificate expiration. I mentioned this in my review of -08, but it hasn't been fixed: Strictly, you want "URIs" here, as tel:, sip:, and mailto: URIs are not URLs [RFC 6068]. 7.3.5. External Account Binding To enable ACME account binding, a CA needs to provide the ACME client with a MAC key and a key identifier, using some mechanism outside of ACME. The key identifier MUST be an ASCII string. The MAC key SHOULD be provided in base64url-encoded form, to maximize compatibility between non-ACME provisioning systems and ACME clients. I *think* what this means is that the service providing the external account provides the ACME client with a MAC key and a key identifier, which the client then uses in constructing its request to the ACME server. If that is correct, this text is not making clear (to me) the distinction between the CA that operates the ACME server (which I take as the default meaning of "CA" in this document) and the service that provides the "external account". I think two different terms are needed for the two services so as to make the processing described in this section clear. 7.4. Applying for Certificate Issuance This section seems to describe both the process of creating an order and the process of finalizing an order. The initial paragraphs regard creating an order, but the text starting with "Once the client believes it has fulfilled the server's requirements..." it talks about finalization. The text continues to discuss finalization until it gets to this confusing item: o "ready": The server agrees that the requirements have been fulfilled, and is awaiting finalization. Submit a finalization request. I *think* what is going on is that the text from "The status of the order will indicate..." through the 5 following items are a *general* description of the status field which is limited to neither the order creation step nor the order finalization step. It seems to me that this section should be divided into three parts, one describing creating an order, one describing finalizing an order, and the generalized description of the status values and the next-steps that each value implies. The first two parts might be better expressed as two subsections of 7.4, to clarify which part of the order life cycle contains which actions. It's not clear to me the best way to handle the third part; it may be most useful placed somewhere else in the document. 7.6. Certificate Revocation It is implicit but might be worth mentioning that the server should only accept revocation requests for certificates that it itself has issued. I mention this because the revocation request signed by the certificate's key does not directly reference an account in the server, but (I think) provides enough information that the server could construct an apparently legitimate CRL entry for the certificate using just the information in the revocation request. Thus, a sloppy implementation might skip verifying that it is dealing with a certificate that it issued. 8.3. HTTP Challenge On receiving a response, the server constructs and stores the key authorization from the challenge "token" value and the current client account key. I'm not sure this storage step is necessary, or even visible in the protocol operation. (E.g., the server can calculate the key authorization at any time that it needs to know the value.) So you might want to remove this sentence. Similarly for section 8.4. 9.1. MIME Type: application/pem-certificate-chain Each following certificate SHOULD directly certify one preceding it. This is grammatical, but it leaves me wondering if a typo was made, because "certify one preceding" and "certify the one preceding" are both grammatical but have different meanings. I suggest you replace it with one of the following: Each following certificate SHOULD directly certify the one preceding it. Each following certificate SHOULD directly certify some certificate preceding it. 9.7.8. Validation Methods | tls-sni-01 | RESERVED | N | N/A | | | | | | | tls-sni-02 | RESERVED | N | N/A | It seems pointless to insert two entries into a registry with no documentation reference, unless the intention is to reserve these identifiers from future use. But in that case, this document is actively prescribing that the identifiers are reserved, and this document is the reference for that action. [END]
