[core] Review of draft-ietf-core-oscore-groupcomm-05

[Ludwig Seitz <[e-mail]>]

Hello Marco, Göran, Francesca and Jiye,

here are some review comments on draft-ietf-core-oscore-groupcomm-05:


==
All document:
Update references to OSCORE to point to RFC 8613

==
1.1

Why allow collisions between Group Identifiers managed by a single GM?
This seems unnecessary and dangerous, is there a good use-case where 
this is warranted?

==
2.

Why not require the application to prevent collisions between Gids?

==
2.
"If not already stored in the Recipient Context associated to the 
sender, the recipient retrieves the sender's public key from the Group 
Manager, which collects public keys upon endpoints' joining the   group, 
acts as trusted key repository and ensures the correct association 
between the public key and the identifier of the sender, for instance by 
means of public key certificates."

Note that this requires a device to have two parallel sessions open (one 
with the GM and one on which the message linked to the potentially 
missing public key. This may not always be feasible on very constrained 
devices.

==
2.1.
"After a new Gid has been distributed, a same Recipient ID ('kid')
should not be considered as a persistent and reliable indicator of
the same group member.  Such an indication can be actually achieved
only by verifying countersignatures of received messages."

This is really counter-intuitive and dangerous. Why can identifiers be 
redistributed within the same group? Shouldn't the GM make sure that the 
same identifier is assigned to a group member, even if the security 
context is updated? If the only reliable identifier is the public key, 
why not make the sender/recipient ID a hash of that public key?

==
3.
"... with an Authenticated Encryption with Additional Data (AEAD) algorithm"

AEAD expands to "Authenticated Encryption with Associated Data"

==
3.1
"The external_aad in the Additional Authenticated Data (AAD) is extended 
as follows."

As far as I can tell the external_add is not "in" the AAD, it is the 
AAD, so I'd suggest to rephrase to "The external_aad structure of the 
Additional Authenticated Data (AAD) ..."

==
7.
"2. [...] Such policies can be enforced locally by the Group Manager, or 
by a third party in a trust relation with the Group Manager and 
entrusted to enforce join policies on behalf of the Group Manager."

This feels unnecessary in the context of this document, since it doesn't 
deal at all with the how/where/why of the policies governing access to a 
group. I would suggest to delete this sentence.

==
7.
"3.   Driving the join process to add new endpoints as group members."

This sounds weird. What does "driving the process" mean? Perhaps just 
rephrase to "handle the join process"?

==
7.
"4.   Establishing Security Common Contexts ..."

This part of the sentence sounds like there is an unintentional word 
swap. Did you mean "Establishing the Common Context part of the Security 
Context"?

Note that the term "Security Common Context" appears in several other 
places throughout the document.

==
8.3
"... a key management scheme for secure revocation and renewal of 
Security Contexts and group keying material ..."

Isn't the "group keying material" part redundant? If I understood 
everything correctly it is part of the Security Contexts.

==
8.4 Second paragraph.

If I read this correctly you propose that applications
allow recipients to continue using invalid security contexts for a 
limited time after a security context update. This sounds dangerous and 
unnecessary. Why would we want to propose such a measure? We could adopt 
the equivalent procedure here to the one proposed in the third 
paragraph, that is "If a sender receives a Security Context update 
shorty after having sent a message, it can re-send that message using 
the new Security Context."

==
8.5

Why would an application even allow non-unique group identifiers? This 
makes the whole concept of the group identifier useless. Is there a 
realistic use case where this could happen? Otherwise I suggest removing 
this text.

==
8.9
"Group OSCORE derives the Security Context using the same construction 
of OSCORE"

should be: ...same construction as ...

==
8.10
"Thus, other that in such unlikely case, it cannot be defined that only 
messages with sequence number which are equal to previous sequence 
number + 1 are accepted."

I cannot make sense of this sentence, please rephrase, possibly 
splitting it into several sentences.

==
8.14
"...this parameter explicitly relate two or more communicating endpoints,"

should be "...explicitly relates two ..."

==
9.1 and 9.2

Would be nice if there was a sentence or two explaining the difference 
between the two registries, as it is not immediately obvious.

==
9.3
"The IANA Registry established in this document is defined"

should be "Registries ... are defined"

==
Appendix B 5th bullet

The abbreviation LLN should be expanded.

==
Appendix B 6th bullet

" The latters may reply back ..."

Should be "The latter ..."

==
Appendix B 6th bullet

Expand the abbreviation "MPL"



Regards,

Ludwig



-- 
Ludwig Seitz, PhD
Security Lab, RISE
Phone [phone]