Tests for Andrew's talloc security work

[Andrew Bartlett]

On Fri, 2015-09-04 at 11:18 -0700, Jeremy Allison wrote:

> Adding the above global static
> breaks that - so it's an ABI
> breakage IMHO.
> 
> Now in common use this variable
> is only read, not written, and
> only initialized once in a constructor
> attribute when the library is loaded,
> so this may mitigate the problem.

That is indeed the design pattern.  If you are concerned that the
library can reasonably be initialised twice then the second patch using
'rand()' could be skipped or re-written to use the sum of the version
numbers, and therefore get an always-constant result. 

The variable could be declared as sig_atomic_t but per 
https://www.gnu.org/software/libc/manual/html_node/Atomic-Types.html in
t is essentially safe.

> But I'd need to check that running
> existing talloc + threaded programs under
> valgrind hellgrind and drd to ensure
> we don't get any error messages before
> I can be convinced this is safe.

We would very much appreciate if you could do that.  How do you think
this would happen?  

Unlike previous patches, no attempt is made to do this 'on first use',
the only support is for the library constructor.  

That is, in my understanding, triggered once per library load, and
would be at startup, except when a program loads a plugin with
dlopen().  In that case, a thread may be present, but I can't see how
another thread could expect to start calling talloc functions until the
dlopen() completes, and this has been run.  Perhaps two different
threads call dlopen() on two different objects?  The talloc lib can't
feasably be loaded twice and if it is loaded once I can't see why two
constructors would run. 

The biggest trap I can see is if another library also pulled in by
dlopen() also used talloc in it's own constructor, and ran before
talloc's constructor, because it could see the variable before and
after being set.  I don't think this is reasonable however, but we
could move to the INIT ELF section if we had to.

Andreas,

You have an 'if initialised' check in uid_wrapper, and commented about
needing this in libssh when I raised this previously.  Can you explain
the race you see?  Is it only when the calling program can also access
the crypto libs (rather than this static variable in our own .so)?

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba