[sr-dev] Random number generation

[Hugh Waite]

Hi,
One of the places that uses rand() is 
modules/tm/h_table.c:init_synonym_id . Requests generated by kamailio 
(as a uac) use this to generate the Via branch tag. If any module calls 
srand(), it affects the Via branch for subsequent requests.

The actual bug we found is in rls/notify.c:generate_string(). The 
add_resource_instance() function will re-seed srand() with 0 (zero), 
leading to nearly every NOTIFY sent by rls having the same "random" 
number in the Via branch. I am sure this was the cause of lost replies, 
timeouts and dropped subscriptions that we were seeing (and appears to 
have gone away after removing it).

Although, I could just remove the srand from rls notify.c, I wondered if 
it should be using a different random function, and also whether 
init_synonym_id should use something more unique for the Via branch 
parameter.

A quick check has shown a few places that call srand() within the code, 
although they probably have less drastic consequences.

Regards,
Hugh

On 13/04/2012 14:01, Daniel-Constantin Mierla wrote:
> Hello,
>
> I haven't looked at the issue reported by Hugh, so by now just 
> comments on srutils/sruid.
>
> The idea was to have an unique id generator without linking to an 
> external library -- my first purpose was to use it for temporary GRUU 
> ids (RFC5627), I am just about to push to master the gruu support in 
> registrar/usrloc.
>
> Then I thought it might be useful in other places, such as dialog 
> unique id.
>
> I added it as part of lib, since its target usage was for modules so 
> far, but if needed for some core processing, the two files (rather 
> small by now) can be moved in the core.
>
> Cheers,
> Daniel
>
> On 4/13/12 2:50 PM, Henning Westerholt wrote:
>> On Friday 13 April 2012, Hugh Waite wrote:
>>> I have a question about random number generation within kamailio.
>>>
>>> A number of modules use rand() to get a random value and in some places
>>> is re-seeding with srand(). I believe this is dangerous because rand()
>>> is used in the Via branch tag generator.
>>> We have detected some real bugs (where srand is reseeding with 0 for
>>> every message, causing transaction mis-matching) but I'm not sure of 
>>> the
>>> correct way to fix this (other than remove srand()).
>>>
>>> Should all modules be using a 'core' random function (e.g. in srutils?)
>>> ? And if so, is this library documented?
>>>
>>> Regards,
>>> Hugh
>> Hi Hugh,
>>
>> for the purpose getting a pseudo-random number (i.e. not for 
>> cryptographic
>> functionality) we should consolidate on a single random function. 
>> There is the
>> recent introduced srutils/sruid code, then there exists a (IMHO 
>> stronger)
>> pseudo-random number generator in rand/fastrand and then there is of 
>> course
>> rand().
>>
>> Maybe Daniel can comment about the purpose of the srutils function, IMHO
>> consolidating on fastrand or one of the stronger function (d_rand 
>> etc..) from
>> stdlib.h would be fine.
>>
>> The re-seeding the internal state of rand() with srand during runtime 
>> sounds
>> wrong toe me and should be removed/ fixed.
>>
>> Viele Grüße/ best regards,
>>
>> Henning Westerholt
>>
>> _______________________________________________
>> sr-dev mailing list
>> sr-dev at lists.sip-router.org
>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>


-- 
Hugh Waite
Senior Design Engineer
Crocodile RCS Ltd.