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

Fix invalid parent hash with empty root left subtree #430

Merged
merged 9 commits into from
Aug 20, 2024

Conversation

birarda
Copy link
Contributor

@birarda birarda commented Aug 20, 2024

Fixes a bug with parent hash generation when the left subtree of the root node is empty.

If left subtree of the root node becomes empty (for example, when first two members are removed by a third group member) the parent hash computed during TreeKEMPublicKey::update will be invalid. A subsequent welcome will also have an invalid tree, and the welcome is unprocessable.

This occurred because parent_hashes assumed that the root node was always present in the filtered direct path, and therefore used the root node as the initial parent when doing a top-down generation of parent hashes.

@bifurcation
Copy link
Contributor

Thanks for finding this. Just to recap / restate the issue here:

The problem arises when the whole left subtree of the ratchet tree is empty:

      _ = G
      |
    .-+-.    
   /     \   
  _       F
 / \     / \ 
_   _   C   D

This can arise, for example, via the following sequence of events:

  • A creates a group and adds B, C, D
  • C removes A and B

In this case, right now, the algorithms for creating and verifying parent hashes disagree. On the creation side, the parent_hashes function assumes that the top node of the filtered direct path is the root, so the top parent-hash link it forms is from the penultimate node to the root -- not to its actual parent. When someone goes to verify this tree, they will find that the top-level node in the committer's direct path does not have a valid parent-hash link to either of its children, since the link was instead built to the root, and thus that the tree is parent-hash invalid.

In the above example tree, C.parent_hash would be the parent hash over G and the blank left subtree. It should be over F and the (one-node) subtree at D.

Copy link
Contributor

@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.

Couple of minor simplifications to the test case, but otherwise LGTM.

It seems like we could actually do this as a test case on TreeKEMPublicKey::update, but we can do that in a follow-on.

test/state.cpp Outdated Show resolved Hide resolved
test/state.cpp Outdated Show resolved Hide resolved
@birarda birarda changed the title Fix for invalid parent hash when root left subtree is empty Fix invalid parent hash with empty root left subtree Aug 20, 2024
@bifurcation bifurcation merged commit fd3fc39 into cisco:main Aug 20, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants