zeroize: add a big fat warning that it's a low-level primitive #659
Comments
|
Could you provide a complete blurb you'd like to see added, and make suggestions as to where it should go relative to the other parts of the documentation? Possibly open a PR? Much of what you're describing is already covered in the already voluminous existing documentation, however it's buried at the bottom and it sounds like you'd like it closer to the top? |
vsrinivas
pushed a commit
to vsrinivas/fuchsia
that referenced
this issue
Apr 11, 2022
It would be dangerous for Fuchsia engineers to rely on zeroize for enforcing security invariants, but it is difficult to remove it from our build graph entirely (see alternatives). **Background** zeroize can't handle a variety of cases like a dynamic buffer which was reallocated partway through writing a secret into it, or a buffer with a secret small enough for the containing buffer to implement Copy. The guarantees it attempts to provide are implemented by inhibiting compiler optimizations that would eliminate dead stores, since to LLVM what zeroize does looks like writing to memory that no one will read. A new LLVM release can always optimize zeroize out of the resulting binary, meaning that not even a best- effort attempt to conceal secrets would be made. Some evidence from upstream that its guarantees can be confusing: RustCrypto/utils#659 RustCrypto/utils#702 And that they're difficult to implement without invoking UB: RustCrypto/utils#653 **Alternatives** *** (1) Make zeroize optional in RustCrypto crates *** We could fork the crates which depend on zeroize locally and work with upstream to release versions where the dependency is optional with the goal of unforking once they were released. Making the dependency optional requires cargo's weak and namespaced deps features that were just stabilized in 1.60, while RustCrypto maintains an MSRV of 1.41, released in February 2020. Bumping MSRV is a significant action for a widely-used Rust crate, and we should not expect maintainers to do so lightly or to be able to bump to 1.60 any time soon. *** (2) Fork zeroize to remove UB and restrict visibility *** We could add support to cargo-gnaw for configurable visibility limits that would allow our transitive RustCrypto crates to use zeroize but not for it to be added as a dep within the main build graph. This approach also forks zeroize, but instead of removing all of its functionality we would fix any UB in the library. From upstream issues its not clear this can be done in the current semantics of Rust. Even if it were possible, it will be difficult to be certain we've addressed all possible UB and this approach is ultimately higher effort than removing all of the crate's code. Fixed: 96317 Change-Id: Ia5419d3cf73ef7f971e8a72a56e8ece495078395 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/667952 Reviewed-by: Tyler Mandry <[email protected]> Reviewed-by: Chris Palmer <[email protected]> Commit-Queue: Adam Perry <[email protected]> Fuchsia-Auto-Submit: Adam Perry <[email protected]>
|
I think we can close this issue. We will be happy to receive documentation improvement PRs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
zeroizeis a low-level crate and does not guard e.g. against implicitly copying the value. However, people sometimes assume that it does: https://benma.github.io/2020/10/16/rust-zeroize-move.htmlWhile
secrecycrate is recommended in the docs, it's buried below the sales pitch. Perhaps a big warning right at the top thatzeroizeis a low-level crate and most people should usesecrecyinstead would prevent such misunderstanding.The text was updated successfully, but these errors were encountered: