Handling sensitive data in memory

I'm working on a simple cryptography abstraction package, and as part of it I was considering making a type for securely storing sensitive data (such that its memory is zeroed before deallocation, so that future processes can't scan memory for these sensitive values).

I'm wondering if the following is sufficient to guarantee that a value placed in the Secure type is not copied as part of the construction of the Secure (on a native level, I want the data to reside in exactly the same memory as before the construction of the Secure)...

use std::ops::{Deref, DerefMut};

pub trait Securable: Sized + Default {}
pub struct Secure<T: Securable>(T);

impl<T> Drop for Secure<T>
where
    T: Securable,
{
    fn drop(&mut self) {
        self.0 = T::default();
    }
}

impl<T> Deref for Secure<T>
where
    T: Securable
{
    type Target = T;
    fn deref(&self) -> &T {
        &self.0
    }
}

impl<T> DerefMut for Secure<T>
where
    T: Securable
{
    fn deref_mut(&mut self) -> &mut T {
        &mut self.0
    }
}

I'm also wondering what other properties I might be missing that are important when handling sensitive data in memory.

1 Like

I think at the very least you’ll need to std::ptr::write_volatile in Drop so that compiler doesn’t elide the write.

In terms of not being copyable while Secure is in scope, someone can use unsafe code to crack into your type manually. Not sure if that matters or not to you.

2 Likes

Thanks! I'll look into that!

I don't mind it being breakable when using the type unsafely. After all, the user has to say "unsafe" themselves to do it. Hopefully they know what they're doing (as always). Especially since this library is aimed at simplifying usage, rather than implementing new, cryptographic libraries.

I've also added a std::ptr::drop_in_place() for types that have their own drop methods

You may want to combine ptr::drop_in_place() with ptr::write_bytes() so you can explicitly zero the memory on Drop. It also helps in that you no longer need to add the Default bound on your Securable trait.

Interestingly, I tried this out on the playpen and you seem to still be able to read the inner variable after you drop your Secure<T>... This is pretty much the definition of UB, but does anyone know why my Secure wrapper isn't working as it should? Inspecting the assembly shows that secure.inner is in fact being overwritten with zeroes.

2 Likes

I would either port sodium_memzero or include the rust bindings and use it. The problem is preventing the compiler from optimizing it out.

https://github.com/jedisct1/libsodium/blob/1d85e73d8fb99f85371e78170bf0f15521c5c6ee/src/libsodium/sodium/utils.c#L103

1 Like

I suspect that some of the UB comes from the fact that you are mutating data while holding an *const pointer to it. You need to put the data in an UnsafeCell (or higher-level abstraction based on it) for that, otherwise the Rust compiler is allowed to assume that the data behind the *const pointer won't change and to elide or reorder the read.

EDIT: The problem is actually even simpler than that. In main(), you are using drop, not drop_in_place, so the code makes a copy of "secure" and erases it instead of erasing the original as you intended. Your inner_ptr thus points to a non-zeroed copy of "secure".

2 Likes

Argh, something extra will need to be done for heap data - the dropping is just overwriting the stack portion of the String (which is just a Vec<u8> internally) with 0s, but it's not zeroing the heap data.

3 Likes

A crate already exists for this: https://github.com/myfreeweb/secstr

2 Likes

This looks pretty good as it also mlock()'s the page(s) containing the heap data to prevent them from going to swap.

2 Likes

Shouldn't drop_in_place() deal with that? Or does it just deallocate without zeroing the heap memory first?

I guess there's always the nuclear option, which is you create your own allocator which just wraps the normal allocator and zeroes everything after it's free'd. The standard collections aren't (yet) generic over their allocator though, so you'd need to install your "secure" allocator globally, which would most probably result in a big performance hit.

2 Likes

That just executes the destructor, which we already executed anyway. It's useful, mostly, for unsized types that can't be put on stack to drop as normal.

The issue here isn't that we're not calling the destructor (we are), but the code in the destructor isn't actually zeroing the sensitive memory (which lives elsewhere in the heap). You can say we're doing a "shallow" zeroing, but we need a "deep" zeroing.

1 Like

Relevant discussion:
https://github.com/isislovecruft/curve25519-dalek/issues/11

2 Likes

newpavlov's link led me to an RFC that referred to the following library, which seems to have this covered...
https://github.com/stouset/secrets

This conversation has a bunch of useful links...
https://github.com/briansmith/ring/issues/566

1 Like

Would using a crate like this one be overkill just for handling passwords? It seems used for crypto stuff.

I was thinking of using it for a web service but I realized that it would be pointless unless I also spend some time modifying the web framework and a LDAP crate.

If reading CVE's and tech news coverage has taught me anything, it is that, in security, the only "kill" worth discussing is "overkill", with a preference for nuclear options. There is no such thing as "just".

Anything "lightweight" or "partially secure" is bound to be used wrongly by someone, somewhere who was either too lazy or too overworked to realise he/she is using the thing in exactly the way that says in the manual: "do NOT do this".

Partial security in that respect is actively worse than no security at all, due to the false sense of safety it spreads. The "partial" tends to be overlooked. (Even if the programmer gets it, his manager will probably answer "but it says 'secure' right there in the name!"

3 Likes