Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
NickeZ opened this issue Oct 16, 2020 · 1 comment
Open

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

NickeZ opened this issue Oct 16, 2020 · 1 comment

Comments

@NickeZ
Copy link

NickeZ commented Oct 16, 2020

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,
}
@tony-iqlusion
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants