Tree Operations test vector includes an Update proposal with leaf_node_source = key_package

[LukaJCB]

According to the spec section 7.3:

f the LeafNode appears in an Update proposal, verify that leaf_node_source is set to update and that encryption_key represents a different public key than the encryption_key in the leaf node being replaced by the Update proposal.

It looks like the 3rd test vector (0002209be6295faf0814054cdef6a80c75a32ea834e634b41da42f5701a6d1f2f0e530205f002e8ef08a200f648ee70fb1c74c3d67b49affd5474607e228bf3e8d5de05e00012028fb5ae1c23e40d0ac1bc2af7239efb36d81265a3bf00ab1b345f010bfdcf0f70200010e0001000200030004eaea0005000604caca1a1a02aaaa0600013a3a0002010000000000000000ffffffffffffffff081a1a058c8a717cc440401c216d950b5fb7eb768d66c011cb6433a0df46e32f5d54f803318ece6b33255264e0a127781167e7837400258c457415d98b73c262baa99ea6ae53d318a50e06) includes an Update proposal with leaf_node_source = key_package.

[raphaelrobert]

It appears you are right. Surprisingly, none of the implementations caught that. While we do check the leaf node source in OpenMLS, we didn't apply that check to the test vectors.

[LukaJCB]

What are the next steps for this one? I could potentially try to generate the test vector myself and submit it as a PR here?

[raphaelrobert]

Sure, that sounds great.

[LukaJCB]

Added a PR here: #196

[bitbltr]

Perhaps there should be more explicit negative tests in the suite so failure modes can be validated? In other words, keep an update proposal with the wrong source, or expired leaf nodes, and require the tests to fail so these don't slip through the cracks.

[LukaJCB]

@bitbltr I think that would be ideal, there are a lot of validations in the protocol and IMO there should be a test vector testing every one of these. Though of course that would be a lot of work

[bitbltr]

@LukaJCB I'm happy to volunteer PRs, but it would be great to have some direction as to how to fit them into the test schema

[LukaJCB]

Perhaps we should open a different issue to discuss