secrecy: How to construct SecretBox

[troublescooter]

Am I missing something obvious here, or how is a SecretBox constructed in practice?

fn foo {
    let secret_byte = Box::new(0_u8);
    let secret_box = Secret::new(secret_byte);
}

error[E0277]: the trait bound `std::boxed::Box<u8>: zeroize::DefaultIsZeroes` is not satisfied
   --> src/handshake.rs:151:37
    |
151 |        let secret_box = Secret::new(secret_byte);
    |                                     ^^^^^^^^^^^ the trait `zeroize::DefaultIsZeroes` is not implemented for `std::boxed::Box<u8>`
    |
    = note: required because of the requirements on the impl of `zeroize::Zeroize` for `std::boxed::Box<u8>`
    = note: required by `secrecy::Secret::<S>::new`

There seems to be an impl missing for Box.

[tony-iqlusion]

The Box impls in zeroize are gated on the alloc feature in zeroize.

Are you using secrecy with --no-default-features?

[troublescooter]

Adding this to the tests in the zeroize crate

fn impls_zeroize<Z: Zeroize>(_: &Z) {}

#[cfg(feature = "alloc")]
#[test]
fn zeroize_box() {
      let mut boxed_arr = Box::new([42u8; 3]);
      // works
      <[u8; 3] as Zeroize>::zeroize(&mut *boxed_arr);
      // works
      boxed_arr.zeroize();
      // doesn't work
      <Box<[u8; 3]> as Zeroize>::zeroize(&mut boxed_arr);
      // doesn't work
      impls_zeroize(&boxed_arr);
      assert_eq!(boxed_arr.as_ref(), &[0u8; 3]);
 }

shows that Box appears to implement Zeroize due to deref coercion. Adding

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
impl Zeroize for Box<[u8; 3]>
{
    fn zeroize(&mut self) {
       (**self).zeroize()
  }
}

gets the test to compile, but adding

#[cfg(feature = "alloc")]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
impl<Z> Zeroize for Box<Z>
where
    Z: Zeroize,
{
    fn zeroize(&mut self) {
       (**self).zeroize()
  }
}

fails with the error

   Compiling zeroize v1.1.1 (/home/c/pjts/secretbox/src/vendor/zeroize)
error[E0119]: conflicting implementations of trait `Zeroize` for type `alloc::boxed::Box<_>`:
   --> src/lib.rs:375:1
    |
237 | / impl<Z> Zeroize for Z
238 | | where
239 | |     Z: DefaultIsZeroes,
240 | | {
...   |
244 | |     }
245 | | }
    | |_- first implementation here
...
375 | / impl<Z> Zeroize for Box<Z>
376 | | where
377 | |     Z: Zeroize,
378 | | {
...   |
385 | |     }
386 | | }
    | |_^ conflicting implementation for `alloc::boxed::Box<_>`
    |
    = note: downstream crates may implement trait `DefaultIsZeroes` for type `alloc::boxed::Box<_>`

Which is a false positive, DefaultIsZeroes being a subtrait of Copy.

[tony-iqlusion]

Thanks for reporting this and the writeup. It seems there are major issues with both zeroize and secrecy.

Per the conflicting implementations error you're getting, I'm not sure how to address the Box situation in zeroize without specialization. I'll try to take a look at both of these issues in depth when I have some more spare time.

[troublescooter]

While trying to see what can be done about this, I ran into a differently worded error:

291 |   impl Zeroize for Box<[u8;3]> {
    |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `alloc::boxed::Box<[u8; 3]>`
    |
    = note: upstream crates may add a new impl of trait `core::marker::Copy` for type `alloc::boxed::Box<[u8; 3]>` in future versions

So apparently Box could in the future implement Copy, making this not a false positive, but I haven't been able to find an issue or link with more information. This doesn't help solve the problem, but it surprised me enough to share.

[ia0]

Isn't the problem that SecretBox<T> should not be implemented as Secret<Box<T>> but as a custom type instead? The reason is that we want T: Zeroize not Box<T>: Zeroize.

pub struct SecretBox<T: Zeroize>(Box<T>);

[tony-iqlusion]

@ia0 yes, I've wanted to refactor it to something like that, but haven't had time

[ia0]

Cool! Thanks for the feedback and no worries. Nothing urgent, just wanted to check my understanding.