Randomized peerids

[dan-bishopfox]

No description provided.

[lsalzman]

Right now it's hard to evaluate what's going on in the PR because most of the diff is line-ending changes. I would appreciate if all the changes could be squashed into one commit and all the line-ending diffs removed from it regardless so that I can easily read this.

[dan-bishopfox]

Removed whitespace changes, squashed down to one commit.

[lsalzman]

On balance I think the idea of the patch seems to not be bad, but there are some improvements here we should do to make this better. incomingPeerID needs to be handled with more care. We should store the randomized ID in a separate field (probably in the 16-bit space that is currently "reserved" in ENetPeer so we don't change its ABI size), and ensure that incomingPeerID just stores the index in the peers array like most code expects, but then we can transparently map that before the user sees it, and use the value stored in that previously reserved field for the pre-mapped randomized value.

The other thing that needs some thought is the map code is currently very expensive in the collision case, so it would be nice if we could find some way to chain the collision cheaply or at least more sanely bound the search.

I would also like to see this integrate with the session ID so that we can gain another 2 bits of randomness for the id.

We probably need to get a better random function for the peer ID and for enet in general. Using a LCG basing off the current random seed would probably be good enough here. I have some Mersenne Twister code I could rig up, but that might be overkill here.

[lsalzman]

We also have the problem of dealing with collisions in the random ID itself, such that we need to ensure no two peers that are live have the same one.

[dan-bishopfox]

Those all sound like good ideas, though straining my understanding of some of the ENet fields. I'm not sure I'd be the right person to make those changes.

Some thoughts:

• The map code is a really quick-and-dirty implementation for sure. O(n) in the worst case is obviously not great, but the logic was simple enough for the PR, which I wanted to demonstrate as viable more than anything. A better map can get O(log(n)), but data structures like that in C get hairy quick.
• The randomness was an issue I noticed, too. Neither an LCG nor the Mersenne twister are a secure source of randomness and can be predicted without much difficulty. (I have a separate tool which does exactly this, in fact https://github.com/altf4/untwister/) To get a source of secure random numbers, you have to ask the OS. On unix, you can read from /dev/urandom, Windows has its own thing. This would get complicated for porters to odd environments, though. I made a port of ENet that runs on the Nintendo Wii, for example, and hooking into the OS's source of entropy would be tricky.
• Avoiding collisions is a good point. Probably needs a Fisher–Yates_shuffle https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle Basically, you pick one of the available slots at random, so you never pick the same one twice.