Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zeroize mem / proj status re safe-oqs? #269

Open
jac-cbi opened this issue Oct 11, 2024 · 2 comments
Open

Zeroize mem / proj status re safe-oqs? #269

jac-cbi opened this issue Oct 11, 2024 · 2 comments

Comments

@jac-cbi
Copy link

jac-cbi commented Oct 11, 2024

All,

I'm in the early stages of writing my project, which is in Rust, and relies on ML-KEM / Kyber (both, one for FIPS world, the other for non-fips).

safe-oqs Q

I originally incorporated a fork of liboqs-rust, safe-oqs because it had updated the submodule of liboqs to include the recently released ML-KEM. However, I now have a few questions, and @Scarjit and @joernheinemann don't have issues open on their fork...

@thomwiggers should liboqs-rust just pull in the relevant changes from safe-oqs? I looked at the diff between main branches, there's not much there. Just updated liboqs commit and exposing ML-KEM / ML-SIG. The rest is mostly renaming their crate

Real Q re Zeroize

Outside of that, my real question is this: Does liboqs-rust zeroize sensitive buffers on Drop? I see OQS_MEM_secure_free(), and a build check to ensure it's used over free(), but I don't see the Drop trait implemented on the Rust side in the macros. Drop only appears to be implemented for Kem and Sig, which, afaict, are just handles. Additionally, that Drop implementation is calling OQS_KEM_free(), which does not securely zero the memory.

Would it make sense to just #[derive(Zeroize, ZeroizeOnDrop)] from the zeroize crate?

@jac-cbi jac-cbi changed the title Zeroize mem / proj status re [safe-oqs](https://github.com/sentclose/liboqs-rust)? Zeroize mem / proj status re safe-oqs? Oct 11, 2024
@joernheinemann
Copy link

@jac-cbi Hello,

sorry forgot to enable the issues. Now it is enabled.

@jac-cbi
Copy link
Author

jac-cbi commented Oct 11, 2024

I should also mention I just completed reading through RustCrypto #1046 regarding redoing the interface for zeroize. Regardless of the outcome, it seems stalled, there's a lot of great insight in there about the intended uses of Zeroize and friends.

Based on this discussion, it would seem than zeroize / ZeroizeOnDrop should only be implemented on sensitive flat types. Would it make sense to have a newtype_zbuffer!() for PrivateKey and SharedSecret which implements Drop and zeroizes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants