Re: [Ace] second AD evaluation of draft-ietf-ace-mqtt-tls-profile-13

[Cigdem Sengul <[e-mail]>]

Hello Ben,
Thank you for your Pull request. I have asked for clarifications in the
following.

On Tue, Dec 7, 2021 at 8:27 PM Benjamin Kaduk <[e-mail]> wrote:

> Hi all,
>
> As promised, here are my comments on the -13.
>
>
> I put some text to this effect in my pull request
> (https://github.com/ace-wg/mqtt-tls-profile/pull/96), but technically
> RFC 7250 allows independent negotiation of the client using a RPK and
> the server using a RPK, but our text is written as if it's
> all-or-nothing.  We can probably just make that an assumption of how we
> work and not have to go into detail about how a mixed mode would work.
>
>
[CS:  Yes, that's the assumption made

"If the Client authentication uses TLS:Known(RPK/PSK), then the Broker is

   authenticated using the respective method.  Otherwise, to
   authenticate the Broker, the client MUST validate a public key from a
   X.509 certificate or an RPK from the Broker against the 'rs_cnf'
   parameter in the token response."


Thank you for the suggested text in the pull request.




> Section 1
>
> > Publish requests from the Clients to the Broker and from the Broker to
> > the Clients
>
> I'm not entirely sure how much we say about the second half specifically
> -- do we need to keep that in?
>

[CS: We do cover that in section 3.2 - the Broker needs to check all
subscribing clients to the topic are
still authorised.]

>
> Section 2.2.3.2
>
> The procedures we list here only make sense for TLS 1.3
> If we wanted to support TLS 1.2 with them we'd need to note that a PSK
> ciphersuite is needed and how to populate the PSK identity in the
> ClientKeyExchange, and reference RFC 4279.  I put some text in my PR
> that qualifies this scenario as being TLS 1.3-only on the basis that we
> don't need to go to so much effort to support TLS 1.2+PSK.
>
>

[Making ietf-ace-dtls-authorise normative, and explaining the extent that
profile
applies (now that there is a short extension draft for TLS use), could we
rely on the text for 3.3.2 psk identity field, and
the recommended cipher suite?]



> Section 2.2.4
>
> Figure 7 and its adjacent prose shows the client nonce as variable
> length (or at least, explicit length with no mention of being constant),
> but Figure 8 shows the client nonce as fixed length (8 bytes).
>
> [CS: True. Will revise. We went back and forth variable=length nonce vs
fixed length nonce in the group discussions.
It ended up being fixed-length nonce for the Broker challenge, and
variable-length for Client challenge.
I suggest to have both fixed or both variable?  The argument for 8 bytes
was that it was sufficient as it's a time-bound request/response]



> Section 2.3
>
> We say that the JWT scope value is a single value with internal
> structure as a JSON array, but I think we have to interpret that in the
> context of RFC 6749's definition of scope as a list of space-delimted
> case-sensitive strings.  That is, in the formal grammar, the "scope"
> parameter value is a single string, and any internal spaces in it are
> delimters between the individual scope values.  So, we need to make our
> JWT scope value fit into the expected shape, which in particular means
> we need to be encoded as a string with no internal spaces.  However,
> MQTT does allow spaces in topic names, so the naive formulation of
> encoding the JSON object as a string would artificially limit what
> topics we can authorize the use of.  If we stick with using the "scope"
> like this, we may find ourselves needing to do some percent- or base64-
> encoding of the JSON array to produce a usable scope value...but we
> could also consider something more drastic and switch to using the OAuth
> "Rich Authorization Requests"
> (https://datatracker.ietf.org/doc/draft-ietf-oauth-rar/) that defines a
> new "authorization_details" parameter that is like scope but allows a
> much richer structure.  My understanding is that, while use of
> structured "scope" like we propose is regularly observed in the wild,
> OAuth purists really don't like it, since it's not what scope was
> intended to be about.
>
> (Also, the example in Figure 10 does include spaces in it.)
>

[CS: That's right. I lean towards encoding JSON array base64.  It's because
I understand in RAR, we
need to format the "authorization_details" using the authorization data
elements defined in that draft.

I feel, with AIF-MQTT, we have the sufficient detail for MQTT
permissions on topic filters.]


> Section 5
>
> Apparently I didn't comment on it the first time I read this, but it's
> very interesting to say that the Broker "MUST continue publishing the
> retained messages **as long as the associated tokens are valid**"
> (emphasis mine).  That seems to imply a model where the authorization
> check happens at the Broker->consumer leg, in addition to (or instead
> of?) at the publisher->Broker leg.

To some extent the right behavior
> here depends on what one uses as the definition for "publishing"
> something, but this does set us up for a scenario where a retained
> message will disappear after some amount of time.  That seems to be a
> divergence from stock MQTT, and in some sense also from what we've
> specified for Will messages, so if we're going to keep it, I think we
> should mention in the security considerations that performing
> authorization checks in this way will result in future new subscribers
> not receiving a retain message that was valid when originally sent.
> If, on the other hand, this "as long as the associated tokens are valid"
> refers to the tokens of the subscribers, that would be a less surprising
> situation, but still merit some clarification.
>

[Yes, I will clarify.

We have considered the will messages different as once accepting the
CONNECT with a Will, the Broker agrees to send that message
on an unexpected disconnection.

We assumed for any other "publication" (including retained wills) the
publisher should still be authorised to send that message, and the
subscribers should still be authorised to receive those messages.  When a
subscriber subscribes to a topic filter, where there are retained messages,
according to subscription options for retained messages, we expect the
broker would publish some messages. For that, we assumed it would do a
check for those retained messages to see if they are still OK to publish.

The other option was that broker does not allow retained messages and
signal that in connack setting RETAIN AVAILABLE to 0. That could be cleaner
but more restrictive.
We thought in constrained scenarios, a client may choose to briefly
connect, publish with retain 1 e.g., their sensor state, and then
disconnect, expecting the broker to publish that info to any new
subscriber.  However, we did not want to allow a scenario a publisher
client connects, publish a message with RETAIN 1, disconnects, and never
shows up again. In this case, we felt the broker should not
continue publishing indefinitely, especially when the token of the
publisher expires.

The MQTT draft says this with regards to RETAIN messages.
"If the Server receives a PUBLISH packet with the RETAIN flag set to 1, and
QoS 0 it SHOULD store the new QoS 0 message as the new retained message for
that topic, but MAY choose to discard it at any time. If this happens there
will be no retained message for that topic."
"If the current retained message for a Topic expires, it is discarded and
there will be no retained message for that topic."
So, I assume for QoS>1, the message expiry interval would dictate how long
the retained message is kept; if it is not set, it should be kept
indefinitely. This would be something we would like to avoid.

In the draft, we do deviate from MQTT, where the server MUST forward the
publications to all matching subscriptions i.e., the Server does not always
forward a publication message if the publication is not authorised or the
subscriber is not authorised.
So, we considered having an authorisation policy on retained messages in
the same vain.
]


>
> Appendix A
>
> It looks like the template doesn't actually have a "Content format" line
> in it, so I'm not sure if we need to keep that in here.
>
>
[+1]

Thanks again.
Kind regards,
-Cigdem