Hello, Yes, I interpreted it as it can provide null value if it wants to. And, since it is listed as one of the parameters to add, I assumed it would, otherwise, put the profile value. In any case, it is not a huge problem from my side to remove it from the mqtt draft. Kind regards, --Cigdem
On Thu, Sep 24, 2020 at 3:05 PM Francesca Palombini < francesca.palomb...@ericsson.com> wrote: > Hi Ludwig, > > > > Thank you for the lightning quick reply! I think that maybe something on > the lines of “The client MUST NOT send a non-empty “ace_profile” > parameter.” in 5.6.1. I think the confusion might have come from the term > “can”, “it can send an empty parameter” does not imply that “it must not > send the parameter if the value is non-empty”. Also Cigdem maybe has a > better proposal on how to clarify, since she interpreted the text that way. > > > > Thanks, > Francesca > > > > *From: *Seitz Ludwig <ludwig.se...@combitech.se> > *Date: *Thursday, 24 September 2020 at 15:39 > *To: *Francesca Palombini <francesca.palomb...@ericsson.com>, Cigdem > Sengul <cigdem.sen...@gmail.com> > *Cc: *"draft-ietf-ace-mqtt-tls-prof...@ietf.org" < > draft-ietf-ace-mqtt-tls-prof...@ietf.org>, Ace Wg <ace@ietf.org> > *Subject: *RE: WGLC review of draft-ietf-ace-mqtt-tls-profile-07 > > > > Hello Francesca, Cigdem, > > I believe I know the reason for the confusion: Earlier versions of the > framework allowed the clients to indicate a preference for a specific > profile by sending in values with the “ace_profile” parameter in the access > token request. > > This option was removed because we came to the conclusion that the AS > would determine the profile to be used based on the registration > information from both client and RS. Instead the only option for the client > is (as Francesca wrote at *** below) to send an empty “ace_profile” > parameter in the access token request in order to query the selected > profile (here the assumption was that this would be hard-coded in the > client in most cases and therefore querying is optional). > > > > I believe the text in the framework is quite clear: > > > > “5.6.1. Client-to-AS Request > > […] > > o The client can send an empty (null value) "ace_profile" parameter > > to indicate that it wants the AS to include the "ace_profile" > > parameter in the response. See Section 5.6.4.3. > > “ > > > > And later: > > > > “5.6.4.3. Profile > > […] Clients that want the AS to provide them with the "ace_profile" > > parameter in the access token response can indicate that by sending a > > ace_profile parameter with a null value (for CBOR-based interactions) > > or an empty string (for JSON based interactions) in the access token > > request.” > > > > What kind of extra clarification would you suggest I add? > > > > /Ludwig > > > > > > > > *From:* Ace <ace-boun...@ietf.org> *On Behalf Of *Francesca Palombini > *Sent:* den 24 september 2020 15:13 > *To:* Cigdem Sengul <cigdem.sen...@gmail.com> > *Cc:* draft-ietf-ace-mqtt-tls-prof...@ietf.org; Ace Wg <ace@ietf.org> > *Subject:* Re: [Ace] WGLC review of draft-ietf-ace-mqtt-tls-profile-07 > > > > Hi Cigdem, > > > > Thanks for the quick reply! Answers inline. > > > > To Ludwig and the WG, which might not otherwise see the comment: Cigdem > and I have a different interpretations of what the framework allows to put > in the “ace_profile” parameter in the request from Client to AS (See (***) > below). I think this might require clarifications in the framework either > way. > > > > Thanks, > > Francesca > > > > *From: *Cigdem Sengul <cigdem.sen...@gmail.com> > *Date: *Sunday, 20 September 2020 at 23:02 > *To: *Francesca Palombini <rancesca.palomb...@ericsson.com> > *Cc: *Ace Wg <ace@ietf.org>, “draft-ietf-ace-mqtt-tls-prof...@ietf.org” < > draft-ietf-ace-mqtt-tls-prof...@ietf.org> > *Subject: *Re: WGLC review of draft-ietf-ace-mqtt-tls-profile-07 > > > > > > > > > > Hello Francesca, > > > > Thank you for your review. My responses are inside; in the cases I have not > > understood a comment, I asked for clarifications. > > Kind regards, > > --Cigdem > > > > > > On Tue, Sep 15, 2020 at 2:38 PM Francesca Palombini < > > rancesca.palomb...@ericsson.com> wrote: > > > > > > > > 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. > > > > > > [CS: Added to https://github.com/ace-wg/mqtt-tls-profile/issues/65, and > > will be fixed in the next revision.] > > > > > > > -- > > > > > > “ 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? > > > > > > [CS: I followed the core document statement, which also says: “It is also > > REQUIRED that the > > communicating endpoints perform mutual authentication.”. > > ] > > > > FP: Right. However I think that the framework’s requirement is ok even if > it is not implementable because the framework is not supposed to be > implemented by itself. So the framework’s requirement is more a requirement > on what the profile must specify. What I was looking for in the profile was > some indication on how to perform the mutual authentication, hence the > suggested rephrasing. It’s not a major change though, so feel free to > disregard. > > > > > -- > > > > > > 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. > > > > > > [CS: This is because I’ve interpreted what I read in 5.6.4 as the > > ace_profile can be used as a parameter in the request. > > > > “5.6.4. Request and Response Parameters: This section provides more > > detail about the new parameters that can be used in access token requests > > and responses” > > 5.6.4.3. Profile > > “ > > So, I did not think that specifying what the profile should be contradicted > > the document. > > ] > > > > (***) > > FP: Your interpretation was surprising to me. I always understood that the > ace_profile parameter can only be included in the request with the null > value, so that the client can indicate to the AS that it wants to receive > the ace_profile back. I went through the relevant sections again, and the > following text in 5.6.1. "Client-to-AS Request" is what I based my > understanding on. > > > > o The client can send an empty (null value) "ace_profile" parameter > > to indicate that it wants the AS to include the "ace_profile" > > parameter in the response. See Section 5.6.4.3. > > > > And in 5.6.4.3. "Profile": > > > > Clients that want the AS to provide them with the "ace_profile" > > parameter in the access token response can indicate that by sending a > > ace_profile parameter with a null value (for CBOR-based interactions) > > or an empty string (for JSON based interactions) in the access token > > request. > > > > There is no text about allowing any other value than the null value in the > ace_profile, or how the AS should react to receiving such a parameter with > non-null value (what if it does not support the profile? That at least > should be documented) so I just assumed that it was not supported. > > > > I'd like to get the opinion of Ludwig and the WG on this, but I think this > requires some clarifications in the framework in both cases: > > * if your interpretation is correct, there should be more text in the > framework about how that works > > * if my interpretation is correct, it should be clarified that in the > request the only acceptable value for the "ace_profile" is null. > > > > > > -- > > > > > > When CBOR is used, the > > > interactions must implement Section 5.6.3 of the ACE framework > > > [I-D.ietf-ace-oauth-authz]. > > > > > > normative MUST? > > > > > > [CS: Added to https://github.com/ace-wg/mqtt-tls-profile/issues/65 > <https://protect2.fireeye.com/v1/url?k=2e78cfdb-70d875b5-2e788f40-861d41abace8-48f0e34a919bfb7b&q=1&e=8f170309-5d08-4515-b712-5c09d1a0daa1&u=https%3A%2F%2Fgithub.com%2Face-wg%2Fmqtt-tls-profile%2Fissues%2F65> > , > > and will be fixed in the next revision.] > > > > -- > > > > > > 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. > > > > > > > > [CS: My response is the same as above. Since the core document capitalised > > REQUIRED, I kept MUST.] > > > > FP: What I did following Ben's review of the OSCORE Profile was to > un-capitalize the normative statements from the framework (i.e. s/MUST/must > when it was a requirement inherited from the framework). But again, your > decision here. > > > > > -- > > > > > > "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) > > > > > > [CS: It is overhead as any token validation done in TLS is overwritten by > > what ever token is provided in ace. > > Will add a sentence around that. > > https://github.com/ace-wg/mqtt-tls-profile/issues/70 > <https://protect2.fireeye.com/v1/url?k=1ae6e7d7-44465db9-1ae6a74c-861d41abace8-cec58ae15cb03910&q=1&e=8f170309-5d08-4515-b712-5c09d1a0daa1&u=https%3A%2F%2Fgithub.com%2Face-wg%2Fmqtt-tls-profile%2Fissues%2F70> > > ] > > > > > > > > > -- > > > > > > 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. > > > > > > [CS: I am not sure I understand the comment. The other methods are > > described, but TLS:Anon-MQTT:ace is recommended. > > You suggest I reverse the order, and start with recommended version?] > > > > FP: No, what I meant is that for example when publishing to authz-info the > client MUST use TLS:Anon-MQTT:None, as described in 2.2.2, so the > "RECOMMENDED that the Client follows TLS:Anon-MQTT:ace" does not apply to > that particular communication. I was just hoping you could add some context > about when it is recommended to use TLS:Anon-MQTT:ace (this is basically > just an editorial/clarification). > > > > > -- > > > > > > 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)? > > > > > [CS: Yes, if it does not want to support session continuation, it always > > starts clean session. > > If necessary was added due to providing potentially support for better QoS, > > but it may be other reasons. So, instead of prescribing a reason, I opted > > for "If necessary", where the need is determined by the Broker provider. > > ] > > > > FP: Ok, thanks for the clarification. Changing "If necessary" to "If the > Broker provider requires it" (or something of the sort) would have helped > me in this case, as that removes the vagueness of "necessary". > > > > -- > > > > > > 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. > > > > > > [CS: QoS level is defined in MQTT terminology in Section 1.3. " QoS level > > The level of assurance for the delivery of an Application > > Message. The QoS level can be 0-2, where "0" indicates "At > > most once delivery", "1" "At least once delivery", and "2" > > "Exactly once delivery"." > > ] > > > > FP: Ah ok. Then I would suggest s/QoS/QoS level. > > > > > -- > > > > > > 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. > > > > > > [CS: Authentication data is defined in 2.2.4 before these subsections as: > > "The Authentication Method is followed by the Authentication Data, > > which has a property identifier 22 (0x16) and is binary data. The > > binary data in MQTT is represented by a two-byte integer length, > > which indicates the number of data bytes, followed by that number of > > bytes." > > ] > > > > FP: Right, what I was wondering is for the "that number of bytes", so the > actual data part of the Authentication Data. For example in 2.2.4.1, you > write: > > > > the Authentication Data MUST contain the two-byte > > integer token length, the token, and the keyed message digest (MAC) > > or the Client signature. > > > > I assume you are concatenating the bytes string of length, token and MAC? > That should be specified, because "contain" is not clear enough. > > > > > -- > > > > > > 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. > > > > > > > [CS: Noted. https://github.com/ace-wg/mqtt-tls-profile/issues/71 > <https://protect2.fireeye.com/v1/url?k=8770862d-d9d03c43-8770c6b6-861d41abace8-9f35c2c5bf438c44&q=1&e=8f170309-5d08-4515-b712-5c09d1a0daa1&u=https%3A%2F%2Fgithub.com%2Face-wg%2Fmqtt-tls-profile%2Fissues%2F71> > ] > > > > > > > > > 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. > > > > > [CS: Not sure I follow the comment. The draft says: " If the Client does > > not provide a valid token or omits the > > Authentication Data field], > > so includes the option of invalid token. I can add to that "does not > > provide a valid token or authentication data" to make it more clear. > > ] > > > > FP: This comment comes from my interpretation of "valid token", which to > me comes from the framework, where the token is validated to make sure that > it is integrity protected, comes from the AS and covers the resource > requested (that also includes not expired). What I was asking you to add > is the case where the token cannot be validated, because it is malformed so > it cannot be parsed or validated, and same for the Authentication data. > > > > > > > > -- > > > > > > Figure 5: AIF-MQTT data model > > > > > > Need to reference CDDL. > > > > > [CS: Added to https://github.com/ace-wg/mqtt-tls-profile/issues/65 > <https://protect2.fireeye.com/v1/url?k=fab2a61f-a4121c71-fab2e684-861d41abace8-f87bb44ce2228a89&q=1&e=8f170309-5d08-4515-b712-5c09d1a0daa1&u=https%3A%2F%2Fgithub.com%2Face-wg%2Fmqtt-tls-profile%2Fissues%2F65>, > and > > will be fixed in the next revision.] > > > > > > > > > > -- > > > > > > Section 3: > > > What does it mean to have an empty array for scope? (as that is allowed) > > > > > > [CS: Do you mean I should add that as an explanation? It means no > > permissions?] > > > > FP: I was just noting that the empty array is a possible option according > to the draft's CDDL (i.e. a Broker would not reject with error such an > empty array), and I was wondering how that would be interpreted. If it's no > permission, it would be good to clarify. Otherwise you could add a > requirement to the AS that it MUST NOT be the empty array. > > > > > -- > > > > > > Scope appears in section 2.1 for the first time, but its encoding is only > > > defined in section 3. > > > > > > > [CS: Do you suggest doing a forward reference to section 3 from section > > 2.1?] > > > > FP: Yes. > > > > > > > > -- > > > > > > 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. > > > > > > > [CS: This was explained in the 2.2.3 - the Will Flag is set in CONNECT > > message. The Will Topic filter would be added like any other permission to > > the scope. Will Topic is set in the CONNECT message. When the time comes to > > publish the will, the RS checks whether the will topic is covered in the > > scope permissions. Will is like any other PUBLISH message, but I will add a > > few more clarifying sentences. > > https://github.com/ace-wg/mqtt-tls-profile/issues/72 > <https://protect2.fireeye.com/v1/url?k=8bb8563f-d518ec51-8bb816a4-861d41abace8-1c5eaf4c777c547a&q=1&e=8f170309-5d08-4515-b712-5c09d1a0daa1&u=https%3A%2F%2Fgithub.com%2Face-wg%2Fmqtt-tls-profile%2Fissues%2F72> > > ] > > > > FP: Thanks! > > > > > > > > -- > > > > > > 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. > > > > > > > > > > [CS: Yes, that would update the access rights. ] > > > > FP: Will you add a sentence about that in the spec? > > > > > -- > > > > > > 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 > needs > > > some clarification. > > > > > > > [CS: When the resource owner changes the access policy for the client at > > the AS, but the client has still a valid token with old access rights? > > Revocation would assume there is an AS-> RS channel for pushing > > notifications. There isn't. The only option is that RS does regular token > > introspections, and not rely on cached data that much. > > > > That sentence was in the context of cached tokens/token introspection > > responses. The next sentence was: "If the RS caches the token introspection > > responses, then the RS should use a reasonable cache timeout to introspect > > tokens regularly." > > ] > > > > FP: Ah of course. My confusion came from the "gets revoked" as for some > reason I assumed it meant that "it got revoked at the RS", so the RS knew > in some way (maybe with introspection). Nevermind, thanks for the answer. > > > > > > > > > > > > > > ============= > > > > > > Nits: > > > > > > framework to enable > > > authorization in an Message Queuing Telemetry Transport (MQTT) > > > > > > s/an/a > > > > > > -- > > > > > > The token may be a reference or JSON Web Token > > > (JWT) > > > > > > add a reference to rfc7519 > > > You could also add a informative reference to 8392 for the CWT. > > > > > > -- > > > > > > Missing references to CoAP (informative) and HTTPS (or rather references > > > to HTTP and TLS). > > > > > > [CS: Added to https://github.com/ace-wg/mqtt-tls-profile/issues/65 > <https://protect2.fireeye.com/v1/url?k=f09648cd-ae36f2a3-f0960856-861d41abace8-d1d59a87976ae56d&q=1&e=8f170309-5d08-4515-b712-5c09d1a0daa1&u=https%3A%2F%2Fgithub.com%2Face-wg%2Fmqtt-tls-profile%2Fissues%2F65>, > and > > will be fixed in the next revision.] > > > > > -- > > > > > > Topic Filters may include wildcards > > > > > > This might be me, but I would have liked some explanation of what > > > wildcards are in MQTT. > > > > > > [CS: Wildcards are explained in Section 3 - " The multi-level wildcard, > > '#', matches any > > number of levels within a topic, and the single-level wildcard, '+', > > matches one topic level." > > Do you mean add it to its definition? > > ] > > > > FP: These are definitions of multi-level and single-level wildcard, I was > talking about wildcard in general. It could also be that I am not a native > English speaking person, feel free to disregard if you think that is clear. > > > > > -- > > > > > > AUTH Properties > > > include Authentication Method and Authentication Data. > > > > > > I assume "Properties" is specific term from MQTT terminology, but I don't > > > see it defined in this section, so maybe either generalize > > > ("field/parameter"? or I see you also use "information" for the Will > > > message) or explain it. > > > > > [CS: Property will mean something specific to MQTT developers. I will add > > it to MQTT Terminology in section 1.3 "A Property consists of an Identifier > > which defines its usage and data type, followed by a value. The Identifier > > is encoded as a Variable Byte Integer. "] > > > > FP: Thanks! Looks good. > > > > > > > > -- > > > > > > If the Client is resource-constrained, a Client Authorisation Server > > > may carry out the token request on behalf of the Client, > > > > > > I might have missed this, where is a "Client Authorization Server" > > > defined? I am fine with the terminology used in the figure though, > "Client > > > - Authorization Server interface". > > > > > > [CS: Client Authorisation Server was defined in the old actors draft - > > Daniel made a similar comment. Not sure what terminology the group settled > > on after the draft expired.] > > > > FP: I see. I suggest using "Client-Authorisation Server interface", which > to me is more intuitive. > > > > > -- > > > > > > If the > > > Client authentication uses TLS:Known(RPK/PSK), then the Broker is > > > authenticated using the respective method. > > > > > > s/respective/same? (just to understand better, please disregard if you > > > think it is clear as is) > > > > > > > [CS: Respective to mean RPK or PSK, respectively] > > > > > > > > -- > > > > > > "authz-info" is a public topic, this response is not expected to > > > cause confusion. > > > > > > Maybe s/public/not protected ? > > > > > [CS: I prefer public, but can change if the group prefers not protected.] > > > > FP: Again this comment stems from my familiarity with ACE terminology. > Public might be clearer for MQTT people. Feel free to disregard, or use > both. > > > > > > > > -- > > > > > > Similarly, the server MUST NOT process > > > any packets other than DISCONNECT or an AUTH that is sent in response > > > to its AUTH before it has sent a CONNACK. > > > > > > add "from that client" > > > > > > [CS: Added to https://github.com/ace-wg/mqtt-tls-profile/issues/65 > <https://protect2.fireeye.com/v1/url?k=c851e393-96f159fd-c851a308-861d41abace8-d3f4b044ec71786e&q=1&e=8f170309-5d08-4515-b712-5c09d1a0daa1&u=https%3A%2F%2Fgithub.com%2Face-wg%2Fmqtt-tls-profile%2Fissues%2F65>, > and > > will be fixed in the next revision.] > > > > > > > -- > > > > > > If the Will Flag is set to 1 and the > > > Broker accepts the connection request, the Broker must store the Will > > > message > > > > > > I understand that these are not requirements set by this document but > > > rather inherited by the MQTT spec, so I would suggest remove the use of > > > must etc (here and in other parts of the document) to avoid any question > > > about if this should be normative or not later on. > > > > > > > [CS: Daniel had a similar comment - I was wondering whether I should > > explain lower case must is referring to MQTT spec, and uppercase for the > > spec.] > > > > FP: Or you could just point to the section of the spec where these are > inherited from. This would have been enough for me. Otherwise it's also > fine with just explaining future reviewers why they are lower case. > > > > > > > > -- > > > > > > which have have not been completely acknowledged or pending > > > > > > remove one of the "have" > > > > > > > -- > > > > > > In case of an invalid > > > PoP token, the CONNACK reason code is '0x87 (Not Authorized)'. > > > > > > This is in the section about success, while there is a separate section > > > for unauthorized request, where imo this would fit better. > > > > > > [CS: Added to https://github.com/ace-wg/mqtt-tls-profile/issues/65 > <https://protect2.fireeye.com/v1/url?k=902b6298-ce8bd8f6-902b2203-861d41abace8-20b0e6c3c778108f&q=1&e=8f170309-5d08-4515-b712-5c09d1a0daa1&u=https%3A%2F%2Fgithub.com%2Face-wg%2Fmqtt-tls-profile%2Fissues%2F65>, > and > > will be fixed in the next revision.] > > > > > > > -- > > > > > > To transport the token to the Broker inside the CONNECT message, the > > > Client uses the username and password fields. > > > > > > This is just a question rather than a nit: is there no way to register > new > > > names for these fields in MQTT? > > > > > [CS: Not to my knowledge] >
_______________________________________________ Ace mailing list Ace@ietf.org https://www.ietf.org/mailman/listinfo/ace