oak icon indicating copy to clipboard operation
oak copied to clipboard

Associated data API for encryption is not very ergonomic

Open tiziano88 opened this issue 1 year ago • 2 comments

https://github.com/project-oak/oak/blob/e9be418ca9b6ae820b4cfc5fb5ba9a19a0ec736d/oak_crypto/src/encryptor.rs#L196-L202

Currently, additional authenticated data is passed in by the caller when encrypting, then stored alongside the encrypted message, and then returned to the caller when decrypting. This is not how it is commonly done in most other crypto libraries (e.g. https://docs.rs/ring/latest/ring/aead/struct.OpeningKey.html ); instead, the associated data is explicitly passed in when decrypting, since it usually comes from somewhere else entirely (e.g. a user id from an auth token, or a digest of some other piece of data known to the caller). In my experience, it is highly unusal for the additional data to be an actual self-contained byte string that needs to be stored alongside the ciphertext.

My suggestion is to:

  1. change the Rust API to require the caller to pass in the additional data when decrypting
  2. remove the associated_data field from the proto entirely (no need to shift proto field numbers around, we can just mark as reserved)

I don't believe anyone is actively using it anyways (but let me know if you know of any use case).

@ipetr0v @jul-sh @fernandolobato @conradgrobler WDYT?

tiziano88 avatar Feb 01 '24 20:02 tiziano88

As per Rust SDK we were planning to return only the decrypted message from the decrypt function (i.e. a vec). And since the EncryptedMessage already contains an associated data, we won't need to pass it separately to the decrypt function.

Though in order to extract the associated data, prod teams would need to parse the Proto message.

The plan was also to use https://docs.rs/aead/latest/aead/struct.Payload.html for encryption.

ipetr0v avatar Feb 02 '24 11:02 ipetr0v

My (unverified) assumption is that clients would not normally expect the AAD to be stored alongside the ciphertext -- perhaps it would come from somewhere else entirely (like an auth token or some other part of the message). So they would not have to parse the protobuf message either.

Using Payload for encryption still makes sense to me (in fact I suggest doing so both for encryption and decryption). The question is whether to store the AAD in the output proto.

tiziano88 avatar Feb 02 '24 14:02 tiziano88