s4-messaging: Use generate_random() to get a unique ID for messaging clients

[Andrew Bartlett]

On Wed, 2012-04-25 at 08:04 -0400, simo wrote:
> On Wed, 2012-04-25 at 11:44 +0200, Andrew Bartlett wrote:
> > commit b8055132b1c62dd19981fea2822ab9e1829a8ded
> > Author: Andrew Bartlett <abartlet at samba.org>
> > Date:   Wed Apr 25 17:53:18 2012 +1000
> > 
> >     s4-messaging: Use generate_random() to get a unique ID for
> > messaging clients
> >     
> >     The call to random() resulted in duplicate values for s3fs
> > configurations
> >     which, due to the forked child, all started with the same random
> > seed.
> >     
> >     A future improvement would be to move to a proven unique value.
> >     
> >     Andrew Bartlett
> >     
> >     Autobuild-User: Andrew Bartlett <abartlet at samba.org>
> >     Autobuild-Date: Wed Apr 25 11:43:40 CEST 2012 on sn-devel-104
> 
> 
> Andrew are you sure you need to use /dev/urandom here ? It doesn't look
> like you need absolutelu unpredictable numbers here, just non-colliding
> numbers.
> You changed the code to draw from urandom, and if it is used often it
> mean it will suck a lot of entropy out of the system, causing any
> application that need to use /dev/random to halt.
> Wouldn't it have been simpler to just run srand(time(NULL)*pid) to get a
> new seed for the process ?

Simo,

You are correct to think carefully about our over-use of random entropy.
In this case, it isn't used often (typically once per child process in
s3fs configurations only), and we have logic in generate_random_buffer()
to attempt to reduce the load on /dev/urandom. We set up our own PRNG,
where a process has read more than 40 bytes, which I added with
6554433fc227baab93398576db703c91db1541f2 back in 2007.

I also worked on a solution using IDR trees to allocate a server_id
number (along side the pid, for the auth_samba4 use case):
https://git.samba.org/?p=abartlet/samba.git/.git;a=shortlog;h=refs/heads/s3-task-id-allocator however I haven't yet made it work.  If I can make it work, I would prefer to use an allocator, for more predictable behaviour under test.  

Another idea I had was to use the combination of pid and memory address
of the event context or messaging context, but currently 'task' is just
32 bits.

Metze has also suggested some improvements on which fields to randomize
in https://bugzilla.samba.org/show_bug.cgi?id=8872#c4 

As to if we should use random() and srand():  I would rather not be
reseeding the srand() as it may be deliberately set by smbtorture
callers wishing to create reproducible runs, and the messaging client is
used by smbtorture. 

Thanks for your careful attention, and I hope I have addressed your
concerns,

Andrew Bartlett
-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org