kem: `EK`/`SS` serialization

[tarcieri]

The one remaining gap in the kem API is a way to obtain the bytes serialization of EK and SS (encapsulated key and shared secret respectively), and for EK, a way to deserialize it from bytes before passing it to Decapsulate::decapsulate.

I was first wondering if there's actually a use case for overlapping impls with these traits and if not, perhaps these should be associated types instead? That would make it easier to give them bounds. Something like:

pub trait Encapsulate: TryKeyInit + KeyExport {
    /// Shared secret after being encrypted by this encapsulator
    type EncapsulatedKey: TryKeyInit + KeyExport;

    /// Shared secret value which results after the `EncapsulatedKey` has been decrypted.
    type SharedSecret: AsRef<[u8]>;

    /// Encapsulates a fresh shared secret
    fn encapsulate_with_rng<R>(&self, rng: &mut R) -> Result<(EK, SS), R::Error>
    where
        R: TryCryptoRng + ?Sized;

    /// Encapsulate a fresh shared secret generated using the system's secure RNG.
    #[cfg(feature = "getrandom")]
    fn encapsulate(&self) -> (EK, SS) {
        let Ok(ret) = self.encapsulate_with_rng(&mut SysRng.unwrap_err());
        ret
    }
}

pub trait Decapsulate {
    /// Encapsulator which corresponds to this decapsulator.
    type Encapsulator: Encapsulate<
        EncapsulatedKey = Self::EncapsulatedKey,
        SharedSecret = Self::SharedSecret
    >;

    /// Shared secret after being encrypted by this encapsulator
    type EncapsulatedKey: TryKeyInit + KeyExport;

    /// Shared secret value which results after the `EncapsulatedKey` has been decrypted.
    type SharedSecret: AsRef<[u8]>;

    /// Decapsulates the given encapsulated key
    fn decapsulate(&self, encapsulated_key: &EK) -> SS;

    /// Retrieve the encapsulator associated with this decapsulator.
    fn encapsulator(&self) -> Self::Encapsulator;
}

We could also consider a third trait that removes the duplicated associated types, perhaps (possibly Kem?)

[rozbb]

I think it’s fine to lock a type into 1 kind of encapsulation. Any other use case feels niche.

For Decapsulate why not remove EncapsulatedKey and SharedSecret entirely? Or just set them to the Encapsulator associated types directly

[tarcieri]

For some precedent from the random traits that crates in https://github.com/RustCrypto/KEMs define to abstract over the KEMs they provide because kem has gaps:

I noticed that the dhkem::DhKem trait looks like what I proposed with associated EncapsulatedKey and SharedSecret types.

The ml_kem::KemCore trait took a slightly different approach:

/// A generic interface to a Key Encapsulation Method
pub trait KemCore: Clone {
    /// The size of a shared key generated by this KEM
    type SharedKeySize: ArraySize;

    /// The size of a ciphertext encapsulating a shared key
    type CiphertextSize: ArraySize;

    [...]
}

I like that a lot! Then these types are just byte arrays and we don't need to worry about defining serialization traits.

[tarcieri]

For Decapsulate why not remove EncapsulatedKey and SharedSecret entirely? Or just set them to the Encapsulator associated types directly

Ooh, good idea

Edit: hmm, on second thought, there's one reason to use a common trait like Kem to define them: then you could define type aliases like EncapsulatedKey and SharedSecret that would work with either the encapsulator or decapsulator.

[rozbb]

We do still need deserialization somewhere, right? Since decap is infallible, there’s nowhere else to put a malformedness error

[tarcieri]

The deserialization would just be to hybrid_array::Array<u8, <Self as Kem>::EncapsulatedKeySize> or thereabouts.

[rozbb]

Isn’t there such thing as a malformed encapsulation key or a malformed ciphertext? If so, where is that error produced?

[tarcieri]

For malformed encapsulation keys, as in the public key, Encapsulate is now bounded by the TryKeyInit trait which is fallible and you can return kem::InvalidKey in that case.

With implicit rejection the only way a ciphertext can be malformed, AFAIK, is being the wrong length, and that's handled by it being a typed array with a fixed size. We could potentially add a helper method that accepts a slice and handles converting it to an array and the length error, returning a Result.

Sidebar: ugh "encapsulation key" and "encapsulated key" is so confusing, I think I'll adopt "ciphertext" instead of the latter which I've seen in a few of our implementations and elsewhere.

[rozbb]

Ciphertexts can be invalid in other ways. For HPKE DHKEMs, ciphertexts are curve points. Not every byte string is a valid P-256 point (uncompressed or compressed), for example

Sidebar: ugh "encapsulation key" and "encapsulated key" is so confusing, I think I'll adopt "ciphertext" instead of the latter which I've seen in a few of our implementations and elsewhere.

Agree. I’m happy using “encapsulation key”, “decapsulation key”, and “ciphertext”. Also it is “shared secret” and NOT “shared key” which I think is floating around somewhere.

[tarcieri]

@rozbb okay, I can pre-emptively add TryDecapsulate to cover those cases, and we can have a blanket impl of TryDecapsulate for types which impl Decapsulate

[tarcieri]

Aah okay, so the reason our dhkem is infallible is because it uses elliptic_curve::PublicKey<C> as the ciphertext / encapsulated key type, so yes making it always a byte array would introduce a fallible case that needs handled there

[tarcieri]

Alright, here's my attempt at addressing all of these concerns: #2220

Unfortunately it's more complicated than before (5 traits instead of two, plus two type aliases) but should actually provide a comprehensive solution for using KEMs in a generic way.