Shared secret stored on stack, making zeroization ineffective?

[faern]

Awesome that you added zeroize support in #51! But it seems to only clear internal state. The most important data, the actual decapsulated shared secret returned from Decapsulate::decapsulate is returned in an array placed directly on the stack.

This probably means the compiler sprinkles copies of this data in many places in memory, with no effective way to clear it out. As a user of the library can I feel safe that after I have performed a decapsulation and returned control upstream, other code running in the same thread can't find the secret on stack? Or other processes running after my process exits. I currently don't think so? Since you did add the zeroize feature I trust you acknowledge the core problem and want to address it? I'd love to be told I'm wrong and that I missed some part of this, making the current API not leak secrets into memory.

The best way I know of to solve this is to let the encapsulate and decapsulate methods take mutable references to the buffers where the output secrets should be stored. And potentially with more ergonomic helper variants just returning the secrets in a Box<Secretz> type that properly implements zeroization. The Classic McEliece library did exactly this.

I reported the same issue towards the Kyber implementation we used before trying to move over to ml-kem.

[tarcieri]

The most important data, the actual decapsulated shared secret

Our zeroize features generally focus on long-term as opposed to ephemeral secrets.

There will be quite a bit of secret data left on the stack after decapsulation. Optimizing compiler features like stack spilling allow rustc to make copies at will.

If you want to ensure the stack is clear of secrets, it would be best to explicitly clear it yourself.

Here is a similar issue where we have discussed this sort of thing in the past: RustCrypto/traits#1494

[faern]

Thanks for the link to the previous discussion.

There will be quite a bit of secret data left on the stack after decapsulation. Optimizing compiler features like stack spilling allow rustc to make copies at will.

Yes, this was my point exactly. My point was to stop doing that. But my understanding of the linked discussion is that you think it's pointless to try to avoid the stack, because the cryptographic code will inevitably spill some internal computations to the stack anyway while running, so stack zeroing would be needed no matter what, if you care about secrets being left in memory?

If you want to ensure the stack is clear of secrets, it would be best to explicitly clear it yourself.

I don't want to. My point was that I should not have to 😅
There seems to be no good way of clearing the stack anyway? The thread only points to architecture specific asm stuff and a "research" crate.

[newpavlov]

stack zeroing would be needed no matter what, if you care about secrets being left in memory?

Yes. This is why spilling of secrets onto stack is out of scope for us. We try to minimize it when it's possible to do without any adverse effects, but the compiler effectively works against us, so we do not spend too much effort on this.

There seems to be no good way of clearing the stack anyway?

The main reason is that it's borderline impossible to generically find an upper bound on how much stack code may use. Compilers have the necessary information in the vary late compilation stages (and tools like cargo-call-stack can retrieve it), but it's simply not available at our level. So you either have to use some "reasonable" guess (e.g. 4 KiB) which may not apply for some algorithms, or use very platform-specific code to erase whole thread stack (which can be REALLY inefficient, imagine zeroizing ~8 MB after each cryptographic function call).

I think we can close this issue as "not planned".

[faern]

Ok. Yeah I understand it can be hard for RustCrypto in itself to do this perfectly. But I would really like to see better support for memory cleaning in the language overall. I understand your decision to close this issue.

For our use case it would be perfect if there was a std::thread::Builder::zeroize_stack(bool) method on the thread builder. And this would make the standard library clear the stack once a thread exits.

EDIT: Here is an IRLO thread on the topic: https://internals.rust-lang.org/t/annotations-for-zeroing-the-stack-of-sensitive-functions-which-deal-in-transient-secrets/11588/9

[newpavlov]

If your code is not generic over cryptographic algorithms, then you can use code like this: https://rust.godbolt.org/z/E6c68hcdh The necessary size for stack erasure can be found experimentally by inspecting stack state after execution of your algorithm. Note the #[inline(never)] annotations, they are important for proper erasure of stack.

This code is not 100% robust, since black_box may be ignored by alternative compilers (but it works in practice for the mainline Rust compiler), so if you target only x86/ARM/RISC-V, it may be worth to replace it with an empty asm! block.

[faern]

Thank you! Yeah I stumbled upon code similar to that in the linked thread above. I guess something like that is the best we can do right now.

[mouse07410]

Please explain the threat model. Your VPN presumably needs to keep the session keys, presumably derived from the shared secret in question, in memory for the entire duration of the session. Which, as you said, may last hours or days. If a hostile application running on the same CPU or otherwise having access to your memory, decides to examine it to extract your keys - how are you going to protect against it? With or without stack zeroization?

[faern]

@mouse07410 You are correct that the session keys need to exist in memory for the entire duration of the tunnel. But just because we need them in one location in memory that is not justification for letting the guard down completely and be fine with it being copied freely and spread all over the place. I still see a lot of value in trying to limit the spread of the key material to the single place where they are strictly needed. This significantly reduces the risk of heartbleed style attacks. If the key material exists in many places in memory, it is more likely that things that should not have access to the key will be able to get access to it.

For the platforms where we use WireGuard in the kernel (Linux and Windows) we could clean all user space memory from the key material once the tunnel is set up, and only let it live in kernel memory.

Defense in depth argues there should be as many protections in place as possible, even if we can't guarantee everything is secure against every possible attack. Having the keys in as few places in memory as possible is one of those layers.

[mouse07410]

I understand the "defense in depth" concept. But how did "two places" (one - on the stack, the other - presumably your copy on the heap) become "all over the place"?
Also, what are you going to do about it, given that the source code is available?

Also, if a hostile is sharing your CPU (and presumably physical RAM), don't you think you've got bigger problems?