zeroize: potetial redesign ideas

[newpavlov]

Right now I see the following issues with public API of zeroize:

• Derived and manually implemented Zeroize can be inefficient since they rely on zeroization of fields one by one or on zeroizing Drop impls.
• Procedural macros are relatively heavy compile time-wise, so usually we do not derive Zeroize ad rely on manual implementations.
• ZeroizeOnDrop is a somewhat useless trait, I haven't seen it used in practice.
• Zeroizing usefulness is limited. It can be used only for "primitive" types (e.g. raw keys), complex types do not implement Zeroize and instead implement zeroizing Drop.
• Zeroization is enabled for a whole crate using features. It's not possible to use selective zeroization and zeroization is not reflected in any way in user code.

I think most structures should be zeroized using the "flat" zeroization (see #1045) and that it can be useful to have explicit indication in user code of structs being zeroized on drop.

So I would like to suggest roughly this API:

/// Zeroizes memory pointed by `data_ptr`, but not memory
/// potentially reference by `T`.
pub unsafe fn zeroize_flat<T>(data_ptr: *mut T) {
    // ...
}

/// Zeroize-on-drop wrapper.
///
/// Note that this wrapper zeroizes only data owned by `T`
/// and does nothing with data referenced by it.
#[repr(transparent)]
pub struct ZeroizeOnDrop<T, const IS_ENABLED: bool = true>(T);

/// Abbreviated alias for `ZeroizeOnDrop`. 
pub type Zod<T, const IS_ENABLED: bool = true> = ZeroizeOnDrop<T, IS_ENABLED>;

impl<T, const IS_ENABLED: bool> Drop for ZeroizeOnDrop<T, IS_ENABLED> {
    fn drop (&mut self) {
        unsafe {
            std::ptr::drop_in_place(&mut self.0);
            if IS_ENABLED { zeroize_flat(&mut self.0); }
        }
        
    }
}

// Impl Deref and DerefMut for `ZeroizeOnDrop`

UPD: FlatPod and FlatZod are removed.

It can be introduced in a backward-compatible way, but for clarity it's probably worth to release it as v2.0.

Users would write code like this:

pub struct Foo {
    secret_cipher: Zod<Aes128>,
    secret_key: Box<Zod<[u8; 16]>>,
    non_secret_hasher: Sha256,
    // other fields
}

const ZOD: bool = cfg!(feature = "zeroize");

pub struct Bar1 {
    // This field will be zeroized only if `zeroize` feature is enabled
    cipher: Zod<Aes128, ZOD>,
}

// Alternatively:
pub struct Bar2<const ZOD: bool = true> {
    cipher: Zod<Aes128, ZOD>,
}

While it will be a bit less convinient, I think it's useful to have explicit indication in source code that secret types will be zeroized on drop. We may provide aliases like type ZodAes128 = Zod<Aes128> to improve visibility, but I don't think they are worth the trouble and it should be sufficient to simply references zeroize in docs.

[daxpedda]

• Procedural macros are relatively heavy compile time-wise, so usually we do not derive Zeroize ad rely on manual implementations.
• ZeroizeOnDrop is a somewhat useless trait, I haven't seen it used in practice.

The point of ZeroizeOnDrop is to run zeroize() on Drop, which is a very common thing to do with the current design of zeroize. A quick search on GitHub (or in its old Zeroize(drop) form) shows that its widely used.

As long as Zeroize exists, ZeroizeOnDrop seems to make a lot of sense to me.

[newpavlov]

The point of ZeroizeOnDrop is to run zeroize() on Drop

No, it's not. zeroize::ZeroizeOnDrop is nothing more than a marker trait which indicates (but does not guarantee in any way) that type has a zeroizing Drop impl. Most of "uses" which your search query has found are simply implementation of this trait, not uses which influence anything practical. On first 5 pages I found only 2 uses which rely on it for bounds.

[daxpedda]

The point of ZeroizeOnDrop is to run zeroize() on Drop

No, it's not. zeroize::ZeroizeOnDrop is nothing more than a marker trait which indicates (but does not guarantee in any way) that type has a zeroizing Drop impl. Most of "uses" which your search query has found are simply implementation of this trait, not uses which influence anything practical. On first 5 pages I found only 2 uses which rely on it for bounds.

I was talking about the proc-macro, not the trait itself. I guess your original point then was about the trait not the proc-macro?

Unfortunately I don't remember the motivation behind the ZeroizeOnDrop trait anymore, but I'm happy to dig into it again when we get there.

[newpavlov]

I guess your original point then was about the trait not the proc-macro?

Yes. As I wrote in the OP, in RustCrypto we usually do not rely on proc macro and instead manually implement Drop and ZeroizeOnDrop.

[tarcieri]

I would probably suggest against any breaking changes to zeroize.

Due to the trait-based nature of its API, which is used across crates, it makes breaking changes extremely difficult due to the requirement for coordinated upgrades.

I find names like FlatPod and Zod to be rather confusing and jargony. "Pod" doesn't intuitively imply "plain old data" to me but rather some sort of container like a "cell". At least the existing traits/structs have "Zeroize" in the name to make it clear what they do.

DefaultIsZeros already seems to cover many of the cases you have in mind for this.

I think #1045 is fine, but should be done in an additive manner.

[tarcieri]

dsa and rsa are some examples of where data isn't flat. Keys have a single secret field, but the type which stores them is a newtype for e.g. Box/Vec like num_bigint_dig::BigUint or crypto_bigint::BoxedUint. We don't want to just blanket add a Drop impl to these types because not all of them are secrets and it would negatively impact performance. The knowledge of which specific fields are secrets exists in the Drop impls in dsa/rsa.

I also think it would be good to be able to silently and transparently promote secret storage for other types to be heap-backed as well when available when the alloc feature is available, so as to better avoid making copies on moves.

[newpavlov]

Due to the trait-based nature of its API, which is used across crates, it makes breaking changes extremely difficult due to the requirement for coordinated upgrades.

For most of the RustCrypto crates upgrade should be quite painless since they do not provide Zeroize impls and instead only have zeroizing Drop and ZeroizeOnDrop. As argued above, the latter is not used much in practice for trait boudns, so it can be removed with minimal consequences. The biggest breaking change will be removal of zeroize crate features.

I find names like FlatPod and Zod to be rather confusing and jargony.

I do not insist on this exact names, think of them as of placehoders. FlatPod probably should not be introduced in the first place, since it's only advisory and its usefulness is extremely limited. As for Zod, I used short name only for brevity. We probably should name it fully as ZeroizeOnDrop with additional type Zod<T> = ZeroizeOnDrop<T>; alias.

DefaultIsZeros already seems to cover many of the cases you have in mind for this.

But it should not be implemented for types like Aes128. The main idea is that Zod will perform indiscriminate zeroization without any need for support in crates which introduce "flat" types (so most of our crates).

Keys have a single secret field, but the type which stores them is a newtype for e.g. Box/Vec like num_bigint_dig::BigUint or crypto_bigint::BoxedUint.

Such types can have ZOD constant as in the example above, but with default set to false. And we can define type BoxedSecretUint = BoxedUint<true>; alias which will be used for secret keys.

[tarcieri]

For most of RustCrypto crates upgrade should be quite painless since they do not provide Zeroize impls and instead only have zeroizing Drop and ZeroizeOnDrop. As argued above, the latter is not used much in practice for trait boudns, so it can be removed with minimal consequences. The biggest breaking change will be removal of zeroize crate features.

You're completely redesigning the API, so this will be a massive breaking change (and I'm not sure I'm particularly pleased with the new design).

Unlike our other trait crates where we generally control all the downstream trait impls, Zeroize and ZeroizeOnDrop impls exist in downstream crates we don't control, so the blast radius of any such change is significantly larger.

Also if we're thinking about a zeroize redesign, I'm not sure this is solving the right problems. There are many highlighted in this paper:

https://eprint.iacr.org/2023/1713.pdf

[newpavlov]

I have updated draft in the OP. FlatPod is removed and Zod is now a type alias.

Unlike our other trait crates where we generally control all the downstream trait impls, Zeroize and ZeroizeOnDrop impls exist in downstream crates we don't control, so the blast radius of any such change is significantly larger.

I think you overestimate potential consequences. Downstream crates which derive Zeroize/ZeroizeOnDrop on their structs will continue to work without any changes. If they decide to move to zeroize v2.0, they would need to decide where Zod should be applied.

Users may have issues if they include our type which implement Zeroize, but there is just several such types in our organization and some of those are private types.

Another potential issue is that users may rely on Drop zeroization, but with the new API they would need to use Zod, so without reading changelogs they may not notice this. I think it will be somewhat addressed by removal of zeroize features from new releases. Users will inevitable notice it during migration to new breaking releases and likely will look into changelogs to learn more about the change.

Also if we're thinking about a zeroize redesign, I'm not sure this is solving the right problems. There are many highlighted in this paper:

I've skimmed the paper and I am not sure it's relevant for deciding how public API of zeroize should look like. IIUC it's mostly concerned about stack bleaching and how to reliably implement zeroization to prevent undesirable compiler optimizations. I will read it more carefully a bit later.

[tarcieri]

I think you overestimate potential consequences. Downstream crates which derive Zeroize/ZeroizeOnDrop on their structs will continue to work without any changes.

There are a lot of uses which aren't custom derive. I suggest looking through this:

https://github.com/search?q=%22impl+zeroize%22&type=code

[newpavlov]

There are a lot of uses which aren't custom derive. I suggest looking through this:

It does not matter whether it's derive or manual implementations. The uses will continue to work until maintainers will decide to migrate to zeroize v2.0. Even if an upstream crate has migrated to v2.0, downstream may delay full migration to v2.0 by simply wrapping upstream types by Zod in their structs.

[tarcieri]

The problem is when you have cross-crate interdependencies, especially when there are multiple different vendors of crates at various levels, e.g.:

protocol implementation -> elliptic curve library -> numerical library

Breaking changes on trait-based APIs are incredibly intrusive in these sorts of environments. It's why serde can't bump to 2.0 to fix const generics support. The error messages for traits wit the same name across breaking releases of the same crate can be quite inscrutible.

I guess there's the semver trick to work around this, but with this sort of API, I think the amount of downstream churn and toil needs to be carefully weighed against the benefits that breaking changes might bring.

[tarcieri]

Yet another approach is to simply make a new library with a different name which provides the same functionality / end goals, which can be vetted independently, and if it seems like a better approach, you can deprecate the old one.

This is what I've explored with e.g. cmov versus subtle, where subtle is also in the position of being the sort of trait-based crate with complex interrelationships across crates with multiple vendors that make it difficult to make breaking changes (and we need breaking changes for e.g. heap-backed big integers).

[newpavlov]

The error messages for traits wit the same name across breaking releases of the same crate can be quite inscrutible.

Note that the proposed re-design does not have Zeroize and ZeroizeOnDrop traits at all, so users should not encounter this issue.

Yet another approach is to simply make a new library with a different name which provides the same functionality / end goals, which can be vetted independently, and if it seems like a better approach, you can deprecate the old one.

Yes, it's a viable option. The only question is whether the RustCrypto crates should keep zeroize support in future breaking releases or not, if the new crate will prove itself a good solution. One of motivation factors for the proposed redesign is desire to remove a lot of zeroize boilerplate from our crates.

[tarcieri]

One of motivation factors for the proposed redesign is desire to remove a lot of zeroize boilerplate from our crates.

I think we can make progress there without breaking changes. #1045 is a good start.