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 +46(0)70-349 92 51
_______________________________________________
Ace mailing list
Ace@ietf.org
https://www.ietf.org/mailman/listinfo/ace
