On Thu, 30 Apr 2020, 07:39 Arthur O'Dwyer via SG12, <sg12_at_[hidden]>
wrote:

> On Thu, Apr 30, 2020 at 2:19 AM Jens Maurer <Jens.Maurer_at_[hidden]> wrote:
>
>> On 30/04/2020 02.10, Arthur O'Dwyer via SG12 wrote:
>> >
>> > It's easy to ensure that an operation */is/* performed; all you have to
>> do is insert an optimization barrier like Google Benchmark's DoNotOptimize,
>> which is basically an `asm volatile("")` statement.
>>
>> Yes.
>> Paul's P1382
>> <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1382r1.pdf> is
>> much easier in this regard, and very close to traditional volatile
>> semantics.
>>
>
> I wouldn't call P1382 "easier" on the human programmer; it's a lot more
> fiddly than a simple "escape" or "DoNotOptimize" function that can fence
> any side-effect at all. But it's certainly close to traditional `volatile`.
>
> The equivalent of `memset(passwordbuffer, 0x00, sizeof passwordbuffer)`
> under P1382 would be
> char passwordbuffer[100];
> ...
> for (auto& ch : passwordbuffer) {
> volatile_store(&ch, '\0'); // note that volatile_store(&ch, 0)
> would not compile, and I think that's too bad
> }
> although I think P1382 needs to explicitly explain whether
> char sequence_of_zeros[100] = {};
> std::volatile_store<char[100]>(&passwordbuffer, sequence_of_zeros);
> is intended to be supported. (I predict it's not supported, because SD-8
> <https://isocpp.org/std/standing-documents/sd-8-standard-library-compatibility>,
> but I'd like to see it in writing in the paper.)
>
>
> Tony Van Eerd suggests:
> > a function: volatile_clear(void *ptr, int size), that does volatile
> writes of 0 into the memory.
> > Is this a worthwhile step?
>
> IMHO that would be a useful addition to P1382 (given the fiddliness of
> using `volatile_store` as shown above).
> I would even call it `volatile_memset` with the appropriate signature,
> since we have prior art for memset.
>
> Niall Douglas suggests:
> > I'd gladly take into the standard an:
> > assume_escaped(const byte *, size_t);
> > I can build stuff on top of that.
>
> That signature should be simply `void assume_escaped(const void*)`. The
> `size_t` parameter is pointless, because the entire point of this function
> (from the compiler's point of view) is that *everything reachable from
> that pointer* must be assumed to have escaped.
> int a[2];
> int *p = &a[0];
> int *q = &a[1];
> assume_escaped(p);
> assume_escaped(q);
> Those two lines should have the same effect from the compiler's point of
> view, since the memory at `*p` is also reachable via the expression
> `*(q-1)`. If `q` escapes into
> a-function-that-the-compiler-doesn't-know-what-it-does, then the compiler
> must assume that `*(q-1)` might be read by that function.
> Here is Google's `DoNotOptimize(const T&)`.
> <https://github.com/google/benchmark/blob/master/include/benchmark/benchmark.h#L307-L345>
> If we had a simple DoNotOptimize-style library function, we wouldn't need
> `volatile_store` *for Miguel's purpose*. But I believe Paul wants
> `volatile_store` for more "lock-free atomic access patterns" kinds of
> things; `volatile_store` has other benefits for his domain, such as
> non-tearing.
>
>
> > memset(passwordbuffer, 0x00, sizeof passwordbuffer);
>> > DoNotOptimize(&passwordbuffer); // escape this variable to force
>> the memset to be respected
>> >
>> > What secure_clear is trying to do is to ensure that a "sufficiently
>> dumb compiler" does not compile the above code into the equivalent of
>> >
>> > char *secondcopy = strdup(passwordbuffer); free(secondcopy); //
>> oops, leaked a copy of the data onto the heap
>> > static char secondcopy[100]; strcpy(secondcopy, passwordbuffer);
>> // oops, leaked a copy of the data into the data section
>> > memset(passwordbuffer, 0x00, sizeof passwordbuffer);
>> > DoNotOptimize(&passwordbuffer); // the memset is respected, but
>> useless
>>
>> And then, the compiler does
>> strcpy(passwordbuffer, secondcopy);
>> rigbt before "passwordbuffer" ends its lifetime. Also undesirable and
>> totally defeating the purpose, but fine under the status quo as-if rule.
>>
>
> Sure; but my point was that the purpose of "not leaking the secret" has *already
> *been defeated, even if the compiler *doesn't *do what you said. As soon
> as the very first copy is made, the security has gone out the window. And
> those copies can be made under the as-if rule.
>

Right. Rather than pursuing an option that we don't know of any way to
extend to being an actually secure clear mechanism, we should find some way
to express the actually desired semantics of "don't leak the secret stuff".

For example, suppose we had a function attribute:

[[secure_clear]]

... with no normative semantics (because it has no effect on the abstract
machine), but with recommended behaviour that it ensures to the extent
possible that data used in the function is not accessible after it
terminates, by any means -- in particular, that any stack space or
registers used by the function are cleared when the function returns or
propagates an exception. That seems implementable, and seems like it would
actually give the desired depth of protection.

We'd still need some way to securely clear heap allocations. Maybe the
proposed std::secure_clear function would be useful for that, but there
might be an argument that what we actually want is a secure allocator. For
example, maybe you don't want your secrets written to a page file -- we
could take care of that for the stack automatically via the function
attribute, but probably not for the heap if the regular allocator is used.
And you probably don't want a read-after-free elsewhere in your program to
be able to read your secure secrets, which would be the case if you used
the regular allocator.

> The above is permitted today under the as-if rule; but secure_clear is
>> asking for some way to /*ensure, formally,*/ that it /*never */happens.
>> >
>> > Ensuring that something /doesn't /happen is much much harder than
>> ensuring that it /does /happen.
>>
>> Agreed.
>>
>
> _______________________________________________
> SG12 mailing list
> SG12_at_[hidden]
> Subscription: https://lists.isocpp.org/mailman/listinfo.cgi/sg12
> Link to this post: http://lists.isocpp.org/sg12/2020/04/0886.php
>