Error in .limit() function

bytecod3r commented 4 years ago

I am not sure how to elaborate this one. but when there is many frequent concurrent connections from both sides, server to client, and client to server. Sometimes the following error showing up.

java.lang.IllegalArgumentException: newLimit < 0: (-33395131 < 0)
    at java.base/java.nio.Buffer.createLimitException(Buffer.java:388)
    at java.base/java.nio.Buffer.limit(Buffer.java:362)
    at java.base/java.nio.ByteBuffer.limit(ByteBuffer.java:1322)
    at java.base/java.nio.MappedByteBuffer.limit(MappedByteBuffer.java:383)
    at java.base/java.nio.MappedByteBuffer.limit(MappedByteBuffer.java:70)
    at net.simplenet.Client$Listener.completed(Client.java:608)
    at net.simplenet.Client$Listener.completed(Client.java:567)
    at java.base/sun.nio.ch.Invoker.invokeUnchecked(Invoker.java:127)
    at java.base/sun.nio.ch.Invoker$2.run(Invoker.java:219)
    at java.base/sun.nio.ch.AsynchronousChannelGroupImpl$1.run(AsynchronousChannelGroupImpl.java:112)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
    at java.base/java.lang.Thread.run(Thread.java:832)

Client.Java:608

 while (buffer.remaining() >= (key = peek.getKey())) {
                    var wrappedBuffer = buffer.duplicate().mark().limit(buffer.position() + key); //Line 608 is here

                    if (shouldDecrypt) {
                        try {
                            wrappedBuffer = client.decryptionFunction.apply(client.decryptionCipher, wrappedBuffer)

Any idea what can cause the issue? and how to fix?

jhg023 commented 4 years ago

I don't think I'll be able to be of much help unless you can provide a snippet of code (form both the client and server) that reproduces this issue.

If the value being provided to limit() is negative, that most-likely means that key is negative, which indicates that you're trying to read a negative amount of bytes (which doesn't make sense).

bytecod3r commented 4 years ago

I think you should be able to reproduce this issue by having 20 clients, connected to a server, with AES encryption, the server sends a packet to all 20 clients every 0.5 sec, and the 20 clients sends packet to server every 0.5 sec too... And each client disconect from the server every 0.5 second and connect back.

Server Side :

private void StartServer(){
    Server server = new Server();
    server.onConnect(client -> {
                try {
                    Cipher[] ciphers = initCiphers("AES/CFB8/NoPadding", key);
                    client.setEncryption(ciphers[0], UPDATE);
                    client.setDecryption(ciphers[1], UPDATE);
                } catch (GeneralSecurityException e) {
                    e.printStackTrace();
                }

                client.preDisconnect(() -> {
                    System.out.println("Client disconnected..");
                });

                client.readByteAlways(opcode -> {
                   log.info("Opcode= " + opcode);
                    switch (opcode) {
                        case 1: 
                            client.readInt(len -> {
                                client.readBytes(len, data -> {
                                    ....

                            });
                        });
                    });
                });
                server.bind(serverIP, Integer.parseInt(serverPort));
        }
}

Client Side:

private void Connect(){
    client = new Client();
    client.onConnect(() -> {
        try {
            var ciphers = initCiphers("AES/CFB8/NoPadding", key);
            client.setEncryption(ciphers[0], UPDATE);
            client.setDecryption(ciphers[1], UPDATE);
        } catch (GeneralSecurityException e) {
            e.printStackTrace();
        }

        client.readByteAlways(opcode -> {
            log.info("Opcode= " + opcode);
            switch (opcode) {
                case 1: 
                    client.readInt(len -> {
                        client.readBytes(len, data -> {
                            ...
                        });
                    });
                    break;
                default:
                    log.info("Unexpected value: " + opcode);
                }
            });

    });
    client.connect(serverIP, Integer.parseInt(port));

}

Common between Server and Client:

private Cipher[] initCiphers(String cipher, String key1) throws GeneralSecurityException {
        byte[] ivTest = new byte[]{ //I know hardcoding this is very bad practice, but I am testing at the moment.
                0x41, 0x41, 0x41, 0x41,
                0x41, 0x41, 0x41, 0x41,
                0x41, 0x41, 0x41, 0x41,
                0x41, 0x41, 0x41, 0x41,
        };

        Keys keys = keygen(128, key1.toCharArray(), key1.getBytes());

        var ivSpec = new IvParameterSpec(ivTest);

        var serverEncryption = Cipher.getInstance(cipher);
        var serverDecryption = Cipher.getInstance(cipher);

        serverEncryption.init(Cipher.ENCRYPT_MODE, keys.encryption, ivSpec);
        serverDecryption.init(Cipher.DECRYPT_MODE, keys.encryption, ivSpec);

        return new Cipher[]{serverEncryption, serverDecryption};
}

private static Keys keygen(int keyLength, char[] password, byte[] salt) {
        SecretKeyFactory factory;
        try {
            factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");
        } catch (NoSuchAlgorithmException impossible) {
            return null;
        }
        // derive a longer key, then split into AES key and authentication key
        KeySpec spec = new PBEKeySpec(password, salt, 32768, keyLength + 8 * 8);
        SecretKey tmp = null;
        try {
            tmp = factory.generateSecret(spec);
        } catch (InvalidKeySpecException ignored) {
        }
        byte[] fullKey = tmp.getEncoded();
        SecretKey authKey = new SecretKeySpec( // key for password authentication
                Arrays.copyOfRange(fullKey, 0, 8), "AES");
        SecretKey encKey = new SecretKeySpec( // key for AES encryption
                Arrays.copyOfRange(fullKey, 8, fullKey.length), "AES");
        return new Keys(encKey, authKey);
   }

And here is the error I am receiving, basically I receive the opcodes that never were sent as the only opcode that has been sent from the server to the client is opcode 1:

[[SocketClient-10] INFO org.test.TestApp - Opcode= 121
[SocketClient-10] INFO org.test.TestApp - Unexpected value: 121
[SocketClient-12] INFO org.test.TestApp - Opcode= 62
[SocketClient-12] INFO org.test.TestApp - Unexpected value: 62
[SocketClient-10] INFO org.test.TestApp - Opcode= 47
[SocketClient-10] INFO org.test.TestApp - Unexpected value: 47
[SocketClient-12] INFO org.test.TestApp - Opcode= -57
[SocketClient-12] INFO org.test.TestApp - Unexpected value: -57
[SocketClient-10] INFO org.test.TestApp - Opcode= 52
[SocketClient-10] INFO org.test.TestApp - Unexpected value: 52
[SocketClient-12] INFO org.test.TestApp - Opcode= -42
[SocketClient-12] INFO org.test.TestApp - Unexpected value: -42
[SocketClient-10] INFO org.test.TestApp - Opcode= -22
[SocketClient-10] INFO org.test.TestApp - Unexpected value: -22
[SocketClient-12] INFO org.test.TestApp - Opcode= 37
[SocketClient-12] INFO org.test.TestApp - Unexpected value: 37
[SocketClient-10] INFO org.test.TestApp - Opcode= -47
[SocketClient-10] INFO org.test.TestApp - Unexpected value: -47
[SocketClient-12] INFO org.test.TestApp - Opcode= 65
[SocketClient-12] INFO org.test.TestApp - Unexpected value: 65
[SocketClient-10] INFO org.test.TestApp - Opcode= 34
[SocketClient-10] INFO org.test.TestApp - Unexpected value: 34
[SocketClient-12] INFO org.test.TestApp - Opcode= -53
[SocketClient-12] INFO org.test.TestApp - Unexpected value: -53
[SocketClient-10] INFO org.test.TestApp - Opcode= 90
[SocketClient-10] INFO org.test.TestApp - Unexpected value: 90
[SocketClient-12] INFO org.test.TestApp - Opcode= 123
[SocketClient-12] INFO org.test.TestApp - Unexpected value: 123
[SocketClient-10] INFO org.test.TestApp - Opcode= 52
[SocketClient-10] INFO org.test.TestApp - Unexpected value: 52
[SocketClient-12] INFO org.test.TestApp - Opcode= -65
[SocketClient-12] INFO org.test.TestApp - Unexpected value: -65
[SocketClient-10] INFO org.test.TestApp - Opcode= 116
[SocketClient-10] INFO org.test.TestApp - Unexpected value: 116
[SocketClient-12] INFO org.test.TestApp - Opcode= -10
[SocketClient-12] INFO org.test.TestApp - Unexpected value: -10
[SocketClient-10] INFO org.test.TestApp - Opcode= 29
[SocketClient-10] INFO org.test.TestApp - Unexpected value: 29
[SocketClient-12] INFO org.test.TestApp - Opcode= 42
[SocketClient-12] INFO org.test.TestApp - Unexpected value: 42
[SocketClient-10] INFO org.test.TestApp - Opcode= 4
java.lang.IllegalArgumentException: newLimit < 0: (-1717140139 < 0)
    at java.base/java.nio.Buffer.createLimitException(Buffer.java:391)
    at java.base/java.nio.Buffer.limit(Buffer.java:365)
    at java.base/java.nio.ByteBuffer.limit(ByteBuffer.java:1382)
    at java.base/java.nio.MappedByteBuffer.limit(MappedByteBuffer.java:304)
    at java.base/java.nio.MappedByteBuffer.limit(MappedByteBuffer.java:70)
    at net.simplenet.Client$Listener.completed(Client.java:123)
    at net.simplenet.Client$Listener.completed(Client.java:82)
    at java.base/sun.nio.ch.Invoker.invokeUnchecked(Invoker.java:127)
    at java.base/sun.nio.ch.Invoker$2.run(Invoker.java:219)
    at java.base/sun.nio.ch.AsynchronousChannelGroupImpl$1.run(AsynchronousChannelGroupImpl.java:112)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
    at java.base/java.lang.Thread.run(Thread.java:832)

jhg023 commented 4 years ago

@bytecod3r In your example above, you have server.bind(serverIP, Integer.parseInt(serverPort)); within the Server#onConnect callback when it should be placed outside of it.

Can you post the actual packets that you're sending back and forth that reproduce this problem, as well as where you disconnect clients every 0.5 seconds?

jhg023 commented 3 years ago

I'll be happy to reopen this issue if you can get back to me with the data requested in my previous response 😃

jhg023 / SimpleNet

Error in .limit() function #20