rust-hpke icon indicating copy to clipboard operation
rust-hpke copied to clipboard

Proposed API change: accept key material by reference

Open marlonbaeten opened this issue 1 year ago • 2 comments

Thank you for this useful library!

I noticed while using the library that the interface forces the caller to transfer memory ownership of the key material to an instance of the operation mode enum (OpModeS).

Since the PSK is also part of this data structure, one cannot reuse the OpModeS::AuthPsk value for multiple sessions and is thus forced to clone the private / public key for each PSK / OpModeS::AuthPsk instance.

The API can be changed to take a reference to the key material; see this PR as an example.

Would you consider such a change to the API? I could assist in updating examples and documentation.

marlonbaeten avatar Jan 30 '24 09:01 marlonbaeten

I would also be curious to know if this was a deliberate design decision, especially since in the Auth and AuthPSK case, this forces a caller to make copies of secret key material; an application might want to keep those in a "zeroizing" data structure for some extra guardrails.

tweedegolf-marc avatar Jun 17 '24 12:06 tweedegolf-marc

Hi, thank you for this! I agree there is unnecessary cloning happening. IIRC the reason I made the API this way was purely convenience. I'm not super concerned with copying secret keys, since all of those types are zeroize-on-drop anyway.

Right now the only reason I could see to modify the API is to avoid an unnecessary clone of a public key. Given that public keys in post-quantum KEMs can be up to 1.5kB, this is slightly nontrivial. Not clear though if this really matters.

That said, the reasons against it are:

  1. This would be a breaking change, and
  2. It kinda breaks the agility code. The try_lift functions can no longer return an OpMode type because they're not owned anymore. I'm not sure if anyone was relying on this behavior tbh

Do you have a strong opinion here? Does the current API impede a use case of yours?

rozbb avatar Jul 03 '24 15:07 rozbb