Subject: [Ace] FW: Review draft-ietf-ace-mqtt-tls-profile-05

From: Jim Schaad <[email-masked]>
Date: Mon, 8 Jun 2020 15:49:23 -0700

Let's see if I can get the mailing list right this time

-----Original Message-----
From: Jim Schaad <[email-masked]> 
Sent: Monday, June 8, 2020 3:02 PM
To: [email-masked]'
<[email-masked]>
Cc: [email-masked]' <[email-masked]>
Subject: Review draft-ietf-ace-mqtt-tls-profile-05

* Style Issue. "Abbreviations should be expanded in document title and upon
first use in the document.  Full expansion of the text should be followed by
the abbreviation in parentheses."  Some items such as TLS are considered to
be known by everybody and thus do not need to be expanded.
https://www.rfc-editor.org/materials/abbrev.expansion.txt contains both
"known" and "unknown" abbreviations.  (Yes this means you are going to have
a really long title.)  (I might argue that  OASIS should be listed as well
known.)

* Section 1 - para 1 - Sentence two has a plurality mismatch between client
and server.  I think in this case server (and broker) should be pluralized.

* Section 1.3 - Broker - s/publishes/publish/  The set of pluralized items
seems to be too large by one.

* Section 1.3 - I think we ought to define Session given that we no longer
require that Clean Session be used.

* Section 1.3 - I do not believe that CONNACK is always the first packet
from the broker to a client.  An AUTH message may be the first one.

* Section 2.1 - para 3 s/the public key used by the RS/the public key to be
used by the RS/

* Section 2.1 - I think (but am not sure) that we may need to do some
registrations around the use of thumbprints in a confirmation claim.  If I
forget, please remind me to track this down.

* Section 2.2.2 - para #1 - s/using client authentication/using server
authentication/  If we are doing MQTT as the default then the server not the
client is authenticated here.  You may not want to have lost the potentially
here.

* Section 2.2.3 - I think that the client state MUST include the token and
either the same token is supplied or a token with the same key is supplied
in order to match with an existing state.

* Section 6.2 - I think it makes sense to put in a note that between the
fact that the server will disconnect on almost any error and is not required
to keep session state between connections, clients need to make a greater
effort to ensure that tokens remain valid and they do not attempt to publish
to topics that they do not have permissions for.

* Section 8 - The storage of tokens long term can be restricted to only
current valid ones if an immediate validation of the token is done.  This
means that the RS spends time doing the validation, but does not need to
consume memory.

This is looking really good.  I think we have only two technical issues open
at this point.  The question of registrations for hash identifications in
confirmation claims and if changes to the default scope structure are to be
done.

Jim

---

From: Cigdem Sengul <[email-masked]>
Date: Sat, 20 Jun 2020 15:38:34 +0100

Hello Jim,

Thank you for the comments. Responses inline.
Changes in this commit
<https://github.com/ace-wg/mqtt-tls-profile/commit/c1e18dba6ce65866ad348dd42a9a96ec348a8937>

> a really long title.)  (I might argue that  OASIS should be listed as well

[CS] Fixed (All style issues are in
https://github.com/ace-wg/mqtt-tls-profile/issues/53)

> [CS] Fixed.

> seems to be too large by one.

[CS] Fixed

> require that Clean Session be used.

 [CS] Added
https://github.com/ace-wg/mqtt-tls-profile/issues/55

> [CS] Removed that sentence.

> * Section 2.1 - para 3 s/the public key used by the RS/the public key to be

[CS] Fixed.

> forget, please remind me to track this down.

[CS] Opened Github issue so that I do not forget.
https://github.com/ace-wg/mqtt-tls-profile/issues/54

> client is authenticated here.  You may not want to have lost the

[CS] What I meant here was this: Given that the client pushed the token
using authz-info,
we expect it to use authenticate itself over TLS next time it connects
(otherwise it would have carried the token in the MQTT connect). I will fix
it as using client authentication over TLS (i.e, TLS:Known(RPK/PSK)-MQTT:none)
but wanted to check first if I missed your meaning.
https://github.com/ace-wg/mqtt-tls-profile/issues/56

> in order to match with an existing state.

I used a MAY because:
1) MQTT originally identifies resuming client with only client id.
2) The client MUST still provide a token when reconnecting and continuing a
session, and the broker MUST perform a validation.

If the broker additionally keeps the former token then it can identify the
client better.

However, the client may have gotten a new token, as it may know its token
has expired, or it just expanded its permissions between sessions.
Also, the CONNECT message does not specify how clients can push a
historical and a new token.

It may be seen as an issue that the broker authorises the current session
(not the continuation of the session) and uses the MQTT way to access the
session state by only the Client ID.
A problem may be that client A uses client_id1, disconnects and then client
B connects with client_id1, both using session continuation.
Then, the risk is client A's subscribed topics may be delivered to client B
because client B's token has the same set of permissions.
I think this is acceptable as client B's token already covers those topics
(although it may mean QoS issues for client A).

https://github.com/ace-wg/mqtt-tls-profile/issues/57

> to topics that they do not have permissions for.

[CS] Added.
https://github.com/ace-wg/mqtt-tls-profile/issues/58

> means that the RS spends time doing the validation, but does not need to

[CS] Just want to confirm; when you say storage of tokens restricted to
current valid ones.
 -> do you mean RS stores the valid tokens and does not store
introspection/validation result, and hence validate each time.
https://github.com/ace-wg/mqtt-tls-profile/issues/59

> Thank you, Jim.

> Jim
>
>
> _______________________________________________
> Ace mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ace
>