-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Associated data API for encryption is not very ergonomic #4742
Comments
As per Rust SDK we were planning to return only the decrypted message from the 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. |
oak/oak_crypto/src/encryptor.rs
Lines 196 to 202 in e9be418
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:
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?
The text was updated successfully, but these errors were encountered: