Re: [Ace] Review of draft-sengul-ace-mqtt-tls-profile-02

[Cigdem Sengul <[e-mail]>]

Hello Ludwig,

Thank you for reviewing our draft. We will start working on addressing your
comments asap.

Thanks,
--Cigdem



On Tue, May 15, 2018 at 10:04 AM, Ludwig Seitz <[e-mail]> wrote:

> Hello ACE,
>
> I've reviewed draft-sengul-ace-mqtt-tls-profile-02. I think this is a
> very relevant draft, due to the amount of IoT devices that use MQTT. I
> would encourage the WG to pick this up as the third profile of
> draft-ietf-ace-oauth-authz
>
> Detailed comments below.
>
>
> /Ludwig
>
>
>
> 1. Introduction
>
> " Section 4 of the ACE framework [I-D.ietf-ace-oauth-authz]"
>
> If you make that "Section 4 of [I-D.ietf-ace-oauth-authz]" the IETF
> html-izer will not turn this into a link to section 4 of your draft (I
> think).
> See: https://xml2rfc.tools.ietf.org/xml2rfcFAQ.html#anchor18
>
> 1.1. Requirements Language
>
> There is a new recommendation how this section should be worded:
>
>  "The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>    "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
>    "OPTIONAL" in this document are to be interpreted as described in BCP
>    14 [RFC2119] [RFC8174] when, and only when, they appear in all
>    capitals, as shown here."
>
> 1.2.  ACE-Related Terminology
>
> "The term "Resource" is used to refer to an MQTT "topic", which is
>    defined in Section 1.2."
>
> This references back to the same section, perhaps this was meant to
> reference to some other part of the draft?
>
> 2.1.  Authorizing Connection Establishment
>
> Figure 1 could clarify that steps E and F are optional.
>
> 2.1.1.  Client Authorization Server (CAS) and Authorization Server (AS)
>         Interaction
>
> "and the client is authorized to obtain a token for the indicated
>    audience (e.g., topics) and scopes (e.g., publish/subscribe
>    permissions)"
>
> Note that the audience claim is typically used to identify the RS (so in
> this case the MQTT broker), while the scope is intended to identify both
> the resource (=topic) and the actions (=publish, subscribe).
> See this for how OAuth scopes are typically used:
> https://www.brandur.org/oauth-scope
>
>
> "This includes a token, which is assumed to be PoP by default."
>
> The abbreviation PoP is not defined at this point, I would write it out as
> in "... assumed to be a Proof-of-Possession (PoP) token ..."
>
> 2.1.2.  Client connection request to the broker
>
> " For the basic operation described in this section,
>    the Username field MUST be set to the token."
>
> I think this refers to the access token right? I'd suggest to be more
> specific, since there is room for confusion (CoAP has also the concept of a
> token, and then OAuth has different types of tokens, the access token being
> one of them).
>
> "The Password field MUST be set to the keyed message digest (MAC) or
> signature."
>
> Please make it explicit already here that this is the Proof-of-possession
> mechanism for the key associated to the token.
>
> "(The Username field is a UTF-8 encoded string,"
>
> You need to specify how the (possibly binary) access token is supposed to
> be converted into an UTF-8 string. Base64 encoding would be obvious, but it
> needs to be explicitly stated.
>
> General comment:
>
> An example of how the CONNECT message could look like would be good.
>
>
> 2.1.3.  Token validation
>
> "The broker MAY cache the introspection result because it will need to
>    decide whether to accept subsequent PUBLISH and SUBSCRIBE messages"
>
> This could warrant a section in the security considerations about the
> trade-off of caching introspection results (freshness vs. connectivity
> requirements).
>
> "If the introspection result is not cached, then the RS needs to
> introspect the saved token for each request."
>
> This is only true if the token is not self-contained. I would rephrase
> this statement
>
> "Note: Scope strings MAY follow ..."
>
> Note that a scope is a "space-delimited list strings" (OAuth 2.0 3.3.)
> so you could include several topics in one scope as in e.g. "connect
> publish_topic1 publish_topic2 subsrcibe_topic3"
>
> The use of 'aud' as described in this paragraph is definitely not the
> intended use, and I would not recommend this type of use. See
> https://tools.ietf.org/html/rfc7519#section-4.1.3 for the definition of
> aud.
>
>
> 2.4.  Token expiration
>
> "The token validation is done either by checking the 'exp' claim of a
>    CWT/JWT or via performing an introspection request with the
>    Authorization server as described in Section 8.2 of the ACE framework
>    [I-D.ietf-ace-oauth-authz]."
>
> The word "token validation" here could be misconstrued to ignoring crypto
> wrappers, token issuers ect. While it is clear from the context (to me)
> that you only refer to checking the expiration of a token, the full
> validation of a token would include the following steps:
>
> 1. Check that the issuer of the token (= AS) is acceptable
> 2. Check the cryptographic wrapper of the token (may be MAC or signature or
> AEAD encryption)
> 3. Check that the token contains all expected claims
> 4. Check that the "exp" and "nbf" claims are fulfilled if present
> 5. Check that the "aud" claim applies to the recipient if present
>
>
> 3.1.  Token Transport via Authentication Exchange (AUTH)
>
> "The first option is that Authentication data contains both the token
>    and the keyed message digest (MAC) or signature as described in
>    Section 2.1.2."
>
> You need to specify how these two items are encoded. I would suggest a
> CBOR array since this property expects binary data. An example would also
> be very helpful here.
>
> "... RS responds with a CONNACK reason code '0x87 (Not Authorized)' and
>    includes a User Property set to the address of the AS."
>
> You need to specify the format of this user property, in order to mirror
> the framework's section 5.1.2. I guess you could use either a JSON map
> or binary data containing a CBOR map for this.
>
>
> 4.  IANA Considerations
>
> "   This memo includes no request to IANA."
>
> This should instead register the new profile identifier "mqtt_tls" in the
> soon-to-be-created IANA registry for ACE profile identifiers. See the DTLS
> profile for example:
>
> https://tools.ietf.org/html/draft-ietf-ace-dtls-authorize-03#section-7
>
>
> 5. Security Considerations
>
> You should add some text here about the security implications of the
> limitations of MQTT v3.1 (client disconnect only).
> I could imagine that you could perform some attacks on a client that
> hasn't realized it was disconnected for example.
>
>
> --
> Ludwig Seitz, PhD
> Security Lab, RISE SICS
> Phone [phone]
>
> _______________________________________________
> Ace mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/ace
>