From 0bb96516ce308b6fb837696338b492d3c9a9f429 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Fri, 6 Oct 2017 17:24:16 +0200 Subject: ssl: Extend hostname check to fallback to checking IP-address If no SNI is available and the hostname is an IP-address also check for IP-address match. This check is not as good as a DNS hostname check and certificates using IP-address are not recommended. --- lib/ssl/doc/src/ssl.xml | 52 ++++++++++++++------- lib/ssl/src/ssl.erl | 2 +- lib/ssl/src/ssl_certificate.erl | 38 ++++++++++++--- lib/ssl/src/ssl_connection.erl | 3 +- lib/ssl/src/ssl_handshake.erl | 18 ++++++-- lib/ssl/test/ssl_sni_SUITE.erl | 100 +++++++++++++++++++++++++++++++++++++++- 6 files changed, 182 insertions(+), 31 deletions(-) diff --git a/lib/ssl/doc/src/ssl.xml b/lib/ssl/doc/src/ssl.xml index ca2dcbb761..e80fd59a7f 100644 --- a/lib/ssl/doc/src/ssl.xml +++ b/lib/ssl/doc/src/ssl.xml @@ -589,22 +589,19 @@ fun(srp, Username :: string(), UserState :: term()) -> {server_name_indication, HostName :: hostname()}

Specify the hostname to be used in TLS Server Name Indication extension. - Is usefull when upgrading a TCP socket to a TLS socket or if the hostname can not be - derived from the Host argument to ssl:connect/3. - Will also cause the client to preform host name verification of the peer certificate - public_key:pkix_verify_hostname(PeerCert, [{dns_id, HostName}]) -

during the x509-path validation. If the check fails the error {bad_cert, hostname_check_failiure} will be - propagated to the path validation fun verify_fun - - - {server_name_indication, disable} - -

When starting a TLS connection without upgrade, the Server Name - Indication extension is sent if possible that is can be derived from the Host argument - to ssl:connect/3. - This option can be used to disable that behavior.

-

Note that this also disables the default host name verification check of the peer certificate.

 + If not specified it will default to the Host argument of connect/[3,4] + unless it is of type inet:ipaddress().

+

+ The HostName will also be used in the hostname verification of the peer certificate using + public_key:pkix_verify_hostname/2. +

+ {server_name_indication, disable} + +

Prevents the Server Name Indication extension from being sent and + disables the hostname verification check + public_key:pkix_verify_hostname/2

+ {fallback, boolean()}

Send special cipher suite TLS_FALLBACK_SCSV to avoid undesired TLS version downgrade. @@ -881,6 +878,12 @@ fun(srp, Username :: string(), UserState :: term()) ->

Upgrades a gen_tcp, or equivalent, connected socket to an SSL socket, that is, performs the client-side ssl handshake.

+ +

If the option verify is set to verify_peer + the option server_name_indication shall also be specified, + if it is not no Server Name Indication extension will be sent, + and public_key:pkix_verify_hostname/2 + will be called with the IP-address of the connection as ReferenceID, which is proably not what you want.

 @@ -897,7 +900,24 @@ fun(srp, Username :: string(), UserState :: term()) -> SslSocket = sslsocket() Reason = term() -

Opens an SSL connection to Host, Port.

 +

Opens an SSL connection to Host, Port.

+ +

When the option verify is set to verify_peer the check + public_key:pkix_verify_hostname/2 + will be performed in addition to the usual x509-path validation checks. If the check fails the error {bad_cert, hostname_check_failed} will + be propagated to the path validation fun verify_fun, where it is possible to do customized + checks by using the full possibilitis of the public_key:pkix_verify_hostname/2 API. + + When the option server_name_indication is provided, its value (the DNS name) will be used as ReferenceID + to public_key:pkix_verify_hostname/2. + When no server_name_indication option is given, the Host argument will be used as + Server Name Indication extension. The Host argument will also be used for the + public_key:pkix_verify_hostname/2 check and if the Host + argument is an inet:ip_address() the ReferenceID used for the check will be {ip, Host} otherwise + dns_id will be assumed with a fallback to ip if that fails.

+

According to good practices certificates should not use IP-addresses as "server names". It would + be very surprising if this happen outside a closed network.

+ diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl index 4e592c02ec..054e3b7ae3 100644 --- a/lib/ssl/src/ssl.erl +++ b/lib/ssl/src/ssl.erl @@ -1003,7 +1003,7 @@ validate_option(server_name_indication = Opt, Value) when is_list(Value) -> validate_option(server_name_indication, undefined = Value) -> Value; validate_option(server_name_indication, disable) -> - undefined; + disable; validate_option(sni_hosts, []) -> []; diff --git a/lib/ssl/src/ssl_certificate.erl b/lib/ssl/src/ssl_certificate.erl index 0dd5e5c5cf..a3333d35e9 100644 --- a/lib/ssl/src/ssl_certificate.erl +++ b/lib/ssl/src/ssl_certificate.erl @@ -138,13 +138,8 @@ validate(_, {bad_cert, _} = Reason, _) -> {fail, Reason}; validate(_, valid, UserState) -> {valid, UserState}; -validate(Cert, valid_peer, UserState = {client, _,_, Hostname, _, _}) when Hostname =/= undefined -> - case public_key:pkix_verify_hostname(Cert, [{dns_id, Hostname}]) of - true -> - {valid, UserState}; - false -> - {fail, {bad_cert, hostname_check_failed}} - end; +validate(Cert, valid_peer, UserState = {client, _,_, Hostname, _, _}) when Hostname =/= disable -> + verify_hostname(Hostname, Cert, UserState); validate(_, valid_peer, UserState) -> {valid, UserState}. @@ -337,3 +332,32 @@ new_trusteded_chain(DerCert, [_ | Rest]) -> new_trusteded_chain(DerCert, Rest); new_trusteded_chain(_, []) -> unknown_ca. + +verify_hostname({fallback, Hostname}, Cert, UserState) when is_list(Hostname) -> + case public_key:pkix_verify_hostname(Cert, [{dns_id, Hostname}]) of + true -> + {valid, UserState}; + false -> + case public_key:pkix_verify_hostname(Cert, [{ip, Hostname}]) of + true -> + {valid, UserState}; + false -> + {fail, {bad_cert, hostname_check_failed}} + end + end; + +verify_hostname({fallback, Hostname}, Cert, UserState) -> + case public_key:pkix_verify_hostname(Cert, [{ip, Hostname}]) of + true -> + {valid, UserState}; + false -> + {fail, {bad_cert, hostname_check_failed}} + end; + +verify_hostname(Hostname, Cert, UserState) -> + case public_key:pkix_verify_hostname(Cert, [{dns_id, Hostname}]) of + true -> + {valid, UserState}; + false -> + {fail, {bad_cert, hostname_check_failed}} + end. diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index 2dbe08e0a7..2fed7d864f 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -496,12 +496,13 @@ certify(internal, #certificate{}, certify(internal, #certificate{} = Cert, #state{negotiated_version = Version, role = Role, + host = Host, cert_db = CertDbHandle, cert_db_ref = CertDbRef, crl_db = CRLDbInfo, ssl_options = Opts} = State, Connection) -> case ssl_handshake:certify(Cert, CertDbHandle, CertDbRef, - Opts, CRLDbInfo, Role) of + Opts, CRLDbInfo, Role, Host) of {PeerCert, PublicKeyInfo} -> handle_peer_cert(Role, PeerCert, PublicKeyInfo, State#state{client_certificate_requested = false}, Connection); diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index b1661624b5..0ee9ee3322 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -50,7 +50,7 @@ finished/5, next_protocol/1]). %% Handle handshake messages --export([certify/6, client_certificate_verify/6, certificate_verify/6, verify_signature/5, +-export([certify/7, client_certificate_verify/6, certificate_verify/6, verify_signature/5, master_secret/4, server_key_exchange_hash/2, verify_connection/6, init_handshake_history/0, update_handshake_history/3, verify_server_key/5 ]). @@ -389,21 +389,21 @@ verify_signature(_, Hash, {HashAlgo, _SignAlg}, Signature, %%-------------------------------------------------------------------- -spec certify(#certificate{}, db_handle(), certdb_ref(), #ssl_options{}, term(), - client | server) -> {der_cert(), public_key_info()} | #alert{}. + client | server, inet:hostname() | inet:ip_address()) -> {der_cert(), public_key_info()} | #alert{}. %% %% Description: Handles a certificate handshake message %%-------------------------------------------------------------------- certify(#certificate{asn1_certificates = ASN1Certs}, CertDbHandle, CertDbRef, - Opts, CRLDbHandle, Role) -> + Opts, CRLDbHandle, Role, Host) -> + ServerName = server_name(Opts#ssl_options.server_name_indication, Host, Role), [PeerCert | _] = ASN1Certs, try {TrustedCert, CertPath} = ssl_certificate:trusted_cert_and_path(ASN1Certs, CertDbHandle, CertDbRef, Opts#ssl_options.partial_chain), ValidationFunAndState = validation_fun_and_state(Opts#ssl_options.verify_fun, Role, - CertDbHandle, CertDbRef, - Opts#ssl_options.server_name_indication, + CertDbHandle, CertDbRef, ServerName, Opts#ssl_options.crl_check, CRLDbHandle, CertPath), case public_key:pkix_path_validation(TrustedCert, CertPath, @@ -1528,6 +1528,8 @@ select_shared_curve([Curve | Rest], Curves) -> sni(undefined) -> undefined; +sni(disable) -> + undefined; sni(Hostname) -> #sni{hostname = Hostname}. @@ -2353,3 +2355,9 @@ available_signature_algs(#hash_sign_algos{hash_sign_algos = ClientHashSigns}, Su available_signature_algs(_, _, _, _) -> undefined. +server_name(_, _, server) -> + undefined; %% Not interesting to check your own name. +server_name(undefined, Host, client) -> + {fallback, Host}; %% Fallback to Host argument to connect +server_name(SNI, _, client) -> + SNI. %% If Server Name Indication is available diff --git a/lib/ssl/test/ssl_sni_SUITE.erl b/lib/ssl/test/ssl_sni_SUITE.erl index 03676cb828..e080de95f6 100644 --- a/lib/ssl/test/ssl_sni_SUITE.erl +++ b/lib/ssl/test/ssl_sni_SUITE.erl @@ -25,6 +25,8 @@ -include_lib("common_test/include/ct.hrl"). -include_lib("public_key/include/public_key.hrl"). +-include_lib("kernel/include/inet.hrl"). + %%-------------------------------------------------------------------- %% Common Test interface functions ----------------------------------- @@ -55,7 +57,10 @@ sni_tests() -> sni_no_match, no_sni_header_fun, sni_match_fun, - sni_no_match_fun]. + sni_no_match_fun, + dns_name, + ip_fallback, + no_ip_fallback]. init_per_suite(Config0) -> catch crypto:stop(), @@ -112,6 +117,65 @@ sni_no_match(Config) -> sni_no_match_fun(Config) -> run_sni_fun_handshake(Config, "c.server", undefined, "server Peer cert"). +dns_name(Config) -> + Hostname = "OTP.test.server", + #{server_config := ServerConf, + client_config := ClientConf} = public_key:pkix_test_data(#{server_chain => + #{root => [], + intermediates => [[]], + peer => [{extensions, [#'Extension'{extnID = + ?'id-ce-subjectAltName', + extnValue = [{dNSName, Hostname}], + critical = false}]}]}, + client_chain => + #{root => [], + intermediates => [[]], + peer => []}}), + unsuccessfull_connect(ServerConf, [{verify, verify_peer} | ClientConf], undefined, Config), + successfull_connect(ServerConf, [{verify, verify_peer}, {server_name_indication, Hostname} | ClientConf], undefined, Config), + unsuccessfull_connect(ServerConf, [{verify, verify_peer}, {server_name_indication, "foo"} | ClientConf], undefined, Config), + successfull_connect(ServerConf, [{verify, verify_peer}, {server_name_indication, disable} | ClientConf], undefined, Config). + +ip_fallback(Config) -> + Hostname = net_adm:localhost(), + {ok, #hostent{h_addr_list = [IP |_]}} = inet:gethostbyname(net_adm:localhost()), + IPStr = tuple_to_list(IP), + #{server_config := ServerConf, + client_config := ClientConf} = public_key:pkix_test_data(#{server_chain => + #{root => [], + intermediates => [[]], + peer => [{extensions, [#'Extension'{extnID = + ?'id-ce-subjectAltName', + extnValue = [{dNSName, Hostname}, + {iPAddress, IPStr}], + critical = false}]} + ]}, + client_chain => + #{root => [], + intermediates => [[]], + peer => []}}), + successfull_connect(ServerConf, [{verify, verify_peer} | ClientConf], Hostname, Config), + successfull_connect(ServerConf, [{verify, verify_peer} | ClientConf], IP, Config). + +no_ip_fallback(Config) -> + Hostname = net_adm:localhost(), + {ok, #hostent{h_addr_list = [IP |_]}} = inet:gethostbyname(net_adm:localhost()), + #{server_config := ServerConf, + client_config := ClientConf} = public_key:pkix_test_data(#{server_chain => + #{root => [], + intermediates => [[]], + peer => [{extensions, [#'Extension'{extnID = + ?'id-ce-subjectAltName', + extnValue = [{dNSName, Hostname}], + critical = false}]} + ]}, + client_chain => + #{root => [], + intermediates => [[]], + peer => []}}), + successfull_connect(ServerConf, [{verify, verify_peer} | ClientConf], Hostname, Config), + unsuccessfull_connect(ServerConf, [{verify, verify_peer} | ClientConf], IP, Config). + %%-------------------------------------------------------------------- %% Internal Functions ------------------------------------------------ @@ -217,3 +281,37 @@ run_handshake(Config, SNIHostname, ExpectedSNIHostname, ExpectedCN) -> ssl_test_lib:check_result(Server, ExpectedSNIHostname, Client, ExpectedCN), ssl_test_lib:close(Server), ssl_test_lib:close(Client). + +successfull_connect(ServerOptions, ClientOptions, Hostname0, Config) -> + {ClientNode, ServerNode, Hostname1} = ssl_test_lib:run_where(Config), + Hostname = host_name(Hostname0, Hostname1), + Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, + {from, self()}, {mfa, {ssl_test_lib, send_recv_result_active, []}}, + {options, ServerOptions}]), + 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, send_recv_result_active, []}}, + {options, ClientOptions}]), + ssl_test_lib:check_result(Server, ok, Client, ok), + ssl_test_lib:close(Server), + ssl_test_lib:close(Client). + +unsuccessfull_connect(ServerOptions, ClientOptions, Hostname0, Config) -> + {ClientNode, ServerNode, Hostname1} = ssl_test_lib:run_where(Config), + Hostname = host_name(Hostname0, Hostname1), + Server = ssl_test_lib:start_server_error([{node, ServerNode}, {port, 0}, + {from, self()}, + {options, ServerOptions}]), + Port = ssl_test_lib:inet_port(Server), + Client = ssl_test_lib:start_client_error([{node, ClientNode}, {port, Port}, + {host, Hostname}, + {from, self()}, + {options, ClientOptions}]), + + ssl_test_lib:check_result(Server, {error, {tls_alert, "handshake failure"}}, + Client, {error, {tls_alert, "handshake failure"}}). +host_name(undefined, Hostname) -> + Hostname; +host_name(Hostname, _) -> + Hostname. -- cgit v1.2.3