From 45b6afd71bbb7c4b9a0678067bbb7f9276be5605 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Wed, 13 Jan 2016 22:04:00 +0100 Subject: ssl: Handle incomplete and unorded chains If the peer sends an incomplete chain that we can reconstruct with our known CA-certs it will be accepted. We will assume that the peer honors the protocol and sends an orded chain, however if validation fails we will try to order the chain in case it was unorded. Will also handle that extraneous cert where present. See Note form RFC 8446 Note: Prior to TLS 1.3, "certificate_list" ordering required each certificate to certify the one immediately preceding it; however, some implementations allowed some flexibility. Servers sometimes send both a current and deprecated intermediate for transitional purposes, and others are simply configured incorrectly, but these cases can nonetheless be validated properly. For maximum compatibility, all implementations SHOULD be prepared to handle potentially extraneous certificates and arbitrary orderings from any TLS version, with the exception of the end-entity certificate which MUST be first. --- lib/ssl/src/ssl_certificate.erl | 97 ++++++++++++++++++--------- lib/ssl/src/ssl_handshake.erl | 54 ++++++++++++--- lib/ssl/test/ssl_certificate_verify_SUITE.erl | 36 +++++++++- lib/ssl/test/ssl_handshake_SUITE.erl | 26 ++++++- 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()]. %% @@ -186,10 +202,21 @@ public_key_type(?'id-dsa') -> 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), -- cgit v1.2.3