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

Use path secret instead of full DirectPath #295

Merged
merged 2 commits into from Feb 6, 2020
Merged

Use path secret instead of full DirectPath #295

merged 2 commits into from Feb 6, 2020

Conversation

bifurcation
Copy link
Collaborator

@bifurcation bifurcation commented Feb 1, 2020

This PR adapts the Welcome logic so that the new joiners get their path secrets directly, rather than from a DirectPath. This cleans things up in a few ways. The new joiners no longer have to have the context for the previous epoch (which was used to encrypt the DirectPath). And we can get away with one HPKE operation per new joiner instead of two (one for the EncryptedKeyPackage and one for the DirectPath).

The major cost is that there's a bit more complexity in the TreeKEM. Instead of just producing and consuming DirectPaths, you now need to know which path secrets go with which nodes. You also need tree math to identify the common ancestor of two leaves, but there's a simple closed-form formula for that, expressed in the Python code here.

Corresponding PR on the Go implementation.

@bifurcation bifurcation requested a review from raphaelrobert Feb 1, 2020
@bifurcation
Copy link
Collaborator Author

@bifurcation bifurcation commented Feb 1, 2020

@raphaelrobert - Curious if this seems implementable to you. Clearly it's possible, since I did it in Go :) But it also needs to make sense to other people.

message from the encrypted GroupInfo object and the encrypted key packages.
* For each new member in the group:
* Identify the lowest common ancestor in the tree of the new member's
leaf node and the member sending the Commit
Copy link
Member

@raphaelrobert raphaelrobert Feb 1, 2020

Choose a reason for hiding this comment

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

I think there might be more math involved here. Unless it's a group of two, the direct path of the new member should be blank until it intersects with the direct path of the adder (who populated their path with the Commit). In other words, the common ancestor node has to be resolved, leading to an array of resolved nodes. The new member should be one of these resolved nodes.
To me this looks like you end up KEMing to the new member's leaf node directly.

Copy link
Collaborator Author

@bifurcation bifurcation Feb 1, 2020

Choose a reason for hiding this comment

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

You're kind of making a point I should have made, namely that sending the path secret of the common ancestor as we are here is equivalent to sending the direct path -- in the direct path encryption, you would encrypt the path secret to the corresponding copath node, which would ultimately resolve to the leaf node (and possibly some others).

What this PR is doing is taking the KEMing to the new member's leaf and taking it out of the context of the DirectPath. Where before you were doing an HPKE to the new member once for EncryptedKeyPackage and again in the DirectPath, now you just do it for the EncryptedKeyPackage.

Copy link
Member

@raphaelrobert raphaelrobert Feb 1, 2020

Choose a reason for hiding this comment

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

Sure, I think that makes sense.

But there might be another issue: I forgot that we don't blank the direct path of new members anymore and instead use unmerged_leaves. This list should now be shorter and only include nodes from leaf to common ancestor. This is however something every member in the group processing the Add has to calculate as it is non explicit.

Copy link
Collaborator Author

@bifurcation bifurcation Feb 4, 2020

Choose a reason for hiding this comment

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

This list should now be shorter and only include nodes from leaf to common ancestor

This is already true. Even before this PR, the DirectPath processing would reset the unnmerged_leaves arrays to [] for nodes in the direct path of the committer.

Copy link
Collaborator Author

@bifurcation bifurcation Feb 4, 2020

Choose a reason for hiding this comment

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

In any case, we should be clear that you reset unmerged_leaves whenever you write to a node. I don't think that needs to be blocking on this PR.

return L + R
# The common ancestor of two leaves is the lowest node that is in the
# lowest-level node that is in the direct paths of both leaves.
def common_ancestor(x, y):
Copy link
Member

@raphaelrobert raphaelrobert Feb 1, 2020

Choose a reason for hiding this comment

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

For simplicity I would just iterate over parent() until the value is equal.

Copy link
Collaborator Author

@bifurcation bifurcation Feb 1, 2020

Choose a reason for hiding this comment

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

That actually doesn't work, because the distance to the parent might not be equal. Consider, for example, the three-member tree:

    ABC
   /   \
 AB     |
 / \    |
A   B   C

The common ancestor of A and B is ABC = parent(parent(A)) = parent(C).

You would have to first compute the direct path of one node, then iterate parent() on the other until you end up in the direct path. The formula here is a little bit of index magic, but I think it can be proven correct, and it's simpler/faster.

Copy link
Member

@raphaelrobert raphaelrobert Feb 1, 2020

Choose a reason for hiding this comment

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

True, my idea only works reliably on the left part of the tree. On the right part you'd have to also use level() to skip some nodes. If you're confident your math works, it might just be simpler to use that.

Copy link
Collaborator Author

@bifurcation bifurcation Feb 3, 2020

Choose a reason for hiding this comment

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

The algorithm passes tests on a non-trivial tree, so I have pretty good confidence that it works. Proof sketch:

  • Each subtree of the tree is identified by a common prefix
    • The root of the subtree is <prefix>0<1 * level>
    • The nodes within the subtree are <prefix>0...0 to 1...10
  • Therefore every common ancestor of A and B will have a prefix that is common to A and B, since a common ancestor is the root of a subtree containing both nodes
  • This algorithm finds the longest prefix shared by A and B, then computes the root of the corresponding subtree

Of course, all of this is just a recommended implementation. Implementations are free to use a more explicit algorithm like yours if they prefer.

@bifurcation bifurcation added the today! (?) label Feb 4, 2020
@bifurcation bifurcation merged commit db9ab6a into master Feb 6, 2020
0 of 5 checks passed
* Tree: The group's ratchet tree after the commit has been applied
* Prior confirmed transcript hash: The confirmed transcript hash for the
current state of the group (not the provisional state)

* Create a DirectPath using the new tree (which includes any new members). The
GroupContext for this operation uses the `group_id`, `epoch`, `tree`, and
`prior_confirmed_transcript_hash` values in the initial GroupInfo object.
Copy link
Contributor

@ericcornelissen ericcornelissen Feb 7, 2020

Choose a reason for hiding this comment

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

This change leaves a reference to a GroupInfo object that is not mentioned before. If I'm not mistaken, I think this can be rewritten as:

Create a DirectPath using the new tree (which includes any new members). The GroupContext for this operation uses the group_id, epoch, and tree in the provisional GroupContext and the confirmed_transcript_hash for the current state of the group (not the provisional state).

On this note, I'm not entirely sure I understand why an entire GroupContext (specifically the group_id, epoch, and confirmed_transcript_hash) is needed to compute a DirectPath...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
today! (?)
Development

Successfully merging this pull request may close these issues.

None yet

3 participants