Hi,
Thank you for this work! Here is my review of the document.
Thanks,
Francesca
The response includes the
parameters described in Section 5.6.2 of the ACE framework
[I-D.ietf-ace-oauth-authz].
The fact that the profile parameter with value "mqtt_tls" is included in this
response must be specified here.
--
" The Client and the AS MUST
perform mutual authentication."
I am not sure this is a testable statement. What about specifying that they
MUST use a protocol that provides mutual authentication instead?
--
The Client requests an access token
from the AS as described in Section 5.6.1 of the ACE framework
[I-D.ietf-ace-oauth-authz], however, it MUST set the profile
parameter to 'mqtt_tls'.
Why adding this here? This is in practice implementing a negotiation of
profile. If this is necessary in this MQTT profile I am wondering why it is
defined here and not in the framework. In general, I dislike the profile to
contradict what defined in the framework.
--
When CBOR is used, the
interactions must implement Section 5.6.3 of the ACE framework
[I-D.ietf-ace-oauth-authz].
normative MUST?
--
The Client and the Broker MUST perform mutual authentication.
Same as before, is this a testable statement? Maybe this sentence does not need
to use normative language, but rather descriptive. The details of what Client
and Broker MUST do is in the following sentences.
--
"TLS:Known(RPK/PSK)-MQTT:ace": This option SHOULD NOT be chosen.
Any reason why this is a SHOULD NOT? Can we add motivation of when this would
be allowed (although not recommended)
--
It is RECOMMENDED that the Client follows TLS:Anon-MQTT:ace.
I think it would be helpful to say when this option is recommended vs the
others. In the next section, it is described in more details that the
communication with authz-info is done with "TLS:Anon-MQTT:None" and then after
that TLS:Known(RPK/PSK)-MQTT:none. This seems to not agree with the
recommendation, which is why I think the recommendation needs more context.
--
In this profile, the Broker SHOULD always start with a
clean session regardless of how these parameters are set.
Does this mean that when the Broker recognize that this spec is used, it SHOULD
disregard the parameters value and start a clean session? What are the cases
where this is not done ("If necessary" in the next paragraph)?
--
includes state on client subscriptions, QoS 1 and QoS 2 messages
At this point I don't know what QoS 1 and QoS messages are, they have not been
introduced. Only QoS levels have. Maybe a reference or a pointer would help.
--
RE Authentication Data (Sections 2.2.4.1, 2.2.4.2 etc): I haven't read MQTT in
details, but I don't really see any encoding of the Authentication Data field
in this spec, only that it contains one or more parameters. I would assume that
is should be in scope of this doc, but if it isn't, that should be clarified.
--
2.2.6.1. Unauthorised Request: Authorisation Server Discovery
If the Client does not provide a valid token or omits the
Authentication Data field, the Broker triggers AS discovery.
Following discussion about AS discovery in this ML: I think this should change
to being possible, not mandatory. That means changing: s/the Broker triggers AS
discovery/the Broker MAY trigger AS discovery. I still think it is useful to
have this section, I am just suggesting it becomes optional.
Also note that not providing a token or omittion the Authentication Data field
are not the possible error messages. A malformed token or Authenticated Data
are also possible.
--
Figure 5: AIF-MQTT data model
Need to reference CDDL.
--
Section 3:
What does it mean to have an empty array for scope? (as that is allowed)
--
Scope appears in section 2.1 for the first time, but its encoding is only
defined in section 3.
--
If the Will Flag is set, then the Broker MUST check that the
token allows the publication of the Will message (i.e. the Will Topic
filter is found in the scope).
This sentence is not super clear to me: "if the Will Flag is set" in which
message? How is the Will Topic filter added to scope? This comes from my
ignorance of MQTT, so feel free to skip it, but I think an example would have
helped me here.
--
Section 4 talks about reauthentication. What described for reauthentication
should work also for update of access rights, is that correct? Does that add
any considerations? Anyway I think update of access rights should be discussed
in this document.
--
If a client's permissions get revoked but the access
token has not expired, the RS may still grant publish/subscribe to
revoked topics.
What does it mean in practice that client's permissions get revoked? (also nit
s/permissions get/permission gets) Do you mean that there is no way to do
access right revocation? If that's the case, I did not understand "the RS may
still grant ..." as being a negative consequence, so maybe it
