Hello Jim,

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

On Mon, Jun 8, 2020 at 11:49 PM Jim Schaad <[email protected]> wrote:

> Let's see if I can get the mailing list right this time
>
> -----Original Message-----
> From: Jim Schaad <[email protected]>
> Sent: Monday, June 8, 2020 3:02 PM
> To: '[email protected]'
> <[email protected]>
> Cc: '[email protected]' <[email protected]>
> 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.)
>

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

>
> * 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.
>
> [CS] Fixed.


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

[CS] Fixed

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

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

>
> * 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.
>
> [CS] Removed that sentence.


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

[CS] Fixed.

>
> * 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.
>

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

>
> * 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.
>

[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


> * 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.
>

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

>
> * 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.


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


>
> * 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.
>

[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

>
> 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.
>
> Thank you, Jim.



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

Reply via email to