On Thu, 2 Aug 2018, John P. Rouillard wrote:

> >In password.py we use os.urandom (for SSHA) without checking if it is 
> >available.
> 
> Wow, yeah that would generate a nice exception at the very least.
> Question, does the salt needs to be generated using os.urandom at all?
> Everything else in that module seems to use just random. Is the
> randomness of the salt that critical?

Optimally for salts, no two passwords hashed with the same algorithm 
(ever, anywhere) would use the same salt, so there is no benefit to an 
attacker precomputing hashes / rainbow tables.  Typically that should mean 
salts of at least 64 bits.  The security model here is protecting against 
attackers who somehow acquire a large number of password hashes and 
corresponding salts.  Since the attacker is presumed to know the salt, its 
unpredictability should not be so critical, unless it results in the same 
salt being likely to be reused (including on different sites).

Things that need secure randomness include password generation 
(generatePassword), OTKs, session IDs for cookies, CSRF nonces.  It's less 
critical, I think, for salts and for message-ids.  (I don't think Roundup 
should do its own message-id generation anyway - use 
email.utils.make_msgid instead from the standard library.)

When secure randomness is needed, you can either use system random number 
generation directly, or use pseudorandom number generation having recently 
seeded from the system random number generator.  What's not safe is using 
pseudorandom number generation having not reseeded that generator for a 
long time (because if too many random numbers have been generated from the 
same generator, an attacker may be able to determine its state, and thus 
predict the random numbers used for other users).

Rather than trying to decide that particular places do not need secure 
randomness, it's certainly safer always to use the system random number 
generator.

> >The places where we reseed the random number generator seem to be a 
> >bit, well, random. We should probably only reseed after forking, i.e. 
> >in ForkingServer (roundup_server.py).
> 
> There are at least 5 deployment mechanisms for roundup (see "Configure
> a Web Interface" in doc/installation.txt . Only one of which is the

That's actually a list of six, despite saying five above the list.

> server. What happens when roundup is used as a cgi or under
> mod_python, zope or wsgi? Cgi is probably ok since we get a default
> seed when the process is started that is thrown away when the process
> exits. But mod_python, wsgi and zope are IIUC long lived processes
> which do not go through scripts/roundup_server.py, but do go through
> the cgi/client.py module which is where I did the seeding.
> 
> I think the seeding there isn't the best as in theory the seeding data
> source could be known (IIRC it's the timestap when seed is
> called). That could be known by an attacker as it is some small window

When os.urandom is available, it should be used by Python for reseeding by 
default.  Except that this code is using SystemRandom when available, so 
the only time when reseeding is not a no-op, it's also using a weak seed.

I'd suggest simply making the code fail if SystemRandom / os.urandom is 
not available, rather than trying to have fallbacks (without SystemRandom 
there is no good way to get a strong seed).  Once you're always using 
SystemRandom (directly or indirectly), you no longer need to deal with 
reseeding.