Subject: [Gen-art] Genart last call review of draft-ietf-acme-authority-token-tnauthlist-07

From: Pete Resnick via Datatracker <[email-masked]>
Date: Mon, 15 Mar 2021 13:06:58 -0700

Reviewer: Pete Resnick
Review result: Ready with Nits

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://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-acme-authority-token-tnauthlist-07
Reviewer: Pete Resnick
Review Date: 2021-03-15
IETF LC End Date: 2021-03-16
IESG Telechat date: Not scheduled for a telechat

Summary:

Looks fine. Some of the MUSTs look weird or superfluous to me and could
probably use a scrub, and a couple are a bit confusing, but none is so bad that
I would raise them as an "issue"; call them "nits/editorial comments".

Major issues:

None

Minor issues:

None

Nits/editorial comments:

Section 1: It's not clear to me what the purpose of the third paragraph in the
intro is. It sounds like it's just describing section 9 of RFC 8226, but it is
not distinguishing it from or comparing it to this document. Is it really
needed?

Section 3:

Instead of a reference to 7.4 of RFC 8555, perhaps a reference to section 7
generally would help, or perhaps a reference later in this section to 7.1.4.
Once I got down to the examples, I had to go look at 7.1.4 to familiarize
myself with the operation to understand what I was looking at.

Total nit, and just a personal pet peeve: It always seems silly to me to use
MUST where the meaning of that word is "MUST do what the protocol we are hereby
defining says to do". So instead of "MUST include", it could simply be
"includes", and "MUST be" could be "is" in the two places it occurs. These
three did not cause any significant confusion, whereas the ones is section 4
and 5.4 did cause some (see below). Either way, you should review all of them
in the document and decide what is truly needed.

Section 4:

Where it says, "a CA MUST use the Authority Token challenge type of "tkauth-01"
with a "tkauth-type" of "atc"", I am left to wonder what other choice the CA
might make such that you have to warn it that it MUST use these. Why is "uses"
not sufficient?

Conversely, when you say that the "token-authority" parameter is "optional"
(did you mean OPTIONAL): Is that really true? Is it that it MUST be used "in
cases where the VoIP telephone network requires the CA to identify the Token
Authority" (in which case it's not OPTIONAL), or is that simply an operational
consideration, and protocol-wise it is truly OPTIONAL? On the other hand, the
MAY and MUST at the end of the paragraph seem more appropriately to be "can"
and "can only". And the MUST in the following paragraph seems like another of
the ones in which you could change "MUST respond" to "responds".

Section 5:

The last paragraph seems superfluous.

Section 5.4:

The MUST NOT in the third bullet actually caused me a bit of confusion: I tried
to read it as a requirement of this document. I think you mean "is not" instead
of "MUST NOT be".

Section 5.5:

   The response to the POST request if successful MUST return a 200 OK
   with a JSON body that contains, at a minimum, the TNAuthList...

I think instead you mean:

   The response to the POST request if successful returns a 200 OK with
   a JSON body that MUST contain, at a minimum, the TNAuthList...

Then you won't need the "...however..." bit at the end of the next sentence.

In the last paragraph, why "SHOULD" and not "MUST"?

---

From: Chris Wendt <[email-masked]>
Date: Fri, 26 Mar 2021 12:08:18 -0400

Thanks Pete for the review i have addressed the nits, including the abundance of MUSTs :) and will release in a next version 08 include other review comments.

-Chris

> On Mar 15, 2021, at 4:06 PM, Pete Resnick via Datatracker <[email-masked]> wrote:
> 
> Reviewer: Pete Resnick