[PATCH] memset_s() and talloc_set_secure()

[Andreas Schneider]

On Thursday, 11 October 2018 13:15:17 CEST Stefan Metzmacher wrote:
> Am 11.10.2018 um 13:07 schrieb Andrew Bartlett via samba-technical:
> > On Thu, 2018-10-11 at 12:17 +0200, Andreas Schneider via samba-
> > 
> > technical wrote:
> >> Hello,
> >> 
> >> the attached patch adds memset_s() [1] and talloc_set_secure(). It will
> >> make sure that memory is zeroed/erased before freeing to not keep
> >> secrets around.> 
> > Stepping back a moment, how do you handle talloc_realloc()?
> > 
> > That either needs to be banned or handled to ensure the old memory is
> > wiped after a memcpy() to new memory (with performance losss).
> > 
> > (And that will all need tests).
> > 
> > Sorry this is turning into a can of worms, but if we do this we need to
> > do it completely.
> 
> Yes, I also discussed privately with Andreas that we need to make sure
> talloc_report() doesn't leak the content.
> 
> The current idea is:
> 
> #define talloc_keep_secret(ptr) _talloc_keep_secret(ptr, #ptr);
> void _talloc_keep_secret(const void *ptr, const char *name);
> 
> While I may prefer to pass name explicit.
> I guess talloc_asprintf_append* also needs special handling.
> We need to decide what to do with talloc_strdup() and even more complex
> talloc_asprintf(..., "%s", secret_talloc_string).

What do you mean? That in case we pass a pointer to such a function it should 
be inherited?

I think if you do that you should do it like:

char *password = talloc_strdup(talloc_secret_string);
talloc_keep_secret(password);

And not magically to it. I think explicitly calling it is better.

> Do we force the caller to reuse talloc_keep_secret() or do we want to
> somehow inherit the secret state.

I would not inherit it, only talloc_realloc() should inherit it.

The current state is here:

https://git.samba.org/?p=asn/samba.git;a=shortlog;h=refs/heads/master-memset_s


-- 
Andreas Schneider                      asn at samba.org
Samba Team                             www.samba.org
GPG-ID:     8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D