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 new_proposal_member SenderType #716

Merged
merged 3 commits into from Jun 2, 2022

Conversation

rohan-wire
Copy link
Contributor

Add new new_proposal_member SenderType and rename new_member to new_commit_member.

This restores the External Proposal Add functionality which was present in the protocol for some years but clarifies how the authorization and signing takes place in this case.

Comment on lines 1436 to 1437
new_commit_member(3),
new_proposal_member(4),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I would flip these around, new_member_commit and new_member_proposal.

Comment on lines 1450 to 1451
case new_proposal_member:
KeyPackageID proposed_member;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can just be struct{}, since because the KeyPackage you apply is the one in the Add proposal.

Comment on lines 1566 to 1568
case new_proposal_member:
// The KeyPackage included in embedded Add proposal
KeyPackage key_package;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't seem all that useful, since the KeyPackage is already being signed via the Add proposal. I would just make this struct{}.

@bifurcation
Copy link
Collaborator

Interim 2022-06-02:

  • Could punt to extensions, but this is partly just because we moved a little too fast on the removal
  • No objections to reverting
  • Once conflicts and comments resolved, good to merge

@bifurcation
Copy link
Collaborator

Closes #693

Copy link
Collaborator

@bifurcation bifurcation left a comment

Choose a reason for hiding this comment

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

LGTM, though would still prefer new_member_proposal etc. One minor suggestion to lock down the content.

draft-ietf-mls-protocol.md Outdated Show resolved Hide resolved
…er to new_member_commit and specify proposal type must be an Add
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.

None yet

2 participants