How to zeroize an Option<Z: ZeroizeOnDrop> ?

[mkj]

I have some ciphers stored as an Option<Ctr32BE<Aes256>>. I can't simply call .zeroize() after .take() because the cipher wrapper only implements ZeroizeOnDrop, not Zeroize (for good reason, blank keys are dangerous).

535 | #[derive(Zeroize, ZeroizeOnDrop)]
    |          ^^^^^^^ method cannot be called on `&mut StreamCipherCoreWrapper<CtrCore<Aes256, Ctr32BE>>` due to unsatisfied trait bounds

It seems like it should be fine for Option<Z: ZeroizeOnDrop> to implement Zeroize, but I can't add a blanket impl since there's already one for Option<Z: Zeroize>. Any ideas how to proceed?

I've tried a quick TakeZeroize trait adding a .take_zeroize() method which works for my purpose, but it seems a bit too specific. mkj@074cec5

[newpavlov]

Why do you want to call .zeroize() manually instead of relying on the cipher's Drop impl? Are you concerned about None containing sensitive data from the time it was Some?

I think we should simply modify the Zeroize impl for Option by removing the Z: Zeroize bound. If a value has Some vaiant, the impl would replace it with None after running Drop impl for Z and fully zeroizing the underlying memory. On None it would zeroize the memory and leave it as None.

[tarcieri]

In the case of Option<Z: ZeroizeOnDrop>, @newpavlov is correct that you can rely on the drop glue for zeroization and don't need to do anything else. In general the difference between Zeroize and ZeroizeOnDrop is the latter requires no manual steps on the user's part, and may indeed leave the resulting object in an invalid state the same way Drop would, but that's okay because there are no remaining references to the invalid object.

That said, this is the second recent suggestion I'v seen to remove the Z: Zeroize bound from the Zeroize impl on Option, and that would break actually calling Zeroize on the inner type. And that is important if the inner type has any kind of pointer, like Vec and Box do, because you need to run custom logic to zeroize the data the pointer points to, not just the value itself.

[newpavlov]

And that is important if the inner type has any kind of pointer, like Vec and Box do, because you need to run custom logic to zeroize the data the pointer points to, not just the value itself.

Shouldn't it be handled by Drop impl of Z? It's impossible to inspect contents of Z inside impl for Option<Z>.

By the way, I think Zeroize is somewhat misleading. In most cases users should rely on ZeroizeOnDrop (i.e. on zeroization inside Drop impl), while Zeroize should be used only for "plain old data" like keys and as implementation detail for types which implement ZeroizeOnDrop.

[tarcieri]

Shouldn't it be handled by Drop impl of Z?

We're talking about the Zeroize impl on Option and how the inner value is handled. Drop is irrelevant in that case. The example I just gave was something like Option<Vec<u8>>. Vec<u8> can't be ZeroizeOnDrop.

The Zeroize trait is needed so it can be impl'd on types like Option, Vec, and Box (not to mention [Z; N]), so that the Drop glue of a ZeroizeOnDrop type can correctly zero these types.

It's not possible, nor does it make sense, to be able to use ZeroizeOnDrop with these types.

[mkj]

Why do you want to call .zeroize() manually instead of relying on the cipher's Drop impl? Are you concerned about None containing sensitive data from the time it was Some?

If I use Option::take() on an Option<Z: ZeroizeOnDrop> I don't think the source will ever get zeroed. My use case is I have a long-lived

struct Session {
    current_keys: Keys,
    new_keys: Option<Keys>,
}

impl Session {
    fn switch_keys(&mut self) {
        self.current_keys = new_keys.take().unwrap();
    }
}

with Keys: ZeroizeOnDrop (it contains the Ctr<Aes> etc). After switch_keys() the key content might remain in new_keys, never dropped.

Another workaround would be making Keys: Clone and doing

self.curr_keys = new_keys.unwrap().clone();
self.new_keys = None; // force the drop here

but it would be nice to be able to just use take() and avoid making it Clone.

[tarcieri]

@mkj there's nothing we can do there. Move semantics may make a copy in that case. It's a documented sharp edge of zeroize.

Making a copy and dropping the original value as you suggested will work around the issue.

[newpavlov]

The example I just gave was something like Option<Vec>. Vec can't be ZeroizeOnDrop.

In cases like this users should use a wrapper around Vec<u8> which implements ZeroizeOnDrop (SecretVec<u8> or something like it). Same goes for the other std types listed by you.

[tarcieri]

While that might make sense in some cases, that isn't necessarily a better option in all cases.

[tarcieri]

Also zeroize is post-1.0 and the Zeroize impl on Option has been stable for over 3 years.

The best we could do is deprecate the Zeroize impl on Option, and I really don't think it would make any sense to do that and just create needless churn.

[mkj]

@tarcieri OK thanks. I'd been assuming that warning was to do with remnants on the local stack (#810), hadn't considered Option there.

[tarcieri]

The Zeroize impl on Option will handle clearing the remaining data left over from a move when it’s called (i.e. the leftover data from when the option is set to None)

However you won’t get the same benefits with a ZeroizeOnDrop type if you take from an Option, which as it were is a good example of when using an outer option on an inner ZeroizeOnDrop type is inferior to zeroizing the Option as it were.