mlswg / mls-implementations

Coordination of implementation and interop specific details
113 stars 14 forks source link

transcript hash update #96

Closed franziskuskiefer closed 1 year ago

franziskuskiefer commented 1 year ago

There are lots of different ways to do this and some of the names are confusing. But here's a proposal.

We can keep only the authenticated_content and skip some of the other information if no one pieces it together.

franziskuskiefer commented 1 year ago

Is there something else that needs testing here? Or can we strip this down?

I agree, there's more in here than needed. But I figured it might be easier for some to assemble the structs by hand rather than deserialising. (Like OpenMLS doesn't implement deserialising of structs like AuthenticatedContent.) But I pushed a simplified version that's testing the same thing. I kept the confirmation key to assemble the InterimTranscriptHashInput instead of giving the input directly. Seems easy enough and tests the confirmation tag computation here as well.

bifurcation commented 1 year ago

BTW, wrt deserializing AuthenticatedContent -- you could just include an MLSMessage(MLSPlaintext(Commit)) (which you'll need to be able to deserialize anyway), and that would have everything, including the confirmation_tag. You could still include the confirmation_key to verify the confirmation_tag.

bifurcation commented 1 year ago

I went ahead and prototyped this in MLSpp with (a) MLSMessage and (b) interim_transcript_hash_before. It works, with a test vector of the following form:

  {
    "cipher_suite": 1,
    "confirmation_key": "83f50b3e27e96e272242c56462b82fadf2c948725f945b6b5931ff679c6c7eb5",
    "interim_transcript_hash_before": "5937ab5f06bda8a011c709f7012c57638b61191d6ea997082ca94b06458e5d59",

    "message": "/* serialized MLSMessage */"

    "confirmed_transcript_hash_after": "b977fd60192426f2db907171dd6738d566836145d753e6d98216ab7bf02e29e4",
    "interim_transcript_hash_after": "0ddfee2a640b880deec5bc0e1611d973e8466869548220d114d4d89d29fc398a"
  },
franziskuskiefer commented 1 year ago

you could just include an MLSMessage(MLSPlaintext(Commit))

Building full message is much more involved (in OpenMLS) than just building the content because it would do all the other checks etc. as well. I'd prefer keeping this tv simple and not wrap the authenticated content. (Having the deserialisation of the authenticated content for tests is simple in contrast.) Also, this would require us adding the membership tag. I'm not sure if this really helps this tv.

(b) interim_transcript_hash_before

I don't mind adding another round of interim transcript hash. But I wouldn't call it before. It's the one from the current epoch.