On 2018/04/17 13:10, Theo Buehler wrote: > 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 agree with tb@, I don't think we should have what are basically noop patches in the ports tree just to silence linker warnings. If upstream are willing to take them (with proper compile checks etc) then it improves things for other OS users. But just replacing random/srand with arc4random on OpenBSD doesn't give a real benefit. There is one caveat with keeping them as-is; if something uses a library that calls one of the *rand*_deterministic functions. From looking over ports I think the only danger area there is things using lua. There are a bunch of similar patches in-tree from when this *did* make a difference. Perhaps these should be removed. (At least these are less likely to cause major breakage than some of the str* patches that have been in the ports tree in the past...) > 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... This is something where a change to arc4random_uniform would make an actual improvement so this one seems fair enough..
