Re: [Ace] Review of draft-ietf-ace-mqtt-tls-profile-03

[Cigdem Sengul <[e-mail]>]

Thank you for this review, Jim.
Responses inline.

On Wed, Jan 1, 2020 at 10:33 PM Jim Schaad <[e-mail]> wrote:

>
> 2.2.2 - para 1, the last sentence seems to imply that the first connection
> to publish to authz-info is not being done over a TLS connection.  But the
> sentence before that states that a TLS connection MUST be used for this.
> Perhaps s/and is expected to try reconnecting over TLS./and reconnects,
> potentially using client authentication with TLS./
>

[CS] Yes, that part reads confusing.
The aim was to say client authentication is not done over TLS, but
connection can be over TLS.
What it should be saying:
Client can use either mqtt or mqtts to connect to the broker, but does only
server validation, to push the token.
After pushing the token, it is expected to do TLS:Known(RPK/PSK)-MQTT:none.
(Although for PSK the token can be in psk_identity).
Issue created:  https://github.com/ace-wg/mqtt-tls-profile/issues/37


> 2.2.2 - I am unclear under what circumstances you could end put with a
> token
> which does not parse when processing a PUBLISH message.  If the token
> cannot
> be parsed at connection time, that I can understand.  However having parsed
> the token then it does not make sense that the token becomes not parsable
> at
> the time of publishing.  Am I missing something?
>

[CS] There is a misunderstanding. The PUBLISH message refers to the actual
PUBLISH message to the "authz-info" topic which contains the token i.e.,
this may not parse to a token. (The PUBLISH message is not for other
messages that are permissioned in the token.)
The client connects (without much security), publishes the token to the
public "authz-info" topic, which only the broker can read.
Then, disconnects, and tries to connect with ace security.

Since MQTT v5 can return error messages in response to PUBLISH messages,
here, this is used to signal to the publisher that there is something wrong
with its token.

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


>
> 2.2.2 - The last paragraph is causing me confusion.  Is this supposed to be
> referencing the RS or the AS?  If it is the AS, then I don't see how there
> could be any confusion.  Please expand this so it is clearer.
>

[CS] This is a typo - it should be RS.
 https://github.com/ace-wg/mqtt-tls-profile/issues/39


> 2.2.4 - I am having a problem with trying to parse the content of the AUTH
> property.  I have no problems with 2.2.4.3 because this is a zero length
> sequence of bytes.  However for 2.2.4.1 and 2.2.4.2 there is a token
> (possibly binary with no length prefix) followed by an optional binary
> cryptographic value.  For introspection, I would need to figure out the
> length of the token before I could make a guess at the length of the
> cryptographic value.  However given that there is no divider this does not
> seem possible.  This may also become a problem for the response from the
> client in the event that there is a change from an 8-byte nonce to a
> variable length one.
>

[CS] Not specified a  format, because I  thought we discarded the idea of
using the variable-length nonce based on the meeting in Singapore.
What would you suggest - introduce a specific format to accommodate
variable length?
length_of_token+binary data for token+(the rest is cryptographic value)?
 https://github.com/ace-wg/mqtt-tls-profile/issues/40


> 2.2.4.1 - In my view it is not the secret, but the content that is being
> obtained from the TLS exporter.  That is one is signing (or MACing) the
> exporter value not using that value to compute a MAC on something else.
> While it is true that only the two parties know that value, exposure to a
> third party does not lead to a compromise.
>

[CS] I see - so reword "secret" to "content".
Or do you have a suggestion for wording?
https://github.com/ace-wg/mqtt-tls-profile/issues/41

>
> 2.2.4.3 - I am not sure if the text is supposed to require an empty
> authentication data field or to allow for the authentication data field to
> be absent as well.
>

[CS] I checked whether it would be a protocol error to have only
Authentication Method in MQTT, and it is not.
Then it is best to omit authentication data field.
Will correct the text that the document allows the CONNECT message to have
an Authentication Method set to 'ace' and allows to omit the Authentication
Data field.
https://github.com/ace-wg/mqtt-tls-profile/issues/42


>
> 3 - It might be worth while to put a pointer to section 4.7 of the MQTT V5
> spec here so that there is an indication of what the different wild card
> characters do.  I had to pop over there to make sure that I could figure it
> out.
>

[CS] Will do.
https://github.com/ace-wg/mqtt-tls-profile/issues/43


> 3.1 - Should you state that for a QoS of 0, the client should close the
> connection w/ an '0x87' in the event of an authorization failure?  I think
> that this is supposed to happen but you have left it open.
>

[CS]
Yes, I must have cut some text out. QoS of 0, the client is disconnected.
Will add.
https://github.com/ace-wg/mqtt-tls-profile/issues/44


> 6 - It is not clear to me if the authentication method described in this
> section is permitted with MQTT v5 or not.  It does not say, but it appears
> to be a true statement.  This should probably be explicit.
>

[CS] Yes,
MQTT v5 broker can support this method as well - we say at the beginning
something around backward compatibility to MQTT v3.1.1.
Will make it more explicit.
https://github.com/ace-wg/mqtt-tls-profile/issues/45



>
> 2.2.4.3 - What is the name of the user property that is being returned
> here?
>
> [CS]
This is under-defined. It should say, the name pairs follow the same
optional (name, value) parameters for AS request creation hints as defined
in the core document. There could be multiple user properties in a CONNACK,
one needs to make sure the size of the message does not grow beyond the
Maximum Packet Size specified by the client.
 https://github.com/ace-wg/mqtt-tls-profile/issues/46

>
> I am going to play with something else.  I am sure I will find other issues
> at a different time.
>

Thank you for your review. Much appreciated.
--Cigdem


>
> Jim
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> Nits:
> Section 2 Para 1 s/Broker.Figure 1/Broker.  Figure 1/
> Section 2 Para 1  s/setup.The/setup.  The/
> Section 2.2.2 Last Para s/when the AS/when the RS/
>
>
> _______________________________________________
> Ace mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/ace
>