Add Zeroize and Drop implementations #13
Conversation
cfb-mode/Cargo.toml
Outdated
| @@ -12,6 +12,7 @@ categories = ["cryptography", "no-std"] | |||
| [dependencies] | |||
| stream-cipher = "0.3" | |||
| block-cipher-trait = "0.6" | |||
| zeroize = { version = "*", optional = true } | |||
There was a problem hiding this comment.
Would probably prefer locking to a particular version here. As it were I might try to put another one out today... will follow up after I do
There was a problem hiding this comment.
Also note: I'd like to get to 1.0 soon, but I want to verify the generated assembly appears to do the right thing on more platforms than just x86(-64).
There was a problem hiding this comment.
You could maybe write a test that uses unsafe to turn a static pointer into one with a lifetime, then initialize it with non-zero values, then drop it and test that the data got zeroed out.
It's a bit more advanced than my current rust skill level, but I could look into it on a weekend maybe.
There was a problem hiding this comment.
Just a quick note: after reading this thread and seeing this change to the Rust documentation, I'm feeling much better about Zeroize's current semantics and plan on dramatically reducing the length of the explanation along with much of the scary language.
It seems much of what is in there is no longer aligned with the opinions of the Unsafe Code WG, which is to say the behavior is defined and, theoretically, a safe abstraction.
That said, after the 0.8 release I am still tweaking things like trait bounds and the procedural macro, so I'd love to get feedback from people integrating it (I am also attempting to integrate it into curve25519-dalek).
| } | ||
|
|
||
| #[cfg(cargo_feature = "zeroize")] | ||
| impl<C> Drop for Cfb<C> { |
There was a problem hiding this comment.
These could potentially be:
#[cfg_attr(cargo_feature = "zeroize", derive(Zeroize, ZeroizeOnDrop))]On pub struct Cfb<C: ... above.
|
A general note on this is as of v0.8 the custom derive support in There's a few places where I want to tweak trait bounds, but I think a lot of the handwritten drop handlers could be replaced with a proc macro. I'm also interested in refining the proc macro, so let me know if you have any difficulties. As of v0.8 the proc macro's codegen is tested (and implemented at a much higher level thanks to https://github.com/iqlusioninc/crates/blob/develop/zeroize_derive/src/lib.rs#L90 |
I haven't messed with rust macros up to this point, so if you want to do it, go ahead. I will look at writing the test I described this weekend. |
| @@ -51,12 +51,20 @@ | |||
| pub extern crate stream_cipher; | |||
| extern crate block_cipher_trait; | |||
|
|
|||
| #[cfg(cargo_feature = "zeroize")] | |||
There was a problem hiding this comment.
AFAICT #[cfg(cargo_feature = "zeroize")] does absolutely nothing. I think all of these need to be changed to #[cfg(feature = "zeroize")]
|
@emc2 it'd be good if you could rebase on |
|
Closing this out as extremely stale. This is adding an out-of-date If you're still interested in this, please reopen a new PR starting from the latest |
This changeset uses the Zeroize crate combined with implementations of the Drop trait to ensure that sensitive cryptographic state information is zeroed out when cryptographic objects go out of scope. This is a defense against leaking sensitive information into unallocated memory, which is a common bug in cryptographic implementations.