yaronf / I-D

Internet Drafts
Other
5 stars 14 forks source link

Francesca's comments #173

Closed thomas-fossati closed 3 years ago

thomas-fossati commented 3 years ago
1. -----

   the ACME CA and waits for the explicit revocation based on CRL and
   OCSP to propagate to the relying parties.

   ...

   result, the TLS connection to the dCDN edge is done with an SNI equal

FP: Please expand CRL, OCSP and SNI on first use.

2. -----

   *  delegations (required, string): A URL from which a list of
      delegations configured for this account can be fetched via a POST-
      as-GET request.

FP: the second occurrence of "delegation" needs a pointer to the next
subsection - Delegation Objects, otherwise this definition becomes
confusing (delegations is a URL from which a list of delegations can
be fetched).

3. -----

   An example delegation object is shown in Figure 3.

FP: please note that the examples are in JSON.

4. -----

   (Forbidden) and type "urn:ietf:params:acme:error:unknownDelegation".

FP: suggestion to change to:

   (Forbidden), providing a problem document [RFC7807] with type  "urn:ietf:params:acme:error:unknownDelegation", as registered in section 5.5.

The error type appears later on as well (e.g. section 2.3.2), but even
without repeating each time, I think this reference at least here where
it appears first would help the reader.

5. -----

   The Order object created by the IdO:

   ...

   *  MUST copy the "star-certificate" field from the STAR Order.  The

FP: (suggestion for clarification) because there are 2 Orders going on
in sequence, this bullet point and more precisely "from the STAR Order"
is slightly confusing. You could use Order1 and Order2 as you have used
in Figure 1 to clarify things (I believe this should be "from the STAR Order2
into Order1) (Note that this is just a suggestion, the rest of the text is
mostly clear about which Order it refers to) Otherwise, I think it would
be good to add "... from the STAR Order into its Order resource."
The same comment apply to the next occurrence:

   *  MUST copy the "certificate" field from the Order, as well as

6. -----

   uCDN is configured to delegate to dCDN, and CP is configured to
   delegate to uCDN, both as defined in Section 2.3.1.

FP: Re Figure 12: I assume that 0. refers to the configuration CP and uCDN
share? In this case, why is there no arrow between uCDN and dCDN? If my
assumption is wrong, then what's the meaning of 0?

7. -----

FP: This document defines two new registry, one with policy Specification
required and the other Expert review (both of which will need designated
experts). https://tools.ietf.org/html/rfc8126#section-5.3 states that:

   When a designated expert is used, the documentation should give clear
   guidance to the designated expert, laying out criteria for performing
   an evaluation and reasons for rejecting a request.  In the case where

I have noticed that RFC 8555 only provided guidance for one of its registries, and that the registries are quite straight forwards, but I still believe that having some guidance for the experts to evaluate requests helps.
fpalombini commented 3 years ago

Thanks for reporting them here! I also added the following to my ballot (IANA expert comments):

Also wanted to point out the IANA Designated Expert review to make sure it is addressed (found in the datatracker, but which I report here for simplicity as well) - thank you to Richard Barnes for it:

  1. The "delegation" field is currently attached to the "identifier" object, which is a bad semantic fit in a few ways. ACME orders can have multiple identifiers, and delegations can describe multiple SAN values, yet this design assumes singularity on both sides. This field should be moved to the order object; in fact, if you wanted to be more radical, you could even use it to replace the "identifiers" field in the newOrder request.
  1. The "allow-certificate-get" field is listed as configurable. It seems like this is a matter of CA policy, so it should either be non-configurable, or if you allow the client to request a value for it, there should be a clear specification that the server is allowed to ignore the client's preference.
yaronf commented 3 years ago

Thanks for reporting them here! I also added the following to my ballot (IANA expert comments):

Also wanted to point out the IANA Designated Expert review to make sure it is addressed (found in the datatracker, but which I report here for simplicity as well) - thank you to Richard Barnes for it:

Sure. These two are tracked as #171 and #172 .

fpalombini commented 3 years ago

Ah, didn't see them. Thanks!

thomas-fossati commented 3 years ago

  1. uCDN is configured to delegate to dCDN, and CP is configured to delegate to uCDN, both as defined in Section 2.3.1.

FP: Re Figure 12: I assume that 0. refers to the configuration CP and uCDN share? In this case, why is there no arrow between uCDN and dCDN? If my assumption is wrong, then what's the meaning of 0?

Good catch, thanks!

0 is the ACME account setup, which we assume as a precondition (see Section 2.3.1.1), and is therefore redundant. I will remove it.

thomas-fossati commented 3 years ago

FP: This document defines two new registry, one with policy Specification required and the other Expert review (both of which will need designated experts). https://tools.ietf.org/html/rfc8126#section-5.3 states that:

When a designated expert is used, the documentation should give clear guidance to the designated expert, laying out criteria for performing an evaluation and reasons for rejecting a request. In the case where

I have noticed that RFC 8555 only provided guidance for one of its registries, and that the registries are quite straight forwards, but I still believe that having some guidance for the experts to evaluate requests helps.

Yaron has provided a fix for this in #179. (Note that we have dropped one of the two registries because of #171)