Closed ciseng closed 1 year ago
[Marco Review] While it is mentioned later in Section 1, I think that the abstract should already mention that this profile covers both CoAP and MQTT.
[Cigdem] - added The profile covers pub-sub scenarios using either the Constrained Application Protocol (CoAP) {{I-D.ietf-core-coap-pubsub}} or the Message Queue Telemetry Transport (MQTT) {{MQTT-OASIS-Standard-v5}} protocol.
[Marco Review] "This document defines a way to authorize pub-sub clients using the ACE framework ...".
Please, clarify what they are authorized to do. Conceptually, it
should be about joining a security group that uses certain key material and is associated to one or more topics (application groups). Practically, this should mean being authorized to getting access to the broker and to obtaining the key material for protecting the topic content exchanged through the broker.
"... for protecting the communication between them".
I would emphasize that it is about protecting the content, exhanged by communicating through the broker (see also the related comment above for the Abstract).
[Cigdem: Reworded as This document defines a way to authorize pub-sub clients using the ACE framework {{I-D.ietf-ace-oauth-authz}} to obtain the keys for protecting the content of their pub-sub messages when communicating through the broker.]
[Marco Review] I suggest to expand as follows: Note that both publishers and subscribers use the same pub-sub communication protocol and the same transport profile of ACE in their interaction with the broker. The same profile of ACE is also used in the interaction with the KDC
[Cigdem] Yes, except the KDC bit. For the KDC regardless of MQTT or CoAP, the CoAP profile is used as it is what is supported by the KDC. (KDC doesn't speak MQTT like the MQTT broker, which is the resource server)
Added: Note that both publishers and subscribers use the same pub-sub communication protocol and the same transport profile of ACE in their interaction with the broker. However, all clients need to use CoAP when communicating to the KDC.
[Marco Review] Caption of Figure 1: s/Authorization Servers/Authorization Server
[Cigdem] Done. Added Key Distribution Center instead.
[Marco Review]
"... so that RS can authorize the Clients ..."
Well, the AS is the one authorizing the Clients. I think you mean "... so that RS can assert the Clients to be authorized ..."
[Cigdem] - changed to: so that RS can verify that the Clients are authorized
I suggest to move the following points out of the last paragraph, and instead have them after "... to as Client in short."
[Cigdem] - Added after the suggested location: The Broker acts as the ACE RS, and also corresponds to the Dispatcher in {{I-D.ietf-ace-key-groupcomm}}).
[Marco -Review]
[Marco Review] Consistently with the first paragraph, the section title can be "Authorising to the KDC and the Broker".
[Cigdem] Done
[Marco Review]
[Marco Review] Section 3.2
[Cigdem - Done]
[Marco Review]
[Section 4.1]
[CS: Will revise those parts more carefully]
[CS fixed to - format of public keys]
"The KDC verifies that the Client is authorized to access the topic with the requested role."
Well, the KDC is in a trust relation with the AS issuing the posted Token (and only following a successful check of a Token request against stored access policies). So, at this stage on the KDC, isn't this just about verifying that the Token is valid?
[CS: Yes, the language used is weird, what is meant is the KDC verifies the token]
[CS: Changed to: The KDC verifies the token to check of the Client is authorized to access the topic ]
[Marco Review] [Section 4.2]
[Cigdem: Done]
[Marco Review] [Section 4.2]
[Cigdem: Fixed]
[Marco Review] [Section 4.2]
[Cigdem - Fixed to:] 'scope' parameter set to the specific group that the Client is attempting to join, i.e., the group name, and the roles it wishes to have in the group. This value corresponds to one scope entry, as defined in {{auth-request}}.
[Marco Review]
"The Subscriber MUST have access to the public keys of all the Publishers; this MAY be achieved ... using "pub" as the 'role_filter' ..."
Why do you need the filtering feature here? It would still work, but based on the previous sentence in the same paragraph, the KDC has available to provide only public keys of publishers anyway. So, asking for all the public keys in the group with 'get_pub_keys' encoding the CBOR simple value Null will return only the public keys of publishers by construction. [CS: That's true, will revise]
[CS: fixed]
[CS: Done]
[CS: It seems it is in 4.3.1 now]
[CS: Fixed]
[CS: Fixed to AS]
[CS: yes, changed to publishers]
[CS: Done]
[CS: Done]
[Section 5.1]
[CS: Done]
"The protected Headers ... MAY contain the kid parameter, ..."
It is optional for the KDC to provide one in the Joining Response, but I guess that if one is provided, then it MUST be included in the protected header.
[Revised as: MUST contain the kid parameter if it was provided in the Joining Response
"For implementation simplicity, it is RECOMMENDED that the ACE transport profile used and this specification use the same format of "scope".
Is this actually meaningful and enforceable?
The format of scope is related to the application that the Client
and the RS want to run. In fact, these application profiles for group communication are defining formats to use for scope, in Tokens used to access resources related to such applications.
In other words, I think the format of scope is orthogonal to the
used transport profile, that basically does not care (and should not). I also can't find anything suggesting otherwise in the ACE framework [7] or in the DTLS and OSCORE profiles.
Unless I'm missing something, it's probably just fine to remove the
sentence.
[7]
https://datatracker.ietf.org/doc/html/draft-ietf-ace-oauth-authz-45#appendix-C
[CS: I think this was triggered by MQTT-TLS Profile providing a scope format to be used in access tokens, and it would be useful the scope format is used towards KDC. But, it may not be necessary at the end. Removed]
[Marco Review] Section 7 "... by the same entity having control of the topic, i.e. KDC."
Perhaps here you mean "... by the same entity having control of the
key material for that topic, i.e. KDC."
[CS: Yes]
Section 8.2]
[CS: Yes]
== Nits ==
[Section 3.2]
s/data model is described/data model described
[Section 4.2]
s/singature/signature
[Section 6.2] s/To be able join/To be able to join
Appendix A]
s/Specity/Specify
s/tranport/transport
[Marco Review]
Proposed expansion of the last sentence:
"... it describes the use of application layer security to protect the content that pub-sub clients exchange by communicating through the broker."
[Cigdem]
the content of the pub-sub client message exchange through the broker."