feat(mls-rs): Verify the update path even in case of a self removal

[ManevilleF]

Description of changes:

In the case of a self removal commit we early return before verifying the update_path, This PR drops the early return and self removals verify the update_path and adds some checks in following operations to avoid:

• Applying the update_path for a self removal
• Updating the key schedule for a self removal

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT license.

[mulmarta]

I'm wondering @ManevilleF how much security this check is adding? We're verifying a path that we're going to immediately delete since we're removed and its not useful for us. And note that if we're removed, we anyway can't verify everything inside the commit such as the confirmation tag.

[ManevilleF]

@mulmarta

We are verifying that:

• the UpdatePath is well-formed
• that the committer is valid (unexpired, unrevoked)
• that the committer new LeafNode is also valid (unexpired, unrevoked) also also matches the local policy (with 'valid_successor')

All these checks mostly prevent group fragmentation (in this case ghost clients) since the other unremoved members would simply ignore this commit.

It could also let a member whose credential is revoked but not yet removed from the group to remove anyone he wants.

[mulmarta]

It could also let a member whose credential is revoked but not yet removed from the group to remove anyone he wants.

I didn't actually realize we re-check the credential even if it didn't change, that makes sense.

All these checks mostly prevent group fragmentation (in this case ghost clients)

Just for the sake of understanding, fragmentation by malicious committers is unavoidable. For example, flipping a bit in the last ciphertext on the update path splits the group in half -- clients who don't use this ciphertext (those in the same half of the tree as the committer) accept, others reject.

So if the committer is honest, it won't generate a bad path (if it's a ghost, commit will fail). If it's dishonest, it can fragment anyway. So the space of fragmenting attacks this prevents seems rather narrow?

[ManevilleF]

So if the committer is honest, it won't generate a bad path (if it's a ghost, commit will fail). If it's dishonest, it can fragment anyway. So the space of fragmenting attacks this prevents seems rather narrow?

fragmentation by malicious committers is unavoidable, yet. This just closes one small door even though as you pointed out there's a blatant gateway opened to cause fragmentation.
This could also impact ill-implemented clients (committer commits even though its credential is revoked) so I think it's still worth it