Emqx: authentication issue

[khodadadi]

Environment

• OS: UBUNTU 18.04
• Erlang/OTP: 21
• EMQ: 3.1.1

Description

We use emqttd 2.3 and try to upgrade to emqx 3.1.1
In emqx, when auth module return error (eg., bad_username_or_password) client socket closed before CONNACK packet received. This behaviour cause ddos attack by client.
Is this a bug or did I miss something?
client: mqttjs
client mqtt protocol: MQTT 3.1.1
emqx.conf
allow_anonymous = false

[HJianBo]

Hi, @khodadadi Thanks for your feedback, but I can not understand this

Why does this behavior cause a DDOS attack?

I think the client process of EMQ X will shut down and release all resources of this after the client closed the connection. All resources of a client will be released.

[khodadadi]

Hi @HJianBo
1- Client tcp connection opend
2- Client send CONNECT packet with not valid auth data
3- Server send CONNACK pack with reason_code=4 (bad_username_and_password). then Tcp connection closed by server before CONNACK packet received in client and client cannot determine why connection closed, and then retry, retry and retry.
there is no problem in emq 2.3 and connection closed after CONNACK packet received in client.

[khodadadi]

hi @Gilbert-Wong
this bug only occurs in websocket mode

[tradingtrace]

I'm a newcomer to EMQX, just starting to read the code. Please correct me if I have any mistakes.

I guess that when the WebSocket is closed, the CONNACK message is still left in the WS connection process mailbox.

When CONNECT command failed, CONNACK message with the error code will be delivered here https://github.com/emqx/emqx/blob/5025b2f65d1c1d53ff2e0a50d6a16221ce2124d3/src/emqx_protocol.erl#L554-L558
then return {error, Error, ProtoState1}.

And the deviler function will call the send_fun function which is defined in emqx_ws_connection.erl if we use WebSocket mode.
https://github.com/emqx/emqx/blob/0f052ce352597ef09453d66a333563a481c64193/src/emqx_ws_connection.erl#L121-L128
And it sends a message to itself WsPid ! {binary, iolist_to_binary(Data)}.

Let's go back to the return value {error, Error, ProtoState1}, it will be returned to emqx_ws_connection.erl line 155
https://github.com/emqx/emqx/blob/0f052ce352597ef09453d66a333563a481c64193/src/emqx_ws_connection.erl#L149-L159
then call the function shutdown to terminate the current process without processing the remaining messages in its mailbox.

So I think that the line 155 or other places like this could be changed to something like WsPid ! {shutdown, Reason}.

PS: In TCP mode, it will call erlang:port_command/3 to send CONNACK to the client directly, not send a message to itself, so it doesn't have this issue.

[gilbertwong96]

Hi, @tradingtrace, Look here:
https://github.com/emqx/emqx/blob/0f052ce352597ef09453d66a333563a481c64193/src/emqx_ws_connection.erl#L121-L128

The CONNACK packet would be sent to websocket in send_fun.

[tradingtrace]

@Gilbert-Wong Yes, you are right. CONNACK packet would be sent to the process WsPid at line 127.
But the process WsPid is a cowboy_websocket process which has a big loop to receive messages.

And here:
https://github.com/emqx/emqx/blob/5025b2f65d1c1d53ff2e0a50d6a16221ce2124d3/src/emqx_protocol.erl#L554-L558

The return value of this function will cause the process WsPid termination directly. There is no change to run loop again to receive the remain messages in its mailbox.

And I made some test, here is the code and the log in debug mode:

%% cowboy_websocket.erl
-spec terminate(#state{}, any(), terminate_reason()) -> no_return().
terminate(State, HandlerState, Reason) ->
    handler_terminate(State, HandlerState, Reason),

    emqx_logger:debug("---> Process ~p terminating, process_info: ~w~n", [self(),
        erlang:process_info(self(), [current_function, message_queue_len])]),
    receive
        Message ->
            emqx_logger:debug("---> The left message: ~p~n", [Message]),
            websocket_send(Message, State)
    after 0 ->
        ignore
    end,

    exit(normal).

%% log
2019-06-10 11:06:45.743 [debug] 127.0.0.1:39138 [Protocol] RECV CONNECT(Q0, R0, D0, ClientId=clientId-OKRsaldJvW, ProtoName=MQIsdp, ProtoVsn=3, CleanStart=true, KeepAlive=10, Username=user1, Password=******)
2019-06-10 11:06:45.744 [warning] [email protected]:39138 [Protocol] Client clientId-OKRsaldJvW (Username: 'user1') login failed for password_error
2019-06-10 11:06:45.745 [debug] clientId-OKRsaldJvW@:39138 ---> WsPid(<0.31663.0>) ! {binary,<<32,2,0,4>>}
2019-06-10 11:06:45.745 [debug] [email protected]:39138 [Protocol] SEND CONNACK(Q0, R0, D0, AckFlags=0, ReasonCode=4)
2019-06-10 11:06:45.746 [debug] [email protected]:39138 [WS Connection] Terminated for malformed_username_or_password, sockerror: stop
2019-06-10 11:06:45.746 [debug] [email protected]:39138 ---> Process <0.31663.0> terminating, process_info: [{current_function,{cowboy_websocket,terminate,3}},{message_queue_len,1}]
2019-06-10 11:06:45.747 [debug] [email protected]:39138 ---> The left message: {binary,<<32,2,0,4>>}

[gilbertwong96]

I suppose this is the bug of our WebSocket implementation, thanks for your feedback, I'll resolve it as quickly as possible.

[gilbertwong96]

It has been fixed, here is the pull request. https://github.com/emqx/emqx/pull/2615

[gilbertwong96]

@tradingtrace

[HJianBo]

Hi, guys! We have released it at v3.2.beta-3, please check it.

Thanks for attention there!