Use zeroize for best-effort secret zeroisation

[ctz]

As a defence-in-depth measure, we should make a passing, best-effort attempt to clear secrets when we're done with them.

[burdges]

Actually clear_on_drop is outdated and unmaintained cesarb/clear_on_drop#20 and may brake in future rustc versions and causes issues without std.

You should use zeroize instead: https://github.com/iqlusioninc/crates/tree/develop/zeroize

If your dependencies do not implement zeroize then you can impl Zeroize yourself by copying and calling this zeroize_hack fn: https://github.com/w3f/schnorrkel/blob/master/src/lib.rs#L225

Actually all you need is zeroize_hack in a Drop, but adding the zeroize trait might make life easier for your callers, and/or permit them to zeroize even on panics or whatever.

[djc]

I can see how it makes sense to use zeroize for TLS 1.2. For TLS 1.3 the secrets are kept as ring::hkdf::Prks which I don't think we can apply zeroize to on the rustls side. There's an open issue in ring.

[briansmith]

I can see how it makes sense to use zeroize for TLS 1.2. For TLS 1.3 the secrets are kept as ring::hkdf::Prks which I don't think we can apply zeroize to on the rustls side. There's an open issue in ring.

One of my goals for ring is to make it so people can write something like Rustls such that there are no &[u8] of key material exposed to the application except for whatever persistent keys it (de)serializes. I don't think ring quite meets that goal but it's still a goal. I would be happy to address whatever shortcomings there are there, e.g. in the HKDF code.

Along those lines, if there are non-application-data buffers with secrets to zeroize in Rustls's TLS 1.3 code then we should see what we can do to eliminate those buffers completely.

As @djc said, the design of TLS 1.2 may force some of these buffers to be exposed to the TLS 1.2 code. However, we should still try to minimize the lifetime of those buffers.

Regarding the application data buffers, I think it would make sense to have a design for how an application would successfully zeroize those buffers, presumably on demand--much like the proposed API for shrinking these to their minimum size on demand. I don't think automatic zeroization is likely to be practical for performance for these large buffers.

More generally, destructor/drop-based zeroization as people intuitively think about it has limited value because it's impractical to use an allowlist-type approach to make sure every sensitive thing is zeroized. Instead it would be better to use an allocator that automatically zeroizes on deallocation (like BoringSSL does in FIPS mode, IIRC). Also, zeroization can only protect data that is not being used. It doesn't protect data that is still in use. This makes zeroization a mitigation with very limited benefit.