crates icon indicating copy to clipboard operation
crates copied to clipboard

secrecy: Pin doesn't help, can still be moved

Open NickeZ opened this issue 5 years ago • 1 comments

Since the Secret type implements Unpin it can be moved out of a pin and therefore Pin doesn't help to pin a secret in memory AFAICT. (I might be wrong, I'm not an expert on pinning.)

Pin::into_inner() would not have been implemented if Secret didn't implement Unpin.

use pin_utils::pin_mut;
use secrecy::{ExposeSecret, Secret};
use std::mem;
use std::pin::Pin;

fn use_secret(s: Secret<[u8; 4]>) {
    println!("Use secret {:?}", s.expose_secret());
    println!("ptr: {:x?}", s.expose_secret().as_ptr());
}

fn main() {
    let a = Secret::new(*b"aaaa");
    println!("ptr: {:x?}", a.expose_secret().as_ptr());
    pin_mut!(a);
    println!("Use secret {:?}", a.expose_secret());

    let b = Pin::into_inner(a);
    // Moves secret even though we pinned it!
    let c = mem::replace(b, Secret::new(*b"bbbb"));
    use_secret(c);
}

I think the solution would be:

pub struct Secret<S>
where
    S: Zeroize,
{
    /// Inner secret value
    inner_secret: S,
    /// Implement `!Unpin`
    _p: PhantomPinned,
}

NickeZ avatar Oct 16 '20 15:10 NickeZ

Yes, secrecy should have better behavior with Pin.

The recommended way to use it now is with heap-allocated data, e.g. the SecretBox type.

tony-iqlusion avatar Oct 17 '20 14:10 tony-iqlusion