Making sure SecretKey is zeroized on drop

[fjarri]

I was trying to make my newtype of SecretKey zeroize properly, and realized I can't even trigger zeroization for SecretKey itself. Consider the code:

use k256::SecretKey;
use rand_core::OsRng;

fn main() {
    let sk = SecretKey::random(&mut OsRng);

    let ptr = &sk as *const SecretKey;
    let ptr_u8 = ptr as *const u8;

    println!("Pointer: {:p}", ptr);

    drop(sk);

    println!("Memory: {:?}", unsafe {
        core::slice::from_raw_parts(ptr_u8, 4)
    });
}

k256 has zeroize feature enabled in Cargo.toml. This still gives a non-zero output after SecretKey was dropped, despite there being a Drop implementation in secret_key.rs that calls zeroize(). Am I misunderstanding something?

[tarcieri]

I'm guessing this is a bug in the Zeroize impl on ScalarBytes, which was recently (#649) made the internal representation for SecretKey.

My guess would be it's making a copy first, then zeroizing that. I'll take a look soon. It would also be good to add a test for zeroization like you have in the description.

[fjarri]

So, it seems that if you instrument the Drop implementation for SecretKey<C>, it does get called, but with a different pointer compared to the calling code.

Creating secret key 0x7ffeeea44320 # in `SecretKey<C>::random()`
Pointer: 0x7ffeeea44320 # In `main.rs`
Dropping secret key 0x7ffeeea44398 # in `SecretKey<C>::drop()`
# `println!("{:?}", self.inner);` in `SecretKey<C>::drop()` before `self.inner.zeroize();`
ScalarBytes { inner: [85, 62, 127, 206, 24, 17, 156, 47, 124, 186, 149, 72, 56, 194, 18, 236, 219, 195, 90, 24, 212, 245, 106, 34, 148, 197, 156, 30, 114, 247, 178, 140] }
# `println!("{:?}", self.inner);` in `SecretKey<C>::drop()` after `self.inner.zeroize();`
ScalarBytes { inner: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] }
Memory: [85, 62, 127, 206] # in `main.rs`

If one does not call drop explicitly in main.rs, it gets the same pointer SecretKey<C>::random() created (when it is called by sk going out of scope in main()). So in that case it works as expected... but it would be nice to be able to test it somehow.

Edit:

It seems that despite only implementing Clone, a SecretKey (or its data) is still copied on move. The structure is basically SecretKey { inner: ScalarBytes { inner: FieldBytes } }, where FieldBytes is an alias for GenericArray (which does implement Copy). Can it be that the inner inner is actually copied on move? Can it be prevented somehow?

A drop without a copy can be triggered in a test as

fn drop_sk() -> *const u8 {
    let sk = SecretKey::random(&mut OsRng);
    let ptr = &sk as *const SecretKey;
    let ptr_u8 = ptr as *const u8;
    ptr_u8
}

fn main() {
    let ptr_u8 = drop_sk();

    println!("Memory: {:?}", unsafe {
        core::slice::from_raw_parts(ptr_u8, 4)
    });
}

Somewhat unexpectedly, it prints

Memory: [1, 0, 0, 0]

So there's some kind of a flag byte in front of the actual inner. I don't really know enough about Rust internals to understand what's up with that.