[PATCH] memset_s() and talloc_set_secure()

[Jeremy Allison]

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

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.