clear_on_drop and clear_stack_on_return

[cesarb]

This RFC proposes a pair of attributes, #[clear_on_drop] and #[clear_stack_on_return], to make it easy to write code which securely clears sensitive data (for instance, encryption keys) after its use.

Rendered

[oli-obk]

Copy types cannot implement Drop: http://is.gd/PDl3Hu

[cesarb]

The error message links to rust-lang/rust#20126, which says implementing Copy and Drop together is currently disabled because the implementation was broken.

Of course, that would either have to be fixed to be able to implement #[clear_on_drop] on Copy types, or #[clear_on_drop] would have to initially be restricted to non-Copy types. I believe being unable to use it with Copy would not be a big restriction.

[comex]

Keep in mind that if a function uses callee-saved registers, the callee will typically save them to the stack, where they may remain after it returns if no later call overwrites them. Therefore, the attribute should probably either ban them, or force them to be treated as caller-saved and clear them before calls.

Since this is mostly a job for LLVM, it would probably be easier to first get an implementation of this upstream (and add a Clang attribute)...

[main--]

Keep in mind that if a function uses callee-saved registers, the callee will typically save them to the stack, where they may remain after it returns if no later call overwrites them.

This affects not only "accidental" parameters like callee-saved registers but actual (intentional) parameters as well. Quoting the RFC:

• When any function is inlined within a #[clear_stack_on_return] function, the inner (inlined) function MUST be treated as if it also were marked with the #[clear_stack_on_return] attribute. This is important for safety: these are often small accessor functions, for instance a Vec's index().
• For the same reason, functions marked with #[inline] should always be inlined within a #[clear_stack_on_return] function, as if they were #[inline(always)].

While this alleviates the issue for all inlineable functions, all the others are still insecure: They might spill your secrets all over their stack frame and there's nothing you can do about it. While it's always advisable to carefully review all utility libraries when writing security-critical code, this proposal is absolutely unforgiving to a programmer that forgets to apply the necessary attribute to just a single function. Furthermore, as some of the discussion in #243 showed, the absence of something is obviously much harder to detect, making (correct) code reviews/audits much harder.

Transitivity in general is a big problem as even the RFC admits how something like #[clear_on_drop] struct Secret { data: Vec<u8> } can be a fatal mistake.

I'm not sure how to improve the situation, but I do think that there needs to be some mechanism that assists programmers in avoiding these issues.

[cesarb]

I'm not sure how to improve the situation, but I do think that there needs to be some mechanism that assists programmers in avoiding these issues.

Perhaps a pair of lints? One to warn when a #[clear_on_drop] type contains a pointer(&T, &mut T, *const T or *mut T), and another to warn when a #[clear_stack_on_return] function calls something that's not #[clear_stack_on_return] or #[inline]?

Edit: and a third lint to warn when something in a impl block for a #[clear_on_drop] type is not #[clear_stack_on_return], to catch cases where the programmer forgot to add the function attribute?

[cesarb]

I have incorporated all the feedback so far into the RFC. Please take a look.

[matthieu-m]

I am slightly concerned with the fact that those attributes would be mostly useless in absence of correct codegen, and codegen fully implicates LLVM. Is there any benefit in pursuing this discussion without any idea of what LLVM could offer here?

LLVM currently offers nothing (or close to), and I would think that any improvement to LLVM toward this functionality would have to be discussed and refined with the current Clang/LLVM developers. Given their experience, they might both:

• suggest significant changes
• suggest limitations
• ...

I laud the goal of this RFC, but I find it quite premature.

[aturon]

@cesarb Thanks much for this RFC!

The lang team discussed the RFC while attempting to find a shepherd for it. Our feeling is that, while the goal is a good one, there are a few reasons we're not ready to consider the RFC in the near future:

• The biggest one is the lack of needed LLVM support. We are not in a position to make commitments on behalf of LLVM, and until the necessary support is available, we can't make progress along the lines the RFC is proposing.
• We are in the midst of significant changes to how drop works -- in particular, with the upcoming MIR rollout, we plan to move to "non-zeroing drop". We would prefer to wait until these changes have settled before considering further changes in this area.

As such, we're going to close the RFC as postponed for the time being. Aside from the above concerns, in a future RFC we'd also like to see more discussion of prior art and expected usage patterns.

[cesarb]

I wrote a crate which partially implements what I proposed in this RFC: https://crates.io/crates/clear_on_drop