Hi Paul! > Hi, > > Thanks for this clear (but long) document. I think overal it is in very > good shape. Most of my comments are minor. > The one thing I did want to highlight is that the use of the REQxx and > OPTxx values seem to be missing some entries, > possibly due to rewrites and getting lost.
Thank you very much for the in-depth review, and apologies for the delayed response. We have submitted a new version that should hopefully address all your comments. We have taken in your suggestions, and added some clarifications for text that we believed could benefit from it, following some of your points. Please see the detailed responses below, plus pointers to each commit. They are all combined in the (now merged) PR: https://github.com/ace-wg/ace-key-groupcomm-oscore/pull/90 new version: https://datatracker.ietf.org/doc/html/draft-ietf-ace-key-groupcomm-oscore-18 diff with last version: https://author-tools.ietf.org/iddiff?url1=draft-ietf-ace-key-groupcomm-oscore-17&url2=draft-ietf-ace-key-groupcomm-oscore-18&difftype=--html Thanks again, Francesca > Section 1 > > > In the CBOR diagnostic notation used in this document, constructs > > of the form e'SOME_NAME' are replaced by the value assigned to > > SOME_NAME in the CDDL model shown in Figure 12 of Appendix C. For > > example, {e'gp_enc_alg': 10, e'sign_alg': -8} stands for {9: > > 10, 10: -8}. > > > > Note to RFC Editor: Please delete the paragraph immediately > > preceding this note. Also, in the CBOR diagnostic notation used > > in this document, please replace the constructs of the form > > e'SOME_NAME' with the value assigned to SOME_NAME in the CDDL > > model shown in Figure 12 of Appendix C. Finally, please delete > > this note. > > Why not leave the curent syntac in the document for readability? During RFC publication, the codepoints listed in Figure 12 of Appendix C will be registered to the corresponding IANA registries. At that moment, the examples in the document can be updated with the actual registered codepoints, together with an actual comment in CBOR diagnostic notation. The paragraph above is asking the RFC Editor to do just that (update the examples with the actual registered values). For example, assuming that the registered points for 'gp_enc_alg' and 'sign_alg' will be 9 and 10 as suggested, then the temporary notation {e'gp_enc_alg': 10, e'sign_alg': -8} will become as below in an example using it: { / gp_enc_alg / 9: 10, / sign_alg/ 10: -8} This approach was also used in the Internet Draft that has recently led to RFC 9770. E.g., see: - https://datatracker.ietf.org/doc/html/draft-ietf-ace-revoked-token-notification-09#figure-7 - https://www.ietf.org/rfc/rfc9770.html#figure-7 > Section 2. > > > All communications between the involved entities MUST be secured. > > > > In particular, communications between the Client and the Group > > Manager leverage protocol-specific transport profiles of ACE > > to achieve [...] > > The first sentence is a bit confusing. How about merging these sentence to: > > > Communications between the Client and the Group Manager MUST > > leverage protocol-specific transport profiles of ACE > > to achieve [...] The "involved entities" are not only the Client and the Group Manager, but also the Authorization Server. Rephrased to clarify that. [6791f9c](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/6791f9c) > Section 3 > > > Furthermore, this document define > > I would remove "Furthermore," > > I would remove "Furthermore," Ok, done. [1c29401](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/1c29401) > > In particular, the following holds for each scope entry: > > I would say: > > > For each scope entry: Ok [1c29401](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/1c29401) > > More specifically, the following applies when, as defined in > > this document, a scope entry includes as set of permissions the > > set of roles to take in an OSCORE group. > > I would rewrte this as: > > > If the set of roles to take in an OSCORE group is included as > > a set of permissions in the scope entry: The problem with the proposed rephrasing is that the "if" is incorrect - the scope entry is always about a set of roles to take in an OSCORE group. At the same time, we think it is important to highlight that this applies to this document, since for example the companion doc draft-ietf-ace-oscore-gm-admin uses the same AIF data model AIF-OSCORE-GROUPCOMM but a different detailed semantics for the scope entries, to express the authorization information for an Administrator. To clarify, we have applied the following change: OLD More specifically, the following applies when, as defined in this document, a scope entry includes as set of permissions the set of roles to take in an OSCORE group. NEW For the application profile of {{RFC9594}} defined in this document, a scope entry includes the name of an OSCORE group and the set of roles to take in that OSCORE group as a set of permissions. Specifically: [1d4058b](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/1d4058b) > Section 4 > > > As also discussed in > > I would remove "also". Ok [38dc879](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/38dc879) > > If the joining node still provides an authentication credential > > in the 'client_cred' parameter of the Join Request (see Section > > 6.1), the Group Manager silently ignores that parameter and the > > related parameter 'client_cred_verify'. > > The question here is whether the Postel Principle is useful here or not. > With > a new deployment, wouldn't it be better to fail to avoid these kind of bad > implementations from ever gaining market share? I am not sure we want to be stricter here. If we were to make this change, we would just be adding one more way to fail at the server, where it is not really necessary. A "bad implementation" would just provide the Group Manager with information that is not required and meaningful to provide in that situation, but ignoring that information does not create any harm and does not alter the way the joining node joins the group. On the other hand, implementers might reuse client implmentations without major changes for monitors. I am just not convinced this change is justified. > > * The joining node is currently a group member, and it is > > re-joining the group not exclusively as monitor. > > > > In this case, the joining node MAY choose to omit the > > 'client_cred' parameter and the 'client_cred_verify' parameter in > > the Join Request, if it intends to use the same authentication > > credential that it is currently using in the group (see Section > > 4.3.1.1 of [RFC9594]). > > I'm a little confused here. The re-join presumbly is to change its role? > But if it previously joined as Monitor, no credentials were given (or > ignored > if given). So re-joining it as not exclusively a Monitor , surely it MUST > include > a credential? > > In the other cases, it must have presented a credential already, so why the > MAY > choose to omit instead of MUST omit? (again the same Postel Principle > question here) There are multiple possible reasons to rejoin the group: changing role in the group is one, another possibility is to obtain a new node identifier (this can be necessary when the group member approaches the exhaustion of the Sender Sequence Number space), or to simply wanting to use a new authentication credential in the OSCORE group. If it previously joined as Monitor, no credentials were given (or ignored if given), as you say, so re-joining it as not exclusively a Monitor, it is expected to include a credential, as specified in the paragraph above: > Upon joining an OSCORE group, a joining node is thus expected to provide its > authentication credential to the Group Manager (see Section 6.1). We have rephrased the two sentences you quoted to hopefully clarify that this exception applies to non-exclusively-monitors group members, in case they want to use the same credentials they were currently using. [e5bc5c0](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/e5bc5c0) [e69656a](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/e69656a) > Section 5 > > > The following considers a joining node [...] > > "following" is not very specific whether it means, subsection, section or > multiple > sections. Why not (as Section 6 does): > > > This Section considers a joining node [...] Ok [a312097](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/a312097) > > for which this document defines the entries in Table 1 > > This modifier makes it confusing what to do when another document adds > entries > to the Group OSCORE Roles registry that are not in Table 1. I would remove > this > text. Ok, text removed. [ff51d0e](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/ff51d0e) > > The AS MUST include the 'expires_in' parameter. Other means for > > the AS to specify the lifetime of access tokens are out of the > > scope of this document. > > Are these in scope for other documents? If so, would those documents undo > this > MUST entry? Or can those documents only add an _additional" lifetime > method. If > there are multiple kinds of lifetimes, what does one do? Pick the shortest > of the > set? This text was included because Section 3.2 of RFC 9594 (on which this is built) says: > In case of successful verification, the Authorization Response sent from the > AS to the Client is also defined in Section 5.8.2 of [RFC9200]. Note that the > 'expires_in' parameter MAY be omitted if the application defines how the > expiration time is communicated to the Client via other means or if it > establishes a default value. But we agree that it is just confusing, and we have removed that text (starting with "Other means...") [d85d551](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/d85d551) > > id' MUST NOT refer to OSCORE groups that are [pairwise | > > signature-only ] > > What action should be taken if the wrong kind of groups are refered to? > Should just > these groups be ignored and others still processed? Or should the entire > request be > dropped? That is our mistake: we used the normative MUST here where there was no need. In fact, the text is supposed to be descriptive of what the Client receives, the Client is not in the position to check whether what the Group Manager has just sent is correct and consistent with how the referred groups are configured. Like RFC 9594 does in its Section 3.3.1 and the present document does in Section 5.3.1, it is better to stick to the facts of to what is specified in 'id'. We have rephrased accordingly. [998092b](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/998092b) > Similar, what to do for violating the MUST in cred_fmt ? (this might be > obvious to > implementers so this might not require new text, Section 6 does give > specific advise > on what to do on errors) This is the same issue: this text is supposed to be descriptive rather than add a normative requirement. Since the text focuses on the value of 'cred_fmt' being acceptable or not and defines a pre-condition for the value to be acceptable, there should be no need for additional text that explains that an unacceptable value must not be accepted by the recipient Client. If 'cred_fmt' is in a Token Transfer Response (see Section 5.3), then that hint from the Group Manager is useless and ignored. If 'cred_fmt' is in a Join Response (see Section 6.3), then the Client is not supposed to continue using Group OSCORE, since the indicated format of authentication credential is not acceptable. That's already the case anyway, as per https://datatracker.ietf.org/doc/html/draft-ietf-core-oscore-groupcomm-26#section-2.4 [1b519ca](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/1b519ca) > > [ As to C509 certificates, the COSE Header Parameters > > 'c5b' and 'c5c' are under pending registration requested by > > draft-ietf-cose-cbor-encoded-cert. ] > > To me the [ square brackets ] makes this look like a note to the RFC > editor, and > not to the reader. I think the [] are better omited. This same applies to > this > same entry in Section 6. Indeed, this was more of an editorial note, which is now outdated, since these registrations have gone through. We have removed this text altogether. [45acc78](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/45acc78) > Section 6 > > > it is RECOMMENDED to use an 8-byte long random nonce. > > Can 8 zero bytes be used? I assume not? So perhaps this needs to > say a little bit more? Or perhaps this RECOMMENDED is a MUST ? We are not sure to understand the comment. Since the nonce is randomly generated, having all its bytes equal to 0x00 is a legitimate possibility, irrespective of the nonce size. Why would that particular case be a problem? If the question is more about recommended size of nonces, those implications are discussed in the security considerations (see Section1 15.2). > > The response MUST have Content-Format set to > > "application/concise-problem- > > details+cbor" [RFC9290] and is formatted as defined in > > Can we ensure with the RFC Editor that the header does not wrap, so > people don't get confused whether "-" is a line wrap or part of the > media type name? (there are more cases that wrap in this section) RFC Editor note added. [315aebf](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/315aebf) > > In order to prevent the acceptance of Ed25519 and Ed448 > > public keys that cannot be successfully converted to > > Montgomery coordinates, and thus cannot be used for > > the derivation of pairwise keys (see Section 2.5.1 of > > [I-D.ietf-core-oscore-groupcomm]), the Group Manager MAY reply > > with a 4.00 (Bad Request) error response in case all the following > > conditions hold: > > Why is this a MAY and not a MUST ? Because, even if the group uses the pairwise mode, that specific joining node might not support the pairwise mode or might not plan to use the pairwise mode in the group, which is fine. In such a case, unless the Group Manager is strict about that, the joining node will still be able to join the group and to use the group mode for sending messages protected with the group mode, signing those with its EdDSA private key, even though the corresponding public key is not eligible to be converted to Montgomery coordinates. However, your comment made us notice that this same handling should have happened also for the POST handler in Section 9.4 "Upload a New Authentication Credential", so we have added a bullet point to cover that. [ed01b52](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/ed01b52) > > The 'cnonce' parameter MUST include a new dedicated nonce N_C > > generated by the joining node. > > Is the Group Manager supposed to track this to enforce? If so, perhaps > explicit text for that should be added or else implementers won't enforce > this. The Group Manager is not supposed to explicitly check whether the new N_C is different from the previous one or not. It is in the interest of the Client to use a different N_C, in order to "salt" in a different way the computation of the proof-of-possession evidence for its own authentication credential and for the Group Manager's. > If the application requires backward security, the Group > Manager MUST generate updated security parameters and group > keying material, and provide it to the current group members, > upon the new node's joining (see Section 11). > > Is this not a race condition? I find the timing is unclearly specified by > the word "upon". Wouldn't the new member we able to read a sporadic message > from the group? (and every logged historic message?) There should be no race conditions: there is some flexibility left to the Group Manager in terms of if the new keying material is distributed to the group before or after it is sent to the new member, but the new member is only provided with this new keying material, so the new member will not be able to read previous messages. > Section 9.9 > > > Upon learning from a 2.05 (Content) Group Status Response that > > the group is currently inactive, the group member SHOULD stop > > taking part in communications within the group, until the group > > becomes active again. > > How can it know if a group becomes active again, unless it is listening to > the group? And if it is listening, isn't it "taking part in communications"? > Can it ignore/prevent group activity and only listen to the Group Manager's > message that a group becomes active again? > > What if a member ignores this SHOULD, what should other members do? ignore > or > act on such messages? It knows by interacting with the Group Manager. So no, it is not listening to the group (or SHOULD NOT), only individually to the Group Manager. If a member doesn't follow this recommendation, its messages will be silently dropped. We have reformulated the text to hopefully clarify that. [812c6f8](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/812c6f8) > Section 10 > > > The Group Manager frees the OSCORE [...] > > > > [...] > > > > The Group Manager deletes the authentication credential [...] > > Is there a difference between "freeing" and "deleting" ? Yes, there is a small difference. It feels weird to think of a "deleted Sender ID". In the first quoted sentences, "frees" means that the Group Manager relinquishes the Sender ID of the leaving node as "taking it away" from that node (other nodes can get that ID in the future). For an authentication credential that the Group Manager is storing, it makes sense to think about the Group Manager deleting that authentication credential. > Once completed the group rekeying process > > Maybe more clear: > > Once the group rekeying process has completed Ok [24069b9](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/24069b9) > There is also some use of "they" to refer to clients, which is better changed to "the client" or "member", etc, depending on context. Right, we have rephrased to avoid using "they". [663e3c4](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/663e3c4) > Appendix.A > > My understanding is that this appendix is the summary of requirements, > so I would expect all REQxx entries in this list to have an entry in > the main document. > > But this is not true for: REQ2, REQ9, REQ16, REQ17, REQ18, REQ19, REQ20 > REQ22, REQ23, REQ24, REQ27 > And: OPT1, OPT4, OP5, OPT6, OPT8, OPT11, OPT12, OPT13 > > Paul Good point, we have added pointers to all missing REQs and OPTs. [2539c47](https://github.com/ace-wg/ace-key-groupcomm-oscore/commit/2539c47)
_______________________________________________ Ace mailing list -- [email protected] To unsubscribe send an email to [email protected]
