Skip to content

Commit 620fff2

Browse files
lukebakkenlukebakken
committed
Log errors from ranch:handshake
Fixes #11171 An MQTT user encountered TLS handshake timeouts with their IoT device, and the actual error from `ssl:handshake` / `ranch:handshake` was not caught and logged. At this time, `ranch` uses `exit(normal)` in the case of timeouts, but that should change in the future (ninenines/ranch#336)
1 parent 746c06f commit 620fff2

File tree

5 files changed

+104
-80
lines changed

5 files changed

+104
-80
lines changed

‎deps/rabbit/src/rabbit_networking.erl

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ failed_to_recv_proxy_header(Ref, Error) ->
560560
end,
561561
rabbit_log:debug(Msg, [Error]),
562562
% The following call will clean up resources then exit
563-
_ = ranch:handshake(Ref),
563+
_ = catch ranch:handshake(Ref),
564564
exit({shutdown, failed_to_recv_proxy_header}).
565565

566566
handshake(Ref, ProxyProtocolEnabled) ->
@@ -572,14 +572,22 @@ handshake(Ref, ProxyProtocolEnabled) ->
572572
{error, protocol_error, Error} ->
573573
failed_to_recv_proxy_header(Ref, Error);
574574
{ok, ProxyInfo} ->
575-
{ok, Sock} = ranch:handshake(Ref),
576-
setup_socket(Sock),
577-
{ok, {rabbit_proxy_socket, Sock, ProxyInfo}}
575+
case catch ranch:handshake(Ref) of
576+
{'EXIT', normal} ->
577+
{error, handshake_failed};
578+
{ok, Sock} ->
579+
setup_socket(Sock),
580+
{ok, {rabbit_proxy_socket, Sock, ProxyInfo}}
581+
end
578582
end;
579583
false ->
580-
{ok, Sock} = ranch:handshake(Ref),
581-
setup_socket(Sock),
582-
{ok, Sock}
584+
case catch ranch:handshake(Ref) of
585+
{'EXIT', normal} ->
586+
{error, handshake_failed};
587+
{ok, Sock} ->
588+
setup_socket(Sock),
589+
{ok, Sock}
590+
end
583591
end.
584592

585593
setup_socket(Sock) ->

‎deps/rabbit/src/rabbit_reader.erl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,10 @@ shutdown(Pid, Explanation) ->
162162
no_return().
163163
init(Parent, HelperSups, Ref) ->
164164
?LG_PROCESS_TYPE(reader),
165+
%% Note:
166+
%% This function could return an error if the handshake times out.
167+
%% It is less likely to happen here as compared to MQTT, so
168+
%% crashing with a `badmatch` seems appropriate.
165169
{ok, Sock} = rabbit_networking:handshake(Ref,
166170
application:get_env(rabbit, proxy_protocol, false)),
167171
Deb = sys:debug_options([]),

‎deps/rabbitmq_mqtt/src/rabbit_mqtt_reader.erl

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -71,34 +71,39 @@ close_connection(Pid, Reason) ->
7171
init(Ref) ->
7272
process_flag(trap_exit, true),
7373
logger:set_process_metadata(#{domain => ?RMQLOG_DOMAIN_CONN ++ [mqtt]}),
74-
{ok, Sock} = rabbit_networking:handshake(Ref,
75-
application:get_env(?APP_NAME, proxy_protocol, false)),
76-
RealSocket = rabbit_net:unwrap_socket(Sock),
77-
case rabbit_net:connection_string(Sock, inbound) of
78-
{ok, ConnStr} ->
79-
ConnName = rabbit_data_coercion:to_binary(ConnStr),
80-
?LOG_DEBUG("MQTT accepting TCP connection ~tp (~ts)", [self(), ConnName]),
81-
_ = rabbit_alarm:register(self(), {?MODULE, conserve_resources, []}),
82-
LoginTimeout = application:get_env(?APP_NAME, login_timeout, 10_000),
83-
erlang:send_after(LoginTimeout, self(), login_timeout),
84-
State0 = #state{socket = RealSocket,
85-
proxy_socket = rabbit_net:maybe_get_proxy_socket(Sock),
86-
conn_name = ConnName,
87-
await_recv = false,
88-
connection_state = running,
89-
conserve = false,
90-
parse_state = rabbit_mqtt_packet:init_state()},
91-
State1 = control_throttle(State0),
92-
State = rabbit_event:init_stats_timer(State1, #state.stats_timer),
93-
gen_server:enter_loop(?MODULE, [], State);
94-
{error, Reason = enotconn} ->
95-
?LOG_INFO("MQTT could not get connection string: ~s", [Reason]),
96-
rabbit_net:fast_close(RealSocket),
97-
ignore;
74+
ProxyProtocolEnabled = application:get_env(?APP_NAME, proxy_protocol, false),
75+
case rabbit_networking:handshake(Ref, ProxyProtocolEnabled) of
9876
{error, Reason} ->
99-
?LOG_ERROR("MQTT could not get connection string: ~p", [Reason]),
100-
rabbit_net:fast_close(RealSocket),
101-
{stop, Reason}
77+
?LOG_ERROR("MQTT could not establish connection: ~s", [Reason]),
78+
{stop, Reason};
79+
{ok, Sock} ->
80+
RealSocket = rabbit_net:unwrap_socket(Sock),
81+
case rabbit_net:connection_string(Sock, inbound) of
82+
{ok, ConnStr} ->
83+
ConnName = rabbit_data_coercion:to_binary(ConnStr),
84+
?LOG_DEBUG("MQTT accepting TCP connection ~tp (~ts)", [self(), ConnName]),
85+
_ = rabbit_alarm:register(self(), {?MODULE, conserve_resources, []}),
86+
LoginTimeout = application:get_env(?APP_NAME, login_timeout, 10_000),
87+
erlang:send_after(LoginTimeout, self(), login_timeout),
88+
State0 = #state{socket = RealSocket,
89+
proxy_socket = rabbit_net:maybe_get_proxy_socket(Sock),
90+
conn_name = ConnName,
91+
await_recv = false,
92+
connection_state = running,
93+
conserve = false,
94+
parse_state = rabbit_mqtt_packet:init_state()},
95+
State1 = control_throttle(State0),
96+
State = rabbit_event:init_stats_timer(State1, #state.stats_timer),
97+
gen_server:enter_loop(?MODULE, [], State);
98+
{error, Reason = enotconn} ->
99+
?LOG_INFO("MQTT could not get connection string: ~s", [Reason]),
100+
rabbit_net:fast_close(RealSocket),
101+
ignore;
102+
{error, Reason} ->
103+
?LOG_ERROR("MQTT could not get connection string: ~p", [Reason]),
104+
rabbit_net:fast_close(RealSocket),
105+
{stop, Reason}
106+
end
102107
end.
103108

104109
handle_call({info, InfoItems}, _From, State) ->

‎deps/rabbitmq_stomp/src/rabbit_stomp_reader.erl

Lines changed: 46 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -63,51 +63,55 @@ close_connection(Pid, Reason) ->
6363

6464
init([SupHelperPid, Ref, Configuration]) ->
6565
process_flag(trap_exit, true),
66-
{ok, Sock} = rabbit_networking:handshake(Ref,
67-
application:get_env(rabbitmq_stomp, proxy_protocol, false)),
68-
RealSocket = rabbit_net:unwrap_socket(Sock),
69-
70-
case rabbit_net:connection_string(Sock, inbound) of
71-
{ok, ConnStr} ->
72-
ConnName = rabbit_data_coercion:to_binary(ConnStr),
73-
ProcInitArgs = processor_args(Configuration, Sock),
74-
ProcState = rabbit_stomp_processor:initial_state(Configuration,
75-
ProcInitArgs),
76-
77-
rabbit_log_connection:info("accepting STOMP connection ~tp (~ts)",
78-
[self(), ConnName]),
79-
80-
ParseState = rabbit_stomp_frame:initial_state(),
81-
_ = register_resource_alarm(),
82-
83-
LoginTimeout = application:get_env(rabbitmq_stomp, login_timeout, 10_000),
84-
MaxFrameSize = application:get_env(rabbitmq_stomp, max_frame_size, ?DEFAULT_MAX_FRAME_SIZE),
85-
erlang:send_after(LoginTimeout, self(), login_timeout),
86-
87-
gen_server2:enter_loop(?MODULE, [],
88-
rabbit_event:init_stats_timer(
89-
run_socket(control_throttle(
90-
#reader_state{socket = RealSocket,
91-
conn_name = ConnName,
92-
parse_state = ParseState,
93-
processor_state = ProcState,
94-
heartbeat_sup = SupHelperPid,
95-
heartbeat = {none, none},
96-
max_frame_size = MaxFrameSize,
97-
current_frame_size = 0,
98-
state = running,
99-
conserve_resources = false,
100-
recv_outstanding = false})), #reader_state.stats_timer),
101-
{backoff, 1000, 1000, 10000});
102-
{error, enotconn} ->
103-
rabbit_net:fast_close(RealSocket),
104-
terminate(shutdown, undefined);
66+
ProxyProtocolEnabled = application:get_env(rabbitmq_stomp, proxy_protocol, false),
67+
case rabbit_networking:handshake(Ref, ProxyProtocolEnabled) of
10568
{error, Reason} ->
106-
rabbit_net:fast_close(RealSocket),
107-
terminate({network_error, Reason}, undefined)
69+
rabbit_log_connection:error(
70+
"STOMP could not establish connection: ~s", [Reason]),
71+
{stop, Reason};
72+
{ok, Sock} ->
73+
RealSocket = rabbit_net:unwrap_socket(Sock),
74+
case rabbit_net:connection_string(Sock, inbound) of
75+
{ok, ConnStr} ->
76+
ConnName = rabbit_data_coercion:to_binary(ConnStr),
77+
ProcInitArgs = processor_args(Configuration, Sock),
78+
ProcState = rabbit_stomp_processor:initial_state(Configuration,
79+
ProcInitArgs),
80+
81+
rabbit_log_connection:info("accepting STOMP connection ~tp (~ts)",
82+
[self(), ConnName]),
83+
84+
ParseState = rabbit_stomp_frame:initial_state(),
85+
_ = register_resource_alarm(),
86+
87+
LoginTimeout = application:get_env(rabbitmq_stomp, login_timeout, 10_000),
88+
MaxFrameSize = application:get_env(rabbitmq_stomp, max_frame_size, ?DEFAULT_MAX_FRAME_SIZE),
89+
erlang:send_after(LoginTimeout, self(), login_timeout),
90+
91+
gen_server2:enter_loop(?MODULE, [],
92+
rabbit_event:init_stats_timer(
93+
run_socket(control_throttle(
94+
#reader_state{socket = RealSocket,
95+
conn_name = ConnName,
96+
parse_state = ParseState,
97+
processor_state = ProcState,
98+
heartbeat_sup = SupHelperPid,
99+
heartbeat = {none, none},
100+
max_frame_size = MaxFrameSize,
101+
current_frame_size = 0,
102+
state = running,
103+
conserve_resources = false,
104+
recv_outstanding = false})), #reader_state.stats_timer),
105+
{backoff, 1000, 1000, 10000});
106+
{error, enotconn} ->
107+
rabbit_net:fast_close(RealSocket),
108+
terminate(shutdown, undefined);
109+
{error, Reason} ->
110+
rabbit_net:fast_close(RealSocket),
111+
terminate({network_error, Reason}, undefined)
112+
end
108113
end.
109114

110-
111115
handle_call({info, InfoItems}, _From, State) ->
112116
Infos = lists:map(
113117
fun(InfoItem) ->

‎deps/rabbitmq_stream/src/rabbit_stream_reader.erl

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,13 @@ init([KeepaliveSup,
135135
heartbeat := Heartbeat,
136136
transport := ConnTransport}]) ->
137137
process_flag(trap_exit, true),
138-
{ok, Sock} =
139-
rabbit_networking:handshake(Ref,
140-
application:get_env(rabbitmq_stream,
141-
proxy_protocol, false)),
138+
ProxyProtocolEnabled =
139+
application:get_env(rabbitmq_stream, proxy_protocol, false),
140+
%% Note:
141+
%% This function could return an error if the handshake times out.
142+
%% It is less likely to happen here as compared to MQTT, so
143+
%% crashing with a `badmatch` seems appropriate.
144+
{ok, Sock} = rabbit_networking:handshake(Ref, ProxyProtocolEnabled),
142145
RealSocket = rabbit_net:unwrap_socket(Sock),
143146
case rabbit_net:connection_string(Sock, inbound) of
144147
{ok, ConnStr} ->

0 commit comments

Comments
 (0)