From de990f6afaa85a43c01d72b65a5a9de6940317a7 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Thu, 14 Feb 2019 14:38:43 +0100 Subject: Clean up module boundaries Improve the abstraction between the ssl_connection module and dtls_connection, tls_connection and tls_sender, as well as towards the lower level tls_record and ssl_record modules. Remove some dead code. --- lib/ssl/src/dtls_connection.erl | 23 +++++++++-------------- lib/ssl/src/ssl_connection.erl | 25 ++++++++----------------- lib/ssl/src/ssl_connection.hrl | 6 +++--- lib/ssl/src/ssl_record.erl | 4 ++-- lib/ssl/src/tls_connection.erl | 34 +++++++++++++--------------------- lib/ssl/src/tls_record.erl | 11 ++++++++--- lib/ssl/src/tls_sender.erl | 33 ++++++++++++++------------------- 7 files changed, 57 insertions(+), 79 deletions(-) diff --git a/lib/ssl/src/dtls_connection.erl b/lib/ssl/src/dtls_connection.erl index e24961186d..2c6b71c97a 100644 --- a/lib/ssl/src/dtls_connection.erl +++ b/lib/ssl/src/dtls_connection.erl @@ -50,8 +50,7 @@ -export([encode_alert/3, send_alert/2, send_alert_in_connection/2, close/5, protocol_name/0]). %% Data handling --export([encode_data/3, next_record/1, - send/3, socket/5, setopts/3, getopts/3]). +-export([next_record/1, socket/4, setopts/3, getopts/3]). %% gen_statem state functions -export([init/3, error/3, downgrade/3, %% Initiation and take down states @@ -392,16 +391,13 @@ protocol_name() -> %% Data handling %%==================================================================== -encode_data(Data, Version, ConnectionStates0)-> - dtls_record:encode_data(Data, Version, ConnectionStates0). +send(Transport, {Listener, Socket}, Data) when is_pid(Listener) -> % Server socket + dtls_socket:send(Transport, Socket, Data); +send(Transport, Socket, Data) -> % Client socket + dtls_socket:send(Transport, Socket, Data). -send(Transport, {_, {{_,_}, _} = Socket}, Data) -> - send(Transport, Socket, Data); -send(Transport, Socket, Data) -> - dtls_socket:send(Transport, Socket, Data). - -socket(Pid, Transport, Socket, Connection, _) -> - dtls_socket:socket(Pid, Transport, Socket, Connection). +socket(Pid, Transport, Socket, _Tracker) -> + dtls_socket:socket(Pid, Transport, Socket, ?MODULE). setopts(Transport, Socket, Other) -> dtls_socket:setopts(Transport, Socket, Other). @@ -1173,7 +1169,6 @@ log_ignore_alert(false, _, _,_) -> send_application_data(Data, From, _StateName, #state{static_env = #static_env{socket = Socket, - protocol_cb = Connection, transport_cb = Transport}, connection_env = #connection_env{negotiated_version = Version}, handshake_env = HsEnv, @@ -1186,9 +1181,9 @@ send_application_data(Data, From, _StateName, [{next_event, {call, From}, {application_data, Data}}]); false -> {Msgs, ConnectionStates} = - Connection:encode_data(Data, Version, ConnectionStates0), + dtls_record:encode_data(Data, Version, ConnectionStates0), State = State0#state{connection_states = ConnectionStates}, - case Connection:send(Transport, Socket, Msgs) of + case send(Transport, Socket, Msgs) of ok -> ssl_connection:hibernate_after(connection, State, [{reply, From, ok}]); Result -> diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index 61524347eb..02542cb1ab 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -70,7 +70,7 @@ -export([terminate/3, format_status/2]). %% Erlang Distribution export --export([get_sslsocket/1, dist_handshake_complete/2]). +-export([dist_handshake_complete/2]). %%==================================================================== %% Setup @@ -182,19 +182,19 @@ socket_control(Connection, Socket, Pid, Transport) -> %%-------------------------------------------------------------------- socket_control(Connection, Socket, Pids, Transport, udp_listener) -> %% dtls listener process must have the socket control - {ok, Connection:socket(Pids, Transport, Socket, Connection, undefined)}; + {ok, Connection:socket(Pids, Transport, Socket, undefined)}; socket_control(tls_connection = Connection, Socket, [Pid|_] = Pids, Transport, ListenTracker) -> case Transport:controlling_process(Socket, Pid) of ok -> - {ok, Connection:socket(Pids, Transport, Socket, Connection, ListenTracker)}; + {ok, Connection:socket(Pids, Transport, Socket, ListenTracker)}; {error, Reason} -> {error, Reason} end; socket_control(dtls_connection = Connection, {_, Socket}, [Pid|_] = Pids, Transport, ListenTracker) -> case Transport:controlling_process(Socket, Pid) of ok -> - {ok, Connection:socket(Pids, Transport, Socket, Connection, ListenTracker)}; + {ok, Connection:socket(Pids, Transport, Socket, ListenTracker)}; {error, Reason} -> {error, Reason} end. @@ -311,9 +311,6 @@ renegotiation(ConnectionPid) -> internal_renegotiation(ConnectionPid, #{current_write := WriteState}) -> gen_statem:cast(ConnectionPid, {internal_renegotiate, WriteState}). -get_sslsocket(ConnectionPid) -> - call(ConnectionPid, get_sslsocket). - dist_handshake_complete(ConnectionPid, DHandle) -> gen_statem:cast(ConnectionPid, {dist_handshake_complete, DHandle}). @@ -1351,10 +1348,6 @@ handle_call({set_opts, Opts0}, From, StateName, handle_call(renegotiate, From, StateName, _, _) when StateName =/= connection -> {keep_state_and_data, [{reply, From, {error, already_renegotiating}}]}; -handle_call(get_sslsocket, From, _StateName, State, Connection) -> - SslSocket = Connection:socket(State), - {keep_state_and_data, [{reply, From, SslSocket}]}; - handle_call({prf, Secret, Label, Seed, WantedLength}, From, _, #state{connection_states = ConnectionStates, connection_env = #connection_env{negotiated_version = Version}}, _) -> @@ -2723,7 +2716,7 @@ format_reply(_, _, _,#socket_options{active = false, mode = Mode, packet = Packe {ok, do_format_reply(Mode, Packet, Header, Data)}; format_reply(CPids, Transport, Socket, #socket_options{active = _, mode = Mode, packet = Packet, header = Header}, Data, Tracker, Connection) -> - {ssl, Connection:socket(CPids, Transport, Socket, Connection, Tracker), + {ssl, Connection:socket(CPids, Transport, Socket, Tracker), do_format_reply(Mode, Packet, Header, Data)}. deliver_packet_error(CPids, Transport, Socket, @@ -2735,7 +2728,7 @@ format_packet_error(_, _, _,#socket_options{active = false, mode = Mode}, Data, {error, {invalid_packet, do_format_reply(Mode, raw, 0, Data)}}; format_packet_error(CPids, Transport, Socket, #socket_options{active = _, mode = Mode}, Data, Tracker, Connection) -> - {ssl_error, Connection:socket(CPids, Transport, Socket, Connection, Tracker), + {ssl_error, Connection:socket(CPids, Transport, Socket, Tracker), {invalid_packet, do_format_reply(Mode, raw, 0, Data)}}. do_format_reply(binary, _, N, Data) when N > 0 -> % Header mode @@ -2791,12 +2784,10 @@ alert_user(Pids, Transport, Tracker, Socket, Active, Pid, From, Alert, Role, Con case ssl_alert:reason_code(Alert, Role) of closed -> send_or_reply(Active, Pid, From, - {ssl_closed, Connection:socket(Pids, - Transport, Socket, Connection, Tracker)}); + {ssl_closed, Connection:socket(Pids, Transport, Socket, Tracker)}); ReasonCode -> send_or_reply(Active, Pid, From, - {ssl_error, Connection:socket(Pids, - Transport, Socket, Connection, Tracker), ReasonCode}) + {ssl_error, Connection:socket(Pids, Transport, Socket, Tracker), ReasonCode}) end. log_alert(true, Role, ProtocolName, StateName, #alert{role = Role} = Alert) -> diff --git a/lib/ssl/src/ssl_connection.hrl b/lib/ssl/src/ssl_connection.hrl index 83013e7fba..b6b23701bb 100644 --- a/lib/ssl/src/ssl_connection.hrl +++ b/lib/ssl/src/ssl_connection.hrl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2013-2018. All Rights Reserved. +%% Copyright Ericsson AB 2013-2019. All Rights Reserved. %% %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. @@ -83,7 +83,7 @@ downgrade, terminated = false ::boolean() | closed, negotiated_version :: ssl_record:ssl_version() | 'undefined', - erl_dist_handle = undefined :: erlang:dist_handle() | undefined, + erl_dist_handle = undefined :: erlang:dist_handle() | 'undefined', private_key :: public_key:private_key() | secret_printout() | 'undefined' }). @@ -109,7 +109,7 @@ %% Data shuffling %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% connection_states :: ssl_record:connection_states() | secret_printout(), protocol_buffers :: term() | secret_printout() , %% #protocol_buffers{} from tls_record.hrl or dtls_recor.hr - user_data_buffer :: undefined | binary() | secret_printout(), + user_data_buffer :: undefined | {[binary()],non_neg_integer(),[binary()]} | secret_printout(), bytes_to_read :: undefined | integer(), %% bytes to read in passive mode %% recv and start handling diff --git a/lib/ssl/src/ssl_record.erl b/lib/ssl/src/ssl_record.erl index e1b16193bd..1a36b2dba8 100644 --- a/lib/ssl/src/ssl_record.erl +++ b/lib/ssl/src/ssl_record.erl @@ -53,8 +53,8 @@ -type ssl_version() :: {integer(), integer()}. -type ssl_atom_version() :: tls_record:tls_atom_version(). --type connection_states() :: term(). %% Map --type connection_state() :: term(). %% Map +-type connection_states() :: map(). %% Map +-type connection_state() :: map(). %% Map %%==================================================================== %% Connection state handling diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl index 8d125b8a70..5004cac8e8 100644 --- a/lib/ssl/src/tls_connection.erl +++ b/lib/ssl/src/tls_connection.erl @@ -57,11 +57,10 @@ %% Alert and close handling -export([send_alert/2, send_alert_in_connection/2, send_sync_alert/2, - encode_alert/3, close/5, protocol_name/0]). + close/5, protocol_name/0]). %% Data handling --export([encode_data/3, next_record/1, - send/3, socket/5, setopts/3, getopts/3]). +-export([next_record/1, socket/4, setopts/3, getopts/3]). %% gen_statem state functions -export([init/3, error/3, downgrade/3, %% Initiation and take down states @@ -202,14 +201,15 @@ next_record_done(#state{protocol_buffers = Buffers} = State, CipherTexts, Connec next_event(StateName, Record, State) -> next_event(StateName, Record, State, []). +%% next_event(StateName, no_record, State0, Actions) -> case next_record(State0) of {no_record, State} -> {next_state, StateName, State, Actions}; {#ssl_tls{} = Record, State} -> {next_state, StateName, State, [{next_event, internal, {protocol_record, Record}} | Actions]}; - {#alert{} = Alert, State} -> - {next_state, StateName, State, [{next_event, internal, Alert} | Actions]} + #alert{} = Alert -> + {next_state, StateName, State0, [{next_event, internal, Alert} | Actions]} end; next_event(StateName, Record, State, Actions) -> case Record of @@ -311,7 +311,7 @@ renegotiate(#state{static_env = #static_env{role = server, Hs0 = ssl_handshake:init_handshake_history(), {BinMsg, ConnectionStates} = tls_record:encode_handshake(Frag, Version, ConnectionStates0), - send(Transport, Socket, BinMsg), + tls_socket:send(Transport, Socket, BinMsg), State = State0#state{connection_states = ConnectionStates, handshake_env = HsEnv#handshake_env{tls_handshake_history = Hs0}}, @@ -333,7 +333,7 @@ queue_handshake(Handshake, #state{handshake_env = #handshake_env{tls_handshake_h send_handshake_flight(#state{static_env = #static_env{socket = Socket, transport_cb = Transport}, flight_buffer = Flight} = State0) -> - send(Transport, Socket, Flight), + tls_socket:send(Transport, Socket, Flight), {State0#state{flight_buffer = []}, []}. queue_change_cipher(Msg, #state{connection_env = #connection_env{negotiated_version = Version}, @@ -386,7 +386,7 @@ send_alert(Alert, #state{static_env = #static_env{socket = Socket, connection_states = ConnectionStates0} = StateData0) -> {BinMsg, ConnectionStates} = encode_alert(Alert, Version, ConnectionStates0), - send(Transport, Socket, BinMsg), + tls_socket:send(Transport, Socket, BinMsg), StateData0#state{connection_states = ConnectionStates}. %% If an ALERT sent in the connection state, should cause the TLS @@ -440,14 +440,9 @@ protocol_name() -> %%==================================================================== %% Data handling %%==================================================================== -encode_data(Data, Version, ConnectionStates0)-> - tls_record:encode_data(Data, Version, ConnectionStates0). -send(Transport, Socket, Data) -> - tls_socket:send(Transport, Socket, Data). - -socket(Pids, Transport, Socket, Connection, Tracker) -> - tls_socket:socket(Pids, Transport, Socket, Connection, Tracker). +socket(Pids, Transport, Socket, Tracker) -> + tls_socket:socket(Pids, Transport, Socket, ?MODULE, Tracker). setopts(Transport, Socket, Other) -> tls_socket:setopts(Transport, Socket, Other). @@ -486,7 +481,7 @@ init({call, From}, {start, Timeout}, Handshake0 = ssl_handshake:init_handshake_history(), {BinMsg, ConnectionStates, Handshake} = encode_handshake(Hello, HelloVersion, ConnectionStates0, Handshake0), - send(Transport, Socket, BinMsg), + tls_socket:send(Transport, Socket, BinMsg), State = State0#state{connection_states = ConnectionStates, connection_env = CEnv#connection_env{negotiated_version = Version}, %% Requested version session = @@ -711,12 +706,11 @@ connection(internal, #client_hello{} = Hello, }, [{next_event, internal, Hello}]); connection(internal, #client_hello{}, - #state{static_env = #static_env{role = server, - protocol_cb = Connection}, + #state{static_env = #static_env{role = server}, handshake_env = #handshake_env{allow_renegotiate = false}} = State0) -> Alert = ?ALERT_REC(?WARNING, ?NO_RENEGOTIATION), send_alert_in_connection(Alert, State0), - State = Connection:reinit_handshake_data(State0), + State = reinit_handshake_data(State0), next_event(?FUNCTION_NAME, no_record, State); connection(Type, Event, State) -> @@ -827,7 +821,6 @@ initial_state(Role, Sender, Host, Port, Socket, {SSLOptions, SocketOptions, Trac initialize_tls_sender(#state{static_env = #static_env{ role = Role, transport_cb = Transport, - protocol_cb = Connection, socket = Socket, tracker = Tracker }, @@ -841,7 +834,6 @@ initialize_tls_sender(#state{static_env = #static_env{ socket => Socket, socket_options => SockOpts, tracker => Tracker, - protocol_cb => Connection, transport_cb => Transport, negotiated_version => Version, renegotiate_at => RenegotiateAt}, diff --git a/lib/ssl/src/tls_record.erl b/lib/ssl/src/tls_record.erl index cd3c3b7829..5cf238a685 100644 --- a/lib/ssl/src/tls_record.erl +++ b/lib/ssl/src/tls_record.erl @@ -75,7 +75,12 @@ init_connection_states(Role, BeastMitigation) -> pending_write => Pending}. %%-------------------------------------------------------------------- --spec get_tls_records(binary(), [tls_version()] | tls_version(), binary()) -> {[binary()], binary()} | #alert{}. +-spec get_tls_records( + binary(), [tls_version()] | tls_version(), + Buffer0 :: binary() | {'undefined' | #ssl_tls{}, {[binary()],non_neg_integer(),[binary()]}}) -> + {Records :: [#ssl_tls{}], + Buffer :: {'undefined' | #ssl_tls{}, {[binary()],non_neg_integer(),[binary()]}}} | + #alert{}. %% %% and returns it as a list of tls_compressed binaries also returns leftover %% Description: Given old buffer and new data from TCP, packs up a records @@ -132,8 +137,8 @@ encode_change_cipher_spec(Version, ConnectionStates) -> encode_plain_text(?CHANGE_CIPHER_SPEC, Version, ?byte(?CHANGE_CIPHER_SPEC_PROTO), ConnectionStates). %%-------------------------------------------------------------------- --spec encode_data(binary(), tls_version(), ssl_record:connection_states()) -> - {iolist(), ssl_record:connection_states()}. +-spec encode_data([binary()], tls_version(), ssl_record:connection_states()) -> + {[[binary()]], ssl_record:connection_states()}. %% %% Description: Encodes data to send on the ssl-socket. %%-------------------------------------------------------------------- diff --git a/lib/ssl/src/tls_sender.erl b/lib/ssl/src/tls_sender.erl index feda0b8672..c07b7f49cd 100644 --- a/lib/ssl/src/tls_sender.erl +++ b/lib/ssl/src/tls_sender.erl @@ -44,7 +44,6 @@ socket, socket_options, tracker, - protocol_cb, transport_cb, negotiated_version, renegotiate_at, @@ -201,7 +200,6 @@ init({call, From}, {Pid, #{current_write := WriteState, socket := Socket, socket_options := SockOpts, tracker := Tracker, - protocol_cb := Connection, transport_cb := Transport, negotiated_version := Version, renegotiate_at := RenegotiateAt}}, @@ -215,7 +213,6 @@ init({call, From}, {Pid, #{current_write := WriteState, socket = Socket, socket_options = SockOpts, tracker = Tracker, - protocol_cb = Connection, transport_cb = Transport, negotiated_version = Version, renegotiate_at = RenegotiateAt}}, @@ -252,12 +249,11 @@ connection({call, From}, downgrade, #data{connection_states = connection({call, From}, {set_opts, Opts}, StateData) -> handle_set_opts(From, Opts, StateData); connection({call, From}, dist_get_tls_socket, - #data{static = #static{protocol_cb = Connection, - transport_cb = Transport, + #data{static = #static{transport_cb = Transport, socket = Socket, connection_pid = Pid, tracker = Tracker}} = StateData) -> - TLSSocket = Connection:socket([Pid, self()], Transport, Socket, Connection, Tracker), + TLSSocket = tls_connection:socket([Pid, self()], Transport, Socket, Tracker), {next_state, ?FUNCTION_NAME, StateData, [{reply, From, {ok, TLSSocket}}]}; connection({call, From}, {dist_handshake_complete, _Node, DHandle}, #data{static = #static{connection_pid = Pid} = Static} = StateData) -> @@ -407,14 +403,14 @@ handle_common(Type, Msg, _) -> error_logger:error_report(Report), keep_state_and_data. -send_tls_alert(Alert, #data{static = #static{negotiated_version = Version, - socket = Socket, - protocol_cb = Connection, - transport_cb = Transport}, - connection_states = ConnectionStates0} = StateData0) -> +send_tls_alert(#alert{} = Alert, + #data{static = #static{negotiated_version = Version, + socket = Socket, + transport_cb = Transport}, + connection_states = ConnectionStates0} = StateData0) -> {BinMsg, ConnectionStates} = - Connection:encode_alert(Alert, Version, ConnectionStates0), - Connection:send(Transport, Socket, BinMsg), + tls_record:encode_alert_record(Alert, Version, ConnectionStates0), + tls_socket:send(Transport, Socket, BinMsg), StateData0#data{connection_states = ConnectionStates}. send_application_data(Data, From, StateName, @@ -422,7 +418,6 @@ send_application_data(Data, From, StateName, socket = Socket, dist_handle = DistHandle, negotiated_version = Version, - protocol_cb = Connection, transport_cb = Transport, renegotiate_at = RenegotiateAt}, connection_states = ConnectionStates0} = StateData0) -> @@ -432,9 +427,9 @@ send_application_data(Data, From, StateName, {next_state, handshake, StateData0, [{next_event, internal, {application_packets, From, Data}}]}; false -> - {Msgs, ConnectionStates} = Connection:encode_data(Data, Version, ConnectionStates0), + {Msgs, ConnectionStates} = tls_record:encode_data(Data, Version, ConnectionStates0), StateData = StateData0#data{connection_states = ConnectionStates}, - case Connection:send(Transport, Socket, Msgs) of + case tls_socket:send(Transport, Socket, Msgs) of ok when DistHandle =/= undefined -> {next_state, StateName, StateData, []}; Reason when DistHandle =/= undefined -> @@ -502,13 +497,13 @@ dist_data(DHandle) -> %% therefore always have to use {packet,4} Data when is_binary(Data) -> Len = byte_size(Data), - [<>,Data|dist_data(DHandle)]; + [[<>,Data]|dist_data(DHandle)]; [BA,BB] = Data -> Len = byte_size(BA) + byte_size(BB), - [<>,Data|dist_data(DHandle)]; + [[<>|Data]|dist_data(DHandle)]; Data when is_list(Data) -> Len = iolist_size(Data), - [<>,Data|dist_data(DHandle)] + [[<>|Data]|dist_data(DHandle)] end. -- cgit v1.2.3