[PATCH] memset_s() and talloc_set_secure()

[Simo]

On Tue, 2019-03-26 at 14:02 -0700, Jeremy Allison wrote:
> On Tue, Mar 26, 2019 at 11:23:06AM -0400, Simo wrote:
> > Sorry to revive this old thread, but Andreas asked me to review his new
> > implementation[1] and I suggested he does it with a talloc flag
> > instead.
> > At which point he pointed me back to this thread :-)
> > 
> > So with my security hat on let me comment on why this needs to be in
> > talloc and with a flag instead of a destructor.
> > 
> > This functionality is hardening, and hardening is useful only if it is
> > complete and as error free as possible.
> > 
> > As I commented in [1], "The reason I see it as a problem is that there
> > can be only one destructor on a piece of memory, so you can end up with
> > a conflict to deal with (if the programmer is careful), or worse, a
> > silent replacement, where the memsetting destructor is simply
> > overwritten with another."
> > 
> > But this is just one reason why using a destructor is not a good idea.
> > There is an even bigger reason, and that is that the destructor is not
> > called when memory is changed around internally.
> > That is where a flag is necessary. Reallocation in particular can only
> > be dealt with by using a flag that tells the code to zero memory that
> > has to be returned to the system when the chunk needs to grow but does
> > not fit the current allocation. But also for memory shrinking, should
> > it happen, as otherwise the destructor may zero only partial
> > information, leaving part of secret material in memory.
> > 
> > I really do not see any way but a flag to do this job properly.
> 
> Hmmm. OK, that is quite a compelling argument.
> 
> I originally was against adding a flag to talloc as
> I'm concerned about adding complexity which ends
> up not being used (the talloc memory limits which
> you originally coded up being the best example,
> sorry :-) :-).

I understand, in my defense, the LDAP code was supposed to use it to
avoid chances of DoS ... it can still be used there, I just ran out of
time to wire that one in there at the time.
But that's the past, let's look at the bright future :)

> Being able to mark an existing talloc memory
> buffer as holding security sensitive data,
> and having it auto-zero'ed out on realloc/free does
> seem like an addition that will certainly
> be used (let's just make this a one-way
> gate to avoid extra complexity).
> 
> Andreas, do you have a copy of the original
> patch to look at ?
> 
> Jeremy.