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...@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-...@ietf.org <mailto:tsv-...@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-...@ietf.org > https://www.ietf.org/mailman/listinfo/tsv-art
_______________________________________________ Ace mailing list Ace@ietf.org https://www.ietf.org/mailman/listinfo/ace
