[PATCH] memset_s() and talloc_set_secure()
Simo
simo at samba.org
Wed Mar 27 01:03:06 UTC 2019
On Wed, 2019-03-27 at 13:56 +1300, Andrew Bartlett wrote:
> On Tue, 2019-03-26 at 20:33 -0400, Simo wrote:
> > On Wed, 2019-03-27 at 12:02 +1300, Andrew Bartlett wrote:
> > > It still seems, compared the the vast majority of talloc use that this
> > > is a niche feature that can be implemented outside libtalloc using
> > > existing features. We have found it incredibly difficult to remove
> > > additional 'helpful' features from talloc and I'm keen to avoid it
> > > where there is a practical alternative.
> >
> > If it were possible to do this meaningfully outside of talloc proper I
> > would have not brought this up. Nut it isn't. The whole point of
> > zeroization of secrets is to not leave them in memory, and dealing with
> > reallocation is crucial for this to be working.
>
> To be clear we already have this in _talloc_realloc():
>
> /* don't allow realloc on referenced pointers */
> if (unlikely(tc->refs)) {
> return NULL;
> }
>
> So I don't take it as proven that talloc_realloc() proves this must be
> done in talloc.
>
> Likewise we could abuse the must-maligned talloc_memlimit() and trigger
> on this instead:
>
> if (tc->limit && (size > tc->size)) {
> if (!talloc_memlimit_check(tc->limit, (size - tc-
> > size))) {
> errno = ENOMEM;
> return NULL;
> }
> }
>
> I'm hand-waving, I've not tested these and both would clearly be
> categorised as a cludge (and talloc_reference() games probably would
> need talloc_unlink() to free it specifically), and I would still
> seriously suggest we skip both and just clearly document.
>
> In short I would prefer this outside libtalloc for the first
> implementation, and I still think that is viable.
The above are really gross hacks though.
You are asking to build a security feature to rely on side-effects of
two features both of which has been marked as desired to go by various
people.
That's not a good foundation to build anything on, because then people
will change those side effects without realizing they are compromising
a security feature.
I would rather gut away the memlimit feature, I can propose a patch to
remove most of it w/o technically breaking binary compatibility, if
that's the price to pay to get zeroization in. But security features
should be built on good foundations.
More information about the samba-technical
mailing list