From 5a9a2fe727ba43bd6352552d42db85255b4fd4ff Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Thu, 28 May 2020 11:01:00 +0200 Subject: ssl: Avoid possible sending an active message to a passive socket In the corner case that a passive socket is started and ssl:recv never is called and the other side closes the socket, an active close message could incorrectly be sent to a passive socket. --- lib/ssl/src/ssl_connection.erl | 8 ++++---- lib/ssl/test/ssl_api_SUITE.erl | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index 8e2e794280..e1a0ce9079 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -2981,11 +2981,11 @@ header(N, Binary) -> send_or_reply(false, _Pid, From, Data) when From =/= undefined -> gen_statem:reply(From, Data); -%% Can happen when handling own alert or tcp error/close and there is -%% no outstanding gen_fsm sync events -send_or_reply(false, no_pid, _, _) -> +send_or_reply(false, Pid, undefined, _) when is_pid(Pid) -> ok; -send_or_reply(_, Pid, _From, Data) -> +send_or_reply(_, no_pid, _, _) -> + ok; +send_or_reply(_, Pid, _, Data) -> send_user(Pid, Data). send_user(Pid, Msg) -> diff --git a/lib/ssl/test/ssl_api_SUITE.erl b/lib/ssl/test/ssl_api_SUITE.erl index b393ba2f9b..29d1fe6a0d 100644 --- a/lib/ssl/test/ssl_api_SUITE.erl +++ b/lib/ssl/test/ssl_api_SUITE.erl @@ -87,6 +87,7 @@ gen_api_tests() -> recv_active, recv_active_once, recv_active_n, + recv_no_active_msg, recv_timeout, recv_close, controlling_process, @@ -824,6 +825,10 @@ listen_socket(Config) -> ok = ssl:close(ListenSocket). %%-------------------------------------------------------------------- + + +%%-------------------------------------------------------------------- + recv_active() -> [{doc,"Test recv on active socket"}]. @@ -951,6 +956,27 @@ recv_close(Config) when is_list(Config) -> ssl_test_lib:check_result(Server, ok). +%%-------------------------------------------------------------------- +recv_no_active_msg() -> + [{doc,"If we have a passive socket and do not call recv and peer closes we should no get" + "receive an active message"}]. +recv_no_active_msg(Config) when is_list(Config) -> + ClientOpts = ssl_test_lib:ssl_options(client_rsa_opts, Config), + ServerOpts = ssl_test_lib:ssl_options(server_rsa_opts, Config), + + {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), + Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, + {from, self()}, + {mfa, {?MODULE, no_recv_no_active, []}}, + {options, [{active, false} | ServerOpts]}]), + Port = ssl_test_lib:inet_port(Server), + Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port}, + {host, Hostname}, + {from, self()}, + {mfa, {ssl_test_lib, no_result, []}}, + {options, ClientOpts}]), + ssl_test_lib:close(Client), + ssl_test_lib:check_result(Server, ok). %%-------------------------------------------------------------------- controlling_process() -> @@ -2142,6 +2168,14 @@ run_error_server([ Pid | Opts]) -> run_client_error([Port, Opts]) -> ssl:connect("localhost", Port, Opts). +no_recv_no_active(Socket) -> + receive + {ssl_closed, Socket} -> + ct:fail(received_active_msg) + after 5000 -> + ok + end. + honor_cipher_order(Config, Honor, ServerCiphers, ClientCiphers, Expected) -> ClientOpts = ssl_test_lib:ssl_options(client_rsa_opts, Config), ServerOpts = ssl_test_lib:ssl_options(server_rsa_opts, Config), -- cgit v1.2.1 From 2e2e6684b797937d03b6dedd2930be0095000e22 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Thu, 11 Jun 2020 08:28:46 +0200 Subject: ftp: Handle that inet/ssl:setopts can return error --- lib/ftp/src/ftp.erl | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/ftp/src/ftp.erl b/lib/ftp/src/ftp.erl index 36b57837fc..dac316fe0b 100644 --- a/lib/ftp/src/ftp.erl +++ b/lib/ftp/src/ftp.erl @@ -2267,12 +2267,21 @@ activate_data_connection(#state{dsock = DSock} = State) -> State. activate_connection(Socket) -> - ignore_return_value( - case socket_type(Socket) of - tcp -> inet:setopts(unwrap_socket(Socket), [{active, once}]); - ssl -> ssl:setopts(unwrap_socket(Socket), [{active, once}]) - end). + case socket_type(Socket) of + tcp -> + activate_connection(inet, tcp_closed, Socket); + ssl -> + activate_connection(ssl, ssl_closed, Socket) + end. +activate_connection(API, CloseTag, Socket) -> + Socket = unwrap_socket(Socket), + case API:setopts(Socket, [{active, once}]) of + ok -> + ok; + {error, _} -> %% inet can retrun einval instead of closed + self() ! {CloseTag, Socket} + end. ignore_return_value(_) -> ok. -- cgit v1.2.1 From 6881c9c532c7d9d19f039e2aa80c1d740295d220 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Thu, 11 Jun 2020 09:53:30 +0200 Subject: inets: Handle that inet/ssl:setopt can return error --- lib/ftp/src/ftp.erl | 4 +- lib/inets/src/http_client/httpc_handler.erl | 18 +++++---- lib/inets/src/http_lib/http_transport.erl | 6 +++ .../src/http_server/httpd_request_handler.erl | 45 +++++++++++----------- 4 files changed, 41 insertions(+), 32 deletions(-) diff --git a/lib/ftp/src/ftp.erl b/lib/ftp/src/ftp.erl index dac316fe0b..b94c535467 100644 --- a/lib/ftp/src/ftp.erl +++ b/lib/ftp/src/ftp.erl @@ -2274,8 +2274,8 @@ activate_connection(Socket) -> activate_connection(ssl, ssl_closed, Socket) end. -activate_connection(API, CloseTag, Socket) -> - Socket = unwrap_socket(Socket), +activate_connection(API, CloseTag, Socket0) -> + Socket = unwrap_socket(Socket0), case API:setopts(Socket, [{active, once}]) of ok -> ok; diff --git a/lib/inets/src/http_client/httpc_handler.erl b/lib/inets/src/http_client/httpc_handler.erl index 3f91ae062c..1f8806d1d8 100644 --- a/lib/inets/src/http_client/httpc_handler.erl +++ b/lib/inets/src/http_client/httpc_handler.erl @@ -831,8 +831,7 @@ connect_and_send_first_request(Address, Request, #state{options = Options0} = St headers = undefined, body = undefined, status = new}, - http_transport:setopts(SocketType, - Socket, [{active, once}]), + activate_once(Session), NewState = activate_request_timeout(TmpState), {ok, NewState}; {error, Reason} -> @@ -1238,7 +1237,12 @@ case_insensitive_header(Str) -> Str. activate_once(#session{socket = Socket, socket_type = SocketType}) -> - http_transport:setopts(SocketType, Socket, [{active, once}]). + case http_transport:setopts(SocketType, Socket, [{active, once}]) of + ok -> + ok; + {error, _} -> %% inet can return einval instead of closed + self() ! {http_transport:close_tag(SocketType), Socket} + end. close_socket(#session{socket = {remote_close,_}}) -> ok; @@ -1581,8 +1585,7 @@ send_raw(SocketType, Socket, ProcessBody, Acc) -> end end. -tls_tunnel(Address, Request, #state{session = #session{socket = Socket, - socket_type = SocketType} = Session} = State, +tls_tunnel(Address, Request, #state{session = #session{} = Session} = State, ErrorHandler) -> UpgradeRequest = tls_tunnel_request(Request), case httpc_request:send(Address, Session, UpgradeRequest) of @@ -1594,8 +1597,7 @@ tls_tunnel(Address, Request, #state{session = #session{socket = Socket, init_status_line(UpgradeRequest), headers = undefined, body = undefined}, - http_transport:setopts(SocketType, - Socket, [{active, once}]), + activate_once(Session), NewState = activate_request_timeout(TmpState), {ok, NewState#state{status = {ssl_tunnel, Request}}}; {error, Reason} -> @@ -1661,7 +1663,7 @@ tls_upgrade(#state{status = type = SessionType, client_close = ClientClose}, httpc_request:send(Address, Session, Request), - http_transport:setopts(SocketType, TLSSocket, [{active, once}]), + activate_once(Session), NewState = State#state{session = Session, request = Request, mfa = init_mfa(Request, State), diff --git a/lib/inets/src/http_lib/http_transport.erl b/lib/inets/src/http_lib/http_transport.erl index d5e1d71336..d2148a3a8a 100644 --- a/lib/inets/src/http_lib/http_transport.erl +++ b/lib/inets/src/http_lib/http_transport.erl @@ -27,6 +27,7 @@ listen/4, listen/5, accept/2, accept/3, close/2, + close_tag/1, send/3, controlling_process/3, setopts/3, getopts/2, getopts/3, @@ -459,6 +460,11 @@ ipv6_name({A, B, C, D, E, F, G, H}) -> http_util:integer_to_hexlist(H). +close_tag(ip_comm) -> + tcp_closed; +close_tag(_) -> + ssl_closed. + %%%======================================================================== %%% Internal functions %%%======================================================================== diff --git a/lib/inets/src/http_server/httpd_request_handler.erl b/lib/inets/src/http_server/httpd_request_handler.erl index e82b1c46e9..65ca1181e8 100644 --- a/lib/inets/src/http_server/httpd_request_handler.erl +++ b/lib/inets/src/http_server/httpd_request_handler.erl @@ -162,8 +162,9 @@ continue_init(Manager, ConfigDB, SocketType, Socket, Peername, Sockname, mfa = MFA, chunk = chunk_start(MaxChunk)}, - http_transport:setopts(SocketType, Socket, - [binary, {packet, 0}, {active, once}]), + ok = http_transport:setopts(SocketType, Socket, + [binary, {packet, 0}]), + activate_once(Socket, SocketType), NewState = data_receive_counter(activate_request_timeout(State), httpd_util:lookup(ConfigDB, minimum_bytes_per_second, false)), gen_server:enter_loop(?MODULE, [], NewState). @@ -242,7 +243,7 @@ handle_info({Proto, Socket, Data}, NewState = handle_chunk(Module, Function, Args, State), {noreply, NewState}; NewMFA -> - http_transport:setopts(SockType, Socket, [{active, once}]), + activate_once(Socket, SockType), case NewDataSize of undefined -> {noreply, State#state{mfa = NewMFA}}; @@ -363,9 +364,7 @@ handle_msg({{continue, Chunk}, Module, Function, Args}, #state{chunk = {_, CbSta handle_internal_chunk(State#state{chunk = {continue, CbState}, body = Chunk}, Module, Function, Args); handle_msg({continue, Module, Function, Args}, #state{mod = ModData} = State) -> - http_transport:setopts(ModData#mod.socket_type, - ModData#mod.socket, - [{active, once}]), + activate_once(ModData#mod.socket, ModData#mod.socket_type), {noreply, State#state{mfa = {Module, Function, Args}}}; handle_msg({last, Body}, #state{headers = Headers, chunk = {_, CbState}} = State) -> NewHeaders = Headers#http_request_h{'content-length' = integer_to_list(size(Body))}, @@ -471,9 +470,7 @@ handle_body(#state{headers = Headers, body = Body, "chunked" -> try http_chunk:decode(Body, MaxBodySize, MaxHeaderSize) of {Module, Function, Args} -> - http_transport:setopts(ModData#mod.socket_type, - ModData#mod.socket, - [{active, once}]), + activate_once(ModData#mod.socket, ModData#mod.socket_type), {noreply, State#state{mfa = {Module, Function, Args}, chunk = chunk_start(MaxChunk)}}; @@ -502,17 +499,13 @@ handle_body(#state{headers = Headers, body = Body, %% This is the case that the we need more data to complete %% the body but chunking to the mod_esi user is not enabled. {Module, add_chunk = Function, Args} -> - http_transport:setopts(ModData#mod.socket_type, - ModData#mod.socket, - [{active, once}]), + activate_once(ModData#mod.socket, ModData#mod.socket_type), {noreply, State#state{mfa = {Module, Function, Args}}}; %% Chunking to mod_esi user is enabled {ok, {continue, Module, Function, Args}} -> - http_transport:setopts(ModData#mod.socket_type, - ModData#mod.socket, - [{active, once}]), - {noreply, State#state{mfa = + activate_once(ModData#mod.socket, ModData#mod.socket_type), + {noreply, State#state{mfa = {Module, Function, Args}}}; {ok, {{continue, Chunk}, Module, Function, Args}} -> handle_internal_chunk(State#state{chunk = chunk_start(MaxChunk), @@ -588,7 +581,7 @@ handle_chunk(http_chunk = Module, decode_data = Function, socket = Socket} = ModData} = State) -> {continue, NewCbState} = httpd_response:handle_continuation(ModData#mod{entity_body = {continue, BodySoFar, CbState}}), - http_transport:setopts(SockType, Socket, [{active, once}]), + activate_once(Socket, SockType), State#state{chunk = {continue, NewCbState}, mfa = {Module, Function, [ChunkSize, TotalChunk, {MaxBodySize, <<>>, 0, MaxHeaderSize}]}}; handle_chunk(http_chunk = Module, decode_size = Function, @@ -597,11 +590,11 @@ handle_chunk(http_chunk = Module, decode_size = Function, mod = #mod{socket_type = SockType, socket = Socket} = ModData} = State) -> {continue, NewCbState} = httpd_response:handle_continuation(ModData#mod{entity_body = {continue, BodySoFar, CbState}}), - http_transport:setopts(SockType, Socket, [{active, once}]), + activate_once(Socket, SockType), State#state{chunk = {continue, NewCbState}, mfa = {Module, Function, [Data, HexList, 0, {MaxBodySize, <<>>, 0, MaxHeaderSize}]}}; handle_chunk(Module, Function, Args, #state{mod = #mod{socket_type = SockType, socket = Socket}} = State) -> - http_transport:setopts(SockType, Socket, [{active, once}]), + activate_once(Socket, SockType), State#state{mfa = {Module, Function, Args}}. handle_internal_chunk(#state{chunk = {ChunkState, CbState}, body = Chunk, @@ -611,7 +604,7 @@ handle_internal_chunk(#state{chunk = {ChunkState, CbState}, body = Chunk, {continue, NewCbState} = httpd_response:handle_continuation(ModData#mod{entity_body = Bodychunk}), case Args of [<<>> | _] -> - http_transport:setopts(SockType, Socket, [{active, once}]), + activate_once(Socket, SockType), {noreply, State#state{chunk = {continue, NewCbState}, mfa = {Module, Function, Args}}}; _ -> handle_info({dummy, Socket, <<>>}, State#state{chunk = {continue, NewCbState}, @@ -675,8 +668,7 @@ handle_next_request(#state{mod = #mod{connection = true} = ModData, case Data of <<>> -> - http_transport:setopts(ModData#mod.socket_type, - ModData#mod.socket, [{active, once}]), + activate_once(ModData#mod.socket, ModData#mod.socket_type), {noreply, NewState}; _ -> handle_info({dummy, ModData#mod.socket, Data}, NewState) @@ -751,3 +743,12 @@ body_chunk(first, _, Chunk) -> {first, Chunk}; body_chunk(ChunkState, CbState, Chunk) -> {ChunkState, Chunk, CbState}. + +activate_once(Socket, SocketType) -> + case http_transport:setopts(SocketType, Socket, [{active, once}]) of + ok -> + ok; + {error, _} -> %% inet can return einval instead of closed + self() ! {http_transport:close_tag(SocketType), Socket} + end. + -- cgit v1.2.1