stack zeroization #1215
stack zeroization #1215
Conversation
|
I think it might be better if something like this started out as its own experimental crate, especially as |
|
What should the crate be called? |
Regarding the rest, spike it out however you'd like, but the difficult part will be the rationale for why it's sound. |
|
I can go with With |
|
Yeah, I've seen Interesting prototype of the idea. |
|
Does it pass Miri? I left some ideas in the original issue. |
|
Darn. Miri wants me to use the following syntax due to how the psm::psm_stack_manipulation! {
yes {
/// define fn with psm::on_stack
pub unsafe fn blah(...) {...}
}
no {
/// define fn without psm::on_stack, likely panic
pub unsafe fn blah(...) { panic!("Stack manipulation not available on this platform")}
}
}After defining it like this, miri fails with the message "Stack manipulation not available on this platform". Likely because miri doesn't have a stack pointer or something... given that this crate was tied to the rust-lang account, I'd say their code is probably legit, and I'm not sure if Miri would be able to analyze all of the assembly without manually using different targets |
|
You can confirm that the test passes normally using |
|
Further evidence for miri not working can be found in https://github.com/rust-lang/stacker/blob/7a3ff32d72bcd0a12a938abb21deddf9f1449cdc/psm/build.rs#L66 |
… comment to debugging section; make no-panic feature for no branch of psm macro? handle closure panic with match?
… with futures::executor::block_on(f()), add asm alternative(?), handle unwind better(?), use stacker crate to handle stack size management(?) or at least use their code(?)
…marked out TODO task for details; next up: better panics
| ## Likely impossible and/or unsafe | ||
|
|
||
| * Add support for async closures, possibly using a macro to define the functions if necessary. Use `futures::executor::block_on(f())` to poll the entire future completion inside the stack switched context, and avoid `.await` that yields control outside of the `on_stack()` boundary. Something like: | ||
|
|
||
| ```rust | ||
| pub unsafe fn exec_async_on_sanitized_stack<Fut, F, R>( | ||
| stack: &mut [u8], | ||
| f: F, | ||
| ) -> Result<R, Box<dyn std::any::Any + Send>> | ||
| where | ||
| F: FnOnce() -> Fut + UnwindSafe, | ||
| Fut: Future<Output = R>, | ||
| { | ||
| let mut result = None; | ||
|
|
||
| on_stack(stack, || { | ||
| result = Some(catch_unwind(AssertUnwindSafe(|| { | ||
| // Block on the future inside the heap stack | ||
| futures::executor::block_on(f()) | ||
| }))); | ||
| }); | ||
|
|
||
| result.expect("Closure did not run") | ||
| } | ||
| ``` | ||
|
|
||
| Copilot provided that code, but Gemini says that after the future is awaited, there will be no way for the program to know which stack to return to. Also, there is an open issue regarding async closures in `stacker` that has not been resolved after 7 months. https://github.com/rust-lang/stacker/issues/111 |
There was a problem hiding this comment.
Do we agree with Gemini that this is either impossible or unsafe to perform? I can try to implement it and see if I get any segfaults, but async code is a low priority for me since basically all crypto operations are synchronous, and it can be ran in a way such that only crypto operations are done on separate stacks
|
Personally, I do not plan to merge this PR in the near future. Instead I would like to play with the idea I mentioned in the issue. |
That's fine, but |
|
@nstilt1 as some general advice, if you want to work on this sort of concept, I'd suggest keeping it as minimal as possible for an MVP. No Also instead of asking LLMs, which don't actually know anything and can hallucinate answers, I think you'd get a lot more mileage out of actually asking the |
Uses
psmto run a function/closure on a stack that is allocated on the heap, then zeroizes that separate stack when the execution finishes.I attempted to zeroize the space between two pointers: one captured before the closure's call and one after, but upon zeroing the stack, there was a segmentation fault. I will have to remove that code, but it's there in the first commit if anyone thinks they can get it to run without a segmentation fault.
Closes #810 if this is up to standards.