oak
oak copied to clipboard
Associated data API for encryption is not very ergonomic
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:
- change the Rust API to require the caller to pass in the additional data when decrypting
- 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?
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.
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.