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@xxxxxxxx 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
