Hi Rikard,

Thanks a lot for your review! I've addressed your comments in version -16 [1].

Please see detailed replies inline.

Best,
/Marco

[1] https://datatracker.ietf.org/doc/html/draft-ietf-ace-key-groupcomm-oscore-16

On 2023-03-01 22:05, Rikard Höglund wrote:
Hello.

As part of the shepherding process I have performed a review of draft-ietf-ace-key-groupcomm-oscore-15. Overall I think the document is in a good state, see my feedback below.

Best
Rikard Höglund



[Terminology]

*It could be good to explain the REQX terminology used throughout. Perhaps in the following paragraph? (The REQX terminology also comes from ietf-ace-key-groupcomm).

"The terms and concept related to the message formats and processing specified in [I-D.ietf-ace-key-groupcomm], for provisioning and renewing keying material in group communication scenarios."

==>MT
Added the following sentence to the second bullet point of Section 1.1 "Terminology".

"These include the abbreviations REQx and OPTx denoting the numbered mandatory-to-address and optional-to-address requirements, respectively."
<==



[Section 3]

*The following CDDL fails to validate using CDDLC (https://github.com/cabo/cddlc):

/   AIF-OSCORE-GROUPCOMM = AIF-Generic<oscore-gname, oscore-gperm>/
/
/
/   oscore-gname = tstr  ; Group name/
/   oscore-gperm = uint . bits group-oscore-roles/
/
/
/   group-oscore-roles = &(/
/      Requester: 1,/
/      Responder: 2,/
/      Monitor: 3,/
/      Verifier: 4/
/   )/

Suggested fix:
. bits group-oscore-roles -> .bits group-oscore-roles


==>MT
Fixed.
<==


[Section 4]

*"In particular, one of the following four cases can occur when a new node joins an OSCORE group."

General comment on the use of "in particular", it is used a lot in the document and seems like it can be removed for many of the instances without loss of meaning.

==>MT
Removed several occurrences (down from 53 to 8).
<==


*It would be nice to simplify the 4 cases, and sub-cases, that can happen when a node joins a group. It seems to me there are 3 cases: 1. The joining node is joining as monitor and does not have to provide credentials. 2. The GM already has the credentials for the joining node, either from a previous joining or the secure communication association establishment. 3. The GM does not have the authentication credentials, so they must be sent by the joining node.

==>MT
After internal discussion the four cases are actually fine and complete. What was not fine was the text right above those, as it wrongly suggests that they are mutually exclusive. Rephrased as follows:

OLD:
In particular, one of the following four cases can occur when a new node joins an OSCORE group.

NEW:
In particular, the following applies when a node joins an OSCORE group.
<==



[Section 5.3]

*"If the 'sign_info' parameter is present in the response, the following applies for each element 'sign_info_entry'."

For some elements in the list following that sentence the CBOR type is specified, but for some it is not. Either specify it for all, or let it be silently inherited from ace-key-groupcomm for all.

==>MT
The type has been removed and is silently inherited from draft-ietf-ace-key-groupcomm.
<==



[Section 5.3.1]

s/Acces Token/Access Token

==>MT
Fixed.
<==



*The following CDDL fails to validate using CDDLC (https://github.com/cabo/cddlc):

/ecdh_info = ecdh_info_req / ecdh_info_resp/
/
/
/ecdh_info_req = null      ; in the Token Transfer/
/    ; Request to the/
/    ; Group Manager/
/
/
/ecdh_info_res = [ + ecdh_info_entry ] ; in the Token Transfer/
/    ; Response from the/
/    ; Group Manager/
/
/
/ecdh_info_entry =/
/[/
/  id : gname / [ + gname ],/
/  ecdh_alg : int / tstr,/
/  ecdh_parameters : [ any ],/
/  ecdh_key_parameters : [ any ],/
/  cred_fmt = int/
/]/
/
/
/gname = tstr/

Suggested fix:
cred_fmt = int -> cred_fmt : int


==>MT
Fixed.
<==


*"Each element contains information about ECDH parameters as well as about authentication credentials and public keys, for one or more OSCORE groups that use the pairwise mode of Group OSCORE and that the Client has been authorized to join. Each element is formatted as follows."

In the list following this section, it can be good to make it clear already in bullet 1 that 'id' contains on or several 'gname' items.

==>MT
Added the following sentence within the bullet point about the 'id' element.

"In the following, each specified group name is referred to as 'gname'."
<==



[Section 5.3.2]

*The following CDDL fails to validate using CDDLC (https://github.com/cabo/cddlc):

/kdc_dh_creds = kdc_dh_creds_req / kdc_dh_creds_resp/
/
/
/kdc_dh_creds_req = null           ; in the Token Transfer/
/          ; Request to the/
/          ; Group Manager/
/
/
/kdc_dh_creds_res = [ + kdc_dh_creds_entry ] ; in the Token Transfer/
/          ; Response from the/
/          ; Group Manager/
/
/
/kdc_dh_creds_entry =/
/[/
/  id : gname / [ + gname ],/
/  cred_fmt = int,/
/  cred = bstr/
/]/
/
/
/gname = tstr/

Suggested fix:
cred_fmt = int, -> cred_fmt : int,
cred = bstr -> cred : bstr


==>MT
Fixed.
<==


* "Acceptable values denote a format that MUST explicitly provide the public key as well as comprehensive set of information related to the public key algorithm, including, e.g., the used elliptic curve (when applicable). "

Can this sentence be split or rephrased? It is a bit hard to parse currently.

==>MT
Rephrased as follows (three similar instances).

"Acceptable values denote a format that MUST explicitly provide the public key as well as a comprehensive set of information related to the public key algorithm. This information includes, e.g., the used elliptic curve (when applicable)."
<==


*"The Group Manager has specifically a Diffie-Hellman authentication credential in an OSCORE group, and thus a Diffie-Hellman public key in that group, if and only if the group is a pairwise-only group."

Suggested rephrasing:
"The Group Manager has a Diffie-Hellman authentication credential in an OSCORE group only if the group is a pairwise-only group."

==>MT
Rephrased as follows:

OLD:
The Group Manager has specifically a Diffie-Hellman authentication credential in an OSCORE group, and thus a Diffie-Hellman public key in that group, if and only if the group is a pairwise-only group.

NEW:
The Group Manager has a Diffie-Hellman authentication credential in an OSCORE group if and only if the group is a pairwise-only group.
<==



[Section 6.1]

s/a 8-byte long/an 8-byte long

==>MT
Fixed (multiple occurrences)
<==



[Section 6.2]

*"following list: "requester", "responder", "monitor", ("requester", "responder")"

Can it be made clearer here, or earlier, that certain combinations of roles are not allowed when joining? Like trying to join with roles ("monitor, responder"), or using "validator".

==>MT
Revised as follows.

In Section 6.1 "Send the Join Request", extended the bullet point about 'scope' with the following text.

"The 'scope' parameter MUST NOT specify any of the following sets of roles: ("requester", "monitor") and ("responder", "monitor"). Future specifications that define a new role for members of OSCORE groups MUST define possible sets of roles (including the new role and existing roles) that are not acceptable to specify in the 'scope' parameter of a Join Request."

In Section 6.2 "Receive the Join Request", rephrased the bullet point about 'scope' as follows.

OLD:
The 'scope' parameter is not present in the Join Request, or it is present and specifies any set of roles not included in the following list: "requester", "responder", "monitor", ("requester", "responder"). Future specifications that define a new role for members of OSCORE groups MUST define possible sets of roles (including the new role and existing roles) that are acceptable to specify in the 'scope' parameter of a Join Request.

NEW:
"The 'scope' parameter is not present in the Join Request, or it is present and specifies any of the following sets of roles: ("requester", "monitor") and ("responder", "monitor")."
<==



*"possibly replacing the currently stored value."

Could be better to say "replacing the currently stored value (if any)"

==>MT
Fixed.
<==



[Section 6.3]

s/meterial/material

==>MT
Fixed.
<==


*"If the joining node has not taken exclusively the role of monitor, the Group Manager performs also the following actions."

If the node joined as monitor it cannot be anything more right? I would rephrase as: "If the joining node is not a monitor, the Group Manager also performs the following actions."
The first bullet right after also repeats the condition again.

Other parts of the text also uses similar phrasing to "exclusively the role of monitor". Better to make it clear that it is an exclusive role earlier in the text once and for all.

==>MT
Kept as is. This is intended to leave the door open for future combinations of roles that include "monitor" and such that a different behavior is intended.
<==



[Section 6.4]

s/group occurred prior/group that occurred prior

==>MT
Fixed.
<==



[Section 7]

s/prior its joining/prior to its joining

==>MT
Fixed.
<==


*"Extension of group lifetime"

Should this not be "Expiration of group lifetime"? That is the occasion when the Group Manager MUST rekey.

==>MT
Text updated, also to be an outer paragraph instead of a bullet point.

OLD: (as a bullet point)
Extension of group lifetime - That is, the group is rekeyed when the expiration time for the group keying material approaches or has passed, if it is appropriate to extend the group operation beyond that.

NEW: (not as a bullet point)
When the expiration time for the group keying material approaches or has passed, the Group Manager may want to extend the secure group operation, as considered appropriate. If the Group Manager does so, the Group Manager MUST rekey the group.
<==



[Section 7.1]

*Maybe this section could be written in a plainer way, and not as an algorithm. From what I understand:

"For every version of the keying material the GM should keep a set containing Sender IDs of nodes that left or changed Sender ID. This set is distributed to group members upon group rekeying. If the GM has to delete sets due to space-constraints it MUST start with the set associated with the oldest version of the keying material."

==>MT
Section 7.1 has been revised, to be less algorithmic and more factual, and to not refer to a "collection of sets".

Sentences in other sections of the document have been consistently rephrased, see Sections 6.3, 9.2, 10, 11, 11.1, 11.3.1, and 14.1.
<==



[Section 8.5]

s/These data are provided/This data is provided

==>MT
Fixed
<==


s/to support only to signature verifiers/to support only for signature verifiers

==>MT
This should be correct; left as is.
<==



[Section 9.2]

*"2. Otherwise, the Group Manager takes one of the following actions."

How does the GM determine which of the actions to take?

==>MT
Left as is.

This is covered by the explanation of the first option (group rekeying), which normally does not happen (unless already intended anyway for other reasons). That is, normally, the second option happens, i.e., the Group Manager provides a new Sender ID.
<==



[Section 9.4]

s/in the same way taken when/in the same way as when

==>MT
Fixed
<==



[Section 9.6]

s/These parameter MUST NOT/These parameters MUST NOT

==>MT
Fixed
<==


*"Figure 4 shows an example."

Can this be expanded to say it shows an example of a response and request, or a bit more description?

==>MT
Fixed (also for three more occurrences later on).
<==



[Section 9.9]

s/the the OSCORE group/the OSCORE group

==>MT
Fixed
<==


*Looks like the example message exchange is missing Content-Format: "application/ace-groupcomm+cbor"

==>MT
Not applicable here; the request has no payload, and the response is not transporting ACE Groupcomm parameters.
<==



[Section 9.10]

*Any reason it's not possible to get information for all the groups a client has access rights to with a GET? Perhaps due to inheritance from draft-ietf-ace-key-groupcomm?

==>MT
Per the KDC interface inherited from draft-ietf-ace-key-groupcomm, this resource supports only FETCH requests, and a GET handler is not needed (as it would be enabling something along the lines of group discovery, which is handled differently in other documents).
<==


*"join the group as a candidate group member"

Better with simply "join the group as a new group member"?

==>MT
Rephrased as follows

OLD
Then, it can use that information to either join the group as a candidate group member, get ...

NEW
Then, it can use that information to join the group, or get ...
<==



[Section 11]

s/all of which associated with/all associated with

==>MT
Rephrased as "all of which are associated with"
<==



[Section 11.1]

*"The CBOR array has to be intended as a set, i.e., the order of its elements is irrelevant."

Suggested rephrasing:
"The order of elements in the CBOR array is irrelevant."

==>MT
Fixed (also in one more, following occurrence).
<==



[Section 11.2]

s/Once received the/After having received the

==>MT
Fixed
<==



[Section 11.3]

s/MUST request the Group Manager for the authentication/MUST request from the Group Manager the authentication

==>MT
Fixed
<==



[Section 11.3.1]

s/These data are provided/This data is provided

==>MT
Fixed
<==


s/recent set is the one associate to/recent set is the one associated to

==>MT
Fixed (as "... associated with")
<==


*Looks like the example message exchange is missing Content-Format: "application/ace-groupcomm+cbor"

==>MT
Not applicable here; the request has no payload, and the response is not transporting ACE Groupcomm parameters.
<==



[Section 12]

s/that has an own/that has its own

==>MT
This is as intended; kept as is
<==



[Section 13]

s/trying re-sending/trying to re-send

==>MT
Fixed
<==



[Section 15.1]

s/one ore more/one or more

==>MT
Fixed
<==


*"It is up to the application to decide how to handle such collisions of Group Identifiers, e.g., by trying to process the incoming message using one Security Context at the time until the right one is found."

Perhaps the application can also take advantage of the KID in the case of requests? That ID may not exist in one of the groups.

==>MT
Kept as is, as the above is actually covered by the current text. A 'kid' would not be found among the Recipient IDs within a Group OSCORE Security Context indicated by the received Gid for which the processing fails.
<==



[Section 15.2]

*"joinings of OSCORE groups"

Should it be be "joinings of OSCORE group members", or "joinings to OSCORE groups"?

==>MT
Fixed as "joinings to OSCORE groups".
<==



[Section 16.3]

s/these objective/these objectives

==>MT
Fixed
<==


s/point assignment/point assignments

==>MT
Fixed
<==



[Appendix B.1]

*State that the example for ecdh_info_entry is in CDDL notation.

==>MT
Before Figure 13, added:

"The CDDL notation [RFC8610] of the 'ecdh_info_entry' parameter is given below."
<==

Also, in this example the following fix should be needed:
cred_fmt = int / null -> cred_fmt : int / null


==>MT
Fixed
<==



_______________________________________________
Ace mailing list
Ace@ietf.org
https://www.ietf.org/mailman/listinfo/ace

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

Attachment: OpenPGP_0xEE2664B40E58DA43.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

_______________________________________________
Ace mailing list
Ace@ietf.org
https://www.ietf.org/mailman/listinfo/ace

Reply via email to