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

[Cigdem Sengul <[e-mail]>]

Hello Marco,
Apologies for the late response - many thanks for the review.
I think there will be some items to discuss further with Francesca, but I
tried to make sure we have a plan for revisions as much as possible.
Comments as usual uses [CS:] inline.
Kind regards,

On Mon, Aug 30, 2021 at 7:15 PM Marco Tiloca <[e-mail]> wrote:

> Hi all,
>
> Please, see below my review of version -03.
>
> Overall, I think this is on the right track and I do appreciate having
> converged to a single AS :-)
>
> I hope the comments can help improving and progressing the document!
>
> Best,
> /Marco
>
[CS: Many thanks, we appreciate it]

>
>
>
>
> This review uses "KG" to refer to draft-ietf-ace-key-groupcomm-13 [KG],
> and "KGO" to refer to draft-ietf-ace-key-groupcomm-oscore-11 [KGO].
>
> [KG] https://datatracker.ietf.org/doc/html/draft-ietf-ace-key-groupcomm-13
>
> [KGO]
>
> https://datatracker.ietf.org/doc/html/draft-ietf-ace-key-groupcomm-oscore-11
>
>
> [General]
>
> * Please, replace references to RFC 8152 with references to
> draft-ietf-cose-rfc8152bis-algs and I-draft-ietf-cose-rfc8152bis-struct.
>
> [CS: OK, will do]


>
> * This draft is basically using only the ace-group/GROUPNAME resource at
> the KDC defined in KG, and only its POST handler for joining a group.
> Will this draft describe/mention also how the rest of the KDC interface
> is used by the pub-sub clients?
>
> [CS: I think we should definitely look into it; will go through all
resources available, and will try to create
a table of what MUST be used, what is RECOMMENDED to use, and what will not
be used by the profile.]



>
> [Abstract]
>
> * "This profile relies on transport layer or application layer security
> to authorize the pub-sub clients to the broker".
>
>     This is aligned with the currently existing transport profiles of
> ACE (based on DTLS and OSCORE). However, do you actually want to limit
> that security enforcement to those two layers?


>     More in general, I think this application profile can build on any
> transport profile of ACE that the broker and the KDC are fine to use
> with the pub-sub clients. This seems in fact the intention later in
> Section 2 and Section 4.2, where some transport profiles are mentioned
> as a non final set to choose from.


>
> * Proposed expansion of the last sentence:
>
>     "... it describes the use of application layer security to protect
> the content that pub-sub clients exchange by communicating through the
> broker."
>

[CS:  So your suggestion is not to limit the coap pub-sub or mqtt, and how
they are secured
but be more generic?]

>
>
> * While it is mentioned later in Section 1, I think that the abstract
> should already mention that this profile covers both CoAP and MQTT.
>

[CS: OK, will do]

>
>
[Section 1]
>
> * "This document defines a way to authorize pub-sub clients using the
> ACE framework ...".
>
>      Please, clarify what they are authorized to do. Conceptually, it
> should be about joining a security group that uses certain key material
> and is associated to one or more topics (application groups).
> Practically, this should mean being authorized to getting access to the
> broker and to obtaining the key material for protecting the topic
> content exchanged through the broker.
>
> [CS: OK. how the clients get authorisation to talk to the Broker is
actually covered in other profiles (e.g., MQTT-TLS profile)
This one describes how they are authorised to get the keying material to
protect the payload of their communications from the
Broker.]



>
> * "... for protecting the communication between them".
>
>     I would emphasize that it is about protecting the content, exhanged
> by communicating through the broker (see also the related comment above
> for the Abstract).
>

[CS: Yes]


> * After "(CoAP)", please add a reference to RFC 7252.
>
>
[CS: OK]


>
> [Section 2]
>
> * "Note that both publishers and subscribers use the same profiles."
>
>     I suggest to expand as follows: "Note that both publishers and
> subscribers use the same pub-sub communication protocol and the same
> transport profile of ACE in their interaction with the broker. The same
> profile of ACE is also used in the interaction with the KDC."
>
> [CS: OK]


>
> * Caption of Figure 1: s/Authorization Servers/Authorization Server
>
[CS: Will fix]


>
>
> * "... so that RS can authorize the Clients ..."
>
>     Well, the AS is the one authorizing the Clients. I think you mean
> "... so that RS can assert the Clients to be authorized ..."
>
> [CS: yes, that's too much of a short cut, will fix.]


>
> * I suggest to move the following points out of the last paragraph, and
> instead have them after "... to as Client in short."
>
>     - The broker acts as ACE RS.
>     - The broker corresponds to the Dispatcher in KG.
>
> [CS: OK]


>
> * The last sentences can be expanded for clarity, e.g.: "... the Broker
> MAY accept subscriptions from unauthorised Subscribers. In this case, a
> Subscriber client can skip setting up Security Association 1 with the
> Broker, before subscribing to topics of interest at the Broker."
>
> [CS: OK]


>
> [Section 3.2]
>
> * Consistently with the first paragraph, the section title can be
> "Authorising to the KDC and the Broker".
>
> [CS: OK]



> * About 'scope', s/the topic identifier/the topic identifiers
>
> [CS: Will fix]



> * About 'audience', it cannot be an array, but only a single text
> string, see [1]. If you want to keep a single Token intended to both KDC
> and broker, then you neeed to rely on a single audience value that both
> the KDC and the broker recognize as valid. A possible way can rely on
> the KDC identifier and the broker identifier separated by a well-known
> and unambiguous separator.
>
> [1]
>
> https://datatracker.ietf.org/doc/html/draft-ietf-ace-oauth-authz-45#section-5.8.5
>
> [CS: My bad, was thinking here a JWT, where  in the general case, the
> "aud" value is an array of case-sensitive strings.

But I think we won't be able to keep a single token, but will have to use
multiple tokens, once for KDC and one for Broker, where the Client
will have to make two calls two /token to the AS for two different
audiences. Since the AS is the same, we do not think there will be an issue
of policy mix-ups, and hence this should be all cleaner.]



> * More generally about using a single Token intended to both KDC and
> broker, that requires additional security when protecting the Token, as
> per [2].
>
>     First, you would need the AS to also protect the Token with an
> asymmetric signature, that both the KDC and the broker have to verify
> using the AS' public key. See the second paragraph of [2].
>
>     Second, since the same Token is targeting multiple Resource Servers
> as part of a broader audience, you cannot have a same single secret used
> as symmetric PoP key, see the third paragraph of [2]. This seems to
> limit the use of the DTLS profile to its aymmetric mode only, and to
> rule out the use of the OSCORE profile.
>
>     I don't see an obvious way out, other than reverting to two separate
> Tokens, i.e. one to post to the KDC and one to post to the broker.
>
>     [2]
>
> https://datatracker.ietf.org/doc/html/draft-ietf-ace-oauth-authz-45#section-6.1


[CS: Yes, because of all these, we will most probably just go for two
tokens esp. when ACE defaults
to symmetric crypto.]

>
>
>
>
> * If a single Token is used for both the KDC and the broker, one
> additional clarification is probably required.
>
>     Since the 'scope' claim in Token indicates names of application
> groups, posting the the Token to the broker indicates which topics the
> client is authorized to publish/subscribe to.
>
>     However, what does it mean when posted to the KDC? I suppose it
> authorizes the pub-sub client to obtain from the KDC the key material
> for any security group associated to any of the application groups
> mentioned in the Token.
>
>     Note that the 'scope' parameter of the Joining Request to the KDC
> specifies the name of the security group for which the key material is
> requested. So, the KDC will need a kind of internal mapping from names
> of application groups (in the Token) to names of security groups (in the
> 'scope' of the Joining Request). Even in the simple (and most common)
> case where only one security group is associated to an application
> group, the two names might differ and it's up to the client to know both
> of them in advance --- through pre-configuration, discovery, etc.
>

[CS: yes, some kind of internal mapping from scopes to security groups
would be needed.
This possibly answers my previous question to groupcomm Why KDC would keep
a different naming for
groups :) ]

>
>
> * s/'profile' is set/the 'ace_profile' claim is set  (two occurrences)
>
>
[CS: Will fix]


>
> [Section 4.1]
>
> * By using 'sign_info' during the Token post, the client cannot ask for
> the public keys already, but only for the format of public keys used in
> the group.
>
> [CS: Will revise those parts more carefully]



>
> * "The KDC verifies that the Client is authorized to access the topic
> with the requested role."
>
>     Well, the KDC is in a trust relation with the AS issuing the posted
> Token (and only following a successful check of a Token request against
> stored access policies). So, at this stage on the KDC, isn't this just
> about verifying that the Token is valid?
>
>
[CS: Yes, the language used is weird, what is meant is the KDC verifies the
token]


>
> [Section 4.2]
>
> * Considering its content and the examples, the section can be renamed
> as "Join Request and Join Response".
>

[CS: +1]

>
>
> * The content-format of the Joining Request is
> "application/ace-groupcomm+cbor". That's the "namespace" where these new
> ACE Groupcomm parameters have been registered, see Section 7 of KG.
>
> [CS: Will add]


>
> * As to 'scope', here it is not "as defined earlier". In comparison with
> the format defined earlier, it specifies only one scope entry, i.e. the
> one indicating the exact topic for which the client wants to obtain the
> key material with this Joining Request. This is also defined in Section
> 4.1.2.1 of KG.
>

[CS: True, will fix]

>
>
> * "... Client's public key formatted as a COSE_Key"
>
>     In KG, there has been a change about the format of public keys used
> in a group.
>
>     Pure COSE Keys are not admitted anymore, due to their limitations to
> represent identities or other metadata. This is aligned with
> corresponding changes done for the same reasons in draft-ietf-lake-edhoc
> and draft-ietf-core-oscore-groupcomm .
>
>     The format of public keys used in the group and indicated in the
> parameter 'pub_key_enc' now takes value from the "COSE Header
> Parameters" registry, for which some entries are under pending
> registration.
>
>     The easiest way to "somehow still use COSE Keys" is to use the
> public key format Unprotected CWT Claim Set (UCCS) [3], including a COSE
> Key as value of its 'cnf' claim. Please, see an example below. You can
> find some more detail on this point in Sections 6.1 and 6.4 of KGO.
>
> {                                              /UCCS/
>    2 : "42-50-31-FF-EF-37-32-39",               /sub/
>    8 : {                                        /cnf/
>      1 : {                                      /COSE_Key/
>        1 : 1,                                   /alg/
>        3: -8,                                   /kty/
>       -1 : 6,                                   /crv/
>       -2 : h'C6EC665E817BD064340E7C24BB93A11E   /x/
>              8EC0735CE48790F9C458F7FA340B8CA3'
>      }
>    }
> }
>
>     [3] https://datatracker.ietf.org/doc/draft-ietf-rats-uccs/
>
>
> [CS: Will look into aligning with the current drafts]



> * On expectable actions around "TODO: Check 'cnonce'", you may also want
> to look at Sections 6.2.1 and 6.3 of KGO.
>
> [CS: Will do]


>
> * "The Subscriber MUST have access to the public keys of all the
> Publishers; this MAY be achieved ... using "pub" as the 'role_filter' ..."
>
>     Why do you need the filtering feature here? It would still work, but
> based on the previous sentence in the same paragraph, the KDC has
> available to provide only public keys of publishers anyway. So, asking
> for all the public keys in the group with 'get_pub_keys' encoding the
> CBOR simple value Null will return only the public keys of publishers by
> construction.
>
[CS: That's true, will revise]


>
>
> * "Note that the alg parameter in the 'client_cred' COSE_Key ..."
>
>     See the comment above about the format of public keys not using pure
> COSE Keys anymore.
>
> [CS: Will revise, thanks]



>
> * s/The KDC response to Joining Response/The KDC replies with a Joining
> Response
>
> [CS: Will fix]


>
> * The fields of the joining response are defined in Section 4.1.2.1 of
> KG (not in its Section 4.2).
>
> [CS: Will fix]


>
> * The content-format of the Joining Response is
> "application/ace-groupcomm+cbor".
>
> [CS: Will add]



> * When describing 'key', "defined by the AS2" is mentioned for both
> 'alg' and 'Base IV'. I guess this is a remnant from previous versions of
> the draft having both AS1 and AS2.
>
> [CS: Yes, will fix]



>
> * Before defining the format of the Joining Response, it's good to
> introduce the symmetric key as generated by the KDC, later provided to
> all the clients joining the group, and what it is used for.
>
>
[CS: +1]


>
> * "... public keys of all authorized signing members ...", which means
> just the publishers, right?
>
> [CS: yes]


>
> * "... formatted as COSE_Keys ..."
>
>      See the comment above about the format of public keys not using
> pure COSE Keys anymore.
>
>
[CS: Yep, will revise]


>
> * "... For Subscriber Clients, the Joining Response MUST contain the
> 'pub_keys' parameter."
>
>      Why? Even if the client did not ask for them in the Joining
> Request? The sub-resource at ace-group/GROUPNAME/pub-key allows for a
> later retrieval, possibly spreading the provisioning of public keys in
> different exchanges through filtering.
>

     If you really want what you're saying here, it might be just easier
> to say upfront for the Joining Request that Subscriber Clients MUST
> include 'get_pub_keys' --- While that uses a MAY at the moment.
>
>
[CS: I am not sure about this MUST either, it was in the draft with these
conditions, and I did
not question until now. I will check with Francesca]



>
> * Figure 6 and Figure 9 will need to be revised based on the comment
> above about the format of public keys not using pure COSE Keys anymore.
>
> [CS: Added to the to-do list]


>
> * The public key of Figure 6 (in 'client_cred') and of Figure 9 (in
> 'pub_keys') includes also the 'kid' header parameter. This is not
> mentioned earlier in the text describing the Joining Request and Joining
> Response.


>     In particular for 'client_cred' in the Joining Request, this gives
> the impression that it is up to the client to self-assign a 'kid' value
> to its own public key. This can later result in collisions, i.e. two
> publishers may have a public key with the same 'kid' value. Later on,
> this in turn puts a subscriber at least in a situation where it might
> have to try verifying a message signature using different public keys,
> until the right one is used.
>
>     I think it's better that the KDC exclusively determines these
> identifiers, as uniquely associated not only to the public key for its
> retrieval but also to the corresponding publisher client. As to the
> actual provisioning of these identifiers, there are two ways:
>
>     a) The one currently used in the Joining Response, where that
> identifier is the value of the 'kid' header parameter; or, as I believe
> a better option,
>
>     b) The separate 'peer_identifiers' parameter defined in Section
> 4.1.2.1 of KG.
>
>     In either case, I think it's fine later on in Section 5.1 to have a
> published message specifying in the 'kid' COSE parameter the identifier
> to use for retrieving the right public key.
>

[CS: Will need to think about this - but yes, it needs fixing]

>
>
> * Based on a comment above on which public keys the KDC actually stores,
> Figure 8 may have 'get_pub_keys' simply encoding the CBOR simple value
> Null.
>

[CS: OK]




>
> [Section 5]
>
> * s/is protected with COSE/is protected with COSE by the publisher (two
> occurrences)
>
>
>
[CS: Will fix]


> * Figure 11 and the following paragraph should show/mention that, when
> using CoAP, Observe (RFC 7641) is used together with GET in order to
> effectively subscribe. See also [4].
>
> [4]
>
> https://datatracker.ietf.org/doc/html/draft-ietf-core-coap-pubsub-09#section-4.4
>
> [CS: OK]

>
> * I think it can be better to swap this current Section 5 with the
> current Section 6. Even though separately for the CoAP and MQTT case,
> Section 6 covers the Token posting at the broker, which happens before
> the actual pub-sub communication described in Section 5.
>
>
[CS: Let me think about this but I think you are right]

>
> [Section 5.1]
>
> * s/COSE_Encrypt0/COSE_Encrypt0 object
>
> [CS: Will fix]


>
> * I suppose the empty string for the 'external_aad' applies to both the
> encryption and the signing process. Correct?
>
> [CS: That was my understanding]


>
> * In order to encrypt the message, how do you build the AEAD nonce?
>
>     All the group members have the same Base IV received in the Joining
> Response, and clearly they are not expected to synchronize with one
> another about their individual message counter used as Partial IV.
> Hence, the basic Message IV construction constisting in xoring the Base
> IV with the zero-padded Partial IV yields the risk of reusing the same
> (nonce, key) pair.
>
>     If the KDC exclusively assigns the identifiers for public keys and
> group members (see comment above), it should work to build the AEAD
> nonce in a similar way like OSCORE does, see [5].
>
>     That would also mean having a quite smaller Partial IV in the
> unprotected header of the COSE_Encrypt0 object, while now in Figure 12
> it consistently has a size of 7 bytes as expected for the AEAD nonce by
> AES-CCM-64-64-128.
>
>     [5] https://datatracker.ietf.org/doc/html/rfc8613#section-5.2
>
> [CS: I need to have Francesca's opinion on this, as I have not contributed
here.]

>
> * Please, add a reference to draft-ietf-cose-countersign , and specify
> that you are using the structure COSE_Countesignature.
>
>     As a related point, the value 7 used for 'countersign' in Figure 12
> is going to be deprecated [6], so it'll need to be updated later on.
>
>     [6]
>
> https://datatracker.ietf.org/doc/html/draft-ietf-cose-countersign-05#section-5.2
>
> [CS: Will revise]

>
> * "The protected Headers ... MAY contain the kid parameter, ..."
>
>     It is optional for the KDC to provide one in the Joining Response,
> but I guess that if one is provided, then it MUST be included in the
> protected header.
>
> [CS: OK]


>
> [Section 6.1]
>
> * "For implementation simplicity, it is RECOMMENDED that the ACE
> transport profile used and this specification use the same format of
> "scope".
>
>     Is this actually meaningful and enforceable?
>
>     The format of scope is related to the application that the Client
> and the RS want to run. In fact, these _application_ profiles for group
> communication are defining formats to use for scope, in Tokens used to
> access resources related to such applications.
>
>     In other words, I think the format of scope is orthogonal to the
> used transport profile, that basically does not care (and should not). I
> also can't find anything suggesting otherwise in the ACE framework [7]
> or in the DTLS and OSCORE profiles.
>
>     Unless I'm missing something, it's probably just fine to remove the
> sentence.
>
>     [7]
>
> https://datatracker.ietf.org/doc/html/draft-ietf-ace-oauth-authz-45#appendix-C
>
> [CS: I think this was triggered by MQTT-TLS Profile providing a scope
format to be used
in access tokens, and it would be useful the scope format is used towards
KDC. But, it may not
be necessary at the end. Will remove]


>
> [Section 6.2]
>
> * "An application group has relevance at the application level - for
> example, in MQTT an application group could denote all topics stored
> under ""home/lights/"."
>
>     For what is worth, I thought of one exact topic to be one
> application group. So, in the example above, "home/lights/" can be seen
> a reasoned collection of related application groups, but not as an
> application group itself.
>

[That's not how I see it in MQTT context - the broker may be hosting
multiple topics,
and home/lights/* maybe one application group that all the group members
should be aware of.
and then there could be other sub-groups home/lights/kitchen/* etc.]


>
>
> * "... the client needs to discover the (application group, security
> group) association. In MQTT, $SYS/ ... can be used by the broker for
> this purpose."
>
>     The security group here is related to the key material used to
> protect the content with COSE, which is competence of the KDC. That key
> material is intended for the security group members, which do not
> include the broker.
>
>     Hence, unless the KDC and the broker are components of a same system
> under the same authority, I would have expected the broker to have no
> idea of how the application groups it manages are related to the
> security groups that the KDC manages.
>
>     I would rather expect the KDC to be aware of such associations, e.g.
> by learning about them when the security groups are actually created on
> it. The KDC can later on advertise these associations. For the Group
> OSCORE case targeted in KGO, this can happen through a Resource
> Directory, as discussed in [8] and using the approach defined in [9].
>
>     [8] https://datatracker.ietf.org/doc/draft-ietf-ace-oscore-gm-admin/
>
>     [9]
> https://datatracker.ietf.org/doc/draft-tiloca-core-oscore-discovery/
>
> [CS: OK, I agree - I did not see how KDC can support this, so put it under
Broker, but
I acknowledge that this has several shortcomings. Will look into OSCORE
impl.]

>
> * "Client computes the corresponding security groups ..."
>
>     Do you mean "Client considers ..." ?
>
[CS: Nope, it definitely needs to compute. If the client plans to subscribe
to
home/lights/* but home/lights/kitchen/* is one security group, and
home/lights/bathroom/*
is one security group, it needs to ask for these two scopes in the token
and it needs to subscribe to these separately etc.]


> * "RS validates the PUBLISH message by checking the topic's security
> group association and the stored token."
>
>     Even having the broker aware of the association (application group,
> security group) as defined in the draft, how does this validation work?
>

[CS: This will be revised - if the complexity falls on the client, learning
information from KDC, these problems
should go away. But note that when a client makes a PUBLISH or SUBSCRIBE
request, the Broker
checks if it can accept the request by checking the Topic Name/Topic Filter
against the stored token scopes.]


>
>     - The protected message targets a resource at the broker associated
> to the application group.
>     - By assumption, the broker knows the security group(s) associated
> to the application group.
>     - From the broker's point of view, the Token specifies the
> application group.
>
>     I understand checking whether the request to the broker targets a
> topic resource (an application group) where the client is authorized to
> publish as per the Token. But how does the mentioned validation involve
> checking that the request is also related to the security group?
>

[CS: We will move away
from this scheme. We will make the Broker agnostic to security groups, and
let the Client ask with proper topic filter matching the scope in the token
(ie. which consequently matches the security group).]


>
>     Also, is that actually important for the broker, which by the way
> does not have the key material of the security group?
>
> [CS: Well the Broker only cares it only accepts Publishers/Subscribers
publish/subscribe
to their authorised topics]



>
> [Section 7]
>
> * "... by the same entity having control of the topic, i.e. KDC."
>
>     Perhaps here you mean "... by the same entity having control of the
> key material for that topic, i.e. KDC."
>
> [CS: Yes]

>
> * The second paragraph seems to mix together authorization to publish
> and source-authentication of messages.
>
>     If it is not critical that only authorized publishers can publish,
> then the broker might not necessarily enforce access control on
> publishers. That is, a publication request would be accepted and
> processed even if there was no posted Token supporting this operation.
>
>     Instead, not using an additional signature and relying only on
> integrity-protected encryption based on group key material, is about: i)
> accepting that subscribers cannot be able to precisely identify the
> actual publisher that has sent a message to the group; and ii) accepting
> that any group member (subscribers included) can modify content
> published by the actual publisher, in case communications between the
> broker and the subscribers are not protected.
>
>
[CS: Will check with Francesca, but I agree with you. I think what she
describes is that if it's only symmetric crypto
and no publisher authorisation (i.e.,  it is not checked who is allowed to
publish), subscribers can
turn into publishers being able to protect the content of the message
acceptable to the group.]

>
> * "Subscribers can be excluded from future publications through
> re-keying for a certain topic. ... How this could be done is out of
> scope for this work."
>
>      Doesn't the same hold for publishers too?
>
[CS: Will clarify with Francesca. But I do agree. I think she thought the
publishers would know the content of
their messages anyway]


>
>      Is there any plan about defining a rekeying approach in this
> document? I think that a basic point-to-point approach can be pretty
> much like the one defined in Section 20 of KGO, although filling the
> 'key' parameter as defined in this document. It might actually be even
> simpler as to the provided content, since you may not need to provide
> additional information to support a recovery for group members that have
> missed a rekeying instance, like KGO has to do.
>
>
[CS: I think we should]


>
> [Section 8.2]
>
> * Shouldn't the field "Profile" indicate both coap_pubsub_app and
> mqtt_pubsub_app ?
>
>
[CS: Yes]

>
> [Appendix A]
>
> * The list of requirements has been greatly revised and extended in
> Appendix A of KG, with new mandatory and optional requirements. So this
> list will need to be re-aligned with the list in KG, and parts of this
> document will need to be revised/extended to say how the updated set of
> requirements is fulfilled
>

[CS: Will do]

>
>
> == Nits ==
>
> [Section 3.2]
>
> s/data model is described/data model described
>
>
> [Section 4.2]
>
> s/singature/signature
>
>
> [Section 6.2]
> s/To be able join/To be able to join
>
>
> [Appendix A]
>
> s/Specity/Specify
>
> s/tranport/transport
>

[CS: Will fix]

>
> --
> Marco Tiloca
> Ph.D., Senior Researcher
>
> Division: Digital System
> Department: Computer Science
> Unit: Cybersecurity
>
> RISE Research Institutes of Sweden
> https://www.ri.se
>
> Phone: [phone]
> Isafjordsgatan 22 / Kistagången 16
> SE-164 40 Kista (Sweden)
>
>
>