C++ Logo

sg12

Advanced search

Re: [SG12] p1315 secure_clear

From: Arthur O'Dwyer <arthur.j.odwyer_at_[hidden]>
Date: Thu, 30 Apr 2020 10:36:15 -0400
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.


> > 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.
>

Received on 2020-04-30 09:41:45