[PATCH] memset_s() and talloc_set_secure()
jra at samba.org
Tue Mar 26 21:37:29 UTC 2019
On Tue, Mar 26, 2019 at 05:16:27PM -0400, Simo wrote:
> 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 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 , "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 :)
Yeah, sorry. Wasn't really trying to beat you
up over it, but point out how we need compelling
reasons to add low-level complexity these days.
Having said that this does seem to be a good
example of that though (and security 'n all :-).
More information about the samba-technical