Re: [Last-Call] [Gen-art] Genart last call review of draft-ietf-ace-mqtt-tls-profile-15

[Lars Eggert <[e-mail]>]

Theresa, thank you for your review. I have entered a No Objection ballot for this document.

Lars


> On 2022-3-2, at 22:03, Theresa Enghardt via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Theresa Enghardt
> 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-ace-mqtt-tls-profile-15
> Reviewer: Theresa Enghardt
> Review Date: 2022-03-02
> IETF LC End Date: 2022-03-03
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary: This document is fairly clear and concise, and basically ready for
> publication. I found a few minor points and nits that should be addressed to
> further enhance document clarity.
> 
> Major issues: None.
> 
> Minor issues:
> 
> Section 1:
> 
> "The token MAY be a
>   reference or JSON Web Token (JWT) [RFC7519]."
> What is a reference in this context?
> 
> Section 1.3:
> 
> "Will
>                   If the network connection is not closed normally, […]"
> I suggest to make this a bit more specific:
> Does "the network connection" refer to a TCP connection, or a TLS session? Or
> does it refer to MQTT's notion of "connection"? Does "not closed normally" mean
> anything other than a FIN-ACK exchange to close a TCP connection? Or does it
> depend on the used transport protocol (however, in this document, it should all
> refer to TLS over TCP iiuc?) If the notion of a "network connection is not
> closed normally" is a well-defined concept in this context, please provide a
> reference if possible.
> 
> Section 2
> 
> "Whatever protocol is
>   used for C-AS and Broker-AS communications must provide mutual
>   authentication, confidentiality protection, and integrity protection."
> Is this a MUST?
> 
> Section 2.1
> 
> "The PoP token includes a 'cnf' parameter with a
>   symmetric or asymmetric PoP key. "
> The 'cnf' (and 'rs_cnf' in Section 2.2.1) parameter is mentioned here and in
> some other places, but it is not obvious what it means and why it is
> special/important. I suggest to provide a brief explanation or reference.
> 
> 2.2.2
> "If the QoS level is equal to 0, and the token is invalid or the
>   claims cannot be obtained in the case of an introspected token, the
>   Broker MUST send a DISCONNECT packet with the reason code '0x87 (Not
>   authorized)'."
> Does this paragraph apply to all packets or is it specific to the PUBLISH
> packet (like the next paragraph)? I suggest to make this explicit.
> 
> Section 2.2.4.1
> 
> "Given that clients provide a token at each connection,"
> Does "at each connection" mean in each CONNECT packet, or something else?
> Please clarify.
> 
> Nits/editorial comments:
> 
> Section 1:
> 
> "In this profile, Clients and Servers
>   (Brokers) use MQTT to exchange Application Messages.  The protocol
>   relies on TLS for communication security between entities."
> Section 1.3 defines MQTT over TLS as MQTTS, and if I understand correctly, this
> document requires MQTT between Clients and Servers over TLS, effectively,
> making the document about MQTTS, and it does say "MQTTS" in some places. Short
> of substituting all or nearly all "MQTT" with "MQTTS", is it worth mentioning
> "MQTTS" once more here in the introduction?
> 
> "MQTT v3.1.1 clients"
> Here, "clients" is lower case, while it is upper case in most other places in
> the doc. Should it be upper case here as well? There are other occurences of
> lower case "client" in Section 2.2.3.1, 2.2.3.2, 2.2.4.1, 2.2.5 - should these
> all be upper case?
> 
> "This
>   document does not protect the payload of the PUBLISH packet from the
>   Broker."
> Maybe this reads better as "The mechanisms specified in this document do not
> protect […]"?
> 
> Section 2.1:
> "The document follows the procedures
>   defined in Section 3.2.1 of the DTLS profile
>   [I-D.ietf-ace-dtls-authorize] for RPK, and in Section 3.3.1 of the
>   same document for PSK."
> This is the first occurrence of RPK and PSK, yet, these acronyms are only
> expanded later in Section 2.2.1 and 2.2.3. I suggest move the expansion and
> maybe a brief explanation to Section 2.1, and then perhaps the acronyms do not
> need to be expanded again in Sections 2.2.1 and 2.2.3.
> 
> Section 8:
> "Broker does not cache
>   any invalid tokens."
> The broker?
> 
> 
> _______________________________________________
> Gen-art mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/gen-art