Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new Terminology section. #202

Merged
merged 5 commits into from
Nov 7, 2023
Merged

Conversation

ekr
Copy link
Collaborator

@ekr ekr commented Aug 16, 2023

No description provided.

Copy link
Collaborator Author

@ekr ekr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rohan-wire per our discussion last night, I took a look at this. I suggested some changes and I think there's one cross-PR topic to discuss. I think once we resolve these we should be able to land this.

@@ -272,13 +272,44 @@ all users, for all group sizes, including groups of only two clients.

# General Setting

## Terminology
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps rename this to "Protocol Overview"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

The group is represented using a _ratchet tree_, which represents the members
as the leaves of a tree. It is used to efficiently encrypt to subsets of the
members. Each member has a _LeafNode_ object in the tree holding the client's
identity, credentials, and capabilities.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rohan-wire I think this goes back to our discussion on PR #201, so before I propose changes, let's see if we can agree on general principles and then we can make both this and PR#201 match.

My feeling is that we should:

  1. Not force people to be aware of the notion that this is a tree, as this is confusing without context, but rather talk about the "group state"
  2. Where appropriate reference the "ratchet_tree" value but as carrying the group state.

An approximate rubric, then, would be to use the term "ratchet_tree" but never "ratchet tree" (sans underscore).

WDYT of that?

Copy link
Contributor

@rohan-wire rohan-wire Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This depends on who the primary audience is for the architecture document. Is it primarily for implementers and architects (who need to understand the ratchet tree and its implications), or primarily for operators (who can largely ignore the ratchet tree as a detail)? That said, even for operators, deciding whether Welcome and GroupInfo objects contain a ratchet_tree extension or it gets that separately (and how) is something the operator needs to understand.

I think the idea that MLS uses a type of tree to efficiently reach different subsets/subtrees is important for the first category.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that the important thing to convey here is that there's a data structure here that has credentials for members (plus maybe some related public keys). Rather than invent some new terminology, I would be inclined to just call it a ratchet tree, and maybe say that the description of each member is in a leaf node, but not call out LeafNode specifically.

On another, minor note, the ratchet tree doesn't represent the whole state of the group, since it doesn't capture the key schedule / secret tree that the clients share.

Net of all that, I would rephrase this para something like the following:

The membership of the group is represented using a ratchet tree, in which each member is represented by a leaf node that describes the member's identity, credential, and capabilities. The tree also contains public keys that allow for efficient updates to the group.

And if we do this, I would amend my comment on #201 to replace "public information about the group, such as group members' credentials and related public keys" with "ratchet tree".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest that we remove the term "ratchet" from tree here. Everyone knows what a tree is and ratchet isn't needed here. In the interest of progress I guess I can live with LeafNode.

implement a collection of proposals. Proposals and Commits are collectively
called _Handshake messages_.
A _KeyPackage_ provides keys that can be used to add the client to a group,
including its LeafNode, and _Signature Key_.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you agree with me above, then we can remove "LeafNode" here and replace it with something like "encryption key, credentials, and signature key"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the primary audience is implementers, I think using LeafNode is better because the term will get used in the protocol spec over and over and the idea that a KeyPackage contains one has baffled many implementers I spoke with.

@ekr
Copy link
Collaborator Author

ekr commented Oct 21, 2023

@bifurcation @rohan-wire if you are happy with this, I think it is ready to land.

@beurdouche beurdouche merged commit 59b2d24 into mlswg:main Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants