TooTallNate / Java-WebSocket

A barebones WebSocket client and server implementation written in 100% Java.
http://tootallnate.github.io/Java-WebSocket
MIT License
10.53k stars 2.58k forks source link

[Bug?] Failed to receive ping packet if I override broadcast(String) function #933

Closed PeterBethlyFord closed 5 years ago

PeterBethlyFord commented 5 years ago

Describe the bug This is what happened:

I have a VPS and I would like to implement a WS server on it. This server will be used with Apache HTTP server.

I tried to force the messages between client and server encrypted, so I implemented the server as follow. As you can see, this is a very typical WS server implementation. However, it failed to receive & response to "ping" packet (and disconnect automatically of course). According to my test, if I comment-out the broadcast(String) function (which was overridden), everything works well. Now, I am wondering whether it is a bug or not.

I have confirmed this issue with HTTP capturing tool like Fiddler, so it can not likely be a fake phenomenon I think.

The server code looks like this:

package com.wangbo.test;

import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.security.GeneralSecurityException;
import java.util.Locale;

import javax.crypto.AEADBadTagException;

import org.java_websocket.WebSocket;
import org.java_websocket.framing.Framedata;
import org.java_websocket.handshake.ClientHandshake;
import org.java_websocket.server.WebSocketServer;

import com.wangbo.RockeryKeeper.component.AESGCMCrypto;

public class TestWebSocketServer
{
    private static final Charset    CHARSET        = StandardCharsets.UTF_8;
    private static final String        KEY            = "ChangeItChangeIt";

    public static void main(String[] args)
    {
        WebSocketServer server = new WebSocketServer(new InetSocketAddress(2728))
        {
            private final AESGCMCrypto mCrypto = new AESGCMCrypto(KEY.getBytes(CHARSET));

            @Override
            public void onStart()
            {
                System.out.println("WebSocket server start successfully");
            }

            @Override
            public void onOpen(WebSocket connection, ClientHandshake handshake)
            {
                System.out.println("New connection" + (null == handshake.getContent() ? "" : ", param is " + new String(handshake.getContent(), StandardCharsets.UTF_8)));
                System.out.println(String.format(Locale.CHINA, "User %d logon,current online number is %d", connection.getRemoteSocketAddress().getPort(), getConnections().size()));
                broadcast(String.format(Locale.CHINA, "User %d logon,current online number is %d", connection.getRemoteSocketAddress().getPort(), getConnections().size()));
            }

            @Override
            public void onMessage(WebSocket connection, ByteBuffer message)
            {
                // Decrypt
                String strMessage = null;
                try
                {
                    byte[] arrCipherText = new byte[message.remaining()];
                    message.get(arrCipherText);
                    strMessage = new String(mCrypto.decrypt(arrCipherText), CHARSET).intern();
                }
                catch (AEADBadTagException e)
                {
                    System.err.println("Malformed packet (We might under MITM attack)");
                }
                catch (GeneralSecurityException e)
                {
                    System.out.println("Malformed packet");
                }

                if (null == strMessage)
                    return;
                System.out.println("Recv message:" + strMessage);

                // Re-encrypt and broadcast
                broadcast(strMessage);
            }

            @Override
            public void onMessage(WebSocket connection, String strMessage)
            {
                System.out.println("Recv plaintext message:" + strMessage);
            }

            @Override
            public void onError(WebSocket connection, Exception e)
            {
                System.out.println("Error occured");
                e.printStackTrace();
            }

            @Override
            public void onWebsocketPing(WebSocket connection, Framedata frame)
            {
                // XXX debug code
                System.out.println("\tGot a ping packet");
                super.onWebsocketPing(connection, frame);
            }

            @Override
            public void onClose(WebSocket connection, int nCode, String strReason, boolean bRemote)
            {
                System.out.println("Disconnected" + (null == strReason || strReason.trim().isEmpty() ? "" : ", reason is " + strReason.trim()));
                InetSocketAddress address = connection.getRemoteSocketAddress();
                System.out.println(String.format(Locale.CHINA, "User %d logout,current online number is %d", (null == address) ? -1 : address.getPort(), getConnections().size()));
            }

            //////////////////////////////////////////////////////////////////////
            // NOTICE : If I comment-out this "broadcast" function, everything works well !
            //////////////////////////////////////////////////////////////////////
            @Override
            public void broadcast(String strMessage)
            {
                try
                {
                    super.broadcast(mCrypto.encrypt(strMessage.getBytes(CHARSET)));
                }
                catch (GeneralSecurityException e)
                {
                    System.out.println("Encrypt failed");
                }
            }
        };
        server.setConnectionLostTimeout(10);
        server.setTcpNoDelay(true);
        server.setReuseAddr(true);

        System.out.println("Start listening [" + server.getAddress().toString() + "] ...");
        server.start();
    }
}

Environment(please complete the following information):

Additional context

  1. It works well if I override send(String) and onMessage(ByteBuffer) functions on WS client implementation, so it might be a server-only issue.
  2. It cannot be reproduced if you connect to yourself (127.0.0.1), it only happens if you connect to a remote server. (In my case, I connected to a Singapore server from China).
marci4 commented 5 years ago

Please provide a full application example, I was not able to find any documentation on com.wangbo.RockeryKeeper.component.AESGCMCrypto ...

PeterBethlyFord commented 5 years ago

Err sorry... it was implemented by myself, and it is just a typical AES-128-GCM util. (And for some reason, my full application cannot be open-sourced.)

Luckily, you just need these 2 files to reproduce the bug.

package com.wangbo.RockeryKeeper.component;

import java.security.GeneralSecurityException;
import java.security.SecureRandom;

import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.spec.GCMParameterSpec;
import javax.crypto.spec.SecretKeySpec;

public class AESGCMCrypto
{
    // Constant
    private static final int        TAG_LENGTH_BIT        = 128;
    private static final int        IV_LENGTH_BYTE        = 12;

    // Input parameter
    private final SecretKey mKey;

    public AESGCMCrypto(byte[] arrKey)
    {
        this.mKey = new SecretKeySpec(arrKey, "AES");
    }

    public final byte[] encrypt(byte[] arrPlainText) throws GeneralSecurityException
    {
        return encrypt(arrPlainText, null);
    }

    public final byte[] encrypt(byte[] arrPlainText, byte[] arrAAD) throws GeneralSecurityException
    {
        // Generate random IV
        SecureRandom random = SecureRandom.getInstanceStrong();
        byte[] arrIV = new byte[IV_LENGTH_BYTE];
        random.nextBytes(arrIV);

        // Init encryptor
        GCMParameterSpec parameter = new GCMParameterSpec(TAG_LENGTH_BIT, arrIV);
        Cipher encryptor = Cipher.getInstance("AES/GCM/NoPadding");
        encryptor.init(Cipher.ENCRYPT_MODE, mKey, parameter);
        if (null != arrAAD)
            encryptor.updateAAD(arrAAD);

        // Encrypt
        int nPlainTextLength = arrPlainText.length;
        byte[] arrCipherText = new byte[IV_LENGTH_BYTE + nPlainTextLength + (TAG_LENGTH_BIT / 8)];
        System.arraycopy(arrIV, 0, arrCipherText, 0, IV_LENGTH_BYTE);
        encryptor.doFinal(arrPlainText, 0, nPlainTextLength, arrCipherText, IV_LENGTH_BYTE);

        return arrCipherText;
    }

    public final byte[] decrypt(byte[] arrCipherText) throws GeneralSecurityException
    {
        return decrypt(arrCipherText, null);
    }

    public final byte[] decrypt(byte[] arrCipherText, byte[] arrAAD) throws GeneralSecurityException
    {
        // Read IV
        byte[] arrIV = new byte[IV_LENGTH_BYTE];
        System.arraycopy(arrCipherText, 0, arrIV, 0, IV_LENGTH_BYTE);

        // Init decryptor
        GCMParameterSpec parameter = new GCMParameterSpec(TAG_LENGTH_BIT, arrIV);
        Cipher decryptor = Cipher.getInstance("AES/GCM/NoPadding");
        decryptor.init(Cipher.DECRYPT_MODE, mKey, parameter);
        if (null != arrAAD)
            decryptor.updateAAD(arrIV);

        // Decrypt
        int nRawDataLength = arrCipherText.length - IV_LENGTH_BYTE;
        byte[] arrPlainText = new byte[nRawDataLength - (TAG_LENGTH_BIT / 8)];
        decryptor.doFinal(arrCipherText, IV_LENGTH_BYTE, nRawDataLength, arrPlainText);

        return arrPlainText;
    }
}
PeterBethlyFord commented 5 years ago

Now I tested my code with the latest Java-websocket 1.4.0, still the same result.

Before this, I have tested my code with Java-websocket 1.3.9 + OpenJDK-11, the same result.

PeterBethlyFord commented 5 years ago

I think I'd better post client implementation as well (to make the test easier).

package com.wangbo.RockeryKeeper;

import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.ByteBuffer;
import java.nio.channels.NotYetConnectedException;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.security.GeneralSecurityException;

import javax.crypto.AEADBadTagException;

import org.java_websocket.client.WebSocketClient;
import org.java_websocket.handshake.ServerHandshake;

import com.wangbo.RockeryKeeper.component.AESGCMCrypto;

public class Test
{
    private static final Charset CHARSET = StandardCharsets.UTF_8;
    private static final byte[] KEY = "ChangeItChangeIt".getBytes(CHARSET);

    public static void main(String[] args) throws IOException, URISyntaxException, InterruptedException, GeneralSecurityException
    {
        WebSocketClient client = new WebSocketClient(new URI("ws://url.to.server:2728"))
        {
            private final AESGCMCrypto mCrypto = new AESGCMCrypto(KEY);

            @Override
            public void onOpen(ServerHandshake handshake)
            {
                System.out.println("WebSocket connect succeed" + (null == handshake.getContent() ? "" : ", param is " + new String(handshake.getContent(), StandardCharsets.UTF_8)));
                send("I am " + getSocket().getLocalPort() + ", I have just logon");
            }

            @Override
            public void send(String strMessage) throws NotYetConnectedException
            {
                System.out.println("Send message:" + strMessage);
                try
                {
                    super.send(mCrypto.encrypt(strMessage.getBytes(CHARSET)));
                }
                catch (GeneralSecurityException e)
                {
                    System.out.println("Failed to encrypt");
                }
            }

            @Override
            public void onMessage(String strMessage)
            {
                System.out.println("Recv plaintext message:" + strMessage);
            }

            @Override
            public void onMessage(ByteBuffer message)
            {
                // Decrypt
                String strMessage = null;
                try
                {
                    byte[] arrCipherText = new byte[message.remaining()];
                    message.get(arrCipherText);
                    strMessage = new String(mCrypto.decrypt(arrCipherText), CHARSET).intern();
                }
                catch (AEADBadTagException e)
                {
                    System.err.println("Malformed packet (We might under MITM attack)");
                }
                catch (GeneralSecurityException e)
                {
                    System.out.println("Malformed packet");
                }

                if (null == strMessage)
                    return;
                System.out.println("Recv message:" + strMessage);
            }

            @Override
            public void onClose(int nCode, String strReason, boolean bRemote)
            {
                System.out.println("Disconnected" + (null == strReason || strReason.trim().isEmpty() ? "" : ", reason is " + strReason.trim()));
            }

            @Override
            public void onError(Exception e)
            {
                System.out.println("Error occured");
                e.printStackTrace();
            }
        };

        // Open Connection
        System.out.println("Start connecting [" + client.getURI().toString() + "] ...");
        client.setConnectionLostTimeout(10);
        client.setTcpNoDelay(true);
        client.setReuseAddr(true);
        client.connectBlocking();
    }
}
PeterBethlyFord commented 5 years ago

Sorry, it seems ... not your fault. I solved it by myself.

The progress of AES encryption got stuck on random IV generation, that is why the whole WebSocket data-outbound thread was blocked. By installing some extra randomness/entropy collector, the code never blocks anymore.