Add repr(transparent) for Zeroizing?

[ssanderson]

I have an application where I have a type declared like:

/// Shared reference for a fixed-length slice of secret bytes that will be zeroed
/// when all references are dropped.
pub struct FixedLengthSecretBytes<const N: usize>(Arc<Zeroizing<[u8; N]>>);

In my application I need to expose the bytes wrapped by this type over a foreign-function interface exposed by a cdylib. I do so using Arc::into_raw and Arc::from_raw. The C layer doesn't have a way to interpret Zeroizing, so I need to return a raw pointer to primitive bytes.

What I want to write for this is roughly something like:

impl<const N: usize> FixedLengthSecretBytes<N> {
    pub fn into_raw(self) -> *const [u8; N] {
        let inner: * const Zeroizing<[u8; N]> = Arc::into_raw(self.0);

        // cast pointer to discard zeroizing wrapper. This implicitly assumes that the layout of Zeroizing<T> is the
        // same as T.
        inner.cast()
    }

    pub unsafe fn from_raw(ptr: *const [u8; N]) -> Self {
        let ptr = ptr as *const Zeroizing<[u8; N]>;  // same layout assumption required here.
        let arc = unsafe { Arc::from_raw(ptr) };
        Self(arc)
    }
}

In practice the code above works today because Zeroizing<[u8; N]> has the same physical layout as [u8; N], but I believe this code is technically UB (or is at least relying on unspecified compiler behavior) because Zeroizing is repr(Rust), which means its layout is not guaranteed to be the same as its underlying type. I think I can make into_raw work as-is with a bit more cleverness by dereferencing down to the inner slice and then extracting a pointer from there, but from_raw is harder to do in a way that's free of these assumptions.

I believe that adding a repr(transparent) annotation on Zeroizing would make the casts here well-defined. Is that something that could be considered here? I believe such a change would be backwards-compatible. The main downside is that it would make it easier for unsafe code like this to extract values out of a Zeroizing, which could circumvent the guarantees provided by that type, but that's already possible if you're dropping down to unsafe, and providing a transparent guarantee would make it possible for code like this to be as safe as possible given the inherent unsafety of FFI.

[tarcieri]

I think it would probably be okay for Zeroizing to be #[repr(transparent)] and have a public constructor that takes a mutable reference &mut T to its inner value and casts it to &mut Zeroizing<T>.

My main worry would be the drop glue but that shouldn't actually get run until T itself is dropped, I believe.

[ssanderson]

I think it would probably be okay for Zeroizing to be #[repr(transparent)] and have a public constructor that takes a mutable reference &mut T to its inner value and casts it to &mut Zeroizing<T>.

Making sure I follow, are you imagining something like:

impl<T> Zeroizing<T> {
     pub fn from_mut(inner: &mut T) -> &mut Self {
         &*(inner as * mut T as * mut Self)
     }
}

? If so, I'm not sure that would help my specific FFI use-case, because what I ultimately want to be able to do is pass ownership back and forth from C code. For that application, the conversion I generally want to be able to do is Box<Zeroizing<T>> <-> *mut T or Arc<Zeroizing<T>> <-> * const T, mediated by {Box,Arc}::{from,into}_raw. So I think the operations that might be more directly relevant would be something like:

impl<T> Zeroizing<T> {
    pub fn boxed_into_raw(self: Box<Self>) -> * mut T {
        self.into_raw().cast()
    }

    unsafe pub fn boxed_from_raw(val: *mut T) -> Box<Self> {
        Box::from_raw(val.cast())
    }

    // Similar for arc_{from,into}_raw.
}

I'd be happy to put together a PR with the above. The implementations of these are fairly trivial since they're just wrappers around Arc and Box methods plus a cast. The main work is documenting the safety requirements, which should be roughly "same as the underlying box/arc methods".

[tarcieri]

I'm not sure I want to add a bunch of unsafe methods to Zeroizing.

Maybe just add #[repr(transparent)] for now.

[ssanderson]

PR for this here: https://github.com/RustCrypto/utils/pull/1253. I also added a small documentation note on Zeroizing noting that it is declared as repr(transparent), but I'm not sure if where I added it is the best place, or if we should have more or less documentation. I think you probably don't want to have a whole discussion about FFI safety in general in this crate.

[ssanderson]

Closing this since https://github.com/RustCrypto/utils/pull/1253 was merged.