Re: [Ace] WGLC review of draft-ietf-ace-mqtt-tls-profile-07

Cigdem Sengul <cigdem.sengul@gmail.com> Thu, 24 September 2020 14:12 UTC

Return-Path: <cigdem.sengul@gmail.com>
X-Original-To: ace@ietfa.amsl.com
Delivered-To: ace@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id DFCE03A09DA; Thu, 24 Sep 2020 07:12:58 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.997
X-Spam-Level:
X-Spam-Status: No, score=-1.997 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, HTTPS_HTTP_MISMATCH=0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id as7XtvtUmEdW; Thu, 24 Sep 2020 07:12:53 -0700 (PDT)
Received: from mail-vs1-xe2b.google.com (mail-vs1-xe2b.google.com [IPv6:2607:f8b0:4864:20::e2b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C75523A0D41; Thu, 24 Sep 2020 07:12:46 -0700 (PDT)
Received: by mail-vs1-xe2b.google.com with SMTP id j185so2205831vsc.3; Thu, 24 Sep 2020 07:12:46 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=5DD0nssV2tUH4S75hoQm6kEgVxzZkEMkMUxBoLrYLgc=; b=IkxB2dJlBmm8QRr1mCG1mIHNg1yP69fwar9NpKfeUTeVkvfMsVYHrWxXupIRD9V4Bu weTJLOK5paX9TZBcvLww7LTNgO0C+edjofA66hVk0bhlaaq+tnmucfpYuRUj36gBNA6V kaw6MWtbZTqzkrvXhwVnKEmv1f37eeNx4zTyF5tVPa1Qq45UF9NlQOhoGERU9AGwoluJ 5sPNPAV6M9QJWxDH/ZOHAm+T71XVtlHZwvq34qt2vagweqXojJ+DwbTAsUXgdon3tzdt hDKhMHBzgZRdy7DXm6UkJqlF+11TnmQzCKzIq23xqmkUqSe1jcTPITlNcG+AIVkGlykm 3XOg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=5DD0nssV2tUH4S75hoQm6kEgVxzZkEMkMUxBoLrYLgc=; b=qRYggMo449gY48AiCLWyC71TiP11mBlAm64IBXvn0sKDI/5Qv3smZigppFPyvYY2lb jAfTeC8FfwTVav80VP9svYVy2csPdEjuf9eSIoUZ53E9odv22Rr0ZX4u2uo1bZoY6/sy C/MnN9DlZt2xKFt5FjX891zFcYzJo4byFJouwp/8tA3v78bv/tlK5MEotU0tVpQ9MSVc 1VBCE2ANXeYR6b3fW8zwN6SwGNsH+qrgkteaVwIoF0OhBQijoaNDhlw1qPJZg+vqFq+Z JNK4dzFEJTYw2TvX/M4uICbpVhg3sgRXQ4BLGIIUe5M0QX47R7azApZZy6MQsW7pkF9r /lEQ==
X-Gm-Message-State: AOAM531DOaZfKKlHDZ7trgi5C8BN/Fa7TWpC84bOn3Yo6ZW5G2m4l08t qq+CQOo6rUB9KbcAeINxqWVWypEAb3NnP3VJ40Q=
X-Google-Smtp-Source: ABdhPJy3wnx4i3q7zuP6ukUjyyNTC0Juom3fvo2rgssjNaQSvo1fg16Lo8b+xjciaQjyZzB6rhnrV6GfeQD5xUsXchI=
X-Received: by 2002:a67:fb90:: with SMTP id n16mr4350306vsr.22.1600956764719; Thu, 24 Sep 2020 07:12:44 -0700 (PDT)
MIME-Version: 1.0
References: <D68C212C-FD3A-4900-86DE-B138DD0CFCD0@ericsson.com> <CAA7SwCPQjY7fwJrJyfK=kmcmPP3i-6mevwP+64yjc4S_8ezoww@mail.gmail.com> <06BB230C-FEDF-46F2-8B54-525BD9675A27@ericsson.com> <a5a83e6943cb4f2f8ee8dde4cd987b7e@combitech.se> <80061122-7B30-4117-8D2E-AC58BDC3B5AF@ericsson.com>
In-Reply-To: <80061122-7B30-4117-8D2E-AC58BDC3B5AF@ericsson.com>
From: Cigdem Sengul <cigdem.sengul@gmail.com>
Date: Thu, 24 Sep 2020 15:12:33 +0100
Message-ID: <CAA7SwCPQR3pk4NSh8XwaF+GAJ2U9faNp8NHUBdBW=+JhM3UGoQ@mail.gmail.com>
To: Francesca Palombini <francesca.palombini@ericsson.com>
Cc: Seitz Ludwig <ludwig.seitz@combitech.se>, "draft-ietf-ace-mqtt-tls-profile@ietf.org" <draft-ietf-ace-mqtt-tls-profile@ietf.org>, Ace Wg <ace@ietf.org>
Content-Type: multipart/alternative; boundary="00000000000043103d05b00fcb5b"
Archived-At: <https://mailarchive.ietf.org/arch/msg/ace/fFI4x25Y7fScXn_DIrGoQC4hppI>
Subject: Re: [Ace] WGLC review of draft-ietf-ace-mqtt-tls-profile-07
X-BeenThere: ace@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Authentication and Authorization for Constrained Environments \(ace\)" <ace.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ace>, <mailto:ace-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ace/>
List-Post: <mailto:ace@ietf.org>
List-Help: <mailto:ace-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ace>, <mailto:ace-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 24 Sep 2020 14:12:59 -0000

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.palombini@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.seitz@combitech.se>
> *Date: *Thursday, 24 September 2020 at 15:39
> *To: *Francesca Palombini <francesca.palombini@ericsson.com>, Cigdem
> Sengul <cigdem.sengul@gmail.com>
> *Cc: *"draft-ietf-ace-mqtt-tls-profile@ietf.org" <
> draft-ietf-ace-mqtt-tls-profile@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-bounces@ietf.org> *On Behalf Of *Francesca Palombini
> *Sent:* den 24 september 2020 15:13
> *To:* Cigdem Sengul <cigdem.sengul@gmail.com>
> *Cc:* draft-ietf-ace-mqtt-tls-profile@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.sengul@gmail.com>
> *Date: *Sunday, 20 September 2020 at 23:02
> *To: *Francesca Palombini <rancesca.palombini@ericsson.com>
> *Cc: *Ace Wg <ace@ietf.org>, “draft-ietf-ace-mqtt-tls-profile@ietf.org” <
> draft-ietf-ace-mqtt-tls-profile@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.palombini@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]
>