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." [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 [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. *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. [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. [Section 5.3.1] s/Acces Token/Access Token *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 *"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. [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 * "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. *"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." [Section 6.1] s/a 8-byte long/an 8-byte long [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". *"possibly replacing the currently stored value." Could be better to say "replacing the currently stored value (if any)" [Section 6.3] s/meterial/material *"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. [Section 6.4] s/group occurred prior/group that occurred prior [Section 7] s/prior its joining/prior to its joining *"Extension of group lifetime" Should this not be "Expiration of group lifetime"? That is the occasion when the Group Manager MUST rekey. [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." [Section 8.5] s/These data are provided/This data is provided s/to support only to signature verifiers/to support only for signature verifiers [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? [Section 9.4] s/in the same way taken when/in the same way as when [Section 9.6] s/These parameter MUST NOT/These parameters MUST NOT *"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? [Section 9.9] s/the the OSCORE group/the OSCORE group *Looks like the example message exchange is missing Content-Format: "application/ace-groupcomm+cbor" [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? *"join the group as a candidate group member" Better with simply "join the group as a new group member"? [Section 11] s/all of which associated with/all 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." [Section 11.2] s/Once received the/After having received the [Section 11.3] s/MUST request the Group Manager for the authentication/MUST request from the Group Manager the authentication [Section 11.3.1] s/These data are provided/This data is provided s/recent set is the one associate to/recent set is the one associated to *Looks like the example message exchange is missing Content-Format: "application/ace-groupcomm+cbor" [Section 12] s/that has an own/that has its own [Section 13] s/trying re-sending/trying to re-send [Section 15.1] s/one ore more/one or more *"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. [Section 15.2] *"joinings of OSCORE groups" Should it be be "joinings of OSCORE group members", or "joinings to OSCORE groups"? [Section 16.3] s/these objective/these objectives s/point assignment/point assignments [Appendix B.1] *State that the example for ecdh_info_entry is in CDDL notation. Also, in this example the following fix should be needed: cred_fmt = int / null -> cred_fmt : int / null
_______________________________________________ Ace mailing list Ace@ietf.org https://www.ietf.org/mailman/listinfo/ace
