On Tue, Apr 17, 2018 at 12:58:09PM +0200, Renaud Allard wrote: > > > On 04/17/2018 11:34 AM, Theo Buehler wrote: > > On Tue, Apr 17, 2018 at 11:18:50AM +0200, Renaud Allard wrote: > > > Hello, > > > > > > This patch for exim replaces all calls to rand() and random() to the > > > secure > > > OpenBSD version, making the compiler less unhappy. > > > After a discussion with one of the exim devs, this change would not have > > > been accepted in mainstream exim because there is no "need" to use a > > > crypto > > > secure algorithm each time. But we do that anyway on OpenBSD, so here it > > > makes sense. > > > > > > Regards > > > > Since this patch is only for OpenBSD, it is not needed. On OpenBSD, > > rand() and random() internally use arc4random() unless > > srand_deterministic() was called (which exim doesn't do, of course). > > > > The way I understand it, the APIWARN is there to make people aware that > > this might be a problem on other systems. The rand(3) manuals states: > > > > Standards insist that this interface return deterministic results. > > Unsafe usage is very common, so OpenBSD changed the subsystem to > > return > > non-deterministic results by default. > > > > [...] > > > > If srand_deterministic() was called, the result will be computed > > using the > > deterministic algorithm. > > > > Same goes for random(3). > > > > Generally, we are reluctant to change ports locally to use the OpenBSD > > idioms to silence this kind of link time warnings (e.g. string handling). > > > > Hi Theo, > > I understand your point of view, but this patch also removes some useless > checks and calls to srandom(). As discussed with stenh@, I am willing to > become the maintainer of this port, so I will be maintaining those patches > too anyway. >
I'm not going to object strongly, but this occurs twice: for (rnd = arc4random() % weights, i = 0; i < num_servers; i++) The expression arc4random() % weights; is subject to modulus bias. Use arc4random_uniform(weights); instead. I'm not sure this fixes all the problems with randomness in its vicinity, but it should be a bit better...