Skip to main content

Last Call Review of draft-ietf-acme-subdomains-04
review-ietf-acme-subdomains-04-artart-lc-bormann-2022-11-23-00

Request Review of draft-ietf-acme-subdomains
Requested revision No specific revision (document currently at 07)
Type Last Call Review
Team ART Area Review Team (artart)
Deadline 2022-11-21
Requested 2022-10-31
Authors Owen Friel , Richard Barnes , Tim Hollebeek , Michael Richardson
I-D last updated 2022-11-23
Completed reviews Artart Last Call review of -04 by Carsten Bormann (diff)
Genart Last Call review of -04 by Reese Enghardt (diff)
Opsdir Last Call review of -04 by Bo Wu (diff)
Assignment Reviewer Carsten Bormann
State Completed
Request Last Call review on draft-ietf-acme-subdomains by ART Area Review Team Assigned
Posted at https://mailarchive.ietf.org/arch/msg/art/dk8yWKSWxxvVu0CfpuDFzaEVNYA
Reviewed revision 04 (document currently at 07)
Result Ready w/issues
Completed 2022-11-23
review-ietf-acme-subdomains-04-artart-lc-bormann-2022-11-23-00
Thank you for an easy-to-read document that is accessible even to
readers who are not ACME experts.

# Minor technical

## "default value"

subdomainAuthAllowed is said to have a default value (value assumed if
not included) of false, i.e., absence of the field implies the value
to be false (except in metadata, which is a separate inconsistency
that might surprise implementers).

However, in several places, the text seems to instruct the server to
specifically include subdomainAuthAllowed with a value of false in
certain cases, apparently turning this into a three-valued field
(true, false, absent).

Which one is it?

(This could easily become an interoperability problem.)

## security considerations

The security considerations do not discuss the actual security
considerations of enabling operations on subdomains, in particular in
case where a subdomain is delegated in DNS.

# Major editorial

## restatement vs. new normative content

Providing a specification of a new feature added to ACME, the text
explains a number basic ACME mechanisms that are relevant to this
specification.

One pervasive problem is that these restatements of RFC 8555 content
are not always easy to distinguish from new, normative statements made
by this document.
E.g., 4.2 contains a statement about "is defined" that is part of a
paragraph restating RFC 8555 -- this one, however, appears to be new
normative content.
(Languagetool diagnoses overuse of passive voice, which exacerbates
this problem.)

(The first paragraph of section 4 repeats the last paragraph of
section 3.  But that is not a problem; redundancy can be good if it
improves the flow, and this is clearly labeled as a restatement.)
The introduction of section 4 is a summary/restatement of RFC 8555;
section 4.1 introduces new normative content without warning (and
leads the reader astray by actually referencing RFC 8555).

## terminology

### parent

The term "parent" is usually reserved for the direct ancestor (single
edge in the graph).  What is defined here really is an "ancestor
domain".  (Given the definition of subdomain that explicitly includes
self, each domain also is its own parent domain; I'm not clear whether
this is intended here.)

Unfortunately, this unusual terminology is now hard-coded in the names
of fields added by this specification, so it is not a purely editorial
decision to adjust the terminology to common usage.

### subdomain

The definition of subdomain (of a domain given) appears to include the
domain given.  This fine point might be lost on the reader; it can be
surprising (subalpine definitely does not include alpine).
Several pieces of text sound like setting subdomainAuthAllowed to true
only allows subdomains, which actually does not make a difference due
to the subtlety of the meaning of subdomain.

### POST-as-GET request

Term should probably be explicitly imported from Section 6.3 of RFC
8555.

### "standard"

Section 4.2 discusses a "standard ACME workflow".
Please do not use "standard" as a synonym of "basic", "regular", ...

# Minor editorial

## abbrev ACME

ACME is not currently marked as "well-known" (no expansion needed) on
the abbreviations list (https://www.rfc-editor.org/materials/abbrev.expansion.txt).
Both the title and the abstract should be reviewed as to how they will
look like when ACME is expanded by the RFC editor.
(RFC 8555 should probably be mentioned in the abstract.)

## recognizing restatements

The abstract reads as if the statement about certification authority
policy were always true.  But this is just stating the precondition
for the mechanism described here to be authorized?

## References

The "CAB Browser Forum" reference is stated as undated, but does link
to a specific edition 1.7.1 dated 2022-08-20.
Is this intended to be a "latest version" reference?

RFC 8555 clearly is a normative reference.
(Other RFCs listed under informative are actually normative
through this reference as well.)

Compounding the restatement anti-pattern, some restatements are
missing the necessary reference, e.g., the start of Section 3.

# Nits

(editorial: inconsistent use of periods at the end of bullet items)

More nits are fixed in: https://github.com/upros/acme-subdomains/pull/14