Packet encryption is a BITCH..

Networking & Multiplayer
#1

Okay, I have this AES cipher:


package renoriaserver.security;

import java.util.Random;
import javax.crypto.BadPaddingException;
import javax.crypto.Cipher;
import javax.crypto.IllegalBlockSizeException;
import javax.crypto.spec.SecretKeySpec;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
 *
 * @author David
 */
public class AESCrypt {
	private static int cid = 0;
	private Cipher cipher;
	private byte[] iv;
	private boolean testing = true;
	private int rot = 0;
	private int id = 0;
	
	protected final static Logger log = LoggerFactory.getLogger(AESCrypt.class);
	
    private static final byte[] bytes = new byte[] { (byte) 0xCE, 0x4F, 0x77, (byte) 0xA4, 0x45, (byte) 0xD0,
            0x71, (byte) 0xBF, (byte) 0xB7, (byte) 0x98, 0x20, (byte) 0xFC, 0x4B, (byte) 0xE9, (byte) 0xB3, (byte) 0xE1,
            0x5C, 0x22, (byte) 0xF7, 0x0C, 0x44, 0x1B, (byte) 0x81, (byte) 0xBD, 0x63, (byte) 0x8D, (byte) 0xD4,
            (byte) 0xC3, (byte) 0xF2, 0x10, 0x19, (byte) 0xE0, (byte) 0xFB, (byte) 0xA1, 0x6E, 0x66, (byte) 0xEA,
            (byte) 0xAE, (byte) 0xD6, (byte) 0xCE, 0x06, 0x18, 0x4E, (byte) 0xEB, 0x78, (byte) 0x95, (byte) 0xDB,
            (byte) 0xBA, (byte) 0xB6, 0x42, 0x7A, 0x2A, (byte) 0x83, 0x0B, 0x54, 0x67, 0x6D, (byte) 0xE8, 0x65,
            (byte) 0xE7, 0x2F, 0x07, (byte) 0xF3, (byte) 0xA4, 0x27, 0x7B, (byte) 0x85, (byte) 0xB0, 0x26, (byte) 0xFD,
            (byte) 0x8B, (byte) 0xA9, (byte) 0xFA, (byte) 0xBE, (byte) 0xA8, (byte) 0xD7, (byte) 0xCB, (byte) 0xCC,
            (byte) 0x92, (byte) 0xDA, (byte) 0xF9, (byte) 0x93, 0x60, 0x2D, (byte) 0xDD, (byte) 0xD2, (byte) 0xA2,
            (byte) 0x9B, 0x39, 0x5F, (byte) 0x72, 0x11, 0x4C, 0x69, (byte) 0xF8, 0x31, (byte) 0x87, (byte) 0xEE,
            (byte) 0x8E, (byte) 0xAD, (byte) 0x8C, 0x6A, (byte) 0xBC, (byte) 0xB5, 0x6B, 0x59, 0x13, (byte) 0xF1, 0x04,
            0x00, (byte) 0xF6, 0x5A, 0x35, 0x79, 0x48, (byte) 0x8F, 0x15, (byte) 0xCD, (byte) 0x97, 0x57, 0x12, 0x3E, 0x37,
            (byte) 0xFF, (byte) 0x9D, 0x4F, 0x51, (byte) 0xF1, (byte) 0xA3, 0x70, (byte) 0xBB, 0x14, 0x75, (byte) 0xC2,
            (byte) 0xB8, 0x72, (byte) 0xC0, (byte) 0xED, 0x7D, 0x68, (byte) 0xC9, 0x2E, 0x0D, 0x62, 0x46, 0x17, 0x11, 0x4D,
            0x6C, (byte) 0xC4, 0x7E, 0x52, (byte) 0xC1, 0x54, (byte) 0xC7, (byte) 0x9A, 0x1C, (byte) 0x88, 0x58, 0x2C,
            (byte) 0x89, (byte) 0xDC, 0x02, 0x64, 0x40, 0x01, 0x5D, 0x38, (byte) 0xA5, (byte) 0xE2, (byte) 0xAF, 0x55,
            (byte) 0xD5, (byte) 0xEF, 0x2A, 0x7C, (byte) 0xA7, 0x5B, (byte) 0xA6, 0x6F, (byte) 0x86, (byte) 0x9F, 0x73,
            (byte) 0xE6, 0x0A, (byte) 0xDE, 0x2B, (byte) 0x99, 0x4A, 0x47, (byte) 0x9C, (byte) 0xDF, 0x09, 0x76,
            (byte) 0x9E, 0x30, 0x0E, (byte) 0xE4, (byte) 0xB2, (byte) 0x94, (byte) 0xA0, 0x3B, 0x34, 0x1D, 0x28, 0x0F,
            0x36, (byte) 0xE3, 0x23, (byte) 0xB4, 0x03, (byte) 0xD8, (byte) 0x90, (byte) 0xC8, 0x3C, (byte) 0xFE, 0x5E,
            0x32, 0x24, 0x50, 0x1F, 0x3A, 0x43, (byte) 0x8A, (byte) 0x96, 0x41, 0x74, (byte) 0xAC, 0x52, 0x33, (byte) 0xF0,
            (byte) 0xD9, 0x29, (byte) 0x50, (byte) 0xB1, 0x16, (byte) 0xD3, (byte) 0xAB, (byte) 0x91, (byte) 0xB9,
            (byte) 0x84, 0x7F, 0x61, 0x1E, (byte) 0xCF, (byte) 0xC5, (byte) 0xD1, 0x56, 0x3D, (byte) 0xCA, (byte) 0xF4,
            0x05, (byte) 0xC6, (byte) 0xD5, 0x08, 0x49, 0x4F, 0x64, 0x69, 0x6E, 0x4D, 0x53, 0x7E, 0x46, 0x72, 0x7A 
    };
	
	/**
	 * 
	 * @param key 128 bit AES key
	 * @param iv 4 byte Initialization vector
	 */
	public AESCrypt(byte[] key, byte[] iv) {
		try {
			id = cid;
			cid++;
			setIV(iv);
			cipher = Cipher.getInstance("AES");
			SecretKeySpec keySpec = new SecretKeySpec(key, "AES");
			cipher.init(Cipher.ENCRYPT_MODE, keySpec);
		} catch (Exception ex) {
			log.error("ERROR", ex);
		}
	}
	
	public byte[] getIV() {
		return iv;
	}
	
	public void setIV(byte[] iv) {
		this.iv = iv;
	}
	
	public byte[] crypt (byte[] in) {
		byte[] nIv = null;
		try {
			nIv = cipher.doFinal(this.iv);
		} catch (IllegalBlockSizeException ex) {
			log.error("Error", ex);
			return null;
		} catch (BadPaddingException ex) {
			log.error("Error", ex);
			return null;
		}
		nIv = trimToSize(nIv, 4);
		nIv = updateIV(nIv);
		//nIv = getIV(nIv);
		int bh = 0;
		for (int i = 0; i < in.length; i++) {
			if (bh >= iv.length - 1) {
				bh = 0;
			}
			in[i] ^= nIv[0];
		}
		iv = nIv;
		if (testing) {
			System.out.print("New IV: (" + rot + ") (" + id + ") ");
			pb(this.iv);
			rot ++;
		}
		return in;
	}
	
	public byte[] getIV(byte[] iv) {
		byte[] b = new byte[4];
		for (int i = 0; i < 4; i++) {
			b[i] = bytes[Math.abs(iv[i])];
		}
		return b;
	}
	
	public byte[] trimToSize(byte[] in, int size) {
		byte[] b = new byte[size];
		for (int i = 0; i < size; i++) {
			b[i] = in[i];
		}
		return b;
	}
	
	public static void main(String[] argz) {
		String test = "This is a test.";
		byte[] bytez = test.getBytes();
		byte[] key = new byte[16];
		byte[] iv = new byte[4];
		new Random().nextBytes(key);
		new Random().nextBytes(iv);
		AESCrypt ec = new AESCrypt(key, iv);
		byte[] en = ec.crypt(bytez);
		System.out.print("Encrypted:"); 
		pb(en);
		System.out.println(new String(en));
		AESCrypt dc = new AESCrypt(key, iv);
		byte[] dn = dc.crypt(en);
		System.out.print("Decrypted:"); 
		pb(dn);
		System.out.println(new String(dn));
	}
	
	public static void pb(byte[] a) {
		for (int i = 0; i < a.length; i++) {
			System.out.print(Integer.toHexString(a[i]));
			if (i < a.length - 1) {
				System.out.print(", ");
			}
		}
		System.out.println();
	}
	
    public byte[] updateIV(byte[] niv) {
        for (int i = 0; i < niv.length; i++) {
            if (i < niv.length/2) { //16/2
                niv[i] ^= niv[i + 1];
                if ((0xff & niv[i]) > 127) {
                    niv[i] = 0x0A;
                }
                niv[i] ^= niv[i + 2];
                if ((0xff & niv[i]) > 127) {
                    niv[i] = 0x0D;
                }
            } else { //16/2
                niv[i] ^= niv[i - 2];
                if ((0xff & niv[i]) > 127) {
                    niv[i] = 0x0C;
                }
                niv[i] ^= niv[i - 2];
                if ((0xff & niv[i]) > 127) {
                    niv[i] = 0x1D;
                }                
            }
        }
        return niv;
    }
}

With one client connected to the server it works fine, but if I connect more than one client it totally screws up the encryption.

Does anyone know why this happens or how to fix it?

#2

At a guess the factory is sharing the Cipher instance between your clients and you need to synchronise your usage of it.

#3

no its not sharing it, it should create a new Cipher each time I create a new AESCrypto

and its not a factory, its a class…

#4

[edit:]

Nevermind! =D

#5

maybe

#6

Jep. This is definately a problem, since its access is not synchronized in the constructor.

@Renoria
Change your constructor to:


        // Create a special object for synchronization, to ensure that you don't
        // accidently block (or get blocked from) unrelated code
        private static final Object ID_MUTEX = new Object();

	/**
	 * 
	 * @param key 128 bit AES key
	 * @param iv 4 byte Initialization vector
	 */
	public AESCrypt(byte[] key, byte[] iv) {
		try {
			synchronized(ID_MUTEX){ id = cid++; }
			setIV(iv);
			cipher = Cipher.getInstance("AES");
			SecretKeySpec keySpec = new SecretKeySpec(key, "AES");
			cipher.init(Cipher.ENCRYPT_MODE, keySpec);
		} catch (Exception ex) {
			log.error("ERROR", ex);
		}
	}

Note that simply making cid volatile would not be sufficient!
Some useful infos here: http://mindprod.com/jgloss/threadsafe.html

Also using an instance of this AESCrypt class from multiple threads is not safe. You need to create an own instance for every Thread:

#7

The id is only used in debugging output. It might be that everything is working fine and it’s just the debugging output which looks wrong, but that’s not the impression I got from the first post.

Renoria, can you make a test case which fails reliably?

#8

As I’ve said, the AESCipher screws up only when there are TWO or more clients connected. One client works fine.

A person that I know of that is developing a game in C# also has this problem. And cid is not needed, its a debug value to tell me which instance is which…

@pjt33:

Yeah, it says things like this:


Got unhandled message from 127.0.0.1, message: 4A 00 31 B4 32 AA D1 3D

sometimes it will throw an ArrayIndexOutOfBoundsException when the header goes negative or above 32k.

#9

Are you sure, you are using an AESCrypt per Thread? If so, how do you ensure it? Post the code.

#10

okay:


    @Override public void sessionOpened(IoSession session) throws Exception {
        log.info("Connection with " + session.getRemoteAddress() + " opened.");
		byte[] sendIV = generateIV();
		byte[] recvIV = generateIV();
		AESCrypt sendCrypto = new AESCrypt(key, sendIV);
		AESCrypt recvCrypto = new AESCrypt(key, recvIV);
        RenoriaClient client = new RenoriaClient(session, sendCrypto, recvCrypto);
		session.write(PacketCreator.getIVs(sendIV, recvIV, (short) 100));
        session.setAttribute(RenoriaClient.CLIENT_KEY, client);

        storage.registerClient(client);
    }

SessionOpened


/*
 * To change this template, choose Tools | Templates
 * and open the template in the editor.
 */

/*
 * This file is part of the "Renoria" Game.
 * Copyright (C) 2008
 * IDGames.
 */

package renoriaserver.net.mina;

import org.apache.mina.common.ByteBuffer;
import org.apache.mina.common.IoSession;
import org.apache.mina.filter.codec.CumulativeProtocolDecoder;
import org.apache.mina.filter.codec.ProtocolCodecFactory;
import org.apache.mina.filter.codec.ProtocolDecoder;
import org.apache.mina.filter.codec.ProtocolDecoderOutput;
import org.apache.mina.filter.codec.ProtocolEncoder;
import org.apache.mina.filter.codec.ProtocolEncoderAdapter;
import org.apache.mina.filter.codec.ProtocolEncoderOutput;
import renoriaserver.client.RenoriaClient;
import renoriaserver.net.RenoriaPacket;

public class ByteArrayCodecFactory implements ProtocolCodecFactory {

    private ProtocolEncoder encoder;
    private ProtocolDecoder decoder;
	
    public ByteArrayCodecFactory() {
        encoder = new ByteArrayEncoder();
        decoder = new ByteArrayDecoder();
    }

    public ProtocolEncoder getEncoder() throws Exception {
        return encoder;
    }

    public ProtocolDecoder getDecoder() throws Exception {
        return decoder;
    }

    public class ByteArrayEncoder extends ProtocolEncoderAdapter {

        public void encode(IoSession session, Object message, ProtocolEncoderOutput out) throws Exception {
            RenoriaClient client = (RenoriaClient) session.getAttribute(RenoriaClient.CLIENT_KEY);
            if (client != null) {
                byte[] bytes = ((RenoriaPacket) message).getBytes();
		bytes = client.getSendCrypto().crypt(bytes);
                ByteBuffer buffer = ByteBuffer.allocate(bytes.length + 4, false);
                buffer.putInt(bytes.length);
                buffer.put(bytes);
                buffer.flip();
                out.write(buffer);
            } else {
		byte[] bytes = ((RenoriaPacket) message).getBytes();
                ByteBuffer buffer = ByteBuffer.allocate(bytes.length + 4, false);
                buffer.putInt(bytes.length);
                buffer.put(bytes);
                buffer.flip();
		out.write(buffer);
            }
        }
    }

    public class ByteArrayDecoder extends CumulativeProtocolDecoder {

        protected boolean doDecode(IoSession session, ByteBuffer in, ProtocolDecoderOutput out) throws Exception {
            if (in.prefixedDataAvailable(4)) {
				RenoriaClient client = (RenoriaClient) session.getAttribute(RenoriaClient.CLIENT_KEY);
				int length = in.getInt();
				byte[] bytes = new byte[length];
				in.get(bytes);
				bytes = client.getRecvCrypto().crypt(bytes);
				out.write(bytes);
				return true;
            } else {
				return false;
            }
        }
    }
}

Coder factory w/e

#11

Seems ok. I can’t see no cross-thread usage in this code. Try to make the AESCrypt construcor synchronized, to rule out the possibility that simply the Cypher.getInstance(“AES”) isn’t thread safe.

#12

I’m sure its not because of that, because my old crypto class:



package renoriaserver.net;

public class PacketEncoder {
	private int iterationCount;
	
	public PacketEncoder(int iterationCount) {
		this.iterationCount = iterationCount;
	}
	
	public void crypt(byte data[]) {
		for (int i = 0; i < data.length; i++) {
			data[i] = crypt(data[i]);
		}
	}
	
	private byte crypt(byte data) {
		byte ret = data;
		for (int i = 0; i < iterationCount; i++) {
			ret ^= 0x4D * (i ^ 0x3A);
			if(i % 2==1){
				ret ^= 0xffff;
			}
		}
		return ret;
	}
}

that screws up too… :’(

I’m thinking that it possibly might be something to do with the client…

#13

I’ll rephrase the question. Can you post a short self-contained test case which reliably fails?

#14

I don’t understand what you’re saying ???

#15

If I take the code you’ve posted,
a) it doesn’t compile, because it uses a logging framework which I don’t have;
b) when I remove that logging to get it to compile, it doesn’t demonstrate the bug.

I don’t think anyone other than you is going to spend time trying to turn that example into something which actually demonstrates the bug, and at this point it doesn’t appear that anyone will find it by code inspection. Therefore your two alternatives are to find it yourself or to produce some code which the rest of us can copy-paste-compile and watch it fail.

#16

I’ll upload the logger

#17

Logger library: http://www.mediafire.com/?m2mghnjnry0

#18

Thanks for all the help guys, but I worked it out ;D turns out I just forgot to use System.arraycopy to duplicate the array. Lol.