[PATCH] memset_s() and talloc_set_secure()

[Andrew Bartlett]

On Fri, 2018-10-12 at 19:22 +0200, Andreas Schneider wrote:
> On Friday, October 12, 2018 6:14:51 PM CEST Jeremy Allison wrote:
> > 
> > Andreas, can you give me the use-case for this being in talloc ?
> 
> Whatever secret (session key, passwords) we put in talloc memory, we should 
> zero it before it is being freed so that if an attacker is able to read parts 
> of memory is not getting secrets. 
>  
> > I can see the need for memset_s(), but why should this be inside
> > talloc rather than being called from specific talloc destructors ?
> 
> You could do that with a destructor too, but then you have to write a 
> destructor for the secrets and the desctructor needs to call a memset which is 
> not optimized out. talloc_keep_secret() just does the right thing (tm) for 
> you!
> 

Sure, but all this is really is another destructor.

I'm with Jeremy on this one.  Make it like talloc_stackframe(), a Samba
add-on, at least for now.

The talloc_realloc() part can be dealt with by adding a useless
reference to cause the talloc_realloc() to fail. 

If we do keep this in talloc, the talloc_realloc() should be banned.  I
can't see how it can be tested, and the current tests don't nearly
cover enough of the cases, which are at least:
 old > new
 old < new
 old == new
 new == 0 (free)

Being a destructor would also avoid using one more talloc flag, as each
flag reduces the entropy in the talloc magic.  

The talloc_report() issues can be largely worked around by setting the
name to "<secret value>" with talloc_set_name() when setting the
destructor.

Then we can learn about how this works in the real world, and tweak it
without promising it in our talloc ABI forever (like the memlimit stuff
is) if it doesn't turn out. 

Sorry,

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba