[Ace] AD Evaluation of draft-ietf-ace-mqtt-tls-profile-12

[Benjamin Kaduk]

Hi all,

Sorry to have taken so long to get back to this, and thank you for
continuing to make updates in response to the changes in the framework and
other profiles.

In general, the protocol mechansisms defined here are in good shape;
thank you!

I made a github PR with some changes that seemed easier to phrase in the
form of a patch than a prose comment:
https://github.com/ace-wg/mqtt-tls-profile/pull/77

I did find a couple of significant issues that need to be addressed
before IETF LC, but I think any needed changes will be pretty localized.

Specifically, there's no requirement for ACE access tokens to be
self-deliniating, so we can't actually programmatically tell whether
there's content after the token in a CONNECT message; the mechanisms in
Sections 2.2.4.1/2.2.4.2 seem to assume that we can do so for
determining whether the CONNECT is just providing a token or also
providing PoP over a TLS exporter value.  I think that this just means
we need to have an explicit "token length" field (or similar).

There are also a few places where we seem to be putting requirements on
Broker behavior that are in direct conflict with normative requirements
of the MQTT specification.  We can't override the external spec, so
we'll need to check and reword in any places where there are conflicts.
(I'm not an expert on MQTT and only read the spec as part of doing this
review, so it's entirely possible that I'm misinterpreting the MQTT spec
in some or all of these locations.)

A few other general notes before the section-by-section notes:

There is very little in this document about the HTTP-based interactions
with the AS.  I think the intent is to defer to the core framework for
that, but being a little more explicit about what is being pulled in and
how would be helpful.

If we're using TLS Exporters and allow TLS-not-1.3, we need to make some
additional requirements on TLS usage in order for the exporter values to
be safe.  Typically this takes the form of requiring the extended master
secret extension along with guidance on what cipher suites to use; I
guess RFC 7925 (rather than 7525) would be the default reference for
other TLS usage.

This document seems to mostly use British English.  AFAIR, that's okay,
but if it's inconsistent the RFC Editor will prefer American English.  I
didn't attempt to check for this (though there are tools like
https://github.com/larseggert/ietf-reviewtool that will do so).


Section 1

   their subscribers.  In the rest of the document the terms "RS", "MQTT
   Server" and "Broker" are used interchangeably.

We will probably get a reviewer asking why we can't pick a single term
and standardize on it.  However, I expect that there are places where we
want to emphasize on one aspect or another of its behavior, so don't
think we should actually do so.  Similarly for the places where we
mention that CoAP can be used (but don't reference a concrete
specification).

   Clients use MQTT PUBLISH message to publish to a topic.  This
   document does not protect the payload of the PUBLISH message from the
   Broker.  Hence, the payload is not signed or encrypted specifically
   for the subscribers.  This functionality MAY be implemented using the
   proposal outlined in the ACE Pub-Sub Profile
   [I-D.ietf-ace-pubsub-profile].

I suggest s/MAY/may/, which would avoid any need to make
ace-pubsub-profile a normative reference.

   reference or JSON Web Token (JWT) [RFC7519].  For JWTs, this document
   follows [RFC7800] for PoP semantics for JWTs.  The Client-AS and RS-
   AS MAY also use protocols other than HTTP, e.g.  Constrained
   Application Protocol (CoAP) [RFC7252] or MQTT; it is recommended that
   TLS is used to secure these communication channels between Client-AS
   and RS-AS.  Implementations MAY also use "application/ace+cbor"
   content type, and CBOR encoding [RFC8949], and CBOR Web Token (CWT)
   [RFC8392] and associated PoP semantics to reduce the protocol memory
   and bandwidth requirements.  For more information, see Proof-of-
   Possession Key Semantics for CBOR Web Tokens (CWTs) [RFC8747].

One thing that can be surprising to readers not versed in the ecosystem
is that RFCs 7800 and 8747 only talk about the token claims that are
used to build the PoP system, and don't actually define mechanisms for
providing the proof of possession.  We might want a forward-reference to
§ 2 where we do actually specify mechansisms to prove possession of the
indicated key.

Section 2.2.1

The way we name these authentication options with specific (quoted)
strings suggests that they will be used as a protocol element.  But
where?  Is the literal string "Known(RPK/PSK)" used in both cases (vs.
having distinct strings for RPK and PSK)?

Also, the hyphen character tends to more often be used as a joiner than
a separator, so it's easy to misread this as a triple of "TLS",
"Anon-MQTT", "None"/etc..  (I originally was going to ask why these all
had a "TLS" prefix...)  It might be better to use a sem

Re: [Ace] AD Evaluation of draft-ietf-ace-mqtt-tls-profile-12

[Cigdem Sengul]

Hello Ben,
Thank you very much for the review, and I will look into the pull request
as soon as possible.
Below I've gone over your comments and categorised them   as "Discussion"
if I had questions/clarifications/comments, and "Will Do" for the rest.

Discussion:


> Specifically, there's no requirement for ACE access tokens to be
> self-deliniating, so we can't actually programmatically tell whether
> there's content after the token in a CONNECT message; the mechanisms in
> Sections 2.2.4.1/2.2.4.2 seem to assume that we can do so for
> determining whether the CONNECT is just providing a token or also
> providing PoP over a TLS exporter value.  I think that this just means
> we need to have an explicit "token length" field (or similar).
>

[CS: Those sections do make use of an explicitly token length field Both
figures 4 and 5 show a token length in the Authentication Data.
2.2.4.1 - "For this option, the Authentication Data MUST contain the
two-byte
   integer token length, the token, and the keyed message digest (MAC)
   or the Client signature (as shown in Figure 4)."
Am I misunderstanding something?
]

>
> There are also a few places where we seem to be putting requirements on
> Broker behavior that are in direct conflict with normative requirements
> of the MQTT specification.  We can't override the external spec, so
> we'll need to check and reword in any places where there are conflicts.
> (I'm not an expert on MQTT and only read the spec as part of doing this
> review, so it's entirely possible that I'm misinterpreting the MQTT spec
> in some or all of these locations.)
>

[CS: Noted, I will respond to them where you have raised them specifically.]


> A few other general notes before the section-by-section notes:
>
> There is very little in this document about the HTTP-based interactions
> with the AS.  I think the intent is to defer to the core framework for
> that, but being a little more explicit about what is being pulled in and
> how would be helpful.
>

[CS: OK. Does this mean refer/cite the relevant sections in the core
framework?]

Section 1

   their subscribers.  In the rest of the document the terms "RS", "MQTT
   Server" and "Broker" are used interchangeably.

We will probably get a reviewer asking why we can't pick a single term
and standardize on it.  However, I expect that there are places where we
want to emphasize on one aspect or another of its behavior, so don't
think we should actually do so.  Similarly for the places where we
mention that CoAP can be used (but don't reference a concrete
specification).

[CS: This was brought up before. In practice, MQTT Broker or Broker is
widely used, MQTT Server is in MQTT spec, and the
RS is the additional functionality we are putting on the broker with this
spec. I am happy to keep to one if interchangeable use is an issue -
possibly select "MQTT Server" then. ]

Section 2.2.1

The way we name these authentication options with specific (quoted)
strings suggest that they will be used as a protocol element.  But
where?  Is the literal string "Known(RPK/PSK)" used in both cases (vs.
having distinct strings for RPK and PSK)?

Also, the hyphen character tends to more often be used as a joiner than
a separator, so it's easy to misread this as a triple of "TLS",
"Anon-MQTT", "None"/etc..  (I originally was going to ask why these all
had a "TLS" prefix...)  It might be better to use a semicolon or comma
instead of hyphen.

[CS: No they are not protocol elements. These were all the cases that we
came up with Jim thinking about the different possibilities for pushing the
token (Client may push the token before/during TLS session set-up; or
client may defer it to MQTT connect. ). I am open to suggestions to present
them better. I guess removing the "" would help? or any other suggestions
on naming?

 o  "TLS:Anon-MQTT:None": This option is used only for the topics that
  do not require authorization, including the "authz-info" topic.

Are there topics other than "authz-info" that don't require
authorization?  We might need to add some heding language to the earlier
statement that "Client and the Broker MUST perform mutual
authentication" if so.

[CS: Yes, actually we should think that the Broker may be hosting both
public and private topics, and hence, it may accept unauthorised clients if
that's the case. ]

   It is RECOMMENDED that the Client implements "TLS:Anon-MQTT:ace" as a
   first choice when working with protected topics.  However, depending
   on the Client capability, Client MAY implement "TLS:Known(RPK/PSK)-
   MQTT:none", and consequently "TLS:Anon-MQTT:None" to submit its token
   to "authz-info".

It's good that we provide guidance on which of the authentication
schemes are preferred (since we offer both ACE-layer and TLS-layer
schemes).  However, we will surely be asked to defend why there are two
possible ways of doing it instead of just one, and this text doesn't
really do a good job of that.  What might cause a need to imple

Re: [Ace] AD Evaluation of draft-ietf-ace-mqtt-tls-profile-12

[Benjamin Kaduk]

Hi Cigdem,

Hopefully you have not gotten too far along on the few items where I reply
and say that your proposed change may not be needed; I had hoped to write
this message several days ago.  (That said, there really are only a few
such places; the bulk of your proposals look good.)

On Sat, Aug 07, 2021 at 02:54:16PM +0100, Cigdem Sengul wrote:
> Hello Ben,
> Thank you very much for the review, and I will look into the pull request
> as soon as possible.
> Below I've gone over your comments and categorised them   as "Discussion"
> if I had questions/clarifications/comments, and "Will Do" for the rest.
> 
> Discussion:
> 
> 
> > Specifically, there's no requirement for ACE access tokens to be
> > self-deliniating, so we can't actually programmatically tell whether
> > there's content after the token in a CONNECT message; the mechanisms in
> > Sections 2.2.4.1/2.2.4.2 seem to assume that we can do so for
> > determining whether the CONNECT is just providing a token or also
> > providing PoP over a TLS exporter value.  I think that this just means
> > we need to have an explicit "token length" field (or similar).
> >
> 
> [CS: Those sections do make use of an explicitly token length field Both
> figures 4 and 5 show a token length in the Authentication Data.
> 2.2.4.1 - "For this option, the Authentication Data MUST contain the
> two-byte
>integer token length, the token, and the keyed message digest (MAC)
>or the Client signature (as shown in Figure 4)."
> Am I misunderstanding something?
> ]

I think I am misunderstanding (or misreading) something, and you have it
right.  Oops.
I'm not sure what I was looking at when I wrote that comment, but what I'm
looking at now doesn't have the behavior I was apparently concerned about.
Sorry about that.

> >
> > There are also a few places where we seem to be putting requirements on
> > Broker behavior that are in direct conflict with normative requirements
> > of the MQTT specification.  We can't override the external spec, so
> > we'll need to check and reword in any places where there are conflicts.
> > (I'm not an expert on MQTT and only read the spec as part of doing this
> > review, so it's entirely possible that I'm misinterpreting the MQTT spec
> > in some or all of these locations.)
> >
> 
> [CS: Noted, I will respond to them where you have raised them specifically.]
> 
> 
> > A few other general notes before the section-by-section notes:
> >
> > There is very little in this document about the HTTP-based interactions
> > with the AS.  I think the intent is to defer to the core framework for
> > that, but being a little more explicit about what is being pulled in and
> > how would be helpful.
> >
> 
> [CS: OK. Does this mean refer/cite the relevant sections in the core
> framework?]

I think that would help, yes.

> Section 1
> 
>their subscribers.  In the rest of the document the terms "RS", "MQTT
>Server" and "Broker" are used interchangeably.
> 
> We will probably get a reviewer asking why we can't pick a single term
> and standardize on it.  However, I expect that there are places where we
> want to emphasize on one aspect or another of its behavior, so don't
> think we should actually do so.  Similarly for the places where we
> mention that CoAP can be used (but don't reference a concrete
> specification).
> 
> [CS: This was brought up before. In practice, MQTT Broker or Broker is
> widely used, MQTT Server is in MQTT spec, and the
> RS is the additional functionality we are putting on the broker with this
> spec. I am happy to keep to one if interchangeable use is an issue -
> possibly select "MQTT Server" then. ]

Let's leave things as-is for now and see who complains.  I just didn't want
it to come as a surprise later, and it sounds like we have a plan for what
we can do if necessary.

> Section 2.2.1
> 
> The way we name these authentication options with specific (quoted)
> strings suggest that they will be used as a protocol element.  But
> where?  Is the literal string "Known(RPK/PSK)" used in both cases (vs.
> having distinct strings for RPK and PSK)?
> 
> Also, the hyphen character tends to more often be used as a joiner than
> a separator, so it's easy to misread this as a triple of "TLS",
> "Anon-MQTT", "None"/etc..  (I originally was going to ask why these all
> had a "TLS" prefix...)  It might be better to use a semicolon or comma
> instead of hyphen.
> 
> [CS: No they are not protocol elements. These were all the cases that we
> came up with Jim thinking about the different possibilities for pushing the
> token (Client may push the token before/during TLS session set-up; or
> client may defer it to MQTT connect. ). I am open to suggestions to present
> them better. I guess removing the "" would help? or any other suggestions
> on naming?

Yes, removing the quotation marks would help.
I'd also replace the hyphen with a comma in each pairing, and change the
sentence introducing the list to be something like "Thus, client
authe

Re: [Ace] AD Evaluation of draft-ietf-ace-mqtt-tls-profile-12

[Cigdem Sengul]

Hello Ben,


> Hopefully you have not gotten too far along on the few items where I reply
> and say that your proposed change may not be needed; I had hoped to write
> this message several days ago.  (That said, there really are only a few
> such places; the bulk of your proposals look good.)
>

No worries at all. I have been swamped and this time it served a good
purpose that I could not start
immediately addressing the issues raised. :) Hope to do so soon.

I am glad we have cleared out some misunderstandings regarding
token lengths, and MQTT operation.  Majority of required changes are clear
now,
I will have a more detailed e-mail showing how your review comments were
addressed.
Below, there are a few where I ask for some further clarifications, or
express my agreement, if I haven't done so already.

>
>
> >   The Broker MUST support "TLS:Anon-MQTT:ace".  To support Clients with
> > >different capabilities, the Broker MAY provide multiple client
> > >authentication options, e.g. support "TLS:Known(RPK)-MQTT:none" and
> > >"TLS:Anon-MQTT:None", to enable RPK-based client authentication, but
> > >fall back to "TLS:Anon-MQTT:ace" if the Client does not send a
> client
> > >certificate (i.e. it sends an empty Certificate message) during the
> > >TLS handshake.
> > >
> > > [Just a potential nit about the wording: "fall back" implies some
> > > ordering requirements, but IIRC use of RPK requires sending the token
> to
> > > authz-info before starting the new TLS connection, which doesn't quite
> > > match the steps as ordered in this description.]
> > >
> >
> > [CS: True that is not well-written. That was added because to
> potentially handle the Brokers having optional
> > client authentication for TLS. So, client authentication in TLS doesn't
> work, the broker would expect a
> > token in CONNECT; But it doesn't look needed as we do have a MUST for a
> token to be provided
> > for TLS-level client authentication. ]
>
> I think that makes sense to not require the broker to support TLS-level
> client authentication, yes.
>

[CS: OK. So, this should be rewritten not to say fall back?
Such as:
The Broker MUST support TLS:Anon,MQTT:ace.  To support Clients with
 different capabilities, the Broker MAY provide multiple client
authentication options, e.g. support TLS:Known(RPK) MQTT:none and
TLS:Anon,MQTT:None, to enable RPK-based client authentication, but
 if the Client does not send a client certificate (i.e. it sends an empty
Certificate message) during the
TLS handshake, the Broker MAY look for a token in MQTT CONNECT, using the
TLS:Anon,MQTT:ace
combination.
]


>
> >Note that, according to the MQTT standard, the Broker uses the Client
> > >identifier to identify the session state.  In the case of a Client
> > >identifier collision, a client may take over another client's
> > >session.  [...]
> > >
> > > Just to confirm: the ACE token is not used to provide authorization to
> > > use a given client identifer; the client identifier is just used as an
> > > unauthenticated identifier?  We might consider calling that out
> > > explicitly.
> > >
> >
> > [OK. Yes, the MQTT client id is considered different than the Client
> > Identifier that may be in the token.]
>
> They are clearly different identifiers, yes.  I was trying to ask about
> something slightly different, though.  One might imagine treating MQTT
> Client Identifiers as resources in their own right, and having the AS grant
> authorization to use them.  In that case the Client Identifier could be
> encoded in the token and the broker could reject attempts to use a client
> identifier that were not accompanied by a token authorizing use of that
> identifier.  However, such an architecture seems to have some layering
> issues and could be messy to implement, so I was assuming that we were not
> proposing to do so.
>

[CS: No, indeed, we have not considered client-identifiers as a resource. I
think it is an interesting proposal,
but may have further issues with client id collisions, which MQTT handles
best-effort.
Collision-free IDs would be desired but hard to achieve, when tokens given
out by multiple ASes, I assume. ]


>
> >a CONNACK with the appropriate response code.  The client cannot
> > >reauthenticate using this method during the same session ( see
> > >Section 4).
> > >
> > > Depending on what "session" means, this restriction may be too strict.
> > > We should probably be more clear about what "session" means...if it's
> > > the MQTT session, I think it's okay to use this method when
> > > re-connecting to take over the session.
> > >
> >
> > [CS: This is the MQTT session - I believe we discussed this before, and
> the
> > agreement was that this should not be allowed.
> > The reasoning was the client was not necessarily proving anything new
> with
> > the challenge from TLS-Exporter already used.]
>
> I agree that re-using the same challenge value from a TLS-Exporter is
> problematic for tha

Re: [Ace] AD Evaluation of draft-ietf-ace-mqtt-tls-profile-12

[Cigdem Sengul]

Hello Ben,
I thought I should comment on your original review to have the same order
you initially planned.
I went through all the comments, and our discussions of it.
The comparison with Editor's copy and github draft is here

.

In summary, I made the following changes:
(1) Kepts dtls profile informative (going through it, I brought all that
applied to this draft but kept out the ones that didn't apply). For that, I
introduced new sections that explains how we support TLS - PSK and RPK for
client authentication (2.2.3.1 and 2.2.3.2). Aimed to clarify all
TLS-related stuff e.g. added recommended references for using TLS in
constrained environments, followed the cipher suite requirements from these
references.
(2) Added more to the glossary about the MQTT data formats so that it is
clear the data is prefixed with a length field, or how variable length
fields
are computed.
(3) Fixed the MQTT CONNECT figures to show more details, and better
alignment of fields.
(4) Revised the document to explain the Clean Session better.
(5) Revised the document to explain how Will topics are verified to be
authorised, and how the Broker doesn't accept CONNECT if they are not.
(6) Added examples on scopes and wildcard matching for SUBSCRIBE.
(7) Added a section on 2.3 on token scope to explain the scope data
structure.

The detailed changes, and references to Github issues/commits are below.
I've SNIPped the ones that required no action or no
change based on our discussion e-mails.

I will submit a new version - should I wait for your perusal of the changes
(detailed also below)?

Kind regards,
--Cigdem

On Thu, Aug 5, 2021 at 11:39 PM Benjamin Kaduk  wrote:

> Hi all,
>
> Sorry to have taken so long to get back to this, and thank you for
> continuing to make updates in response to the changes in the framework and
> other profiles.
>
> In general, the protocol mechansisms defined here are in good shape;
> thank you!
>
> I made a github PR with some changes that seemed easier to phrase in the
> form of a patch than a prose comment:
> https://github.com/ace-wg/mqtt-tls-profile/pull/77


[Cigdem: Pulled and merged.]

>
>
> I did find a couple of significant issues that need to be addressed
> before IETF LC, but I think any needed changes will be pretty localized.
>
> Specifically, there's no requirement for ACE access tokens to be
> self-deliniating, so we can't actually programmatically tell whether
> there's content after the token in a CONNECT message; the mechanisms in
> Sections 2.2.4.1/2.2.4.2 seem to assume that we can do so for
> determining whether the CONNECT is just providing a token or also
> providing PoP over a TLS exporter value.  I think that this just means
> we need to have an explicit "token length" field (or similar).
>

[CS: This was discussed; no changes as the text already had a "token
length".]


> [SNIP]
>
> A few other general notes before the section-by-section notes:
>
> There is very little in this document about the HTTP-based interactions
> with the AS.  I think the intent is to defer to the core framework for
> that, but being a little more explicit about what is being pulled in and
> how would be helpful.
>
[CS: I assumed this comment was for the token and introspection endpoints.
Added some text to refer to the core document more explicitly.
commit 8a789e024b4576b06a9bccc6673b53ab6a709ab8



>
> If we're using TLS Exporters and allow TLS-not-1.3, we need to make some
> additional requirements on TLS usage in order for the exporter values to
> be safe.  Typically this takes the form of requiring the extended master
> secret extension along with guidance on what cipher suites to use; I
> guess RFC 7925 (rather than 7525) would be the default reference for
> other TLS usage.
>

[CS: Added a new section 2.2.3 Client Authentication over TLS  that makes
these recommendations.]

>
> This document seems to mostly use British English.  AFAIR, that's okay,
> but if it's inconsistent the RFC Editor will prefer American English.  I
> didn't attempt to check for this (though there are tools like
> https://github.com/larseggert/ietf-reviewtool that will do so).
>

[CS: haven't used this particular tool, but I believe inconsistencies are
now fixed.]

>
>
> Section 1
>
>their subscribers.  In the rest of the document the terms "RS", "MQTT
>Server" and "Broker" are used interchangeably.
>
> We will probably get a reviewer asking why we can't pick a single term
> and standardize on it.  However, I expect that there are places where we
> want to emphasize on one aspect or another of its behavior, so don't
> think we should actually do so.  Similarly for the places where we
> mention that CoAP can be used (but don't reference a concrete

Re: [Ace] AD Evaluation of draft-ietf-ace-mqtt-tls-profile-12

[Benjamin Kaduk]

Hi Cigdem,

Oof, has it really been two months since you sent this?  I am sorry to have
let it linger for so long.

I've gone through the published -13 and have a handful or two of comments
left (which I will send separately), but let me just reply here to a few
things first (inline).

On Mon, Oct 18, 2021 at 03:23:12PM +0100, Cigdem Sengul wrote:
> Hello Ben,
> I thought I should comment on your original review to have the same order
> you initially planned.
> I went through all the comments, and our discussions of it.
> The comparison with Editor's copy and github draft is here
> 
> .
> 
> In summary, I made the following changes:
> (1) Kepts dtls profile informative (going through it, I brought all that
> applied to this draft but kept out the ones that didn't apply). For that, I
> introduced new sections that explains how we support TLS - PSK and RPK for
> client authentication (2.2.3.1 and 2.2.3.2). Aimed to clarify all
> TLS-related stuff e.g. added recommended references for using TLS in
> constrained environments, followed the cipher suite requirements from these
> references.

I think I see what you're trying to do here, and it makes sense in a
certain way ... but if I go and compare side-by-side the text we have here
and what's in the DTLS profile, the DTLS profile goes into a lot more
detail.  In particular, the DTLS profile mentions some things that
implementations need to do in order to avoid vulnerabilities, and I'm not
sure that we want to go into so much detail in this document when it's
already recorded elsewhere.  I'm willing to let the document advance to
IETF LC and IESG review with this part as-is (but will not be surprised if
other reviewers raise the topic), but just wanted to check if there's a
particular reason to want the DTLS profile to not be a normative reference.
I think I don't understand that part of things very well, so to me there's
not much harm in making it a normative reference -- maybe I'm missing
something!

> (2) Added more to the glossary about the MQTT data formats so that it is
> clear the data is prefixed with a length field, or how variable length
> fields
> are computed.
> (3) Fixed the MQTT CONNECT figures to show more details, and better
> alignment of fields.
> (4) Revised the document to explain the Clean Session better.
> (5) Revised the document to explain how Will topics are verified to be
> authorised, and how the Broker doesn't accept CONNECT if they are not.
> (6) Added examples on scopes and wildcard matching for SUBSCRIBE.
> (7) Added a section on 2.3 on token scope to explain the scope data
> structure.

I think the restructuring to handle this stuff came out quite well --
thanks for taking the wholistic approach.

> The detailed changes, and references to Github issues/commits are below.
> I've SNIPped the ones that required no action or no
> change based on our discussion e-mails.
> 
> I will submit a new version - should I wait for your perusal of the changes
> (detailed also below)?
> 
> Kind regards,
> --Cigdem
> 
> On Thu, Aug 5, 2021 at 11:39 PM Benjamin Kaduk  wrote:
> 
> > Hi all,
> >
> > Sorry to have taken so long to get back to this, and thank you for
> > continuing to make updates in response to the changes in the framework and
> > other profiles.
> >
> > In general, the protocol mechansisms defined here are in good shape;
> > thank you!
> >
> > I made a github PR with some changes that seemed easier to phrase in the
> > form of a patch than a prose comment:
> > https://github.com/ace-wg/mqtt-tls-profile/pull/77
> 
> 
> [Cigdem: Pulled and merged.]
> 
> >
> >
> > I did find a couple of significant issues that need to be addressed
> > before IETF LC, but I think any needed changes will be pretty localized.
> >
> > Specifically, there's no requirement for ACE access tokens to be
> > self-deliniating, so we can't actually programmatically tell whether
> > there's content after the token in a CONNECT message; the mechanisms in
> > Sections 2.2.4.1/2.2.4.2 seem to assume that we can do so for
> > determining whether the CONNECT is just providing a token or also
> > providing PoP over a TLS exporter value.  I think that this just means
> > we need to have an explicit "token length" field (or similar).
> >
> 
> [CS: This was discussed; no changes as the text already had a "token
> length".]
> 
> 
> > [SNIP]
> >
> > A few other general notes before the section-by-section notes:
> >
> > There is very little in this document about the HTTP-based interactions
> > with the AS.  I think the intent is to defer to the core framework for
> > that, but being a little more explicit about what is being pulled in and
> > how would be helpful.
> >
> [CS: I assumed this comment was for the token and introspection endpoints.
> Added some text to refer to the core document more ex

Re: [Ace] AD Evaluation of draft-ietf-ace-mqtt-tls-profile-12

[Cigdem Sengul]

Hello Ben,
No worries. It's been a very busy period for me as well.
My responses are below. Thank you, as always, for your feedback.

On Tue, Dec 7, 2021 at 8:14 PM Benjamin Kaduk  wrote:

> Hi Cigdem,
>
> Oof, has it really been two months since you sent this?  I am sorry to have
> let it linger for so long.
>
> I've gone through the published -13 and have a handful or two of comments
> left (which I will send separately), but let me just reply here to a few
> things first (inline).
>
> On Mon, Oct 18, 2021 at 03:23:12PM +0100, Cigdem Sengul wrote:
> > Hello Ben,
> > I thought I should comment on your original review to have the same order
> > you initially planned.
> > I went through all the comments, and our discussions of it.
> > The comparison with Editor's copy and github draft is here
> > <
> https://tools.ietf.org/rfcdiff?url1=https://tools.ietf.org/id/draft-ietf-ace-mqtt-tls-profile.txt&url2=https://ace-wg.github.io/mqtt-tls-profile/draft-ietf-ace-mqtt-tls-profile.txt
> >
> > .
> >
> > In summary, I made the following changes:
> > (1) Kepts dtls profile informative (going through it, I brought all that
> > applied to this draft but kept out the ones that didn't apply). For
> that, I
> > introduced new sections that explains how we support TLS - PSK and RPK
> for
> > client authentication (2.2.3.1 and 2.2.3.2). Aimed to clarify all
> > TLS-related stuff e.g. added recommended references for using TLS in
> > constrained environments, followed the cipher suite requirements from
> these
> > references.
>
> I think I see what you're trying to do here, and it makes sense in a
> certain way ... but if I go and compare side-by-side the text we have here
> and what's in the DTLS profile, the DTLS profile goes into a lot more
> detail.  In particular, the DTLS profile mentions some things that
> implementations need to do in order to avoid vulnerabilities, and I'm not
> sure that we want to go into so much detail in this document when it's
> already recorded elsewhere.  I'm willing to let the document advance to
> IETF LC and IESG review with this part as-is (but will not be surprised if
> other reviewers raise the topic), but just wanted to check if there's a
> particular reason to want the DTLS profile to not be a normative reference.
> I think I don't understand that part of things very well, so to me there's
> not much harm in making it a normative reference -- maybe I'm missing
> something!
>

[Cigdem: I wasn't sure if I could make DTLS profile normative when I was
using TLS, and then there were
a few MUSTs in the DTLS profile that I felt didn't apply. But, given there
is a new short draft that says the profile applies to TLS,
things are in a better place (
https://datatracker.ietf.org/doc/draft-ietf-ace-extend-dtls-authorize/).

However, I need input on the following issues before I revise to make DTLS
profile normative, and reorganise the
sections accordingly based on that change i.e., refer more to DTLS sections
in the newly introduced sections,
and shorten them.

The DTLS profile expectedly talks about CoAp, CBOR, COSE (some examples
below).
Also, token expiration is handled differently with MQTT.
Can those be revised for MQTT-TLS profile?
"If the "ace_profile" parameter is present, it is set to "coap_dtls".
"This specification uses CBOR web tokens to convey claims within an access
token issued by the server."
For PSK mode:
"The authorization server adds a "cnf" parameter to the access information
carrying a "COSE_Key" object that informs the client about the shared
secret that is to be used between the client and the resource server. If
the access token carries a symmetric key, the access
   token MUST be encrypted using a "COSE_Encrypt0" structure (see section
7.1 of [RFC8392])."
DTLS channel setup:
 "To do so, it MUST create a "COSE_Key" structure with the "kid" that was
conveyed in the "rs_cnf" claim in the token response from
   the authorization server and the key type "symmetric".
Token expiration:
"As specified in Section 5.10.3 of [I-D.ietf-ace-oauth-authz], the resource
server MUST notify the client with an error response with
   code 4.01 (Unauthorized) for any long running request before terminating
the association."
(The error response for MQTT would be different.)

Also, for PSK,
"If a resource server receives a ClientKeyExchange message that contains a
"psk_identity" with a length greater than zero, it MUST
 parse the contents of the "psk_identity" field as CBOR data structure"
I think this is defined differently in TLS with pre_shared_key extension
etc.
]
 [SNIP]

>
>
> > >
> > >o  "TLS:Anon-MQTT:ace": The token is transported inside the CONNECT
> > >   message, and MUST be validated using one of the methods described
> > >   in Section 2.2.2.  This option also supports a tokenless
> > >   connection request for AS discovery.
> > >
> > > We should probably look carefully at the "for AS discovery" phrasing,
> in
> > > light of the late changes in how the framework tal

Re: [Ace] AD Evaluation of draft-ietf-ace-mqtt-tls-profile-12

[Benjamin Kaduk]

Hi Cigdem,

It seems I have let my todo list get the better of me once again :(
On the grounds of "better late than never", more inline...

On Fri, Dec 10, 2021 at 08:24:07PM +, Cigdem Sengul wrote:
> Hello Ben,
> No worries. It's been a very busy period for me as well.
> My responses are below. Thank you, as always, for your feedback.
> 
> On Tue, Dec 7, 2021 at 8:14 PM Benjamin Kaduk  wrote:
> 
> > Hi Cigdem,
> >
> > Oof, has it really been two months since you sent this?  I am sorry to have
> > let it linger for so long.
> >
> > I've gone through the published -13 and have a handful or two of comments
> > left (which I will send separately), but let me just reply here to a few
> > things first (inline).
> >
> > On Mon, Oct 18, 2021 at 03:23:12PM +0100, Cigdem Sengul wrote:
> > > Hello Ben,
> > > I thought I should comment on your original review to have the same order
> > > you initially planned.
> > > I went through all the comments, and our discussions of it.
> > > The comparison with Editor's copy and github draft is here
> > > <
> > https://tools.ietf.org/rfcdiff?url1=https://tools.ietf.org/id/draft-ietf-ace-mqtt-tls-profile.txt&url2=https://ace-wg.github.io/mqtt-tls-profile/draft-ietf-ace-mqtt-tls-profile.txt
> > >
> > > .
> > >
> > > In summary, I made the following changes:
> > > (1) Kepts dtls profile informative (going through it, I brought all that
> > > applied to this draft but kept out the ones that didn't apply). For
> > that, I
> > > introduced new sections that explains how we support TLS - PSK and RPK
> > for
> > > client authentication (2.2.3.1 and 2.2.3.2). Aimed to clarify all
> > > TLS-related stuff e.g. added recommended references for using TLS in
> > > constrained environments, followed the cipher suite requirements from
> > these
> > > references.
> >
> > I think I see what you're trying to do here, and it makes sense in a
> > certain way ... but if I go and compare side-by-side the text we have here
> > and what's in the DTLS profile, the DTLS profile goes into a lot more
> > detail.  In particular, the DTLS profile mentions some things that
> > implementations need to do in order to avoid vulnerabilities, and I'm not
> > sure that we want to go into so much detail in this document when it's
> > already recorded elsewhere.  I'm willing to let the document advance to
> > IETF LC and IESG review with this part as-is (but will not be surprised if
> > other reviewers raise the topic), but just wanted to check if there's a
> > particular reason to want the DTLS profile to not be a normative reference.
> > I think I don't understand that part of things very well, so to me there's
> > not much harm in making it a normative reference -- maybe I'm missing
> > something!
> >
> 
> [Cigdem: I wasn't sure if I could make DTLS profile normative when I was
> using TLS, and then there were
> a few MUSTs in the DTLS profile that I felt didn't apply. But, given there
> is a new short draft that says the profile applies to TLS,
> things are in a better place (
> https://datatracker.ietf.org/doc/draft-ietf-ace-extend-dtls-authorize/).

(For what it's worth, I think it's entirely permissible to cherry-pick
specific sections of a normative reference without pulling in the whole
thing.  That is, something like "for , follow the procedures
of Section  of " would not make any dependency on sections
other than .)

> However, I need input on the following issues before I revise to make DTLS
> profile normative, and reorganise the
> sections accordingly based on that change i.e., refer more to DTLS sections
> in the newly introduced sections,
> and shorten them.
> 
> The DTLS profile expectedly talks about CoAp, CBOR, COSE (some examples
> below).
> Also, token expiration is handled differently with MQTT.
> Can those be revised for MQTT-TLS profile?
> "If the "ace_profile" parameter is present, it is set to "coap_dtls".
> "This specification uses CBOR web tokens to convey claims within an access
> token issued by the server."

Yes, we can revise them.  That would be something like "follow the
procedures of [DTLS-PROFILE], with the exceptions that the 'ace_profile'
parameter is set to 'mqtt_tls' and JSON Web Tokens can be used (CBOR Web
Token can also be used)."

> For PSK mode:
> "The authorization server adds a "cnf" parameter to the access information
> carrying a "COSE_Key" object that informs the client about the shared
> secret that is to be used between the client and the resource server. If
> the access token carries a symmetric key, the access
>token MUST be encrypted using a "COSE_Encrypt0" structure (see section
> 7.1 of [RFC8392])."
> DTLS channel setup:
>  "To do so, it MUST create a "COSE_Key" structure with the "kid" that was
> conveyed in the "rs_cnf" claim in the token response from
>the authorization server and the key type "symmetric".

Hmm, so this would have to discuss both the JOSE and COSE formulations...

> Token expiration:
> "As specified in Section 5.10.3 of [I-D.ietf