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.

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()) ->
 
       <tag><c>{server_name_indication, HostName :: hostname()}</c></tag>
       <item><p>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 <seealso marker="ssl#connect-3">ssl:connect/3</seealso>.
-      Will also cause the client to preform host name verification of the peer certificate
-      <seealso marker="public_key:public_key#pkix_verify_hostname-2">public_key:pkix_verify_hostname(PeerCert, [{dns_id, HostName}])</seealso>
-      </p> during the x509-path validation. If the check fails the error {bad_cert, hostname_check_failiure} will be
-      propagated to the path validation fun <seealso marker="#verify_fun">verify_fun</seealso>
-      </item>
-
-	<tag><c>{server_name_indication, disable}</c></tag>
-      <item>
-        <p>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 <seealso marker="ssl#connect-3">ssl:connect/3</seealso>.  
-	This option can be used to disable that behavior.</p>
-	<note><p> Note that this also disables the default host name verification check of the peer certificate.</p></note>
+      If not specified it will default to the <c>Host</c> argument of <seealso marker="#connect-3">connect/[3,4]</seealso>
+      unless it is of type inet:ipaddress().</p>
+      <p>
+	The <c>HostName</c> will also be used in the hostname verification of the peer certificate using
+	<seealso marker="public_key:public_key#pkix_verify_hostname-2">public_key:pkix_verify_hostname/2</seealso>.
+      </p>
       </item>
+      <tag><c>{server_name_indication, disable}</c></tag>
+	<item>
+          <p> Prevents the Server Name Indication extension from being sent and
+	  disables the hostname verification check
+	  <seealso marker="public_key:public_key#pkix_verify_hostname-2">public_key:pkix_verify_hostname/2</seealso> </p>
+	</item>
       <tag><c>{fallback, boolean()}</c></tag>
       <item>
 	<p> Send special cipher suite TLS_FALLBACK_SCSV to avoid undesired TLS version downgrade.
@@ -881,6 +878,12 @@ fun(srp, Username :: string(), UserState :: term()) ->
       <desc><p>Upgrades a <c>gen_tcp</c>, or equivalent,
 	  connected socket to an SSL socket, that is, performs the
 	  client-side ssl handshake.</p>
+	  
+	  <note><p>If the option <c>verify</c> is set to <c>verify_peer</c>
+	  the option <c>server_name_indication</c> shall also be specified,
+	  if it is not no Server Name Indication extension will be sent,
+	  and <seealso marker="public_key:public_key#pkix_verify_hostname-2">public_key:pkix_verify_hostname/2</seealso>
+	  will be called with the IP-address of the connection as <c>ReferenceID</c>, which is proably not what you want.</p></note>
     </desc>
     </func>
 
@@ -897,7 +900,24 @@ fun(srp, Username :: string(), UserState :: term()) ->
 	  <v>SslSocket = sslsocket()</v>
 	  <v>Reason = term()</v>
       </type>
-      <desc><p>Opens an SSL connection to <c>Host</c>, <c>Port</c>.</p></desc>
+      <desc><p>Opens an SSL connection to <c>Host</c>, <c>Port</c>.</p>
+      
+      <p> When the option <c>verify</c> is set to <c>verify_peer</c> the check 
+      <seealso marker="public_key:public_key#pkix_verify_hostname-2">public_key:pkix_verify_hostname/2</seealso> 
+      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 <seealso marker="#verify_fun">verify_fun</seealso>, where it is possible to do customized
+      checks by using the full possibilitis of the <seealso marker="public_key:public_key#pkix_verify_hostname-2">public_key:pkix_verify_hostname/2</seealso> API.
+      
+      When the option <c>server_name_indication</c> is provided, its value (the DNS name) will be used as <c>ReferenceID</c>
+      to <seealso marker="public_key:public_key#pkix_verify_hostname-2">public_key:pkix_verify_hostname/2</seealso>.
+      When no <c>server_name_indication</c> option is given, the <c>Host</c> argument will be used as
+      Server Name Indication extension. The <c>Host</c> argument will also be used for the
+      <seealso marker="public_key:public_key#pkix_verify_hostname-2">public_key:pkix_verify_hostname/2</seealso> check and if the <c>Host</c>
+      argument is an <c>inet:ip_address()</c> the <c>ReferenceID</c> used for the check will be <c>{ip, Host}</c> otherwise
+      <c>dns_id</c> will be assumed with a fallback to <c>ip</c> if that fails. </p>
+      <note><p>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. </p></note> 
+      </desc>
     </func>
 
     <func>
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.