[BUG] Two caller in different processes may generate same group id.

[gou4shi1]

Describe the bug
I launched three caller processes on my desktop at the same time, they will push different videos to the same server, but two callers generated same socket id sequence, and server considered they are from same group :(
Here is the server debug log https://1drv.ms/u/s!AuaosNlJ5ELxjyxer5F6zlUwraBx?e=qSoAW4
May be it's because the seed of srand() are same?

   gettimeofday(&t, 0);
   srand((unsigned int)t.tv_usec);

Can I use stream id as group id?

Expected behavior
Different processes should not generate same group id.

Desktop (please provide the following information):

• OS: Linux,
• SRT Version / commit ID: master/bdb3191

[gou4shi1]

I have tried a simple fix by myself https://github.com/gou4shi1/srt/commit/e94227cd98eda1b58bb8872a050e4ca88d3efa1a.diff
Just reject handshake if SID of same group conflict.

[ethouris]

No, streamid is an optional setting to be interpreted by the user application, and it's empty by default. The problem must be somewhere else.

Can you provide me with more information about the case you conducted? Meaning from which host you have callers and where the listener is etc.

I think there's one check lacking - if you found a matching peer group, it should be checked that it has come from the same server as the existing sockets, while peer group ID should not be itself a sufficing peer group ID.

[gou4shi1]

Both callers and listener are localhost.

[ethouris]

Wait. But there are two callers running in two different SRT applications?

[gou4shi1]

YES.

[maxsharabayko]

Potentially some additional Unique Identifier has to be included in the Group Membership extension.
This UID must have a randomly generated value. The chances of having the same Group ID and UID values generated by independent instances are pretty low. It would also mean, however, that SRT will have to look up both GroupID and UID sometimes or always.

    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                           Group ID                            |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |     Type    |     Flags     |             Weight              |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                             UID                               | <--- additional field
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

[ethouris]

Yes, and then the key for the group peer map would be a 64-bit value consisting of the group ID and UID. This way, this facility could be used like before.

[gou4shi1]

Beside the group id conflict problem, does socket id confliction will cause server error?

Also, can we use std::random_device instead of srand(time)?

[ethouris]

I'm afraid, not.

Defined in header <random> |   |  
-- | -- | --
class random_device; |   | (since C++11)

[ethouris]

And whether the problem could exist with two peer sockets with two different IDs I'm only about to check. But I don't think there's a problem here - you can associate a socket with a peer socket only once. Second socket, even if with the same ID, could only be associated with a different socket, as well as when sending a packet the DESTINATION socket ID is used.

[maxsharabayko]

@gou4shi1

Also, can we use std::random_device instead of srand(time)?

Yes, when C++11 is supported. FR #1825.

[ethouris]

Ok, I was unprecise: Not as the only way - it can be added as an alternative and used when compiling in C++11 mode.

[maxsharabayko]

Status log.
PR #1852 removes the check for the size of the Group Membership HS extension.
SRT v1.4.2 checks for the exact size of the extension, which does not allow to extend it, e.g. with UID.