aboutsummaryrefslogtreecommitdiffstats
path: root/lib/ssl
diff options
context:
space:
mode:
authorRaimo Niskanen <[email protected]>2018-04-25 08:51:00 +0200
committerRaimo Niskanen <[email protected]>2018-04-26 12:03:30 +0200
commitba25ad268065f48ad7647464c8f4c237d07d18b1 (patch)
tree7e11c626c8006316c08e267f9d6dfd7401143c69 /lib/ssl
parentc9b2af1f2cecb6565a2c79d683d86a1d7ee746e0 (diff)
downloadotp-ba25ad268065f48ad7647464c8f4c237d07d18b1.tar.gz
otp-ba25ad268065f48ad7647464c8f4c237d07d18b1.tar.bz2
otp-ba25ad268065f48ad7647464c8f4c237d07d18b1.zip
Move check ip to before SSL handshake
Diffstat (limited to 'lib/ssl')
-rw-r--r--lib/ssl/src/inet_tls_dist.erl140
-rw-r--r--lib/ssl/test/ssl_dist_bench_SUITE.erl7
2 files changed, 81 insertions, 66 deletions
diff --git a/lib/ssl/src/inet_tls_dist.erl b/lib/ssl/src/inet_tls_dist.erl
index f43a7b03e6..b084135d1a 100644
--- a/lib/ssl/src/inet_tls_dist.erl
+++ b/lib/ssl/src/inet_tls_dist.erl
@@ -222,46 +222,58 @@ gen_accept(Driver, Listen) ->
accept_loop(Driver, Listen, Kernel) ->
case Driver:accept(Listen) of
{ok, Socket} ->
- {Opts,CertNodesFun} =
- setup_verify_client(
- Driver, Socket, get_ssl_options(server)),
- wait_for_code_server(),
- case
- ssl:ssl_accept(
- Socket,
- trace([{active, false},{packet, 4}|Opts]),
- net_kernel:connecttime())
- of
- {ok, #sslsocket{pid = DistCtrl} = SslSocket} ->
- trace(
- Kernel !
- {accept, self(), DistCtrl,
- Driver:family(), tls}),
- receive
- {Kernel, controller, Pid} ->
- ok = ssl:controlling_process(SslSocket, Pid),
- trace(
- Pid ! {self(), controller, CertNodesFun});
- {Kernel, unsupported_protocol} ->
- exit(trace(unsupported_protocol))
- end,
- accept_loop(Driver, Listen, Kernel);
- {error, {options, _}} = Error ->
- %% Bad options: that's probably our fault.
- %% Let's log that.
+ case check_ip(Driver, Socket) of
+ true ->
+ accept_loop(Driver, Listen, Kernel, Socket);
+ {false,IP} ->
error_logger:error_msg(
- "Cannot accept TLS distribution connection: ~s~n",
- [ssl:format_error(Error)]),
- gen_tcp:close(Socket),
- exit(trace(Error));
- Other ->
- gen_tcp:close(Socket),
- exit(trace(Other))
+ "** Connection attempt from "
+ "disallowed IP ~w ** ~n", [IP]),
+ ?shutdown2(no_node, trace({disallowed, IP}))
end;
Error ->
exit(trace(Error))
end.
+accept_loop(Driver, Listen, Kernel, Socket) ->
+ {Opts,CertNodesFun} =
+ setup_verify_client(
+ Driver, Socket, get_ssl_options(server)),
+ wait_for_code_server(),
+ case
+ ssl:ssl_accept(
+ Socket,
+ trace([{active, false},{packet, 4}|Opts]),
+ net_kernel:connecttime())
+ of
+ {ok, #sslsocket{pid = DistCtrl} = SslSocket} ->
+ trace(
+ Kernel !
+ {accept, self(), DistCtrl,
+ Driver:family(), tls}),
+ receive
+ {Kernel, controller, Pid} ->
+ ok = ssl:controlling_process(SslSocket, Pid),
+ trace(
+ Pid ! {self(), controller, CertNodesFun});
+ {Kernel, unsupported_protocol} ->
+ exit(trace(unsupported_protocol))
+ end,
+ accept_loop(Driver, Listen, Kernel);
+ {error, {options, _}} = Error ->
+ %% Bad options: that's probably our fault.
+ %% Let's log that.
+ error_logger:error_msg(
+ "Cannot accept TLS distribution connection: ~s~n",
+ [ssl:format_error(Error)]),
+ gen_tcp:close(Socket),
+ exit(trace(Error));
+ Other ->
+ gen_tcp:close(Socket),
+ exit(trace(Other))
+ end.
+
+
%% {verify_fun,{fun ?MODULE:verify_client/3,_}} is used
%% as a configuration marker that verify_client/3 shall be used.
%%
@@ -326,6 +338,8 @@ verify_client(PeerCert, valid_peer, {CertNodesFun,Driver,PeerIP} = S) ->
[] ->
{fail,cert_missing_node_name};
CertNodes ->
+ %% Check if the IP address of one of the nodes
+ %% in the peer certificate has the peer's IP address
case filter_nodes_by_ip(CertNodes, Driver, PeerIP) of
[] ->
{fail,cert_no_host_with_peer_ip};
@@ -334,6 +348,8 @@ verify_client(PeerCert, valid_peer, {CertNodesFun,Driver,PeerIP} = S) ->
end
end.
+%% Filter out the nodes that has got the given IP address
+%%
filter_nodes_by_ip(Nodes, Driver, IP) ->
[Node ||
Node <- Nodes,
@@ -405,30 +421,25 @@ do_accept(Driver, Kernel, AcceptPid, DistCtrl, MyNode, Allowed, SetupTime) ->
receive
{AcceptPid, controller, CertNodesFun} ->
Timer = dist_util:start_timer(SetupTime),
- case check_ip(Driver, SslSocket) of
- true ->
- NewAllowed =
- allowed_nodes(
- Driver, CertNodesFun, SslSocket, Allowed),
- HSData0 = hs_data_common(SslSocket),
- HSData =
- HSData0#hs_data{
- kernel_pid = Kernel,
- this_node = MyNode,
- socket = DistCtrl,
- timer = Timer,
- this_flags = 0,
- allowed = NewAllowed},
- link(DistCtrl),
- dist_util:handshake_other_started(trace(HSData));
- {false,IP} ->
- error_logger:error_msg(
- "** Connection attempt from "
- "disallowed IP ~w ** ~n", [IP]),
- ?shutdown2(no_node, trace({disallowed, IP}))
- end
+ NewAllowed =
+ allowed_nodes(
+ Driver, CertNodesFun, SslSocket, Allowed),
+ HSData0 = hs_data_common(SslSocket),
+ HSData =
+ HSData0#hs_data{
+ kernel_pid = Kernel,
+ this_node = MyNode,
+ socket = DistCtrl,
+ timer = Timer,
+ this_flags = 0,
+ allowed = NewAllowed},
+ link(DistCtrl),
+ dist_util:handshake_other_started(trace(HSData))
end.
+%% Return a list of allowed nodes according to
+%% the given Allowed list that matches the peer certificate
+%%
allowed_nodes(_Driver, undefined, _SslSocket, Allowed) ->
Allowed;
allowed_nodes(Driver, CertNodesFun, SslSocket, Allowed) ->
@@ -444,6 +455,10 @@ allowed_nodes(Driver, CertNodesFun, SslSocket, Allowed) ->
[] ->
?shutdown(cert_missing_node_name);
CertNodes ->
+ %% Filter out all nodes in the
+ %% allowed list that is in peer
+ %% certificate and that has got
+ %% the same IP address as the peer
allowed(
filter_nodes_by_ip(
CertNodes, Driver, PeerIP),
@@ -457,7 +472,8 @@ allowed_nodes(Driver, CertNodesFun, SslSocket, Allowed) ->
end.
allowed(CertNodes, []) ->
- %% All allowed -> allow only certificate nodes
+ %% Empty allowed list means all allowed
+ %% -> allow only certificate nodes
CertNodes;
allowed(CertNodes, Allowed) ->
%% Find the intersection of the allowed list and certificate nodes
@@ -602,6 +618,8 @@ verify_server(PeerCert, valid_peer, {CertNodesFun,Node} = S) ->
[] ->
{fail,cert_missing_node_name};
CertNodes ->
+ %% Check that the node we are connecting to
+ %% is in the peer certificate
case dist_util:is_allowed(Node, CertNodes) of
true ->
{valid,S};
@@ -615,15 +633,15 @@ verify_server(PeerCert, valid_peer, {CertNodesFun,Node} = S) ->
%% Do only accept new connection attempts from nodes at our
%% own LAN, if the check_ip environment parameter is true.
%% ------------------------------------------------------------
-check_ip(Driver, SslSocket) ->
+check_ip(Driver, Socket) ->
case application:get_env(check_ip) of
{ok, true} ->
- case get_ifs(SslSocket) of
+ case get_ifs(Socket) of
{ok, IFs, IP} ->
check_ip(Driver, IFs, IP);
Other ->
?shutdown2(
- no_node, trace({check_ip_failed, SslSocket, Other}))
+ no_node, trace({check_ip_failed, Socket, Other}))
end;
_ ->
true
@@ -637,7 +655,7 @@ check_ip(Driver, [{OwnIP, _, Netmask}|IFs], PeerIP) ->
check_ip(_Driver, [], PeerIP) ->
{false, PeerIP}.
-get_ifs(#sslsocket{fd = {gen_tcp, Socket, _}}) ->
+get_ifs(Socket) ->
case inet:peername(Socket) of
{ok, {IP, _}} ->
%% XXX this is seriously broken for IPv6
diff --git a/lib/ssl/test/ssl_dist_bench_SUITE.erl b/lib/ssl/test/ssl_dist_bench_SUITE.erl
index 77c42de3f2..4ffc486fc4 100644
--- a/lib/ssl/test/ssl_dist_bench_SUITE.erl
+++ b/lib/ssl/test/ssl_dist_bench_SUITE.erl
@@ -246,7 +246,7 @@ setup(Config) ->
run_nodepair_test(fun setup/5, Config).
setup(A, B, Prefix, HA, HB) ->
- Rounds = 10,
+ Rounds = 50,
[] = ssl_apply(HA, erlang, nodes, []),
[] = ssl_apply(HB, erlang, nodes, []),
{SetupTime, CycleTime} =
@@ -276,10 +276,7 @@ setup_loop(A, B, T, N) ->
receive
{'DOWN',Mref,process,_,_} ->
[] = erlang:nodes(),
- receive
- after 500 ->
- setup_loop(A, B, Time + T, N - 1)
- end
+ setup_loop(A, B, Time + T, N - 1)
end.