s4-messaging: Use generate_random() to get a unique ID for messaging clients
abartlet at samba.org
Wed Apr 25 07:12:07 MDT 2012
On Wed, 2012-04-25 at 08:59 -0400, simo wrote:
> On Wed, 2012-04-25 at 22:51 +1000, Andrew Bartlett wrote:
> > 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.
> This doesn't really help at process startup, as we reset the global
> variable after fork, so for the first 40 bytes at least we do
> use /dev/urandom, that's why I had a concern.
Certainly. If we were using less than 40 random bytes in total per
process, then we could be reading a little more random data. Do you
have a suggestion for a different heuristic? (It was just a guess at
the time, trying not to always pull 40 bytes per process regardless).
> > 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
> Ok, seem like iot is staying on the radar, that's good enough.
> > 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.
> True, although libraries could change it, so depending on nothing ever
> calling srand() in smbtorture seem a bit fragile.
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
More information about the samba-technical