From 794df8cbba8d7942dcb3bf2cbdfa526b04d41dd3 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Fri, 8 Jun 2018 15:49:06 +0200 Subject: Use public_key to verify client hostname --- lib/kernel/src/net_kernel.erl | 7 +- lib/ssl/src/inet_tls_dist.erl | 286 ++++++++++++---------------------- lib/ssl/test/ssl_dist_bench_SUITE.erl | 13 +- 3 files changed, 109 insertions(+), 197 deletions(-) (limited to 'lib') diff --git a/lib/kernel/src/net_kernel.erl b/lib/kernel/src/net_kernel.erl index 669adefdf8..c4e1a0ce1e 100644 --- a/lib/kernel/src/net_kernel.erl +++ b/lib/kernel/src/net_kernel.erl @@ -53,7 +53,7 @@ %% Documented API functions. --export([allow/1, +-export([allow/1, allowed/0, connect_node/1, monitor_nodes/1, monitor_nodes/2, @@ -171,6 +171,8 @@ kernel_apply(M,F,A) -> request({apply,M,F,A}). Nodes :: [node()]. allow(Nodes) -> request({allow, Nodes}). +allowed() -> request(allowed). + longnames() -> request(longnames). -spec stop() -> ok | {error, Reason} when @@ -528,6 +530,9 @@ handle_call({allow, Nodes}, From, State) -> async_reply({reply,error,State}, From) end; +handle_call(allowed, From, #state{allowed = Allowed} = State) -> + async_reply({reply,{ok,Allowed},State}, From); + %% %% authentication, used by auth. Simply works as this: %% if the message comes through, the other node IS authorized. diff --git a/lib/ssl/src/inet_tls_dist.erl b/lib/ssl/src/inet_tls_dist.erl index a6ceff25cb..aa3d7e3f72 100644 --- a/lib/ssl/src/inet_tls_dist.erl +++ b/lib/ssl/src/inet_tls_dist.erl @@ -31,7 +31,7 @@ -export([nodelay/0]). --export([verify_client/3, verify_server/3, cert_nodes/1]). +-export([verify_client/3, cert_nodes/1]). -export([dbg/0]). % Debug @@ -236,12 +236,10 @@ accept_loop(Driver, Listen, Kernel) -> end. accept_loop(Driver, Listen, Kernel, Socket) -> - {Opts,CertNodesFun} = - setup_verify_client( - Driver, Socket, get_ssl_options(server)), + Opts = setup_verify_client(Socket, get_ssl_options(server)), wait_for_code_server(), case - ssl:ssl_accept( + ssl:handshake( Socket, trace([{active, false},{packet, 4}|Opts]), net_kernel:connecttime()) @@ -255,7 +253,7 @@ accept_loop(Driver, Listen, Kernel, Socket) -> {Kernel, controller, Pid} -> ok = ssl:controlling_process(SslSocket, Pid), trace( - Pid ! {self(), controller, CertNodesFun}); + Pid ! {self(), controller}); {Kernel, unsupported_protocol} -> exit(trace(unsupported_protocol)) end, @@ -278,48 +276,59 @@ accept_loop(Driver, Listen, Kernel, Socket) -> %% as a configuration marker that verify_client/3 shall be used. %% %% Replace the State in the first occurence of -%% {verify_fun,{fun ?MODULE:verify_client/3,State}} and remove the rest. +%% {verify_fun,{fun ?MODULE:verify_client/3,State}} +%% and remove the rest. %% The inserted state is not accesible from a configuration file %% since it is dynamic and connection dependent. %% -setup_verify_client(Driver, Socket, Opts) -> - setup_verify_client(Driver, Socket, Opts, undefined, []). +setup_verify_client(Socket, Opts) -> + setup_verify_client(Socket, Opts, true, []). %% -setup_verify_client(_Driver, _Socket, [], CertNodesFun, OptsR) -> - {lists:reverse(OptsR),CertNodesFun}; -setup_verify_client(Driver, Socket, [Opt|Opts], CertNodesFun, OptsR) -> +setup_verify_client(_Socket, [], _, OptsR) -> + lists:reverse(OptsR); +setup_verify_client(Socket, [Opt|Opts], First, OptsR) -> case Opt of - {verify_fun,{Fun,NewCertNodesFun}} -> + {verify_fun,{Fun,_}} -> case Fun =:= fun ?MODULE:verify_client/3 of - true when is_function(NewCertNodesFun, 1) -> + true -> if - CertNodesFun =:= undefined -> + First -> case inet:peername(Socket) of {ok,{PeerIP,_Port}} -> + {ok,Allowed} = net_kernel:allowed(), + AllowedHosts = allowed_hosts(Allowed), setup_verify_client( - Driver, Socket, Opts, NewCertNodesFun, + Socket, Opts, false, [{verify_fun, - {Fun, - {NewCertNodesFun,Driver,PeerIP}}} + {Fun, {AllowedHosts,PeerIP}}} |OptsR]); {error,Reason} -> exit(trace({no_peername,Reason})) end; true -> setup_verify_client( - Driver, Socket, Opts, CertNodesFun, OptsR) + Socket, Opts, First, OptsR) end; - true -> - exit( - trace( - {verify_client_bad_argument,CertNodesFun})); false -> setup_verify_client( - Driver, Socket, Opts, CertNodesFun, [Opt|OptsR]) + Socket, Opts, First, [Opt|OptsR]) end; _ -> - setup_verify_client( - Driver, Socket, Opts, CertNodesFun, [Opt|OptsR]) + setup_verify_client(Socket, Opts, First, [Opt|OptsR]) + end. + +allowed_hosts(Allowed) -> + lists:usort(allowed_node_hosts(Allowed)). +%% +allowed_node_hosts([]) -> []; +allowed_node_hosts([Node|Allowed]) -> + case dist_util:split_node(Node) of + {node,_,Host} -> + [Host|allowed_node_hosts(Allowed)]; + {host,Host} -> + [Host|allowed_node_hosts(Allowed)]; + _ -> + allowed_node_hosts(Allowed) end. %% Same as verify_peer but check cert host names for @@ -330,48 +339,19 @@ verify_client(_, {extension,_}, S) -> {unknown,S}; verify_client(_, valid, S) -> {valid,S}; -verify_client(PeerCert, valid_peer, {CertNodesFun,Driver,PeerIP} = S) -> - %% - %% Parse out all node names from the peer's certificate - %% - case CertNodesFun(PeerCert) of - undefined -> - %% Certificate allows all nodes +verify_client(_, valid_peer, {[],_} = S) -> + %% Allow all hosts + {valid,S}; +verify_client(PeerCert, valid_peer, {AllowedHosts,PeerIP} = S) -> + case + public_key:pkix_verify_hostname( + PeerCert, + [{ip,PeerIP}|[{dns_id,Host} || Host <- AllowedHosts]]) + of + true -> {valid,S}; - [] -> - %% Certificate allows no nodes - {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}; - _ -> - {valid,S} - end - end. - -%% Filter out the nodes that has got the given IP address -%% -filter_nodes_by_ip(Nodes, Driver, IP) -> - [Node || - Node <- Nodes, - case dist_util:split_node(Node) of - {node,_,Host} -> - filter_host_by_ip(Host, Driver, IP); - {host,Host} -> - filter_host_by_ip(Host, Driver, IP); - {name,_Name} -> - true - end]. - -filter_host_by_ip(Host, Driver, IP) -> - case Driver:getaddr(Host) of - {ok,IP} -> - true; - _ -> - false + false -> + {fail,cert_no_hostname_nor_ip_match} end. @@ -417,19 +397,18 @@ gen_accept_connection( spawn_opt( fun() -> do_accept( - Driver, Kernel, AcceptPid, DistCtrl, - MyNode, Allowed, SetupTime) + Driver, AcceptPid, DistCtrl, + MyNode, Allowed, SetupTime, Kernel) end, [link, {priority, max}])). -do_accept(Driver, Kernel, AcceptPid, DistCtrl, MyNode, Allowed, SetupTime) -> +do_accept( + _Driver, AcceptPid, DistCtrl, MyNode, Allowed, SetupTime, Kernel) -> SslSocket = ssl_connection:get_sslsocket(DistCtrl), receive - {AcceptPid, controller, CertNodesFun} -> + {AcceptPid, controller} -> Timer = dist_util:start_timer(SetupTime), - NewAllowed = - allowed_nodes( - Driver, CertNodesFun, SslSocket, Allowed), + NewAllowed = allowed_nodes(SslSocket, Allowed), HSData0 = hs_data_common(SslSocket), HSData = HSData0#hs_data{ @@ -443,65 +422,67 @@ do_accept(Driver, Kernel, AcceptPid, DistCtrl, MyNode, Allowed, SetupTime) -> 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) -> +allowed_nodes(_SslSocket, []) -> + %% Allow all + []; +allowed_nodes(SslSocket, Allowed) -> case ssl:peercert(SslSocket) of {ok,PeerCertDER} -> case ssl:peername(SslSocket) of {ok,{PeerIP,_Port}} -> - PeerCert = public_key:pkix_decode_cert(PeerCertDER, otp), - %% - %% Parse out all node names from the peer's certificate - %% - case CertNodesFun(PeerCert) of - undefined -> - %% Certificate allows all nodes - Allowed; + PeerCert = + public_key:pkix_decode_cert(PeerCertDER, otp), + case + allowed_nodes( + PeerCert, allowed_hosts(Allowed), PeerIP) + of [] -> - %% Certificate allows no nodes - ?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), - Allowed) + error_logger:error_msg( + "** Connection attempt from " + "disallowed node(s) ~p ** ~n", [PeerIP]), + ?shutdown2( + PeerIP, trace({is_allowed, not_allowed})); + AllowedNodes -> + AllowedNodes end; Error1 -> ?shutdown2(no_peer_ip, trace(Error1)) end; + {error,no_peercert} -> + Allowed; Error2 -> ?shutdown2(no_peer_cert, trace(Error2)) end. -allowed(CertNodes, []) -> - %% Empty allowed list means all allowed - %% -> allow only certificate nodes - CertNodes; -allowed(CertNodes, Allowed) -> - %% Find the intersection of the allowed list and certificate nodes - case - [CertNode || - CertNode <- CertNodes, - dist_util:is_allowed(CertNode, Allowed)] - of - [] -> - error_logger:error_msg( - "** Connection attempt from " - "disallowed node(s) ~p ** ~n", [CertNodes]), - ?shutdown2(CertNodes, trace({is_allowed, not_allowed})); - NewAllowed -> - NewAllowed +allowed_nodes(PeerCert, [], PeerIP) -> + case public_key:pkix_verify_hostname(PeerCert, [{ip,PeerIP}]) of + true -> + Host = inet:ntoa(PeerIP), + true = is_list(Host), + [Host]; + false -> + [] + end; +allowed_nodes(PeerCert, [Node|Allowed], PeerIP) -> + case dist_util:split_node(Node) of + {node,_,Host} -> + allowed_nodes(PeerCert, Allowed, PeerIP, Node, Host); + {host,Host} -> + allowed_nodes(PeerCert, Allowed, PeerIP, Node, Host); + _ -> + allowed_nodes(PeerCert, Allowed, PeerIP) + end. + +allowed_nodes(PeerCert, Allowed, PeerIP, Node, Host) -> + case public_key:pkix_verify_hostname(PeerCert, [{dns_id,Host}]) of + true -> + [Node|allowed_nodes(PeerCert, Allowed, PeerIP)]; + false -> + allowed_nodes(PeerCert, Allowed, PeerIP) end. + setup(Node, Type, MyNode, LongOrShortNames, SetupTime) -> gen_setup(inet_tcp, Node, Type, MyNode, LongOrShortNames, SetupTime). @@ -541,15 +522,7 @@ do_setup(Driver, Kernel, Node, Type, MyNode, LongOrShortNames, SetupTime) -> end. do_setup_connect(Driver, Kernel, Node, Address, Ip, TcpPort, Version, Type, MyNode, Timer) -> - Opts = - trace( - connect_options( - %% - %% Use verify_server/3 to verify that - %% the server's certificate is for Node - %% - setup_verify_server( - get_ssl_options(client), Node))), + Opts = trace(connect_options(get_ssl_options(client))), dist_util:reset_timer(Timer), case ssl:connect( Address, TcpPort, @@ -587,67 +560,6 @@ close(Socket) -> gen_close(Driver, Socket) -> trace(Driver:close(Socket)). -%% {verify_fun,{fun ?MODULE:verify_server/3,_}} is used -%% as a configuration marker that verify_server/3 shall be used. -%% -%% Replace the State in the first occurence of -%% {verify_fun,{fun ?MODULE:verify_server/3,State}} and remove the rest. -%% The inserted state is not accesible from a configuration file -%% since it is dynamic and connection dependent. -%% -setup_verify_server(Opts, Node) -> - setup_verify_server(Opts, Node, true). -%% -setup_verify_server([], _Node, _) -> - []; -setup_verify_server([Opt|Opts], Node, Once) -> - case Opt of - {verify_fun,{Fun,CertNodesFun}} -> - case Fun =:= fun ?MODULE:verify_server/3 of - true when not is_function(CertNodesFun, 1) -> - ?shutdown2( - Node, - {verify_server_bad_argument,CertNodesFun}); - true when Once -> - [{verify_fun,{Fun,{CertNodesFun,Node}}} - |setup_verify_server(Opts, Node, false)]; - true -> - setup_verify_server(Opts, Node, Once); - false -> - [Opt|setup_verify_server(Opts, Node, Once)] - end; - _ -> - [Opt|setup_verify_server(Opts, Node, Once)] - end. - -verify_server(_, {bad_cert,_} = Reason, _) -> - {fail,Reason}; -verify_server(_, {extension,_}, S) -> - {unknown,S}; -verify_server(_, valid, S) -> - {valid,S}; -verify_server(PeerCert, valid_peer, {CertNodesFun,Node} = S) -> - %% - %% Parse out all node names from the peer's certificate - %% - case CertNodesFun(PeerCert) of - undefined -> - %% Certificate allows all nodes - {valid,S}; - [] -> - %% Certificate allows no nodes - {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}; - false -> - {fail,wrong_nodes_in_cert} - end - end. - %% ------------------------------------------------------------ %% Determine if EPMD module supports address resolving. Default diff --git a/lib/ssl/test/ssl_dist_bench_SUITE.erl b/lib/ssl/test/ssl_dist_bench_SUITE.erl index f827ea12bb..66b90a9048 100644 --- a/lib/ssl/test/ssl_dist_bench_SUITE.erl +++ b/lib/ssl/test/ssl_dist_bench_SUITE.erl @@ -117,19 +117,14 @@ init_per_suite(Config) -> ?MODULE_STRING ++ " ROOT CA", CertOptions), SSLConf = [{verify, verify_peer}, - {fail_if_no_peer_cert, true}, {versions, [TLSVersion]}, {ciphers, [TLSCipher]}], ServerConf = - [{verify_fun, - {fun inet_tls_dist:verify_client/3, - fun inet_tls_dist:cert_nodes/1}} - | SSLConf], - ClientConf = - [{verify_fun, - {fun inet_tls_dist:verify_server/3, - fun inet_tls_dist:cert_nodes/1}} + [{fail_if_no_peer_cert, true}, + {verify_fun, + {fun inet_tls_dist:verify_client/3,[]}} | SSLConf], + ClientConf = SSLConf, %% write_node_conf( NodeAConfFile, NodeA, ServerConf, ClientConf, -- cgit v1.2.3