Refactor Group Joining and fix External Inits #529
Conversation
There was a problem hiding this comment.
So basically, we're converging the internal and external join cases as a result of fixing a bug in external join. Seems like a fine idea.
I will note that this promotes GroupInfo into a "public" / "on-the-wire" object, as opposed to always being wrapped in a Welcome. In the internal join case, the GroupInfo is always sent encrypted to the new joiner; in the external join case, it isn't necessarily encrypted (up to the application). Doesn't need to be a blocker, but might be worth elaborating on in an implementation document.
Marking as approved, but I would pretty strongly prefer that external_pub be in an extension vs. an optional field.
draft-ietf-mls-protocol.md
Outdated
| Extension group_context_extensions<0..2^32-1>; | ||
| Extension other_extensions<0..2^32-1>; | ||
| HPKEPublicKey external_pub; | ||
| MAC confirmation_tag; | ||
| optional<HPKEPublicKey> external_pub; |
There was a problem hiding this comment.
I would probably make this an extension (in other_extensions, like ratchet_tree) instead of an optional field.
Co-authored-by: Richard Barnes <rlb@ipv.sx>
…otocol into refactor_pgs_and_welcome
|
Thanks for the suggestions! I agree that it's a bit nicer to have an extension rather then an optional field. |
draft-ietf-mls-protocol.md
Outdated
| Extension group_context_extensions<0..2^32-1>; | ||
| Extension other_extensions<0..2^32-1>; | ||
| MAC confirmation_tag; | ||
| optional<HPKEPublicKey> external_pub; |
There was a problem hiding this comment.
Since it's now an extension, do we still need it here?
There was a problem hiding this comment.
Ah, sorry. Forgot to remove that one. Thanks!
|
I agree it's good to align |
This PR refactors the two sections on Group Joining (via Welcome and via External Init) by changing the
GroupInfostruct that can be used for both cases. Theexternal_pubused in the course of an External Init is added as an optional field. This PR also:confirmed_transcript_hashandconfirmation_tagis now used for external inits instead of theinterim_transcript_hash, which was not sufficient to create a valid commit