Decoding custom packets with Python using Twisted

Question

This server require fast decoding of packets from clients, as I am expecting 5000 concurrent clients that is sending packets possibly every second. This is a game server (5000 players walking send packets each n every step).

I want to see if my implementation is fast enough or there are still room for improvements. The encryption isn't on this code as I only want to tackle the decryption for now.

The decode function is taken from a JavaScript implementation, This is the best output I got after trying to translate it. (though the translation is correct because the results are same from JavaScript and now Python)

def dataReceived(self, data):
 packet = decode(data)
 (counter, packet_id, username, password, unknown) = unpack('<B H 16s 16s 16s', packet)
 print counter
 print packet_id
 print username
 print password
 print unknown

For example the above implementation is pretty straight forward but the concern here is the decode(data) part.

I am new to Python and Twisted so I had to do some maybe absurd way on implementing this.

This is my decode function:

def decode(packet_data):
 cl = 0x6e
 internal_rounds = 2
 i = 0
 seqX = bytearray.fromhex('HEXSTRINGREMOVED')
 seqY = bytearray.fromhex('HEXSTRINGREMOVED')
 result = []
 while i < (len(packet_data) - 4):
 dl = ord(packet_data[i])
 bl = seqX[(i * 2) + 1]
 al = seqX[i * 2]
 bl2 = seqY[(i * 2) + 1]
 dl2 = seqY[i * 2]
 j = internal_rounds - 1
 while j > 0:
 if bl <= dl:
 dl -= bl
 else:
 bl = (~bl) & 0xFF
 bl += 1
 dl += bl
 dl ^= bl2
 j -= 1
 if al <= dl:
 dl -= al
 else:
 al = (~al) & 0xFF
 al += 1
 dl += al
 dl ^= dl2
 dl ^= cl
 result.append(dl)
 cl ^= dl
 i += 1
 return "".join(map(chr, result))

I did this dl = ord(packet_data[i]) because I keep getting int -= char error.

Also did this return "".join(map(chr, result)) because I kept getting errors with unpack because the return was previously return result which result is an array.

So what do you think? Should I also have used unpack_from instead? But I don't know how to make a buffer in Python. Is the data received already a buffer?

• Gareth Rees: Can you say more about this algorithm? How did you choose it? Why did you not use a standard algorithm like AES?
• majidarif: algorithm is something I have to (must) use. It's actually a rc4 xor block cipher. As that is the algorithm used by the client and I cannot modify the client. I'm just updating the server code from way back 2003 to newer technology, choices are node.js and python. Already done with the node.js prototype, now I'm trying python.
• Gareth Rees: If it's RC4, did you consider using Crypto.Cipher.ARC4 in pycrypto?
• majidarif: To be more specific this is the implementation used, with this ARC4 as PRNG. I tried using arc4 implementations but I just cant get the same results. Maybe there is something custom going on those? What do you think?
• majidarif: but actually my issue here is if handling the conversions and the datatypes properly.

Answer 1

It is hard to review something when you can't test whether your reviewed code actually works, so with that disclaimer lets see what we can do with your code.

Regarding style issues, your variable names are not good, and doesn't convey any useful informations. But leave that as it is...

Here are some elements I've incorporated in the refactoring below:

• Your code sets j=internal_rounds - 1 which in effect sets j=1, so that the while loop never executes more than once. In other words it can be removed
• You calculate i*2 and i*2 +1 twice for each loop, which could be simplified to some addition if we add a few more variables. That is i is expanded into i2 and i2_1, in the hope that 3 additions on those is cheaper than 3 additions and 4 multiplications. :)
• Several of your steps change a variable temporary before using the result to change the dl variable, so I've compressed those a little
• Remove the calculation of the loop limit for every loop

Is my refactored code faster than yours? You've got to test, as I can't. The same applies for the correctness, but I think it should be the same.

def decode(packet_data):
 i = 0
 i2 = 0
 i2_1 = 1
 packet_length = len(packet_data) - 4
 seqX = bytearray.fromhex('HEXSTRINGREMOVED')
 seqY = bytearray.fromhex('HEXSTRINGREMOVED')
 result = []
 cl = 0x6e
 while i < packet_length:
 dl = ord(packet_data[i])
 bl = seqX[i2_1]
 al = seqX[i2]
 bl2 = seqY[i2_1]
 dl2 = seqY[i2]
 dl += -bl if bl <= dl else (~bl & 0xFF + 1)
 dl ^= bl2
 dl = -al if al <= dl else (~al & 0xFF + 1)
 dl ^= dl2 ^ cl
 result.append(dl)
 cl ^= dl
 i += 1
 i2 += 2
 i2_1 += 2
 return "".join(map(chr, result))

Another micro-change you could try out, is to change the handling of result to:

result = ''
...
result += chr(dl)
...
return result

It could be faster (or slower), as it skips the array handling and the join/map operations, but it uses a string concatenation instead...