10
\$\begingroup\$

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?

\$\endgroup\$
5
  • \$\begingroup\$ Can you say more about this algorithm? How did you choose it? Why did you not use a standard algorithm like AES? \$\endgroup\$ Commented Jul 7, 2014 at 15:32
  • \$\begingroup\$ 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. \$\endgroup\$
    – majidarif
    Commented Jul 7, 2014 at 16:41
  • \$\begingroup\$ If it's RC4, did you consider using Crypto.Cipher.ARC4 in pycrypto? \$\endgroup\$ Commented Jul 7, 2014 at 16:52
  • \$\begingroup\$ 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? \$\endgroup\$
    – majidarif
    Commented Jul 7, 2014 at 16:56
  • \$\begingroup\$ but actually my issue here is if handling the conversions and the datatypes properly. \$\endgroup\$
    – majidarif
    Commented Jul 7, 2014 at 17:01

1 Answer 1

3
\$\begingroup\$

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...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.