mailing list of musl libc
 help / color / mirror / code / Atom feed
* Compiler support for erasure of sensitive data
@ 2015-09-09 16:36 Zack Weinberg
  2015-09-09 16:39 ` Fwd: " Zack Weinberg
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Zack Weinberg @ 2015-09-09 16:36 UTC (permalink / raw)
  To: gcc, llvmdev, GNU C Library, musl

Over the past couple months I have been working on adding a function to
glibc for erasure of sensitive data after it's used. This function
already exists in various other libraries under various names --
explicit_bzero, explicit_memset, SecureZeroMemory, memset_s,
OPENSSL_cleanse, etc -- and the problem it solves is, in this code:

    void encrypt_with_phrase(const char *phrase, const char *in,
                             char *out, size_t n)
    {
        char key[16];
        genkey(phrase, key);
        encrypt(key, in, out, n);
        memset(key, 0, 16);
    }

the compiler is entitled to delete the final call to memset, on the
grounds that `key` is dead after the call, so no conforming C program
can access that memory to prove that it has been cleared. However, a
crash dump, a debugger, or a plain old bug in the program might expose
that region of the stack to prying eyes. This is listed at
https://cwe.mitre.org/data/definitions/14.html as a "common weakness".
explicit_bzero (under whatever name) is just bzero with an additional,
documented guarantee that the compiler will not optimize it out even
if the memory region is dead afterward.  To further underline that
this is a common scenario, there are 152 uses of either
OPENSSL_cleanse or explicit_bzero in libressl 2.2.3 (they appear to be
in the middle of changing the name).

During development of my patches, we encountered two problems which
cannot be fully resolved without compiler assistance, so I want to
open a conversation about getting that assistance added to GCC and
Clang.

The first, simpler problem is strictly optimization.  explicit_bzero
can be optimized to memset followed by a vacuous use of the memory
region (generating no machine instructions, but preventing the stores
from being deleted as dead); this is valuable because the sensitive
data is often small and fixed-size, so the memset can in turn be
replaced by inline code.  (This also facilitates implementation of
-D_FORTIFY_SOURCE checks for explicit_bzero.)  Again looking at
libressl, 92 of those 152 uses are improved by a crude version of this
optimization:

    void explicit_bzero(void *, size_t);
    extern inline __attribute__((gnu_inline, always_inline))
    void explicit_bzero_constn(void *ptr, size_t len)
    {
      typedef struct {char x[len];} memblk;
      memset(ptr, 0, len);
      asm("" : : "m" (*(memblk __attribute__((may_alias)) *)ptr));
    }
    #define explicit_bzero(s, n)                          \
      (__extension__(__builtin_constant_p(n) && (n) > 0   \
                     ? explicit_bzero_constn(s, n)        \
                     : explicit_bzero(s, n)))

I call this "crude" because it only works in GCC, when compiling C,
and when the length parameter is compile-time constant.  GCC issues no
error for this code when 'len' is not compile-time constant, but it is
not documented to work reliably.  When compiling C++, GCC does not
accept a structure containing an array whose size is not *lexically*
constant; even if the body of explicit_bzero_constn is moved into the
macro so that the whole thing is guarded by __builtin_constant_p,
using explicit_bzero with a non-constant size will cause a compile
error.  The same is true for Clang whether compiling C or C++.

This problem could be solved with a very simple feature addition:

    extern inline __attribute__((gnu_inline, always_inline))
    void explicit_bzero(void *ptr, size_t len)
    {
      memset(ptr, 0, len);
      __builtin_use_memory(ptr, len);
    }

where __builtin_use_memory compiles to no machine instructions, but is
treated as a non-deletable use of all LEN bytes starting at PTR,
whether or not LEN is constant.

The harder problem, unfortunately, is one of correctness.  Consider
now this example (courtesy Florian Weimer):

    extern void explicit_bzero(void *, size_t);

    struct key { unsigned long long hi, lo; };
    extern struct key get_key(void);
    extern void use_key(struct key);

    void without_clear(void)
    {
        struct key k = get_key();
        use_key(k);
    }

    void with_clear(void)
    {
        struct key k = get_key();
        use_key(k);
        explicit_bzero(&k, sizeof k);
    }

On AMD64 this will be compiled to something like

    without_clear:
        subq    $8, %rsp
        call    get_key
        movq    %rdx, %rsi
        movq    %rax, %rdi
        call    use_key
        addq    $8, %rsp
        ret

   with_clear:
        subq    $24, %rsp
        call    get_key
        movq    %rax, %rdi
        movq    %rdx, %rsi
        movq    %rax, (%rsp)
        movq    %rdx, 8(%rsp)
        call    use_key
        movq    %rsp, %rdi
        movl    $16, %esi
        call    explicit_bzero
        addq    $24, %rsp
        ret

The ABI dictates basically everything you see.  The call to
explicit_bzero has forced the compiler to *create* a second copy of
the variable `k` on the stack, just so it can be erased -- and the
copy in registers survives (at least for a short time), which is not
what the programmer wanted.  With or without explicit_bzero, we have
no way of getting rid of the copy in registers.  More complicated
scenarios of course exist.

I think the ideal feature addition to address this would be

    void safe(void)
    {
        struct key __attribute__((sensitive)) k = get_key();
        use_key(k);
    }

which should generate

    safe:
        subq    $8, %rsp
        call    get_key
        movq    %rdx, %rsi
        movq    %rax, %rdi
        call    use_key
        xorq    %rdx, %rdx
        xorq    %rax, %rax
        xorq    %rsi, %rsi
        xorq    %rdi, %rdi
        addq    $8, %rsp
        ret

(use_key *could have* clobbered the values in all four of those
registers, but it is not required to.  If use_key gets inlined, of
course the compiler could track what registers actually contain
sensitive values.)

We would have to nail down precise semantics for __attribute__((sensitive)).
It needs to propagate to invisible copies made by the compiler, and to
*partial* copies in register or stack slots.  Doing a full-on taint
propagation, however, might be too much.  (In the original example,
one would *not* want the compiler to erase `out`, even though what
gets put into `out` is data-dependent on the value of `key`.)
It also needs to work on heap allocations as well as declared variables.
And there are uses in both glibc (crypt(3)) and libressl where the
sensitive datum is one field of a larger struct, other fields of which
must *not* be erased.

For compatibility with existing code that uses explicit_bzero or
similar, a second attribute would be desirable:

    extern void explicit_bzero(void *, size_t)
        __attribute__((erase_sensitive(1, 2)));

informs the compiler that this function performs an erasure of arg#2
bytes of sensitive data pointed to by arg#1.  This has the effect of
back-propagating __attribute__((sensitive)) to the object pointed-to
by argument #1.  The compiler _is_ then allowed to omit a call to the
function provided that its own erasures are at least as thorough.  For
heap allocations (which don't go out of scope) the call to this
function marks the point where erasure of copies should occur.

(I'm proposing an attribute rather than a builtin function because of
the many different names used for this operation by existing code.)

Comments?  Please note that I do not have anything like the time
required to implement any of this myself (and I'm ten years out of
practice on GCC and have no experience whatsoever with Clang,
anyway).  I'm hoping this catches someone's interest.

zw


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Fwd: Compiler support for erasure of sensitive data
  2015-09-09 16:36 Compiler support for erasure of sensitive data Zack Weinberg
@ 2015-09-09 16:39 ` Zack Weinberg
  2015-09-09 16:41   ` Zack Weinberg
  2015-09-09 16:42 ` Rich Felker
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Zack Weinberg @ 2015-09-09 16:39 UTC (permalink / raw)
  To: llvmdev, gcc, GNU C Library, musl

[Sorry for the double-post, but I got the LLVM mailing list address wrong,
and I want to make sure that everyone sees the entire conversation.]

Over the past couple months I have been working on adding a function to
glibc for erasure of sensitive data after it's used. This function
already exists in various other libraries under various names --
explicit_bzero, explicit_memset, SecureZeroMemory, memset_s,
OPENSSL_cleanse, etc -- and the problem it solves is, in this code:

    void encrypt_with_phrase(const char *phrase, const char *in,
                             char *out, size_t n)
    {
        char key[16];
        genkey(phrase, key);
        encrypt(key, in, out, n);
        memset(key, 0, 16);
    }

the compiler is entitled to delete the final call to memset, on the
grounds that `key` is dead after the call, so no conforming C program
can access that memory to prove that it has been cleared. However, a
crash dump, a debugger, or a plain old bug in the program might expose
that region of the stack to prying eyes. This is listed at
https://cwe.mitre.org/data/definitions/14.html as a "common weakness".
explicit_bzero (under whatever name) is just bzero with an additional,
documented guarantee that the compiler will not optimize it out even
if the memory region is dead afterward.  To further underline that
this is a common scenario, there are 152 uses of either
OPENSSL_cleanse or explicit_bzero in libressl 2.2.3 (they appear to be
in the middle of changing the name).

During development of my patches, we encountered two problems which
cannot be fully resolved without compiler assistance, so I want to
open a conversation about getting that assistance added to GCC and
Clang.

The first, simpler problem is strictly optimization.  explicit_bzero
can be optimized to memset followed by a vacuous use of the memory
region (generating no machine instructions, but preventing the stores
from being deleted as dead); this is valuable because the sensitive
data is often small and fixed-size, so the memset can in turn be
replaced by inline code.  (This also facilitates implementation of
-D_FORTIFY_SOURCE checks for explicit_bzero.)  Again looking at
libressl, 92 of those 152 uses are improved by a crude version of this
optimization:

    void explicit_bzero(void *, size_t);
    extern inline __attribute__((gnu_inline, always_inline))
    void explicit_bzero_constn(void *ptr, size_t len)
    {
      typedef struct {char x[len];} memblk;
      memset(ptr, 0, len);
      asm("" : : "m" (*(memblk __attribute__((may_alias)) *)ptr));
    }
    #define explicit_bzero(s, n)                          \
      (__extension__(__builtin_constant_p(n) && (n) > 0   \
                     ? explicit_bzero_constn(s, n)        \
                     : explicit_bzero(s, n)))

I call this "crude" because it only works in GCC, when compiling C,
and when the length parameter is compile-time constant.  GCC issues no
error for this code when 'len' is not compile-time constant, but it is
not documented to work reliably.  When compiling C++, GCC does not
accept a structure containing an array whose size is not *lexically*
constant; even if the body of explicit_bzero_constn is moved into the
macro so that the whole thing is guarded by __builtin_constant_p,
using explicit_bzero with a non-constant size will cause a compile
error.  The same is true for Clang whether compiling C or C++.

This problem could be solved with a very simple feature addition:

    extern inline __attribute__((gnu_inline, always_inline))
    void explicit_bzero(void *ptr, size_t len)
    {
      memset(ptr, 0, len);
      __builtin_use_memory(ptr, len);
    }

where __builtin_use_memory compiles to no machine instructions, but is
treated as a non-deletable use of all LEN bytes starting at PTR,
whether or not LEN is constant.

The harder problem, unfortunately, is one of correctness.  Consider
now this example (courtesy Florian Weimer):

    extern void explicit_bzero(void *, size_t);

    struct key { unsigned long long hi, lo; };
    extern struct key get_key(void);
    extern void use_key(struct key);

    void without_clear(void)
    {
        struct key k = get_key();
        use_key(k);
    }

    void with_clear(void)
    {
        struct key k = get_key();
        use_key(k);
        explicit_bzero(&k, sizeof k);
    }

On AMD64 this will be compiled to something like

    without_clear:
        subq    $8, %rsp
        call    get_key
        movq    %rdx, %rsi
        movq    %rax, %rdi
        call    use_key
        addq    $8, %rsp
        ret

   with_clear:
        subq    $24, %rsp
        call    get_key
        movq    %rax, %rdi
        movq    %rdx, %rsi
        movq    %rax, (%rsp)
        movq    %rdx, 8(%rsp)
        call    use_key
        movq    %rsp, %rdi
        movl    $16, %esi
        call    explicit_bzero
        addq    $24, %rsp
        ret

The ABI dictates basically everything you see.  The call to
explicit_bzero has forced the compiler to *create* a second copy of
the variable `k` on the stack, just so it can be erased -- and the
copy in registers survives (at least for a short time), which is not
what the programmer wanted.  With or without explicit_bzero, we have
no way of getting rid of the copy in registers.  More complicated
scenarios of course exist.

I think the ideal feature addition to address this would be

    void safe(void)
    {
        struct key __attribute__((sensitive)) k = get_key();
        use_key(k);
    }

which should generate

    safe:
        subq    $8, %rsp
        call    get_key
        movq    %rdx, %rsi
        movq    %rax, %rdi
        call    use_key
        xorq    %rdx, %rdx
        xorq    %rax, %rax
        xorq    %rsi, %rsi
        xorq    %rdi, %rdi
        addq    $8, %rsp
        ret

(use_key *could have* clobbered the values in all four of those
registers, but it is not required to.  If use_key gets inlined, of
course the compiler could track what registers actually contain
sensitive values.)

We would have to nail down precise semantics for __attribute__((sensitive)).
It needs to propagate to invisible copies made by the compiler, and to
*partial* copies in register or stack slots.  Doing a full-on taint
propagation, however, might be too much.  (In the original example,
one would *not* want the compiler to erase `out`, even though what
gets put into `out` is data-dependent on the value of `key`.)
It also needs to work on heap allocations as well as declared variables.
And there are uses in both glibc (crypt(3)) and libressl where the
sensitive datum is one field of a larger struct, other fields of which
must *not* be erased.

For compatibility with existing code that uses explicit_bzero or
similar, a second attribute would be desirable:

    extern void explicit_bzero(void *, size_t)
        __attribute__((erase_sensitive(1, 2)));

informs the compiler that this function performs an erasure of arg#2
bytes of sensitive data pointed to by arg#1.  This has the effect of
back-propagating __attribute__((sensitive)) to the object pointed-to
by argument #1.  The compiler _is_ then allowed to omit a call to the
function provided that its own erasures are at least as thorough.  For
heap allocations (which don't go out of scope) the call to this
function marks the point where erasure of copies should occur.

(I'm proposing an attribute rather than a builtin function because of
the many different names used for this operation by existing code.)

Comments?  Please note that I do not have anything like the time
required to implement any of this myself (and I'm ten years out of
practice on GCC and have no experience whatsoever with Clang,
anyway).  I'm hoping this catches someone's interest.

zw


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 16:39 ` Fwd: " Zack Weinberg
@ 2015-09-09 16:41   ` Zack Weinberg
  0 siblings, 0 replies; 18+ messages in thread
From: Zack Weinberg @ 2015-09-09 16:41 UTC (permalink / raw)
  To: gcc, GNU C Library, musl

OK, I have now failed to find the LLVM development list twice in a
row.  Could some kind soul please forward the message to whereever the
heck the proper address is?

zw


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 16:36 Compiler support for erasure of sensitive data Zack Weinberg
  2015-09-09 16:39 ` Fwd: " Zack Weinberg
@ 2015-09-09 16:42 ` Rich Felker
  2015-09-09 16:47   ` [musl] " Zack Weinberg
  2015-09-09 16:52 ` Paul_Koning
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Rich Felker @ 2015-09-09 16:42 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: gcc, llvmdev, GNU C Library, musl

On Wed, Sep 09, 2015 at 12:36:01PM -0400, Zack Weinberg wrote:
> The first, simpler problem is strictly optimization.  explicit_bzero
> can be optimized to memset followed by a vacuous use of the memory
> region (generating no machine instructions, but preventing the stores
> from being deleted as dead); this is valuable because the sensitive
> data is often small and fixed-size, so the memset can in turn be
> replaced by inline code.  (This also facilitates implementation of
> -D_FORTIFY_SOURCE checks for explicit_bzero.)  Again looking at
> libressl, 92 of those 152 uses are improved by a crude version of this
> optimization:
> 
>     void explicit_bzero(void *, size_t);
>     extern inline __attribute__((gnu_inline, always_inline))
>     void explicit_bzero_constn(void *ptr, size_t len)
>     {
>       typedef struct {char x[len];} memblk;
>       memset(ptr, 0, len);
>       asm("" : : "m" (*(memblk __attribute__((may_alias)) *)ptr));
>     }
>     #define explicit_bzero(s, n)                          \
>       (__extension__(__builtin_constant_p(n) && (n) > 0   \
>                      ? explicit_bzero_constn(s, n)        \
>                      : explicit_bzero(s, n)))
> 
> I call this "crude" because it only works in GCC, when compiling C,
> and when the length parameter is compile-time constant.  GCC issues no
> error for this code when 'len' is not compile-time constant, but it is
> not documented to work reliably.  When compiling C++, GCC does not
> accept a structure containing an array whose size is not *lexically*
> constant; even if the body of explicit_bzero_constn is moved into the
> macro so that the whole thing is guarded by __builtin_constant_p,
> using explicit_bzero with a non-constant size will cause a compile
> error.  The same is true for Clang whether compiling C or C++.
> 
> This problem could be solved with a very simple feature addition:
> 
>     extern inline __attribute__((gnu_inline, always_inline))
>     void explicit_bzero(void *ptr, size_t len)
>     {
>       memset(ptr, 0, len);
>       __builtin_use_memory(ptr, len);
>     }

You're making this harder than it needs to be. The "m" constraint is
the wrong thing to use here. Simply use:

	__asm__(""::"r"(ptr):"memory");

The memory constraint implies that the asm can read or write any
memory that's reachable by it. The lack of output constraints implies
__volatile__ which is also needed.

Rich


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [musl] Compiler support for erasure of sensitive data
  2015-09-09 16:42 ` Rich Felker
@ 2015-09-09 16:47   ` Zack Weinberg
  2015-09-09 17:13     ` Rich Felker
  0 siblings, 1 reply; 18+ messages in thread
From: Zack Weinberg @ 2015-09-09 16:47 UTC (permalink / raw)
  To: Rich Felker; +Cc: gcc, GNU C Library, musl

On Wed, Sep 9, 2015 at 12:42 PM, Rich Felker <dalias@libc.org> wrote:
> You're making this harder than it needs to be. The "m" constraint is
> the wrong thing to use here. Simply use:
>
>         __asm__(""::"r"(ptr):"memory");

Please review my earlier conversation with Adhemerval on exactly this point.

zw


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 16:36 Compiler support for erasure of sensitive data Zack Weinberg
  2015-09-09 16:39 ` Fwd: " Zack Weinberg
  2015-09-09 16:42 ` Rich Felker
@ 2015-09-09 16:52 ` Paul_Koning
  2015-09-09 16:58   ` Zack Weinberg
  2015-09-09 17:54 ` David Edelsohn
  2015-10-22 16:09 ` Denys Vlasenko
  4 siblings, 1 reply; 18+ messages in thread
From: Paul_Koning @ 2015-09-09 16:52 UTC (permalink / raw)
  To: zackw; +Cc: gcc, llvmdev, libc-alpha, musl


> On Sep 9, 2015, at 12:36 PM, Zack Weinberg <zackw@panix.com> wrote:
> 
> ...
> I think the ideal feature addition to address this would be
> 
>    void safe(void)
>    {
>        struct key __attribute__((sensitive)) k = get_key();
>        use_key(k);
>    }

That certainly is a cleaner answer.  What is attractive about it is that it expresses the need for variables (data) to be given different treatment, rather than expecting the programmer to code that special treatment in every place where that data becomes dead.  It's also likely to be a whole lot harder to implement, unfortunately.

Then again, suppose all you had is explicit_bzero, and an annotation on the data saying it's sensitive.  Can static code analyzers take care of the rest?  If so, this sort of thing doesn't need to be in the compiler.

	paul


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 16:52 ` Paul_Koning
@ 2015-09-09 16:58   ` Zack Weinberg
  2015-09-09 17:25     ` [musl] " Rich Felker
  0 siblings, 1 reply; 18+ messages in thread
From: Zack Weinberg @ 2015-09-09 16:58 UTC (permalink / raw)
  To: Paul_Koning; +Cc: gcc, llvmdev, libc-alpha, musl

On 09/09/2015 12:52 PM, Paul_Koning@Dell.com wrote:
> Then again, suppose all you had is explicit_bzero, and an annotation
> on the data saying it's sensitive.  Can static code analyzers take
> care of the rest?  If so, this sort of thing doesn't need to be in
> the compiler.

The thing that absolutely has to be implemented in the compiler (AFAICT)
is register clearing.  I'm undecided as to how *necessary* that is.
There certainly can be a lot of sensitive data in registers (e.g. AESNI
puts an entire AES key schedule in xmm registers).  I don't know of any
exploits that depended on salvaging such data from registers, but I
don't follow exploit research closely.

zw


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 16:47   ` [musl] " Zack Weinberg
@ 2015-09-09 17:13     ` Rich Felker
  2015-09-09 18:48       ` [musl] " Zack Weinberg
  0 siblings, 1 reply; 18+ messages in thread
From: Rich Felker @ 2015-09-09 17:13 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: gcc, GNU C Library, musl

On Wed, Sep 09, 2015 at 12:47:10PM -0400, Zack Weinberg wrote:
> On Wed, Sep 9, 2015 at 12:42 PM, Rich Felker <dalias@libc.org> wrote:
> > You're making this harder than it needs to be. The "m" constraint is
> > the wrong thing to use here. Simply use:
> >
> >         __asm__(""::"r"(ptr):"memory");
> 
> Please review my earlier conversation with Adhemerval on exactly this point.

My understanding is that you consider this a "big hammer". Does that
really matter if the intent is that it only be used in isolated,
sensitive contexts? Are you just unhappy with the performance cost, or
concerned that the clobber will cause more spilling of sensitive data?
I'm doubtful that this would happen because a "memory" clobber does
not affect all data cached in registers, only data which is
potentially reachable by the asm.

In any case, I think the intent of my reply was unclear. I did not
mean to detract from the idea of compiler support for handling of
sensitive data, just to point out that the hack with the "m"
constraint is wrong and easily fixed. It still may be possible to get
much better results via other means.

Rich


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [musl] Re: Compiler support for erasure of sensitive data
  2015-09-09 16:58   ` Zack Weinberg
@ 2015-09-09 17:25     ` Rich Felker
  0 siblings, 0 replies; 18+ messages in thread
From: Rich Felker @ 2015-09-09 17:25 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: Paul_Koning, gcc, llvmdev, libc-alpha, musl

On Wed, Sep 09, 2015 at 12:58:36PM -0400, Zack Weinberg wrote:
> On 09/09/2015 12:52 PM, Paul_Koning@Dell.com wrote:
> > Then again, suppose all you had is explicit_bzero, and an annotation
> > on the data saying it's sensitive.  Can static code analyzers take
> > care of the rest?  If so, this sort of thing doesn't need to be in
> > the compiler.
> 
> The thing that absolutely has to be implemented in the compiler (AFAICT)
> is register clearing.  I'm undecided as to how *necessary* that is.
> There certainly can be a lot of sensitive data in registers (e.g. AESNI
> puts an entire AES key schedule in xmm registers).  I don't know of any
> exploits that depended on salvaging such data from registers, but I
> don't follow exploit research closely.

Conceptually you can "clear all registers" by calling an external asm
function that clears all non-call-saved registers internally. Hardened
implementations of explicit_bzero could even do this. The caller would
(or at least should) not save registers whose old contents it does not
intend to use again after the call. And of course all the call-saved
registers will get restored when the function returns, preventing any
leak via them.

I agree it's much better if the compiler can do it, but I think the
approach I described is a viable hardening technique that's
immediately doable.

Rich


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 16:36 Compiler support for erasure of sensitive data Zack Weinberg
                   ` (2 preceding siblings ...)
  2015-09-09 16:52 ` Paul_Koning
@ 2015-09-09 17:54 ` David Edelsohn
  2015-09-09 18:02   ` Paul_Koning
  2015-10-22 16:09 ` Denys Vlasenko
  4 siblings, 1 reply; 18+ messages in thread
From: David Edelsohn @ 2015-09-09 17:54 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: GCC Development, GNU C Library, musl

On Wed, Sep 9, 2015 at 12:36 PM, Zack Weinberg <zackw@panix.com> wrote:

> The ABI dictates basically everything you see.  The call to
> explicit_bzero has forced the compiler to *create* a second copy of
> the variable `k` on the stack, just so it can be erased -- and the
> copy in registers survives (at least for a short time), which is not
> what the programmer wanted.  With or without explicit_bzero, we have
> no way of getting rid of the copy in registers.  More complicated
> scenarios of course exist.

> Comments?  Please note that I do not have anything like the time
> required to implement any of this myself (and I'm ten years out of
> practice on GCC and have no experience whatsoever with Clang,
> anyway).  I'm hoping this catches someone's interest.

What level of erasure of sensitive data are you trying to ensure?
Assuming that overwriting values in the ISA registers actually
completely clears and destroys the values is delusionally naive.

Most modern hardware architectures have hardware capabilities to
encrypt and protect sensitive data.

- David


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 17:54 ` David Edelsohn
@ 2015-09-09 18:02   ` Paul_Koning
  2015-09-09 18:11     ` David Edelsohn
  2015-09-09 19:03     ` Zack Weinberg
  0 siblings, 2 replies; 18+ messages in thread
From: Paul_Koning @ 2015-09-09 18:02 UTC (permalink / raw)
  To: dje.gcc; +Cc: zackw, gcc, libc-alpha, musl


> On Sep 9, 2015, at 1:54 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
> 
> On Wed, Sep 9, 2015 at 12:36 PM, Zack Weinberg <zackw@panix.com> wrote:
> 
>> The ABI dictates basically everything you see.  The call to
>> explicit_bzero has forced the compiler to *create* a second copy of
>> the variable `k` on the stack, just so it can be erased -- and the
>> copy in registers survives (at least for a short time), which is not
>> what the programmer wanted.  With or without explicit_bzero, we have
>> no way of getting rid of the copy in registers.  More complicated
>> scenarios of course exist.
> 
>> Comments?  Please note that I do not have anything like the time
>> required to implement any of this myself (and I'm ten years out of
>> practice on GCC and have no experience whatsoever with Clang,
>> anyway).  I'm hoping this catches someone's interest.
> 
> What level of erasure of sensitive data are you trying to ensure?
> Assuming that overwriting values in the ISA registers actually
> completely clears and destroys the values is delusionally naive.

Could you point to some references about that?

> Most modern hardware architectures have hardware capabilities to
> encrypt and protect sensitive data.

I'm not sure about "most".  I haven't worked on any that could do this.

I agree it would be good to specify the threat model.  Reading between the lines, I believe it is: capture of the software-visible process state after the code is finished with the sensitive data, either via a process dump file, or a debugger.  With an explicitly stated list of goals and non-goals we can see whether a proposed solution addresses all, part, or none of the problem space, and whether it is a small solution or one much more powerful than is actually requested.

If the threat is indeed delayed process state examination in software, then I think your "dangerously naive" does not apply.  If you're talking excavating the chip and doing quantum forensics, that's a different matter.

Another threat that I don't believe is covered here is disclosure of copies of the process state held in the OS, like saved context from thread switching, copies of stuff in the page file or in now-freed memory pages, or things like that.  

	paul


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 18:02   ` Paul_Koning
@ 2015-09-09 18:11     ` David Edelsohn
  2015-09-09 19:03     ` Zack Weinberg
  1 sibling, 0 replies; 18+ messages in thread
From: David Edelsohn @ 2015-09-09 18:11 UTC (permalink / raw)
  To: Paul_Koning; +Cc: Zack Weinberg, GCC Development, GNU C Library, musl

On Wed, Sep 9, 2015 at 2:02 PM,  <Paul_Koning@dell.com> wrote:
>
>> On Sep 9, 2015, at 1:54 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
>>
>> On Wed, Sep 9, 2015 at 12:36 PM, Zack Weinberg <zackw@panix.com> wrote:
>>
>>> The ABI dictates basically everything you see.  The call to
>>> explicit_bzero has forced the compiler to *create* a second copy of
>>> the variable `k` on the stack, just so it can be erased -- and the
>>> copy in registers survives (at least for a short time), which is not
>>> what the programmer wanted.  With or without explicit_bzero, we have
>>> no way of getting rid of the copy in registers.  More complicated
>>> scenarios of course exist.
>>
>>> Comments?  Please note that I do not have anything like the time
>>> required to implement any of this myself (and I'm ten years out of
>>> practice on GCC and have no experience whatsoever with Clang,
>>> anyway).  I'm hoping this catches someone's interest.
>>
>> What level of erasure of sensitive data are you trying to ensure?
>> Assuming that overwriting values in the ISA registers actually
>> completely clears and destroys the values is delusionally naive.
>
> Could you point to some references about that?
>
>> Most modern hardware architectures have hardware capabilities to
>> encrypt and protect sensitive data.
>
> I'm not sure about "most".  I haven't worked on any that could do this.

Intel, Power, z/Arch, and (probably) SPARC.

>
> I agree it would be good to specify the threat model.  Reading between the lines, I believe it is: capture of the software-visible process state after the code is finished with the sensitive data, either via a process dump file, or a debugger.  With an explicitly stated list of goals and non-goals we can see whether a proposed solution addresses all, part, or none of the problem space, and whether it is a small solution or one much more powerful than is actually requested.
>
> If the threat is indeed delayed process state examination in software, then I think your "dangerously naive" does not apply.  If you're talking excavating the chip and doing quantum forensics, that's a different matter.

If this feature is implying / assuring that all values have been
irrecoverably destroyed, and one can find the values in physical
register files, then one is being dangerously naive in the assertions
/ expectations about the feature.  One must specify the threat model.

- David


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [musl] Compiler support for erasure of sensitive data
  2015-09-09 17:13     ` Rich Felker
@ 2015-09-09 18:48       ` Zack Weinberg
  2015-09-09 20:05         ` Rich Felker
  0 siblings, 1 reply; 18+ messages in thread
From: Zack Weinberg @ 2015-09-09 18:48 UTC (permalink / raw)
  To: Rich Felker; +Cc: gcc, GNU C Library, musl

On 09/09/2015 01:13 PM, Rich Felker wrote:
> On Wed, Sep 09, 2015 at 12:47:10PM -0400, Zack Weinberg wrote:
>> On Wed, Sep 9, 2015 at 12:42 PM, Rich Felker <dalias@libc.org> wrote:
>>> You're making this harder than it needs to be. The "m" constraint is
>>> the wrong thing to use here. Simply use:
>>>
>>>         __asm__(""::"r"(ptr):"memory");
>>
>> Please review my earlier conversation with Adhemerval on exactly this point.
> 
> My understanding is that you consider this a "big hammer". Does that
> really matter if the intent is that it only be used in isolated,
> sensitive contexts? Are you just unhappy with the performance cost, or
> concerned that the clobber will cause more spilling of sensitive data?

Please review *all* of my earlier conversation with Adhemerval, in
particular the bit where I compiled libressl three different ways and
analyzed the assembly dumps.  I'm sure there's more to be said on the
topic, but *starting* from there.

> the hack with the "m" constraint is wrong and easily fixed

It's not wrong; it is in fact the documented way to express a fixed-size
read access to one block of memory.  Look for "ten bytes of a string"
within https://gcc.gnu.org/onlinedocs/gcc-4.8.1/gcc/Extended-Asm.html
(sorry, there don't appear to be anchors).

It merely doesn't work in C++, with Clang, or (maybe) with a block of
memory whose size cannot be determined at compile time.

zw


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 18:02   ` Paul_Koning
  2015-09-09 18:11     ` David Edelsohn
@ 2015-09-09 19:03     ` Zack Weinberg
  2015-09-09 20:26       ` Szabolcs Nagy
  1 sibling, 1 reply; 18+ messages in thread
From: Zack Weinberg @ 2015-09-09 19:03 UTC (permalink / raw)
  To: Paul_Koning, dje.gcc; +Cc: gcc, libc-alpha, musl

On 09/09/2015 02:02 PM, Paul_Koning@Dell.com wrote:
>> On Sep 9, 2015, at 1:54 PM, David Edelsohn <dje.gcc@gmail.com>
>> wrote:
>> 
>> What level of erasure of sensitive data are you trying to ensure? 
>> Assuming that overwriting values in the ISA registers actually 
>> completely clears and destroys the values is delusionally naive.
> 
> Could you point to some references about that?

I *assume* David is referring to register renaming, which is not
architecturally visible...

...
> I agree it would be good to specify the threat model.  Reading
> between the lines, I believe it is: capture of the software-visible
> process state after the code is finished with the sensitive data,
> either via a process dump file, or a debugger.

This is correct.  Other avenues for the sensitive data to leak include
use-after-free or out-of-bounds memory reads within the program, and
malicious code (having gained control via some other bug) scanning
memory in bulk.

I would consider data leaks via state inaccessible to a program
executing at the same privilege level as the code to be hardened to be
out of scope.  (Which does mean that *when hardening an OS kernel* one
would possibly have to worry about special mechanisms that reveal the
"true" register file, and the like, but I'd be plenty happy with
something that was good enough for user mode.)  Techniques involving
direct manipulation of the hardware or the microcode, ditto.

> Another threat that I don't believe is covered here is disclosure of
> copies of the process state held in the OS, like saved context from
> thread switching, copies of stuff in the page file or in now-freed
> memory pages, or things like that.

Some of that might conceivably be in-scope; jmp_buf comes to mind.  (I'm
not prepared to make an exhaustive list at the moment.)  Normally,
people programming this kind of code expect to have to lock pages in
memory and so on.

zw


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 18:48       ` [musl] " Zack Weinberg
@ 2015-09-09 20:05         ` Rich Felker
  0 siblings, 0 replies; 18+ messages in thread
From: Rich Felker @ 2015-09-09 20:05 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: gcc, GNU C Library, musl

On Wed, Sep 09, 2015 at 02:48:22PM -0400, Zack Weinberg wrote:
> On 09/09/2015 01:13 PM, Rich Felker wrote:
> > On Wed, Sep 09, 2015 at 12:47:10PM -0400, Zack Weinberg wrote:
> >> On Wed, Sep 9, 2015 at 12:42 PM, Rich Felker <dalias@libc.org> wrote:
> >>> You're making this harder than it needs to be. The "m" constraint is
> >>> the wrong thing to use here. Simply use:
> >>>
> >>>         __asm__(""::"r"(ptr):"memory");
> >>
> >> Please review my earlier conversation with Adhemerval on exactly this point.
> > 
> > My understanding is that you consider this a "big hammer". Does that
> > really matter if the intent is that it only be used in isolated,
> > sensitive contexts? Are you just unhappy with the performance cost, or
> > concerned that the clobber will cause more spilling of sensitive data?
> 
> Please review *all* of my earlier conversation with Adhemerval, in
> particular the bit where I compiled libressl three different ways and
> analyzed the assembly dumps.  I'm sure there's more to be said on the
> topic, but *starting* from there.

OK, sorry for jumping back in without the full context.

> > the hack with the "m" constraint is wrong and easily fixed
> 
> It's not wrong; it is in fact the documented way to express a fixed-size
> read access to one block of memory.  Look for "ten bytes of a string"
> within https://gcc.gnu.org/onlinedocs/gcc-4.8.1/gcc/Extended-Asm.html
> (sorry, there don't appear to be anchors).
> 
> It merely doesn't work in C++, with Clang, or (maybe) with a block of
> memory whose size cannot be determined at compile time.

It relies on structs containing VLAs which are not standard C nor
supported by any "GNU C" compilers except GCC. And features like this
tend to be really fragile even in GCC because nobody uses them (for
good reason -- they can't be expected to work except on certain GCC
versions). You can disagree if you like, but that's why I called it
"wrong".

Rich


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 19:03     ` Zack Weinberg
@ 2015-09-09 20:26       ` Szabolcs Nagy
  2015-10-22 16:02         ` [musl] " Denys Vlasenko
  0 siblings, 1 reply; 18+ messages in thread
From: Szabolcs Nagy @ 2015-09-09 20:26 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: Paul_Koning, dje.gcc, gcc, libc-alpha, musl

* Zack Weinberg <zackw@panix.com> [2015-09-09 15:03:50 -0400]:
> On 09/09/2015 02:02 PM, Paul_Koning@Dell.com wrote:
> >> On Sep 9, 2015, at 1:54 PM, David Edelsohn <dje.gcc@gmail.com>
> >> wrote:
> >> 
> >> What level of erasure of sensitive data are you trying to ensure? 
> >> Assuming that overwriting values in the ISA registers actually 
> >> completely clears and destroys the values is delusionally naive.
> > 
> > Could you point to some references about that?
> 
> I *assume* David is referring to register renaming, which is not
> architecturally visible...
> 

or async signal handler copying all the register state on sigaltstack
or internal counters and debug features making sensitive info observable
or timing/cache-effect side channels that let other processes get info
or compiling to a highlevel language (js) with different kind of leaks
or running under emulator/debugger that can make secrets visible
or...

> I would consider data leaks via state inaccessible to a program
> executing at the same privilege level as the code to be hardened to be
> out of scope.  (Which does mean that *when hardening an OS kernel* one

specifying the info leak at the abstract c machine level is not useful
(the memset is not observable there, unless you assign meaning to
undefined behaviour which is a can of worms), but you do have to specify
the leak on some abstraction level (that is applicable to the targets of
a compiler and gives useful security properties in practice) otherwise
the attribute is not meaningful.

leaks can happen for many reasons that are layers below the control
of the compiler, but still observable by high level code.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [musl] Re: Compiler support for erasure of sensitive data
  2015-09-09 20:26       ` Szabolcs Nagy
@ 2015-10-22 16:02         ` Denys Vlasenko
  0 siblings, 0 replies; 18+ messages in thread
From: Denys Vlasenko @ 2015-10-22 16:02 UTC (permalink / raw)
  To: musl, Zack Weinberg, Paul_Koning, dje.gcc, gcc, libc-alpha

On Wed, Sep 9, 2015 at 10:26 PM, Szabolcs Nagy <nsz@port70.net> wrote:
> * Zack Weinberg <zackw@panix.com> [2015-09-09 15:03:50 -0400]:
>> On 09/09/2015 02:02 PM, Paul_Koning@Dell.com wrote:
>> >> On Sep 9, 2015, at 1:54 PM, David Edelsohn <dje.gcc@gmail.com>
>> >> wrote:
>> >>
>> >> What level of erasure of sensitive data are you trying to ensure?
>> >> Assuming that overwriting values in the ISA registers actually
>> >> completely clears and destroys the values is delusionally naive.
>> >
>> > Could you point to some references about that?
>>
>> I *assume* David is referring to register renaming, which is not
>> architecturally visible...
>>
>
> or async signal handler copying all the register state on sigaltstack
> or internal counters and debug features making sensitive info observable
> or timing/cache-effect side channels that let other processes get info
> or compiling to a highlevel language (js) with different kind of leaks
> or running under emulator/debugger that can make secrets visible
> or...

I think if attacker got that much control of the machine that he can
get, for example, signals to reach your sensitive process, you already lost.
Ditto for running under emulator.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Compiler support for erasure of sensitive data
  2015-09-09 16:36 Compiler support for erasure of sensitive data Zack Weinberg
                   ` (3 preceding siblings ...)
  2015-09-09 17:54 ` David Edelsohn
@ 2015-10-22 16:09 ` Denys Vlasenko
  4 siblings, 0 replies; 18+ messages in thread
From: Denys Vlasenko @ 2015-10-22 16:09 UTC (permalink / raw)
  To: musl; +Cc: gcc, llvmdev, GNU C Library

On Wed, Sep 9, 2015 at 6:36 PM, Zack Weinberg <zackw@panix.com> wrote:
> The first, simpler problem is strictly optimization.  explicit_bzero
> can be optimized to memset followed by a vacuous use of the memory
> region (generating no machine instructions, but preventing the stores
> from being deleted as dead); this is valuable because the sensitive
> data is often small and fixed-size, so the memset can in turn be
> replaced by inline code.

How valuable is that speedup due to possible inlining?

You know, call instruction is not a crime.
In fact, it is *heavily* optimized on any CPU exactly because
calls happen gazillion times every second.

In my measurement, on x86 call+ret pair is cheaper than
a single read-modify-write ALU op on L1 data item!

So, just implement explicit_bzero() as a function which
is prohibited from inlining, and which clears all call-clobbered
registers in addition to clearing memory.

This will probably also be the smallest solution code size wise.


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-10-22 16:09 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-09 16:36 Compiler support for erasure of sensitive data Zack Weinberg
2015-09-09 16:39 ` Fwd: " Zack Weinberg
2015-09-09 16:41   ` Zack Weinberg
2015-09-09 16:42 ` Rich Felker
2015-09-09 16:47   ` [musl] " Zack Weinberg
2015-09-09 17:13     ` Rich Felker
2015-09-09 18:48       ` [musl] " Zack Weinberg
2015-09-09 20:05         ` Rich Felker
2015-09-09 16:52 ` Paul_Koning
2015-09-09 16:58   ` Zack Weinberg
2015-09-09 17:25     ` [musl] " Rich Felker
2015-09-09 17:54 ` David Edelsohn
2015-09-09 18:02   ` Paul_Koning
2015-09-09 18:11     ` David Edelsohn
2015-09-09 19:03     ` Zack Weinberg
2015-09-09 20:26       ` Szabolcs Nagy
2015-10-22 16:02         ` [musl] " Denys Vlasenko
2015-10-22 16:09 ` Denys Vlasenko

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).