Last Call Review of draft-ietf-acme-integrations-12
review-ietf-acme-integrations-12-dnsdir-lc-lemon-2023-01-24-00

I said that this document is "ready with issues" because I think that you
really need to clarify the stuff I called out in the security considerations
section below, and the bit about subdomains and TEAP. Everything else is just a
nit—if it's useful, please don't hesitate to use it, and if it's not, please
feel free to ignore it!

In section 2, Terminology:

> Using graph theory, a label identifies one node in a portion of the graph of
all possible domain names.

This statement is immediately followed by this:

> Domain Name: An ordered list of one or more labels.

> Subdomain ...

I assume that the point you're getting at here is that labels are nodes in a
tree, when talking in terms of the domain naming hierarchy, and nodes in an
ordered list, in the case of a domain name, and "directed graph" is the direct
superset of these two things. So I get why this more general term was used, but
I don't think it's going to clarify anything for the reader. If you want the
reader to really have a mental model that's oriented around graph theory, you
probably need to actually be specific about what each of these things is in
relation to graph theory. E.g., maybe say:

* every node in the tree of the domain hierarchy is a label.
* a domain name describes a particular path along that tree between two nodes
one of which is below the other in the tree hierachy, where each label is
hierarchically below the label to its right (if any) and hierarchically above
the label to its left (if any), with each pair of labels separated by a '.'. *
an FQDN is a path along the tree between the root node and some other node (all
nodes on the tree obviously being hierarchically below the root node), with the
root label being represented by a trailing '.'

I'm not seriously proposing that you make this change, but if you don't, I
think you should delete the sentence about graph theory, because it's just
confusingly broad if you don't then actually describe the subset of graph
theory you're talking about.

--

As a naive reader of this document, the diagrams that show the ACME process
aren't really contextualized as being part of ACME, so when I was reviewing the
document, I found myself hunting for where the DNS operations shown in the
diagrams are actually described in an RFC. I knew it was part of ACME, but
wasn't sure that there weren't additional bits in e.g. the EST or BRSKI
specification. It might be helpful to explicitly say that the flow here is
defined in the ACME spec. Again, this really depends on who your reader is and
how much you want to coddle them, but I thought it was worth mentioning.

--

In the introduction you say "Use of ACME for subdomains is not a necessary
requirement." You should probably delete "necessary" here, but the main reason
I call this out is that later on when you are describing ACME integration with
TEAP, you only describe it for the subdomain case. Which leads me to the
question "which is it?" I.e., are subdomains required for TEAP, and if not, how
does it work when you aren't doing subdomains?

--

In section 9:

> It is expected that the TEAP-EAP server/EST Registrar will perform DNS
dynamic updates to a DNS primary server using [RFC3007] Dynamic updates,
secured with either SIG(0), or TSIG keys.

This seems needlessly prescriptive. I think it's quite reasonable to think that
an implementation might use some mechanism for directly writing to the primary
zone database rather than using either type of DNS update. You are describing a
vulnerability related to updates of this type, so maybe you should just say "in
the case that the TEAP-EAP server/EST registrar performs DNS dynamic updates
..."

--

In section 9:

> A major source of vulnerability is the disclosure of these DNS key records. 
An attacker that has access to them, can provision their own certificates into
the the name space of the entity.

This is somewhat inaccurate. TSIG doesn't have KEY records, so you aren't
pointing to a thing here if you're using TSIG. In the case of SIG(0), the
actual vulbnerability is that the attacker somehow gets the secret key, not the
public key. Only the public key appears in a KEY record. So you should probably
fix this to more accurately describe the data elements that can be compromised
to effect the attack.

Later text made me wonder whether the vulnerability you were talking about here
is not that the attacker is able to get at the secret key, but rather that the
attacker is able to write its own key, and then use that key to publish the DNS
TXT record that's being used for the ACME challenge. If that's the case, this
section should probably be clarified to talk about this more specifically.

--

You probably need a reference to RFC2136 and RFC2931 in addition to the
reference to RFC3007.