Re: [Acme] Alexey Melnikov's No Objection on draft-ietf-acme-acme-14: (with COMMENT)

[Richard Barnes <[e-mail]>]

Hi Alexey,

Thanks for the comments.  A couple of replies are below; resulting edits
are in this PR:

https://github.com/ietf-wg-acme/acme/pull/442

--Richard


On Wed, Aug 29, 2018 at 7:14 AM Alexey Melnikov <[e-mail]>
wrote:

> Alexey Melnikov has entered the following ballot position for
> draft-ietf-acme-acme-14: No Objection
>
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
>
>
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
>
>
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-acme-acme/
>
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Thank you for this very important document. I would like to switch to
> "Yes",
> but please first review and respond to my comments:
>
> First mentions of JSON and HTTPS need references.
>
> 6.4.1.  Replay-Nonce
>
>    The "Replay-Nonce" header field includes a server-generated value
>    that the server can use to detect unauthorized replay in future
>    client requests.  The server MUST generate the value provided in
>    Replay-Nonce in such a way that they are unique to each message, with
>    high probability.  For instance, it is acceptable to generate Replay-
>    Nonces randomly.
>
>    The value of the Replay-Nonce field MUST be an octet string encoded
>    according to the base64url encoding described in Section 2 of
>    [RFC7515].  Clients MUST ignore invalid Replay-Nonce values.  The
>    ABNF [RFC5234] for the Replay-Nonce header field follows:
>
>      base64url = [A-Z] / [a-z] / [0-9] / "-" / "_"
>
> This is not correct ABNF. Change range syntax in Section 3.4 of RFC 5234
>

I've updated to try to fix this, but your review on the PR would be
appreciated.



>      Replay-Nonce = *base64url
>
> You allow for empty nonce here. Should this be "1*base64url"?
>

Done.


Please add normative references for Retry-After and Link header fields.
>

These already have normative references at their first appearance:

https://tools.ietf.org/html/draft-ietf-acme-acme-14#section-6.5

Do you think those references are incorrect?


In Section 6.6:
>
>   | unsupportedIdentifier   | Identifier is not supported, but may be |
>    |                         | in future
>
> Do you mean "identifier type", not identifier?
>

Done.


   This list is not exhaustive.  The server MAY return errors whose
>    "type" field is set to a URI other than those defined above.  Servers
>    MUST NOT use the ACME URN namespace Section 9.6 for errors other than
>    the standard types.
>
> I think this text is misleading, as you create a registry for these.
> I suggest you rephrase and add a reference to the registry.
>

Clarified that the MUST NOT means "don't use unregistered values"



> In 7.1.1:
>
>    caaIdentities (optional, array of string):  Each string MUST be a
>       lowercase hostname which the ACME server recognizes as referring
>
> Is hostname fully qualified? U-label IDNA domains not allowed here? Please
> clarify.
>

These are only provided for matching against CAA records (RFC 6844), so the
simplest thing is to just require that they be equal.  In practice, that
means ASCII-names.



>       to itself for the purposes of CAA record validation as defined in
>       [RFC6844].  This allows clients to determine the correct issuer
>       domain name to use when configuring CAA records.
>
> Example in 7.1.1 (or maybe even earlier): You probably should say that some
> header fields are omitted here.
>

I think that is clear enough from context.



> In 7.1.2:
>
>    contact (optional, array of string):  An array of URLs that the
>
> Which URI schemes are allowed (or at least expected) here?
>

Basically, servers must support "mailto", and may support others; they can
specify their requirements in an error message.

https://tools.ietf.org/html/draft-ietf-acme-acme-14#section-7.3

The WG discussed this and decided not to have more negotiation here.  See,
e.g.:

https://mailarchive.ietf.org/arch/msg/acme/TW8sbspUIGDGbIldaqWW0k9jKYo



>       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.
>
> In 7.4:
>
>    Clients SHOULD NOT make any assumptions about the sort order of
>    "identifiers" or "authorizations" elements in the returned order
>    object.
>
> Why is this a SHOULD NOT and not a MUST NOT?
>

Done.


(Similar text in other sections!)
>

I don't see "sort order" anywhere else, or other relevant usage of
"order".  Do you have other places in mind?



>
> 7.4.2.  Downloading the Certificate
>
>    GET /acme/cert/asdf HTTP/1.1
>    Host: example.com
>    Accept: application/pkix-cert
>
> I think your example is wrong, as Accept value needs to match
> application/pem-certificate-chain returned below:
>

Done.



> In 8.3:
>
>    The server SHOULD follow redirects when dereferencing the URL.
>
> Why only a SHOULD?
>

Some server operators wanted to have the option to require that the
validation work on the first request.



> 9.1.  MIME Type: application/pem-certificate-chain
>
>    The "Media Types" registry should be updated with the following
>    additional value:
>
>    MIME media type name: application
>
>    MIME subtype name: pem-certificate-chain
>
>    Required parameters: None
>
>    Optional parameters: None
>
>    Encoding considerations: None
>
> This value has to be one of "7bit", "8bit", "binary" or "framed". I think
> this
> should be "7bit", as PEM is ASCII.
>

Done.



>    Security considerations: Carries a cryptographic certificate and its
>    associated certificate chain
>
> I suggest you add text saying that this media type doesn't include active
> content.
>

Done.



>    Interoperability considerations: None
>
>    Published specification: draft-ietf-acme-acme [[ RFC EDITOR: Please
>    replace draft-ietf-acme-acme above with the RFC number assigned to
>    this ]]
>
>    Applications which use this media type: Any MIME-compliant transport
>
> This text is not actually useful for Media Type reviewers or users.
> You should say "ACME client and servers" or something like this. Giving
> more
> examples of use would be a plus.
>

Done.



> You are also missing some fields from the registration template. In
> particular,
> who is the Change Controller? (IETF or IESG)
>
> 9.6.  URN Sub-namespace for ACME (urn:ietf:params:acme)
>
>    Repository:  URL-TBD
>
> Who needs to fix this value?
>

There's a request to the RFC editor below.



> 9.7.1.  Fields in Account Objects
>
>    o  Field type: The type of value to be provided, e.g., string,
>       boolean, array of string
>
> Here and in all similar registries: I think you should insert "JSON" before
> "type" to make it clear that types are only restricted to JSON type
> choices.
>

It's a JSON object.  If you define a non-JSON type, you're gonna have a bad
time.



> 9.7.8.  Validation Methods
>
>    Template:
>
>    o  Identifier Type: The type of identifier that this method applies
>       to
>
> I think you should add "or a special keyword RESERVED", as otherwise the
> table
> below might be confusing.
>
>    o  ACME: "Y" if the validation method corresponds to an ACME
>       challenge type; "N" otherwise.
>
> I think this could have been clearer, as it is not obvious when "N" can be
> used. For example you use "N" for "RESERVED" values.
>

Clarified that for non-ACME values, the identifier type is "N/A".