`zeroize::Zeroize` is not properly implemented for `ml_kem::DecapsulationKey`

[arctic-hen7]

The current implementation under the zeroize feature in ml-kem for DecapsulationKey implements a manual zeroize-on-drop pattern where Zeroize is implemented for internal fields, but not for the outer container type. Then ZeroizeOnDrop is implemented, but this is just a marker trait. As such, there is no actual implementation of Zeroize on DecapsulationKey, meaning users who want to put these types inside a larger container type cannot derive Zeroize on that type.

It seems like dhkem's DecapsulationKey, on the other hand, does implement Zeroize.

This could be quite easily fixed for ml-kem by reverting to the original proposed behaviour of #51, where Zeroize was implemented directly. This was replaced by a Drop implementation, which still works, but makes things awkward for library authors who want to wrap ml-kem types in their own containers.

If this is a deliberate decision to avoid users being able to use key material after zeroization, that makes sense, but why the discrepancy with dhkem?

Very happy to do a PR for this?

[arctic-hen7]

Looks like this also affects x-wing's DecapsulationKey too actually.

[tarcieri]

Zeroization occurs here, in the Drop handler: https://github.com/RustCrypto/KEMs/blob/1a09f4f/ml-kem/src/decapsulation_key.rs#L142-L154

That's what ZeroizeOnDrop means: that the type takes care of its own zeroization so no action is required on your part.

We don't impl the Zeroize trait on keys because calling that outside the drop handler would leave the key in an insecure state that violates its own invariants. We don't ever recommend implementing Zeroize in a way that leave a type in a state that violates its invariants, as that can lead to a sort of "use after zeroize" condition.