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

zeroize: how to signal "safeness" to crate's users? #757

Closed
fjarri opened this issue Jun 17, 2021 · 5 comments
Closed

zeroize: how to signal "safeness" to crate's users? #757

fjarri opened this issue Jun 17, 2021 · 5 comments

Comments

@fjarri
Copy link
Contributor

fjarri commented Jun 17, 2021

Person A creates a crate with a type that has some secret data inside, say, SecretKey { inner: SecretDataType }. They implement Zeroize and Drop (or #[zeroize(drop)]) for SecretDataType, and stop at that - the secret data is now safe (to the extend Rust can guarantee it), the drop() of SecretKey will call the zeroizing drop() of SecretDataType, so there's no need to define anything for SecretKey itself.

Person B uses A's crate and wants to use Secret<SecretKey>. But they cannot, because SecretKey does not implement Zeroize - so they have to wrap it in a newtype and implement an empty Zeroize for it. And they still cannot assert in code that SecretKey keeps its secret - they have to rely on the docs.

So what are the responsibilities of A here? Should they define an (empty) Zeroize for SecretKey? Should they export a Secret<Box<SecretKey>>? What's the convention?

I wonder if it would be more convenient to have a marker trait ZeroizedOnDrop, that A could implement for SecretKey, and Secret could require?

@tony-iqlusion
Copy link
Member

It's a good question, and one I've encountered quite a bit. See also RustCrypto/AEADs#65.

The biggest problem is types which want to maintain some kind of invariant which having a Zeroize impl would violate, and therefore only want be responsible for clearing internal fields via a Drop impl.

Indeed it would seem a marker trait like ZeroizedOnDrop would be helpful for this. I can potentially add one before the next release.

@tony-iqlusion
Copy link
Member

tony-iqlusion commented Jun 17, 2021

Another interesting thing: this could be used with the custom derive support in lieu of the current (and somewhat awkward):

#[derive(Zeroize)]
#[zeroize(drop)]

Instead it could just be:

#[derive(ZeroizedOnDrop)]

As these crates are post-1.0, we'd still need to retain support for #[zeroize(drop)] (which could also derive ZeroizedOnDrop), but it could be deprecated.

And of course, Zeroizing would impl it.

@fjarri
Copy link
Contributor Author

fjarri commented Jun 17, 2021

Oh, didn't realize you have two github accounts :)

As you may have guessed, I was asking this in relation to RustCrypto/traits#671. Currently, if I want to wrap SecretKey in Secret<Box<...>>, i have to do something like

struct SecretKeyInner(BackendSecretKey<CurveType>);

impl Zeroize for SecretKeyInner {
    fn zeroize(&mut self) {
        // BackendSecretKey is zeroized on drop, nothing to do
    }
}

impl Zeroize for Box<SecretKeyInner> {
    fn zeroize(&mut self) {
        // BackendSecretKey is zeroized on drop, nothing to do
    }
}

pub struct SecretKey(Secret<Box<BackendSecretKey<CurveType>>>);

because Secret<T> requires T: Zeroize, and there is no blanket impl for Box<T> where T: Zeroize.

@tony-iqlusion
Copy link
Member

tony-iqlusion commented Oct 14, 2021

re: a ZeroizedOnDrop trait, it used to exist: #168

I'm still looking through the commit history for why it was removed.

Edit: aah yes, it was removed with the goal of always adding drop handlers by default. But that also ended up having its own issues: #188

@tony-iqlusion
Copy link
Member

Moved to RustCrypto/utils#652

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