Stream: wg-secure-code

Topic: Zeroize (https://crates.io/crates/zeroize)


Joshua Liebow-Feeser (Oct 16 2018 at 23:13, on Zulip):

I actually talked to Erick Tryzelaar about this. I definitely want to see it ported to Fuchsia.

Tony Arcieri (Oct 17 2018 at 13:31, on Zulip):

awesome, thanks

Tony Arcieri (Oct 17 2018 at 13:33, on Zulip):

the API is potentially RFC-able. just a trait I've implemented for some of the core types: https://docs.rs/zeroize/latest/zeroize/trait.Zeroize.html

Tony Arcieri (Oct 17 2018 at 13:33, on Zulip):

if it got into std, there wouldn't be any need for OS-by-OS support, since it could provide a stable API for an underlying LLVM intrinsic (volatile_set_memory in this case)

Joshua Liebow-Feeser (Oct 17 2018 at 17:16, on Zulip):

Yeah, that sounds like the way to go.

Tony Arcieri (Oct 17 2018 at 18:37, on Zulip):

@nikomatsakis do you have any thoughts regarding stabilizing volatile_set_memory with an API like https://docs.rs/zeroize/latest/zeroize/trait.Zeroize.html

Tony Arcieri (Oct 17 2018 at 18:37, on Zulip):

well, volatile_set_memory that always sets things to 0

Tony Arcieri (Oct 17 2018 at 18:42, on Zulip):

even just a stable wrapper that does a volatile zero on a &mut [u8] would be sufficient

nikomatsakis (Oct 17 2018 at 19:51, on Zulip):

I don't have deep thoughts :)

nikomatsakis (Oct 17 2018 at 19:51, on Zulip):

it seems like a .. probably fine thing; I imagine there will be some objections that the API on [u8] is too specific, but I'm not sure what the more general API should look like

Tony Arcieri (Oct 17 2018 at 21:02, on Zulip):

@nikomatsakis the Zeroize trait I linked above is an idea of what a more general API could look like. I guess the neat part is that much can be built out as a crate so long as there's some access to the intrinsic via core

nikomatsakis (Oct 17 2018 at 21:10, on Zulip):

I think that the idea of adding a stable "intrinsic primitive" is pretty appealing, yes

Tony Arcieri (Dec 18 2018 at 16:41, on Zulip):

@nikomatsakis so std::ptr::write_volatile has the volatile write semantics a "zeroizer" needs, but appears to require alignment. std::ptr::write_unaligned can perform unaligned writes, but they aren't volatile

Tony Arcieri (Dec 18 2018 at 16:42, on Zulip):

I think the minimum viable primitive which could be used for a crate like zeroize but is also generally useful would be something like std::ptr::write_unaligned_volatile or write_volatile_unaligned

nikomatsakis (Dec 18 2018 at 20:12, on Zulip):

interesting

nikomatsakis (Dec 18 2018 at 20:12, on Zulip):

it seems like there is no clear reason to couple those two things

Tony Arcieri (Dec 18 2018 at 20:44, on Zulip):

this would be as a stable wrapper for core::intrinsics::volatile_set_memory

Tony Arcieri (Dec 18 2018 at 20:45, on Zulip):

which AFAIK has write_unaligned_volatile behavior

Tony Arcieri (Dec 18 2018 at 20:45, on Zulip):

but perhaps I'm misunderstanding how one or more of these handle alignment

Tony Arcieri (Dec 18 2018 at 20:45, on Zulip):

perhaps I can dig up some past discussion of this sort of thing

Tony Arcieri (Dec 18 2018 at 20:46, on Zulip):

the use case is zeroing out arbitrary byte slices which are not necessarily aligned

briansmith (Dec 18 2018 at 20:51, on Zulip):

@Tony Arcieri a byte slice is always aligned since bytes have alignment 1.

briansmith (Dec 18 2018 at 20:52, on Zulip):

Other libraries may be different, but in ring if we wanted to zeroize things, we'd never need to do so in an unaligned way. It seems weird that that would be a requirement.

briansmith (Dec 18 2018 at 20:54, on Zulip):

Also, write_volatile is already unsafe so you could just cast the pointer to a *mut u8 and it would work fine without any more unsafe.

Tony Arcieri (Dec 18 2018 at 20:54, on Zulip):

@briansmith so I was a bit confused by write_volatile's documentation

Tony Arcieri (Dec 18 2018 at 20:55, on Zulip):

namely the Safety section: https://doc.rust-lang.org/std/ptr/fn.write_volatile.html#safety

Tony Arcieri (Dec 18 2018 at 20:55, on Zulip):

and comments like this:

Tony Arcieri (Dec 18 2018 at 20:55, on Zulip):

Note that even if T has size 0, the pointer must be non-NULL and properly aligned.

Tony Arcieri (Dec 18 2018 at 20:55, on Zulip):

what does "aligned" mean in the context of a zero-sized type?

Tony Arcieri (Dec 18 2018 at 20:55, on Zulip):

I assumed that meant "word aligned"

briansmith (Dec 18 2018 at 20:56, on Zulip):

Maybe it it referring to things like [u64; 0]?

briansmith (Dec 18 2018 at 20:56, on Zulip):

which presumably has alignment 4 or 8 but size 0

Tony Arcieri (Dec 18 2018 at 20:56, on Zulip):

if write_volatile already does what I want, hallelujah

Tony Arcieri (Dec 18 2018 at 20:57, on Zulip):

so if I have a variable-width byte slice, I could just iterate over it clearing it a byte-at-a-time?

briansmith (Dec 18 2018 at 20:58, on Zulip):

If you have an arbitrary byte slice, you can't really do it any other way since it might not be properly aligned to be accessed any other way.

Tony Arcieri (Dec 18 2018 at 20:59, on Zulip):

yeah not looking for a different way, just wanting to confirm that has the intended semantics

briansmith (Dec 18 2018 at 20:59, on Zulip):

Presumably you are getting the pointer that you pass to write_volatile from a reference.

briansmith (Dec 18 2018 at 20:59, on Zulip):

probably a reference to a slice.

briansmith (Dec 18 2018 at 21:00, on Zulip):

References are (assumed to be) always properly aligned so your pointer will always be properly aligned.

briansmith (Dec 18 2018 at 21:03, on Zulip):

FWIW, I still prefer the ISO C semantics for volatile where it is used to describe objects, not accesses to objects.

Tony Arcieri (Dec 19 2018 at 05:13, on Zulip):

well, if this is semantically correct... https://github.com/iqlusioninc/crates/pull/142/files

Tony Arcieri (Dec 19 2018 at 05:13, on Zulip):

not sure what else to say but "wow"

Tony Arcieri (Dec 19 2018 at 05:13, on Zulip):

Screen-Shot-2018-12-18-at-9.12.47-PM.png

Tony Arcieri (Dec 19 2018 at 21:25, on Zulip):

@nikomatsakis so I was looking at the generated assembly from ^^^ PR... it does more or less what you'd expect on a byte slice: loops over it doing mov and setting each byte to 0

Tony Arcieri (Dec 19 2018 at 21:26, on Zulip):

I totally plan on landing that PR, but I was looking at volatile_set_memory and it just calls memset, so I'm still wondering "what would a stable API for (volatile) memset look like"?

Tony Arcieri (Dec 19 2018 at 21:27, on Zulip):

I could imagine in a future with specialization and a stable wrapper for that I could specialize handling of &[u8] which so happens to be the main case I care about

nikomatsakis (Dec 19 2018 at 21:35, on Zulip):

the only difference really being that the stable API (write_volatile) writes "one T worth" of memory, and you'd prefer to have an n value, basically?

briansmith (Dec 19 2018 at 21:37, on Zulip):

@Tony Arcieri I think it is OK for volatile_set_memory to just call memset as long as the IR indicates that the call cannot be removed. In general, what's really relevant is the IR, not the generated assembly.

Tony Arcieri (Dec 19 2018 at 21:37, on Zulip):

@nikomatsakis yep!

Tony Arcieri (Dec 19 2018 at 21:37, on Zulip):

@briansmith I want memset!

Tony Arcieri (Dec 19 2018 at 21:37, on Zulip):

the problem is volatile_set_memory isn't stable

Tony Arcieri (Dec 19 2018 at 21:39, on Zulip):

the C equivalent of what I'd actually want is:

memset(dest, '\0', n);
asm volatile ("" ::: "memory");
briansmith (Dec 19 2018 at 21:46, on Zulip):

Ideally you'd want semantics that are like "treat this as a volatile write unless something else would overwrite what it would write"

Tony Arcieri (Dec 19 2018 at 21:46, on Zulip):

@nikomatsakis crazy idea: set_volatile() or fill_volatile() on the slice primitive

Tony Arcieri (Dec 19 2018 at 21:48, on Zulip):

or [u8] specifically would be fine

briansmith (Dec 19 2018 at 21:49, on Zulip):

It wouldn't be good to limit it only to u8. In fact most secret data probably shouldn't be in u8 arrays.

Alex Gaynor (Dec 19 2018 at 21:57, on Zulip):

memset_s I assume?

Tony Arcieri (Dec 19 2018 at 22:01, on Zulip):

@Alex Gaynor volatile memset, yeah

Tony Arcieri (Dec 19 2018 at 22:03, on Zulip):

@briansmith yeah, [u8] was in the context of "in a future Rust with specialization, I could specialize the implementation of Zeroize for [u8]". good enough for my purposes but I'd agree a more general primitive would be better. instead of specializing a faster implementation just for [u8], I could have something that's fast for all supported types

Tony Arcieri (Dec 19 2018 at 22:25, on Zulip):

but yeah anyway, this is all a microoptimization. in the meantime I'll be moving forward with what I've got even if it's slower

Tony Arcieri (Dec 19 2018 at 22:27, on Zulip):

getting rid of the yucky OS-specific API bindings would be worth it alone, but it's nice how easily this generalizes to all of the primitive number types

Tony Arcieri (Dec 19 2018 at 22:28, on Zulip):

this was a bit surprising: https://github.com/iqlusioninc/crates/blob/d9ec89ada4e89b6c9a9333ff9c67c650a35455af/zeroize/src/lib.rs#L100

Tony Arcieri (Dec 19 2018 at 22:28, on Zulip):

the same macro works on char :open_mouth:

Tony Arcieri (Dec 19 2018 at 22:28, on Zulip):

not sure that's actually doing what I want

Tony Arcieri (Dec 19 2018 at 22:28, on Zulip):

possibly doing something unsafe :sweat_smile:

briansmith (Dec 19 2018 at 22:35, on Zulip):

@Tony Arcieri Couldn't you impl Zeroize for T where T: Defaultwithout any macro?

Alex Gaynor (Dec 19 2018 at 22:36, on Zulip):

Default is implemented on things like Vec<T>, where this wouldn't work, would it?

briansmith (Dec 19 2018 at 22:38, on Zulip):

Good point.

Tony Arcieri (Dec 20 2018 at 14:04, on Zulip):

@briansmith haha, came across this thread where you seem to be asking the questions I'm curious about now https://internals.rust-lang.org/t/volatile-and-sensitive-memory/3188/24

Tony Arcieri (Dec 20 2018 at 14:05, on Zulip):

was looking specifically at this in the write_volatile docs

Tony Arcieri (Dec 20 2018 at 14:05, on Zulip):

Memory accessed with read_volatile or write_volatile should not be accessed with non-volatile operations.

Tony Arcieri (Dec 20 2018 at 14:08, on Zulip):

Now I better understand what you're getting at here:

FWIW, I still prefer the ISO C semantics for volatile where it is used to describe objects, not accesses to objects.

briansmith (Dec 20 2018 at 18:05, on Zulip):

Yes, I spent a significant amount of time trying this idea before. Note in particular that any movable type (i.e. basically all types) do have non-volatile accesses when they are moved.

briansmith (Dec 20 2018 at 18:05, on Zulip):

And, obviously, the same for Copy

Tony Arcieri (Dec 20 2018 at 18:22, on Zulip):

yep, I already have a whole disclaimer in my README about that sort of thing

Tony Arcieri (Dec 20 2018 at 18:23, on Zulip):

noting the Pin RFC

Tony Arcieri (Dec 20 2018 at 18:25, on Zulip):

here's my WIP text... I'm about to mention ^^^ internals.rust-lang thread and @nikomatsakis's response at the very end:

## What guarantees does this crate provide?

Ideally a memory-zeroing function would do two things:

1. Ensure the zeroing operation can't be "optimized away" by the compiler.
2. Ensure all subsequent reads to the memory following the zeroing operation
   will always see zeroes.

This crate guarantees #1 is true: LLVM's volatile semantics ensure it.

The story around #2 is much more complicated. In brief, it should be true that
LLVM's current implementation does not attempt to perform optimizations which
would allow a subsequent (non-volatile) read to see the original value prior
to zeroization. However, this is not a guarantee, but rather an LLVM
implementation detail.

For more background, we can look to the [std::ptr::write_volatile()]
documentation:

> Volatile operations are intended to act on I/O memory, and are guaranteed
> to not be elided or reordered by the compiler across other volatile
> operations.
>
> Memory accessed with read_volatile or write_volatile should not be accessed
> with non-volatile operations.

Uhoh! This crate does not guarantee all reads to the memory it operates on are
volatile, and explicitly warns against mixing volatile and non-volatile
operations. Perhaps we'd be better off with something like a `VolatileCell`
type which owns the associated data and ensures all reads and writes are
volatile so we don't have to worry about the semantics of mixing volatile and
non-volatile accesses.

While that's a strategy worth pursuing (and something we may investigate
separately from this crate), it comes with some onerous API requirements:
it means any data that we might ever desire to zero is owned by a
`VolatileCell`. However, this does not make it possible for this crate
to act on references, which severely limits its applicability.

It's worth asking what the semantics of mixing volatile and non-volatile reads
actually are, and whether a less obtrusive API which can act entirely on
mutable references is possible, safe, and provides the desired behavior.
Unfortunately, that's a tricky question, because Rust does not have a formally
defined memory model, and the behavior of mixing volatile and non-volatile
memory accesses winds up largely being an LLVM implementation detail.

[To Be Continued]
Tony Arcieri (Dec 20 2018 at 18:27, on Zulip):

err reading over my own text I'm seeing a few things I need to rewrite :wink:

Tony Arcieri (Dec 20 2018 at 18:28, on Zulip):

This crate does not guarantee all reads to the memory it operates on are volatile, and explicitly warns against mixing volatile and non-volatile
operations

Tony Arcieri (Dec 20 2018 at 18:28, on Zulip):

need to reword that in particular

Tony Arcieri (Dec 21 2018 at 00:43, on Zulip):

@nikomatsakis so based on your response here: https://internals.rust-lang.org/t/volatile-and-sensitive-memory/3188/46

Tony Arcieri (Dec 21 2018 at 00:44, on Zulip):

it seems like the best path forward is something that has VolatileCell-like semantics

Tony Arcieri (Dec 21 2018 at 00:44, on Zulip):

i.e. it owns the data, and ensures all accesses are volatile

Tony Arcieri (Dec 21 2018 at 00:44, on Zulip):

something like that, plus Pin

Tony Arcieri (Dec 21 2018 at 00:45, on Zulip):

and I think you can enforce fairly reasonable zeroization semantics, which are:

Tony Arcieri (Dec 21 2018 at 00:45, on Zulip):

1) don't make (non-explicit) copies

Tony Arcieri (Dec 21 2018 at 00:45, on Zulip):

2) don't allow non-volatile accesses

Tony Arcieri (Dec 21 2018 at 00:46, on Zulip):

3) the "zeroization" operation (which can now be performed on Drop) is guaranteed not to be "optimized away" or otherwise elided by the compiler

Tony Arcieri (Dec 21 2018 at 00:46, on Zulip):

would you agree?

Tony Arcieri (Dec 21 2018 at 00:47, on Zulip):

just wondering if I'm missing something here

Tony Arcieri (Dec 21 2018 at 00:47, on Zulip):

re: what I have in ^^^ PR to zeroize it is... "Not Optimal" because it mixes volatile and non-volatile accesses

Tony Arcieri (Dec 21 2018 at 00:49, on Zulip):

also I was wondering if core::sync::atomic::compiler_fence and/or core::sync::atomic::fence can provide reliable, portable semantics for mixing volatile and non-volatile accesses

Tony Arcieri (Dec 21 2018 at 00:49, on Zulip):

I have never used either and do not have a good sense of what either do

Tony Arcieri (Dec 21 2018 at 01:31, on Zulip):

@nikomatsakis also any thoughts on something like VolatileCell in core/std?

Tony Arcieri (Dec 21 2018 at 23:09, on Zulip):

so VolatileCell for this purpose has a big drawback: it reads an owned value. if the goal is to erase a secret, making copies of it to read it seems... suboptimal

Tony Arcieri (Dec 22 2018 at 14:59, on Zulip):

added memory fences... and also a marker trait for zeroizing using the Default value which when used as the trait bound for things like collections allows them to only be used once at the end of the collection https://github.com/iqlusioninc/crates/pull/146/files

Tony Arcieri (Dec 22 2018 at 15:02, on Zulip):

was also able to add special-case optimizations on nightly for [u8] and [u8; N] which are the main cases I care about

Tony Arcieri (Dec 22 2018 at 15:03, on Zulip):

using volatile_set_memory

nikomatsakis (Jan 02 2019 at 22:12, on Zulip):

it seems like the best path forward is something that has VolatileCell-like semantics

I'm trying to bring this back in cache -- I know that @RalfJ and I were talking about this sort of thing from time to time in conjunction with the work on stacked borrows and other parts of the unsafe-code-guidelines. To some extent, a union may already fit the bill here as well, since the compiler is not generally permitted to assume that the contents are valid

nikomatsakis (Jan 02 2019 at 22:13, on Zulip):

I feel like the upcoming topic for the unsafe-code-guidelines discussion -- which will be a focus on the invariants that have to hold for various types -- is going to be very apropos

nikomatsakis (Jan 02 2019 at 22:13, on Zulip):

(that said, the LLVM treatment of unions may not be sufficiently conservative I suppose, so maybe there is still a need for a volatile cell sort of thing)

nikomatsakis (Jan 02 2019 at 22:14, on Zulip):

I guess the question is whether there are optimizations we could do on unions that would be ruled out by volatile cell

Tony Arcieri (Jan 02 2019 at 23:26, on Zulip):

@nikomatsakis upon further reflection VolatileCell is the opposite of what I want... it makes a copy each time data is read, and I'm trying to reduce the number of copies to 0 :wink:

RalfJ (Jan 03 2019 at 10:52, on Zulip):

FWIW, I still prefer the ISO C semantics for volatile where it is used to describe objects, not accesses to objects.

That's not correct: ISO C also allows volatile accesses, you can cast a pointer to a volatile pointer and use that. adding volatile objects adds lots of complexity for no gain at all. In the best case, a definition in terms of objects can coincide with the access-based definition for the case where there is no object with mixed accesses.

Personally, I never understood volatile objects, for many years. then somewhere I found an explanation in terms of accesses and suddenly everything made sense. Also note that the concurrency memory model is entirely defined in terms of accesses, not objects.
I am still curious why you think this makes more sense for objects -- but we've had this discussion before and did not get very far.

RalfJ (Jan 03 2019 at 10:54, on Zulip):

Volatile objects make sense as a high-level API, just like we have "atomic objects" like AtomicUsize. But they are the wrong level of abstraction for defining the semantics.

RalfJ (Jan 03 2019 at 10:56, on Zulip):

it seems like the best path forward is something that has VolatileCell-like semantics
i.e. it owns the data, and ensures all accesses are volatile

agreed. and you can define this entirely as a library, just like AtomicUsize.

so VolatileCell for this purpose has a big drawback: it reads an owned value. if the goal is to erase a secret, making copies of it to read it seems... suboptimal

what do you mean by this?

RalfJ (Jan 03 2019 at 12:21, on Zulip):

one potential issue here is the dereferencable attribute; for that, also see https://github.com/rust-lang/rust/issues/55005

Tony Arcieri (Jan 03 2019 at 14:16, on Zulip):

@RalfJ naively it looks like VolatileCell performs a volatile read but leaves a copy of the data on the stack

Tony Arcieri (Jan 03 2019 at 14:16, on Zulip):

which is literally the opposite of what I want

RalfJ (Jan 03 2019 at 14:28, on Zulip):

VolatileCell is a type, not an operation, it doesn't do anything.^^ which method are you referring to?

RalfJ (Jan 03 2019 at 14:30, on Zulip):

but also, all of rustc, LLVM (and also GCC, ...) dont have a notion of data being secret, so you'll always be fighting their optimizers etc because you are making a difference between behaviors that they deem equivalent

Tony Arcieri (Jan 03 2019 at 14:37, on Zulip):

I'm specifically referring to the accessors, which work on values, not references, e.g. https://github.com/japaric/vcell/blob/master/src/lib.rs#L43

Tony Arcieri (Jan 03 2019 at 14:38, on Zulip):

unless I'm mistaken, that's making a copy of a value onto the stack

RalfJ (Jan 03 2019 at 15:39, on Zulip):

what else would get do? you asked to get access to the value directly, of course that puts it outside the cell

Tony Arcieri (Jan 03 2019 at 15:47, on Zulip):

well going back to my original comment you were asking about...

Tony Arcieri (Jan 03 2019 at 15:47, on Zulip):

so VolatileCell for this purpose has a big drawback: it reads an owned value. if the goal is to erase a secret, making copies of it to read it seems... suboptimal

Tony Arcieri (Jan 03 2019 at 15:47, on Zulip):

I don't want to make copies

Tony Arcieri (Jan 03 2019 at 15:47, on Zulip):

I am trying to delete existing copies

RalfJ (Jan 03 2019 at 16:04, on Zulip):

why do you call get if you want to erase a secret...?

Tony Arcieri (Jan 03 2019 at 16:04, on Zulip):

I think you're misunderstanding what I'm saying

Tony Arcieri (Jan 03 2019 at 16:04, on Zulip):

VolatileCell doesn't solve my problem so I'm not using it

RalfJ (Jan 03 2019 at 16:04, on Zulip):

I think so too :)

briansmith (Jan 03 2019 at 19:34, on Zulip):

FWIW, I still prefer the ISO C semantics for volatile where it is used to describe objects, not accesses to objects.

That's not correct: ISO C also allows volatile accesses, you can cast a pointer to a volatile pointer and use that.

I don't think that is right. The C standard consistently uses the phrase "object defined with a volatile-qualified type", says accessing these kinds of objects through volatile-qualified types is implementation defined, and says using them as non-volatile-qualified lvaules is undefined behavior. It doesn't require an implementation to give volatile semantics to objects it knows are not volatile even if they are exclusively accessed through volatile pointers. However, there is a recent proposal by a Googler to change this to the LLVM semantics.

briansmith (Jan 03 2019 at 19:39, on Zulip):

Regardless of that, I understand that at this point it isn't useful to debate what Rust will do w.r.t. volatile. I prefer the C way, except without the ridiculous ease of (accidentally) wrongly casting from volatile <-> non-volatile in ways that don't work, but I understand that it is unlikely that Rust would change to that any time soon, especially because rustc will be using LLVM; it seems easier to change the C standard than to change LLVM, as evidenced by Google's proposal.

RalfJ (Jan 04 2019 at 11:00, on Zulip):

another reason we should prefer volatile accesses IMO is that there's a trivial translation from a "volatile-object" based world to a "volatile-access" based world -- every access to a volatile object is a volatile access, done. there is no translation possible in general the other way. so, volatile accesses are strictly more general.

RalfJ (Jan 04 2019 at 11:06, on Zulip):

The C standard consistently uses the phrase "object defined with a volatile-qualified type", says accessing these kinds of objects through volatile-qualified types is implementation defined, and says using them as non-volatile-qualified lvaules is undefined behavior.

Indeed, I misremembered.

It doesn't require an implementation to give volatile semantics to objects it knows are not volatile even if they are exclusively accessed through volatile pointers.

A possible reading of the standard is that having a volatile-qualified lvalue makes the object in that lvalue a volatile object.

Notice that it's not just LLVM that thinks of volatile as being an access-qualifier; the Linux kernel does that a lot so this is for sure also the way GCC handles volatile.

Tony Arcieri (May 21 2019 at 17:36, on Zulip):

nice, thanks @RalfJ https://github.com/rust-lang/rust/pull/60972

RalfJ (May 21 2019 at 17:37, on Zulip):

I feel guilty because I was involved in adding this text (and didn't foresee the confusion it would cause), so this is only fair ;)

DevQps (May 22 2019 at 09:50, on Zulip):

Nice work indeed! (Y)

Last update: Nov 11 2019 at 22:50UTC