[PATCH] memset_s() and talloc_set_secure()

[Ralph Böhme]

On Thu, Oct 11, 2018 at 03:26:33PM +0200, Andreas Schneider via samba-technical wrote:
>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

https://git.samba.org/?p=asn/samba.git;a=commitdiff;h=c95e28c4be2ca057061db23237ff6303059c1b13 
still misses the things I mentioned.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46