new treemath and encryption test vectors

[franziskuskiefer]

The treemath and encryption test vectors can stay as they are.
I updated some wording and added new test vectors from OpenMLS for this.
Lets make sure we agree on the test vectors before merging.

Note that these are generated from an openmls branch that is not merged yet (openmls/openmls#1236).

[TWal]

I think the "Commit & Transcript" miss a signature public key, because it was moved out of the credential. Right know we can't check the signature of the Commit.

[franziskuskiefer]

Indeed, there are no signatures here. Encryption is only about the symmetric part.

I'll get to the others as well. But feel free to start updating the other test vectors as well.

[mulmarta]

In tree math, shouldn't there be only one root for each case, denoting the root index for n_leaves? The way it's described in the document seems outdated -- there is no (full) tree with i+1 leaves for most i.

[mulmarta]

Is there a reason why the vectors are so long? Wouldn't it be enough to check say trees for up to 32 leaves and encryption for only one generation (say the 15th)?

[franziskuskiefer]

Is there a reason why the vectors are so long? Wouldn't it be enough to check say trees for up to 32 leaves and encryption for only one generation (say the 15th)?

There could indeed be less values... I just took what I had in openmls.
How about changing the encryption test vector to the following, i.e. pull the generation a level in. This way we can have smaller test vectors that still test larger generations.

{
...
  "leaves": [
    {
      "handshake": [ /* array with `generations` handshake keys and nonces */
        {
          "generation": /* uint32 */,
          "key": /* hex-encoded binary data */,
          "nonce": /* hex-encoded binary data */,
          "plaintext": /* hex-encoded binary data */
          "ciphertext": /* hex-encoded binary data */
        },
        ...
      ],
      "application": [ /* array with `generations` application keys and nonces */
        {
          "generation": /* uint32 */,
          "key": /* hex-encoded binary data */,
          "nonce": /* hex-encoded binary data */,
          "plaintext": /* hex-encoded binary data */
          "ciphertext": /* hex-encoded binary data */
        },
        ...
      ]
    }
  ]
}

In tree math, shouldn't there be only one root for each case, denoting the root index for n_leaves? The way it's described in the document seems outdated -- there is no (full) tree with i+1 leaves for most i.

Hm, I think it's correct unless I misunderstand your question. There is only one root for each tree. Maybe the description isn't great...
The first two values n_leaves and n_nodes define the number of leaves/nodes in the tree. The root array then defines the root indices for a tree of size i where i is in [0, ..., n_leaves]. So root[0] is the node index of the root node of a tree of size 1 etc. (there's no root for an empty tree, hence the +1).

[mulmarta]

How about changing the encryption test vector to the following, i.e. pull the generation a level in. This way we can have smaller test vectors that still test larger generations.

I like it!

The root array then defines the root indices for a tree of size i where i is in [0, ..., n_leaves].

We are now using complete binary trees. But root[2] would denote the index of the root node of a tree of size 3 which is not complete. It seems that your implementation uses the "ceiling" and sets root[2] to be the root of a complete tree of size 4?

In any case, my point is that in the current test we test computing the root of a tree with say 2 leaves many times: once for each n_leaves in [2, 4, 8, ...]. It seems enough to test it once, for n_leaves = 2?

[franziskuskiefer]

It seems that your implementation uses the "ceiling" and sets root[2] to be the root of a complete tree of size 4?

Yes.

my point is that in the current test we test computing the root of a tree with say 2 leaves many times: once for each n_leaves in [2, 4, 8, ...].

Yeah, that's duplicated. But it's cheap to do so I don't mind 🤷🏻 But I'd be happy to change the test vector definition if you have a better alternative.

[mulmarta]

(This is my last attempt, it's not a big deal anyway :) ) My alternative would be to have it

...
  "n_leaves": /* uint32 */,
  "n_nodes": /* uint32 */,
  "root": /* uint32 */,
...

where root is the root node index of the tree with n_nodes leaves. This way we test the root for each tree size once.

[franziskuskiefer]

My alternative would be to have it

Ah, yes of course. That makes sense. I added that change to this PR.