Re: [Last-Call] [Tsv-art] Tsvart last call review of draft-ietf-ace-key-groupcomm-17

Thanks Marco,

I am fine with your provided resolutions/explanations.

After the PR is merged and a new draft version is published, this draft is “Ready” with respect to my review (transport perspective).

Vidhi

> On Dec 15, 2023, at 8:22 AM, Marco Tiloca <marco.tiloca=40ri.se@dmarc.ietf.org> wrote:
> 
> Hello Vidhi,
> 
> Thanks a lot for your review! Please find in line below our detailed replies to your comments.
> 
> A Github PR where we have addressed your comments is available at [PR].
> 
> Unless any concern is raised, we plan to soon merge this PR (and the other ones related to other received reviews), and to submit the result as version -18 of the document.
> 
> Thanks,
> /Marco
> 
> [PR] https://github.com/ace-wg/ace-key-groupcomm/pull/157
> 
> On 2023-10-14 08:07, Vidhi Goel via Datatracker wrote:
>> Reviewer: Vidhi Goel
>> Review result: Ready with Nits
>> 
>> This document has been reviewed as part of the transport area review team's
>> ongoing effort to review key IETF documents. These comments were written
>> primarily for the transport area directors, but are copied to the document's
>> authors and WG to allow them to address any issues raised and also to the IETF
>> discussion list for information.
>> 
>> When done at the time of IETF Last Call, the authors should consider this
>> review as part of the last-call comments they receive. Please always CC
>> tsv-art@ietf.org <mailto:tsv-art@ietf.org> if you reply to or forward this review.
>> 
>> As I am not very familiar with this topic, most of my comments are editorial
>> except a few. My comments are only suggestions to improve the text and I'd
>> happy to discuss them if needed.
>> 
>> COMMENTS:
>> Section 1.1 : “NODENAME: the invariant once established text string used in
>> URIs to identify a member a group.” >> Typo at the end of the sentence.
>> 
>> OLD: to identify a member a group. >> NEW: to identify a member of a group.
> 
> ==>MT
> 
> Fixed as part of the now merged PR 156, see https://github.com/ace-wg/ace-key-groupcomm/pull/156
> 
> <==
> 
>> 
>> Section 2: “Client (C): node that wants to join a group and take part in group
>> communication with other group memebrs.” Typo in the last word- members.
> 
> ==>MT
> 
> Fixed as part of the now merged PR 156, see https://github.com/ace-wg/ace-key-groupcomm/pull/156
> 
> <==
> 
>> 
>> Section 2: “Examples of a Dispatcher are:”
>> Does this need to include anycast addresses?
> 
> ==>MT
> 
> That would be another example of dispatcher, yes. However, in order to not overload the (already too long) text, we don't think that we should be adding it here.
> 
> <==
> 
>> 
>> Section 3.3.1:”in the groups indicated by the transferred acces token as per
>> its 'scope' claim” >> Typo in the word - access.
> 
> ==>MT
> 
> Fixed as part of the now merged PR 156, see https://github.com/ace-wg/ace-key-groupcomm/pull/156
> 
> <==
> 
>> 
>> Section 4.1:
>> For all the resources defined, can authors double check if “Clients may be
>> authorized to access this resource even without being group members” is
>> mentioned for all applicable resources.
> 
> ==>MT
> 
> Good point - we have now added a similar sentence to the /ace-group resource, see https://github.com/ace-wg/ace-key-groupcomm/commit/73c9f7e6b4aab628a185b3ff0d7acc960910a5eb
> 
> <==
> 
>> 
>> Section 4.1.2: “If the request includes unknown or non-expected fields, the
>> handler MUST silently ignore them and continue processing the request.” >> Is this
>> safe to silently ignore such requests? Please double check.
> 
> ==>MT
> 
> We believe this is ok as-is.
> 
> The mentioned sentence comes after a much harder check for malformed messages (due, e.g., to expected fields that are missing). Also, it is followed by a sentence that allows application profiles to be stricter and react with an error in the presence of unexpected/unknown fields.
> 
> <==
> 
>> 
>> Section 4.2.1.1. “In case the joining node only knows the group identifier of
>> the group it wishes to join or about which it wishes to get update information
>> from the KDC” >>  >> Did you mean to say “.. to get updated information..”?
> 
> ==>MT
> 
> Yes, now fixed: https://github.com/ace-wg/ace-key-groupcomm/commit/5278dc8d6c97533bf38baa7c6a1677d3bccc3604
> 
> <==
> 
> 
>>  >>  >> Section
>> 4.4.1.1. Is it possible to include an example of authentication credential
>> request-response for more than 1 group members?
> 
> ==>MT
> 
> Sure, now changed: https://github.com/ace-wg/ace-key-groupcomm/commit/3c8ce8059965da37e647678198e7168edd0868ef
> 
> <==
> 
>> 
>> Section 5:
>> 
>> OLD: “…if it acts as repository of authentication credentials for group
>> members.” NEW: “…if it acts as a repository of authentication credentials for
>> group members.”
>> 
>> Similar text “KDC acts as repository” is present elsewhere and can be updated
>> like above.
> 
> ==>MT
> 
> Now fixed: https://github.com/ace-wg/ace-key-groupcomm/commit/91fd85d4c7a0ff346ba833e52c1a22edf3187df3
> 
> <==
> 
>> 
>> Section 6.1: “This approach consists in the KDC sending one individual rekeying
>> message to each target group member.” The beginning of this sentence doesn’t
>> seem right. Can you double check?
> 
> ==>MT
> 
> Now tried to reformulate, see https://github.com/ace-wg/ace-key-groupcomm/commit/04d4de9e1dd3f65efe89124939f670ede3fdd07a
> 
> <==
> 
>> 
>> Section 6.2.1:  >> OLD: “The used encryption algorithm which SHOULD be the same
>> one used to protect communications in the group.” NEW: “The used encryption
>> algorithm SHOULD be the same one used to protect communications in the group.”
> 
> ==>MT
> 
> Fixed in: https://github.com/ace-wg/ace-key-groupcomm/commit/7bbe27aebdbfd76b8ded27141803fff44b0d27e1
> 
> <==
> 
>> 
>> Section 6.3: Paragraph starting with “In the second case, the sender protects a
>> message using the new group keying material, but the recipient receives that
>> message before having received the new group keying material.” >>  >> Another
>> suggestion to deal with this case: The recipient can store a message that it
>> can’t decrypt temporarily for a limited time so that if it receives new group
>> keying material shortly after, it can then decrypt those stored messages.
> 
> ==>MT
> 
> We have kept the text as is. 
> 
> In principle, the suggested approach is possible, but we don't expect it to be practically enforceable. Typical secure communication protocols are such that a recipient rightly discards an incoming message, if this is not successfully decrypted and verified.
> 
> <==
> 
>> 
>> Section 10. >> DDOS attack possibilities - I can see some possibilities for DDOS
>> where Clients can overwhelm KDC by sending unexpected or unknown fields. There
>> might be more - can authors provide some possible DDOS attack vectors?
> 
> ==>MT
> 
> The second from last paragraph of Section 10.2 (the one starting with "The KDC needs to have a mechanism") already discusses a possible, concrete Denial of Service vector against the KDC, and how the KDC can counteract and mitigate its exploitation.
> 
> More generally, the KDC is expected to be implemented by a host well-equipped in terms of resources and capable to handle a relatively high load. Therefore, we do not expect that Clients can realistically overwhelm the KDC by means of unexpected or unknown fields in their requests.
> 
> Also, a first protection for the KDC is the requirement for a node to have first of all successfully uploaded a valid access token and established a secure communication association.
> 
> With this considered, we do not believe that we need any additional text.
> 
> <==
> 
>> 
>> Thanks,
>> Vidhi
>> 
>> 
> 
> -- 
> Marco Tiloca
> Ph.D., Senior Researcher
> 
> Phone: +46 (0)70 60 46 501
> 
> RISE Research Institutes of Sweden AB
> Box 1263
> 164 29 Kista (Sweden)
> 
> Division: Digital Systems
> Department: Computer Science
> Unit: Cybersecurity
> 
> https://www.ri.se <https://www.ri.se/><OpenPGP_0xEE2664B40E58DA43.asc>_______________________________________________
> Tsv-art mailing list
> Tsv-art@ietf.org
> https://www.ietf.org/mailman/listinfo/tsv-art