Merge branch 'ingela/ssl/unorded-or-incomplete-cert-chain/OTP-12983/OTP-15060' into maint

* ingela/ssl/unorded-or-incomplete-cert-chain/OTP-12983/OTP-15060:
  ssl: Handle incomplete and unorded chains

4 files changed, 169 insertions, 44 deletions

diff --git a/lib/ssl/src/ssl_certificate.erl b/lib/ssl/src/ssl_certificate.erl
index c15e8a2138..549e557beb 100644
--- a/lib/ssl/src/ssl_certificate.erl
+++ b/lib/ssl/src/ssl_certificate.erl
@@ -33,6 +33,7 @@
 
 -export([trusted_cert_and_path/4,
 	 certificate_chain/3,
+	 certificate_chain/4,
 	 file_to_certificats/2,
 	 file_to_crls/2,
 	 validate/3,
@@ -40,7 +41,8 @@
 	 is_valid_key_usage/2,
 	 select_extension/2,
 	 extensions_list/1,
-	 public_key_type/1
+	 public_key_type/1,
+	 foldl_db/3
 	]).
  
 %%====================================================================
@@ -79,7 +81,8 @@ trusted_cert_and_path(CertChain, CertDbHandle, CertDbRef, PartialChainHandler) -
 		    %% Trusted must be selfsigned or it is an incomplete chain
 		    handle_path(Trusted, Path, PartialChainHandler);
 		_ ->
-		    %% Root CA could not be verified
+		    %% Root CA could not be verified, but partial
+                    %% chain handler may trusted a cert that we got
 		    handle_incomplete_chain(Path, PartialChainHandler)
 	    end
     end.
@@ -94,10 +97,23 @@ certificate_chain(undefined, _, _) ->
     {error, no_cert};
 certificate_chain(OwnCert, CertDbHandle, CertsDbRef) when is_binary(OwnCert) ->
     ErlCert = public_key:pkix_decode_cert(OwnCert, otp),
-    certificate_chain(ErlCert, OwnCert, CertDbHandle, CertsDbRef, [OwnCert]);
+    certificate_chain(ErlCert, OwnCert, CertDbHandle, CertsDbRef, [OwnCert], []);
 certificate_chain(OwnCert, CertDbHandle, CertsDbRef) ->
     DerCert = public_key:pkix_encode('OTPCertificate', OwnCert, otp),
-    certificate_chain(OwnCert, DerCert, CertDbHandle, CertsDbRef, [DerCert]).
+    certificate_chain(OwnCert, DerCert, CertDbHandle, CertsDbRef, [DerCert], []).
+
+%%--------------------------------------------------------------------
+-spec certificate_chain(undefined | binary() | #'OTPCertificate'{} , db_handle(), certdb_ref(), [der_cert()]) ->
+			  {error, no_cert} | {ok, #'OTPCertificate'{} | undefined, [der_cert()]}.
+%%
+%% Description: Create certificate chain with certs from 
+%%--------------------------------------------------------------------
+certificate_chain(Cert, CertDbHandle, CertsDbRef, Candidates) when is_binary(Cert) ->
+    ErlCert = public_key:pkix_decode_cert(Cert, otp),
+    certificate_chain(ErlCert, Cert, CertDbHandle, CertsDbRef, [Cert], Candidates);
+certificate_chain(Cert, CertDbHandle, CertsDbRef, Candidates) ->
+    DerCert = public_key:pkix_encode('OTPCertificate', Cert, otp),
+    certificate_chain(Cert, DerCert, CertDbHandle, CertsDbRef, [DerCert], Candidates).
 %%--------------------------------------------------------------------
 -spec file_to_certificats(binary(), term()) -> [der_cert()].
 %%
@@ -187,9 +203,20 @@ public_key_type(?'id-ecPublicKey') ->
     ec.
 
 %%--------------------------------------------------------------------
+-spec foldl_db(fun(), db_handle() | {extracted, list()}, list()) ->
+ {ok, term()} | issuer_not_found.
+%%
+%% Description:
+%%--------------------------------------------------------------------
+foldl_db(IsIssuerFun, CertDbHandle, []) ->
+    ssl_pkix_db:foldl(IsIssuerFun, issuer_not_found, CertDbHandle);
+foldl_db(IsIssuerFun, _, [_|_] = ListDb) ->
+    lists:foldl(IsIssuerFun, issuer_not_found, ListDb).
+
+%%--------------------------------------------------------------------
 %%% Internal functions
 %%--------------------------------------------------------------------
-certificate_chain(OtpCert, BinCert, CertDbHandle, CertsDbRef, Chain) ->
+certificate_chain(OtpCert, BinCert, CertDbHandle, CertsDbRef, Chain, ListDb) ->
     IssuerAndSelfSigned = 
 	case public_key:pkix_is_self_signed(OtpCert) of
 	    true ->
@@ -200,12 +227,12 @@ certificate_chain(OtpCert, BinCert, CertDbHandle, CertsDbRef, Chain) ->
     
     case IssuerAndSelfSigned of 
 	{_, true = SelfSigned} ->
-	    certificate_chain(CertDbHandle, CertsDbRef, Chain, ignore, ignore, SelfSigned);
+	    do_certificate_chain(CertDbHandle, CertsDbRef, Chain, ignore, ignore, SelfSigned, ListDb);
 	{{error, issuer_not_found}, SelfSigned} ->
-	    case find_issuer(OtpCert, BinCert, CertDbHandle, CertsDbRef) of
+	    case find_issuer(OtpCert, BinCert, CertDbHandle, CertsDbRef, ListDb) of
 		{ok, {SerialNr, Issuer}} ->
-		    certificate_chain(CertDbHandle, CertsDbRef, Chain,
-				      SerialNr, Issuer, SelfSigned);
+		    do_certificate_chain(CertDbHandle, CertsDbRef, Chain,
+					 SerialNr, Issuer, SelfSigned, ListDb);
 		_ ->
 		    %% Guess the the issuer must be the root
 		    %% certificate. The verification of the
@@ -214,19 +241,19 @@ certificate_chain(OtpCert, BinCert, CertDbHandle, CertsDbRef, Chain) ->
 		    {ok, undefined, lists:reverse(Chain)}
 	    end;
 	{{ok, {SerialNr, Issuer}}, SelfSigned} -> 
-	    certificate_chain(CertDbHandle, CertsDbRef, Chain, SerialNr, Issuer, SelfSigned)
+	    do_certificate_chain(CertDbHandle, CertsDbRef, Chain, SerialNr, Issuer, SelfSigned, ListDb)
     end.
   
-certificate_chain(_, _, [RootCert | _] = Chain, _, _, true) ->	  
+do_certificate_chain(_, _, [RootCert | _] = Chain, _, _, true, _) ->	  
     {ok, RootCert, lists:reverse(Chain)};		      
 
-certificate_chain(CertDbHandle, CertsDbRef, Chain, SerialNr, Issuer, _SelfSigned) ->
+do_certificate_chain(CertDbHandle, CertsDbRef, Chain, SerialNr, Issuer, _, ListDb) ->
     case ssl_manager:lookup_trusted_cert(CertDbHandle, CertsDbRef,
 						SerialNr, Issuer) of
 	{ok, {IssuerCert, ErlCert}} ->
 	    ErlCert = public_key:pkix_decode_cert(IssuerCert, otp),
 	    certificate_chain(ErlCert, IssuerCert, 
-			      CertDbHandle, CertsDbRef, [IssuerCert | Chain]);
+			      CertDbHandle, CertsDbRef, [IssuerCert | Chain], ListDb);
 	_ ->
 	    %% The trusted cert may be obmitted from the chain as the
 	    %% counter part needs to have it anyway to be able to
@@ -234,7 +261,8 @@ certificate_chain(CertDbHandle, CertsDbRef, Chain, SerialNr, Issuer, _SelfSigned
 	    {ok, undefined, lists:reverse(Chain)}		      
     end.
 
-find_issuer(OtpCert, BinCert, CertDbHandle, CertsDbRef) ->
+
+find_issuer(OtpCert, BinCert, CertDbHandle, CertsDbRef, ListDb) ->
     IsIssuerFun =
 	fun({_Key, {_Der, #'OTPCertificate'{} = ErlCertCandidate}}, Acc) ->
 		case public_key:pkix_is_issuer(OtpCert, ErlCertCandidate) of
@@ -252,26 +280,29 @@ find_issuer(OtpCert, BinCert, CertDbHandle, CertsDbRef) ->
 		Acc
 	end,
 
-    if is_reference(CertsDbRef) -> % actual DB exists
-	try ssl_pkix_db:foldl(IsIssuerFun, issuer_not_found, CertDbHandle) of
-	    issuer_not_found ->
-		{error, issuer_not_found}
-	catch
-	    {ok, _IssuerId} = Return ->
-		Return
-	end;
-       is_tuple(CertsDbRef), element(1,CertsDbRef) =:= extracted -> % cache bypass byproduct
-	{extracted, CertsData} = CertsDbRef,
-	DB = [Entry || {decoded, Entry} <- CertsData],
-	try lists:foldl(IsIssuerFun, issuer_not_found, DB) of
-	    issuer_not_found ->
-		{error, issuer_not_found}
-	catch
-	    {ok, _IssuerId} = Return ->
-		Return
-	end
+    Result = case is_reference(CertsDbRef) of
+		 true ->
+		     do_find_issuer(IsIssuerFun, CertDbHandle, ListDb); 
+		 false ->
+		     {extracted, CertsData} = CertsDbRef,
+		     DB = [Entry || {decoded, Entry} <- CertsData],
+		     do_find_issuer(IsIssuerFun, CertDbHandle, DB)
+	     end,
+    case Result of
+        issuer_not_found ->
+	    {error, issuer_not_found};
+	Result ->
+	    Result
     end.
 
+do_find_issuer(IssuerFun, CertDbHandle, CertDb) ->
+    try 
+	foldl_db(IssuerFun, CertDbHandle, CertDb)
+    catch
+	throw:{ok, _} = Return ->
+	    Return
+    end.
+	
 is_valid_extkey_usage(KeyUse, client) ->
     %% Client wants to verify server
     is_valid_key_usage(KeyUse,?'id-kp-serverAuth');
@@ -300,7 +331,7 @@ other_issuer(OtpCert, BinCert, CertDbHandle, CertDbRef) ->
 	{ok, IssuerId} ->
 	    {other, IssuerId};
 	{error, issuer_not_found} ->
-	    case find_issuer(OtpCert, BinCert, CertDbHandle, CertDbRef) of
+	    case find_issuer(OtpCert, BinCert, CertDbHandle, CertDbRef, []) of
 		{ok, IssuerId} ->
 		    {other, IssuerId};
 		Other ->
diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index 3888f9dcf6..dc89fb0029 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -338,7 +338,7 @@ certify(#certificate{asn1_certificates = ASN1Certs}, CertDbHandle, CertDbRef,
         Opts, CRLDbHandle, Role, Host) ->    
 
     ServerName = server_name(Opts#ssl_options.server_name_indication, Host, Role),
-    [PeerCert | _] = ASN1Certs,       
+    [PeerCert | ChainCerts ] = ASN1Certs,       
     try
 	{TrustedCert, CertPath}  =
 	    ssl_certificate:trusted_cert_and_path(ASN1Certs, CertDbHandle, CertDbRef,  
@@ -347,14 +347,14 @@ certify(#certificate{asn1_certificates = ASN1Certs}, CertDbHandle, CertDbRef,
                                                          CertDbHandle, CertDbRef, ServerName,
                                                          Opts#ssl_options.customize_hostname_check,
                                                          Opts#ssl_options.crl_check, CRLDbHandle, CertPath),
-	case public_key:pkix_path_validation(TrustedCert,
-					     CertPath,
-					     [{max_path_length,  Opts#ssl_options.depth},
-					      {verify_fun, ValidationFunAndState}]) of
+        Options = [{max_path_length, Opts#ssl_options.depth},
+                   {verify_fun, ValidationFunAndState}],
+	case public_key:pkix_path_validation(TrustedCert, CertPath, Options) of
 	    {ok, {PublicKeyInfo,_}} ->
 		{PeerCert, PublicKeyInfo};
 	    {error, Reason} ->
-		path_validation_alert(Reason)
+		handle_path_validation_error(Reason, PeerCert, ChainCerts, Opts, Options, 
+                                             CertDbHandle, CertDbRef)
 	end
     catch
 	error:{badmatch,{asn1, Asn1Reason}} ->
@@ -363,7 +363,6 @@ certify(#certificate{asn1_certificates = ASN1Certs}, CertDbHandle, CertDbRef,
         error:OtherReason ->
             ?ALERT_REC(?FATAL, ?INTERNAL_ERROR, {unexpected_error, OtherReason})
     end.
-
 %%--------------------------------------------------------------------
 -spec certificate_verify(binary(), public_key_info(), ssl_record:ssl_version(), term(),
 			 binary(), ssl_handshake_history()) -> valid | #alert{}.
@@ -1312,6 +1311,45 @@ apply_user_fun(Fun, OtpCert, ExtensionOrError, UserState0, SslState, _CertPath)
 	    {unknown, {SslState, UserState}}
     end.
 
+handle_path_validation_error({bad_cert, unknown_ca} = Reason, PeerCert, Chain,  
+                             Opts, Options, CertDbHandle, CertsDbRef) ->
+    handle_incomplete_chain(PeerCert, Chain, Opts, Options, CertDbHandle, CertsDbRef, Reason);
+handle_path_validation_error({bad_cert, invalid_issuer} = Reason, PeerCert, Chain0, 
+			     Opts, Options, CertDbHandle, CertsDbRef) ->
+    case ssl_certificate:certificate_chain(PeerCert, CertDbHandle, CertsDbRef, Chain0) of
+	{ok, _, [PeerCert | Chain] = OrdedChain} when  Chain =/= Chain0 -> %% Chain appaears to be unorded 
+            {Trusted, Path} = ssl_certificate:trusted_cert_and_path(OrdedChain,
+                                                                    CertDbHandle, CertsDbRef,
+                                                                    Opts#ssl_options.partial_chain),
+            case public_key:pkix_path_validation(Trusted, Path, Options) of
+		{ok, {PublicKeyInfo,_}} ->
+		    {PeerCert, PublicKeyInfo};
+                {error, PathError} ->
+		    handle_path_validation_error(PathError, PeerCert, Path,
+                                                 Opts, Options, CertDbHandle, CertsDbRef)
+	    end;
+        _ ->
+            path_validation_alert(Reason)
+    end;
+handle_path_validation_error(Reason, _, _, _, _,_, _) ->
+    path_validation_alert(Reason).
+
+handle_incomplete_chain(PeerCert, Chain0, Opts, Options, CertDbHandle, CertsDbRef, PathError0) ->
+    case ssl_certificate:certificate_chain(PeerCert, CertDbHandle, CertsDbRef) of
+        {ok, _, [PeerCert | _] = Chain} when Chain =/= Chain0 -> %% Chain candidate found          
+            {Trusted, Path} = ssl_certificate:trusted_cert_and_path(Chain,
+                                                                    CertDbHandle, CertsDbRef,
+                                                                    Opts#ssl_options.partial_chain),
+            case public_key:pkix_path_validation(Trusted, Path, Options) of
+		{ok, {PublicKeyInfo,_}} ->
+		    {PeerCert, PublicKeyInfo};
+                {error, PathError} ->
+		    path_validation_alert(PathError)
+	    end;
+        _ ->
+            path_validation_alert(PathError0)
+    end.
+
 path_validation_alert({bad_cert, cert_expired}) ->
     ?ALERT_REC(?FATAL, ?CERTIFICATE_EXPIRED);
 path_validation_alert({bad_cert, invalid_issuer}) ->
@@ -1324,8 +1362,6 @@ path_validation_alert({bad_cert, unknown_critical_extension}) ->
     ?ALERT_REC(?FATAL, ?UNSUPPORTED_CERTIFICATE);
 path_validation_alert({bad_cert, {revoked, _}}) ->
     ?ALERT_REC(?FATAL, ?CERTIFICATE_REVOKED);
-%%path_validation_alert({bad_cert, revocation_status_undetermined}) ->
-%%   ?ALERT_REC(?FATAL, ?BAD_CERTIFICATE);
 path_validation_alert({bad_cert, {revocation_status_undetermined, Details}}) ->
     Alert = ?ALERT_REC(?FATAL, ?BAD_CERTIFICATE),
     Alert#alert{reason = Details};
diff --git a/lib/ssl/test/ssl_certificate_verify_SUITE.erl b/lib/ssl/test/ssl_certificate_verify_SUITE.erl
index 63e9d07d0b..b387feb97a 100644
--- a/lib/ssl/test/ssl_certificate_verify_SUITE.erl
+++ b/lib/ssl/test/ssl_certificate_verify_SUITE.erl
@@ -88,7 +88,8 @@ tests() ->
      critical_extension_verify_client,
      critical_extension_verify_server,
      critical_extension_verify_none,
-     customize_hostname_check
+     customize_hostname_check,
+     incomplete_chain
     ].
 
 error_handling_tests()->
@@ -1198,6 +1199,39 @@ customize_hostname_check(Config) when is_list(Config) ->
     ssl_test_lib:close(Server),
     ssl_test_lib:close(Client).
 
+incomplete_chain() ->
+    [{doc,"Test option verify_peer"}].
+incomplete_chain(Config) when is_list(Config) ->
+    DefConf = ssl_test_lib:default_cert_chain_conf(),
+    CertChainConf = ssl_test_lib:gen_conf(rsa, rsa, DefConf, DefConf),
+    #{server_config := ServerConf,
+      client_config := ClientConf} = public_key:pkix_test_data(CertChainConf),
+    [ServerRoot| _] = ServerCas = proplists:get_value(cacerts, ServerConf),
+    ClientCas = proplists:get_value(cacerts, ClientConf),
+
+    Active = proplists:get_value(active, Config),
+    ReceiveFunction =  proplists:get_value(receive_function, Config),
+    {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+    Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+					{from, self()},
+                                        {mfa, {ssl_test_lib, ReceiveFunction, []}},
+                                        {options, [{active, Active}, {verify, verify_peer},
+                                                   {cacerts, [ServerRoot]} |  
+                                                   proplists:delete(cacerts, ServerConf)]}]),
+    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, ReceiveFunction, []}},
+                                        {options, [{active, Active}, 
+                                                   {verify, verify_peer},
+                                                   {cacerts,  ServerCas ++ ClientCas} | 
+                                                   proplists:delete(cacerts, ClientConf)]}]),
+    ssl_test_lib:check_result(Server, ok, Client, ok),
+    ssl_test_lib:close(Server),
+    ssl_test_lib:close(Client).
+
+
 %%--------------------------------------------------------------------
 %% Internal functions ------------------------------------------------
 %%--------------------------------------------------------------------
diff --git a/lib/ssl/test/ssl_handshake_SUITE.erl b/lib/ssl/test/ssl_handshake_SUITE.erl
index 9ae04184e2..b8b9989d30 100644
--- a/lib/ssl/test/ssl_handshake_SUITE.erl
+++ b/lib/ssl/test/ssl_handshake_SUITE.erl
@@ -40,7 +40,8 @@ all() -> [decode_hello_handshake,
 	  decode_single_hello_sni_extension_correctly,
 	  decode_empty_server_sni_correctly,
 	  select_proper_tls_1_2_rsa_default_hashsign,
-	  ignore_hassign_extension_pre_tls_1_2].
+	  ignore_hassign_extension_pre_tls_1_2,
+          unorded_chain].
 
 %%--------------------------------------------------------------------
 init_per_suite(Config) ->
@@ -173,6 +174,29 @@ ignore_hassign_extension_pre_tls_1_2(Config) ->
     {md5sha, rsa} = ssl_handshake:select_hashsign(HashSigns, Cert, ecdhe_rsa, tls_v1:default_signature_algs({3,2}), {3,2}),
     {md5sha, rsa} = ssl_handshake:select_hashsign(HashSigns, Cert, ecdhe_rsa, tls_v1:default_signature_algs({3,0}), {3,0}).
 
+unorded_chain(Config) when is_list(Config) ->
+    DefConf = ssl_test_lib:default_cert_chain_conf(),
+    CertChainConf = ssl_test_lib:gen_conf(rsa, rsa, DefConf, DefConf),
+    #{server_config := ServerConf,
+      client_config := _ClientConf} = public_key:pkix_test_data(CertChainConf),
+    PeerCert = proplists:get_value(cert, ServerConf),
+    CaCerts = [_, C1, C2] = proplists:get_value(cacerts, ServerConf),
+    {ok,  ExtractedCerts} = ssl_pkix_db:extract_trusted_certs({der, CaCerts}),
+    UnordedChain = case public_key:pkix_is_self_signed(C1) of
+                       true ->
+                           [C1, C2];
+                       false ->
+                           [C2, C1]
+                   end,
+    OrderedChain = [PeerCert | lists:reverse(UnordedChain)],
+    {ok, _, OrderedChain} = 
+        ssl_certificate:certificate_chain(PeerCert, ets:new(foo, []), ExtractedCerts, UnordedChain).
+
+
+%%--------------------------------------------------------------------
+%% Internal functions ------------------------------------------------
+%%--------------------------------------------------------------------
+
 is_supported(Hash) ->
     Algos = crypto:supports(),
     Hashs = proplists:get_value(hashs, Algos),