From a6de8740405037bad55c09089f1d69c8c5511d6c Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Fri, 27 Aug 2010 10:06:22 +0200 Subject: Empty certificate chain Handling of unkown CA certificats was changed in ssl and public_key to work as intended. In the process of doing this some test cases has been corrected as they where wrong but happened to work together with the incorrect unknown CA handling. --- lib/public_key/src/public_key.erl | 18 +++++---- lib/public_key/test/public_key_SUITE.erl | 5 +++ lib/ssl/src/ssl_certificate.erl | 31 ++++++--------- lib/ssl/src/ssl_handshake.erl | 15 ++++---- lib/ssl/test/erl_make_certs.erl | 51 +++++++++++++++---------- lib/ssl/test/ssl_basic_SUITE.erl | 65 +++++++++++++++++++++++++++++--- lib/ssl/test/ssl_test_lib.erl | 2 +- 7 files changed, 128 insertions(+), 59 deletions(-) diff --git a/lib/public_key/src/public_key.erl b/lib/public_key/src/public_key.erl index 95c3d714d3..f9b992afd3 100644 --- a/lib/public_key/src/public_key.erl +++ b/lib/public_key/src/public_key.erl @@ -373,11 +373,9 @@ pkix_verify(DerCert, #'RSAPublicKey'{} = RSAKey) pkix_is_issuer(Cert, IssuerCert) when is_binary(Cert) -> OtpCert = pkix_decode_cert(Cert, otp), pkix_is_issuer(OtpCert, IssuerCert); - pkix_is_issuer(Cert, IssuerCert) when is_binary(IssuerCert) -> OtpIssuerCert = pkix_decode_cert(IssuerCert, otp), pkix_is_issuer(Cert, OtpIssuerCert); - pkix_is_issuer(#'OTPCertificate'{tbsCertificate = TBSCert}, #'OTPCertificate'{tbsCertificate = Candidate}) -> pubkey_cert:is_issuer(TBSCert#'OTPTBSCertificate'.issuer, @@ -438,7 +436,7 @@ pkix_normalize_name(Issuer) -> pubkey_cert:normalize_general_name(Issuer). %%-------------------------------------------------------------------- --spec pkix_path_validation(der_encoded()| #'OTPCertificate'{}, +-spec pkix_path_validation(der_encoded()| #'OTPCertificate'{} | unknown_ca, CertChain :: [der_encoded()] , Options :: list()) -> {ok, {PublicKeyInfo :: term(), @@ -447,10 +445,16 @@ pkix_normalize_name(Issuer) -> {error, {bad_cert, Reason :: term()}}. %% Description: Performs a basic path validation according to RFC 5280. %%-------------------------------------------------------------------- -pkix_path_validation(TrustedCert, CertChain, Options) - when is_binary(TrustedCert) -> - OtpCert = pkix_decode_cert(TrustedCert, otp), - pkix_path_validation(OtpCert, CertChain, Options); +pkix_path_validation(unknown_ca, [Cert | Chain], Options) -> + case proplists:get_value(verify, Options, true) of + true -> + {error, {bad_cert, unknown_ca}}; + false -> + pkix_path_validation(Cert, Chain, [{acc_errors, [{bad_cert, unknown_ca}]}]) + end; +pkix_path_validation(TrustedCert, CertChain, Options) when + is_binary(TrustedCert) -> OtpCert = pkix_decode_cert(TrustedCert, + otp), pkix_path_validation(OtpCert, CertChain, Options); pkix_path_validation(#'OTPCertificate'{} = TrustedCert, CertChain, Options) when is_list(CertChain), is_list(Options) -> diff --git a/lib/public_key/test/public_key_SUITE.erl b/lib/public_key/test/public_key_SUITE.erl index 1d32e989a9..ee5e939476 100644 --- a/lib/public_key/test/public_key_SUITE.erl +++ b/lib/public_key/test/public_key_SUITE.erl @@ -374,6 +374,11 @@ pkix_path_validation(Config) when is_list(Config) -> {ok, {_,_,[E]}} = public_key:pkix_path_validation(Trusted, [Cert1, Cert3,Cert4], [{verify,false}]), + + {error, {bad_cert,unknown_ca}} = public_key:pkix_path_validation(unknown_ca, [Cert1, Cert3, Cert4], []), + + {ok, {_,_,[{bad_cert,unknown_ca}]}} = + public_key:pkix_path_validation(unknown_ca, [Cert1], [{verify, false}]), ok. %%-------------------------------------------------------------------- diff --git a/lib/ssl/src/ssl_certificate.erl b/lib/ssl/src/ssl_certificate.erl index 917e75157b..a42cd0c10d 100644 --- a/lib/ssl/src/ssl_certificate.erl +++ b/lib/ssl/src/ssl_certificate.erl @@ -31,7 +31,7 @@ -include("ssl_debug.hrl"). -include_lib("public_key/include/public_key.hrl"). --export([trusted_cert_and_path/3, +-export([trusted_cert_and_path/2, certificate_chain/2, file_to_certificats/1, validate_extensions/6, @@ -47,14 +47,14 @@ %%==================================================================== %%-------------------------------------------------------------------- --spec trusted_cert_and_path([der_cert()], certdb_ref(), boolean()) -> - {der_cert(), [der_cert()], list()}. +-spec trusted_cert_and_path([der_cert()], certdb_ref()) -> + {der_cert() | unknown_ca, [der_cert()]}. %% %% Description: Extracts the root cert (if not presents tries to %% look it up, if not found {bad_cert, unknown_ca} will be added verification %% errors. Returns {RootCert, Path, VerifyErrors} %%-------------------------------------------------------------------- -trusted_cert_and_path(CertChain, CertDbRef, Verify) -> +trusted_cert_and_path(CertChain, CertDbRef) -> [Cert | RestPath] = lists:reverse(CertChain), OtpCert = public_key:pkix_decode_cert(Cert, otp), IssuerAnPath = @@ -71,24 +71,22 @@ trusted_cert_and_path(CertChain, CertDbRef, Verify) -> {ok, IssuerId} -> {IssuerId, [Cert | RestPath]}; Other -> - {Other, RestPath} + {Other, [Cert | RestPath]} end end end, case IssuerAnPath of - {{error, issuer_not_found}, _ } -> - %% The root CA was not sent and can not be found, we fail if verify = true - not_valid(?ALERT_REC(?FATAL, ?UNKNOWN_CA), Verify, {Cert, RestPath}); + {{error, issuer_not_found}, Path} -> + %% The root CA was not sent and can not be found. + {unknown_ca, Path}; {{SerialNr, Issuer}, Path} -> - case ssl_manager:lookup_trusted_cert(CertDbRef, - SerialNr, Issuer) of + case ssl_manager:lookup_trusted_cert(CertDbRef, SerialNr, Issuer) of {ok, {BinCert,_}} -> - {BinCert, Path, []}; + {BinCert, Path}; _ -> - %% Fail if verify = true - not_valid(?ALERT_REC(?FATAL, ?UNKNOWN_CA), - Verify, {Cert, RestPath}) + %% Root CA could not be verified + {unknown_ca, Path} end end. @@ -244,11 +242,6 @@ find_issuer(OtpCert, PrevCandidateKey) -> end end. -not_valid(Alert, true, _) -> - throw(Alert); -not_valid(_, false, {ErlCert, Path}) -> - {ErlCert, Path, [{bad_cert, unknown_ca}]}. - is_valid_extkey_usage(KeyUse, client) -> %% Client wants to verify server is_valid_key_usage(KeyUse,?'id-kp-serverAuth'); diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index ee725997a4..add5147fb4 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -203,18 +203,15 @@ certify(#certificate{asn1_certificates = ASN1Certs}, CertDbRef, end end, try - %% Allow missing root_cert and check that with VerifyFun - ssl_certificate:trusted_cert_and_path(ASN1Certs, CertDbRef, false) of - {TrustedErlCert, CertPath, VerifyErrors} -> + ssl_certificate:trusted_cert_and_path(ASN1Certs, CertDbRef) of + {TrustedErlCert, CertPath} -> Result = public_key:pkix_path_validation(TrustedErlCert, CertPath, [{max_path_length, MaxPathLen}, {verify, VerifyBool}, {validate_extensions_fun, - ValidateExtensionFun}, - {acc_errors, - VerifyErrors}]), + ValidateExtensionFun}]), case Result of {error, Reason} -> path_validation_alert(Reason, Verify); @@ -474,7 +471,7 @@ get_tls_handshake(Data, Buffer) -> get_tls_handshake_aux(list_to_binary([Buffer, Data]), []). %%-------------------------------------------------------------------- --spec dec_client_key(binary(), key_algo(), tls_version()) -> +-spec decode_client_key(binary(), key_algo(), tls_version()) -> #encrypted_premaster_secret{} | #client_diffie_hellman_public{}. %% %% Description: Decode client_key data and return appropriate type @@ -510,6 +507,8 @@ path_validation_alert({bad_cert, unknown_critical_extension}, _) -> ?ALERT_REC(?FATAL, ?UNSUPPORTED_CERTIFICATE); path_validation_alert({bad_cert, cert_revoked}, _) -> ?ALERT_REC(?FATAL, ?CERTIFICATE_REVOKED); +path_validation_alert({bad_cert, unknown_ca}, _) -> + ?ALERT_REC(?FATAL, ?UNKNOWN_CA); path_validation_alert(_, _) -> ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE). @@ -1129,7 +1128,7 @@ sig_alg(_) -> key_exchange_alg(rsa) -> ?KEY_EXCHANGE_RSA; key_exchange_alg(Alg) when Alg == dhe_rsa; Alg == dhe_dss; - Alg == dh_dss; Alg == dh_rsa; Alg == dh_anon -> + Alg == dh_dss; Alg == dh_rsa -> ?KEY_EXCHANGE_DIFFIE_HELLMAN; key_exchange_alg(_) -> ?NULL. diff --git a/lib/ssl/test/erl_make_certs.erl b/lib/ssl/test/erl_make_certs.erl index c9db0d3851..f8aef55754 100644 --- a/lib/ssl/test/erl_make_certs.erl +++ b/lib/ssl/test/erl_make_certs.erl @@ -66,9 +66,9 @@ make_cert(Opts) -> %% @end %%-------------------------------------------------------------------- write_pem(Dir, FileName, {Cert, Key = {_,_,not_encrypted}}) when is_binary(Cert) -> - ok = ssl_test_lib:der_to_pem(filename:join(Dir, FileName ++ ".pem"), + ok = der_to_pem(filename:join(Dir, FileName ++ ".pem"), [{'Certificate', Cert, not_encrypted}]), - ok = ssl_test_lib:der_to_pem(filename:join(Dir, FileName ++ "_key.pem"), [Key]). + ok = der_to_pem(filename:join(Dir, FileName ++ "_key.pem"), [Key]). %%-------------------------------------------------------------------- %% @doc Creates a rsa key (OBS: for testing only) @@ -144,34 +144,39 @@ encode_key(Key = #'DSAPrivateKey'{}) -> make_tbs(SubjectKey, Opts) -> Version = list_to_atom("v"++integer_to_list(proplists:get_value(version, Opts, 3))), - {Issuer, IssuerKey} = issuer(Opts, SubjectKey), + + IssuerProp = proplists:get_value(issuer, Opts, true), + {Issuer, IssuerKey} = issuer(IssuerProp, Opts, SubjectKey), {Algo, Parameters} = sign_algorithm(IssuerKey, Opts), SignAlgo = #'SignatureAlgorithm'{algorithm = Algo, parameters = Parameters}, - + Subject = case IssuerProp of + true -> %% Is a Root Ca + Issuer; + _ -> + subject(proplists:get_value(subject, Opts),false) + end, + {#'OTPTBSCertificate'{serialNumber = trunc(random:uniform()*100000000)*10000 + 1, signature = SignAlgo, issuer = Issuer, validity = validity(Opts), - subject = subject(proplists:get_value(subject, Opts),false), + subject = Subject, subjectPublicKeyInfo = publickey(SubjectKey), version = Version, extensions = extensions(Opts) }, IssuerKey}. -issuer(Opts, SubjectKey) -> - IssuerProp = proplists:get_value(issuer, Opts, true), - case IssuerProp of - true -> %% Self signed - {subject(proplists:get_value(subject, Opts), true), SubjectKey}; - {Issuer, IssuerKey} when is_binary(Issuer) -> - {issuer_der(Issuer), decode_key(IssuerKey)}; - {File, IssuerKey} when is_list(File) -> - {ok, [{cert, Cert, _}|_]} = public_key:pem_to_der(File), - {issuer_der(Cert), decode_key(IssuerKey)} - end. +issuer(true, Opts, SubjectKey) -> + %% Self signed + {subject(proplists:get_value(subject, Opts), true), SubjectKey}; +issuer({Issuer, IssuerKey}, _Opts, _SubjectKey) when is_binary(Issuer) -> + {issuer_der(Issuer), decode_key(IssuerKey)}; +issuer({File, IssuerKey}, _Opts, _SubjectKey) when is_list(File) -> + {ok, [{cert, Cert, _}|_]} = public_key:pem_to_der(File), + {issuer_der(Cert), decode_key(IssuerKey)}. issuer_der(Issuer) -> Decoded = public_key:pkix_decode_cert(Issuer, otp), @@ -179,8 +184,8 @@ issuer_der(Issuer) -> #'OTPTBSCertificate'{subject=Subject} = Tbs, Subject. -subject(undefined, IsCA) -> - User = if IsCA -> "CA"; true -> os:getenv("USER") end, +subject(undefined, IsRootCA) -> + User = if IsRootCA -> "RootCA"; true -> os:getenv("USER") end, Opts = [{email, User ++ "@erlang.org"}, {name, User}, {city, "Stockholm"}, @@ -267,7 +272,7 @@ publickey(#'DSAPrivateKey'{p=P, q=Q, g=G, y=Y}) -> #'OTPSubjectPublicKeyInfo'{algorithm = Algo, subjectPublicKey = Y}. validity(Opts) -> - DefFrom0 = date(), + DefFrom0 = calendar:gregorian_days_to_date(calendar:date_to_gregorian_days(date())-1), DefTo0 = calendar:gregorian_days_to_date(calendar:date_to_gregorian_days(date())+7), {DefFrom, DefTo} = proplists:get_value(validity, Opts, {DefFrom0, DefTo0}), Format = fun({Y,M,D}) -> lists:flatten(io_lib:format("~w~2..0w~2..0w000000Z",[Y,M,D])) end, @@ -406,3 +411,11 @@ extended_gcd(A, B) -> {X, Y} = extended_gcd(B, N), {Y, X-Y*(A div B)} end. + +pem_to_der(File) -> + {ok, PemBin} = file:read_file(File), + public_key:pem_decode(PemBin). + +der_to_pem(File, Entries) -> + PemBin = public_key:pem_encode(Entries), + file:write_file(File, PemBin). diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl index c6807c7b32..d50b34b6ac 100644 --- a/lib/ssl/test/ssl_basic_SUITE.erl +++ b/lib/ssl/test/ssl_basic_SUITE.erl @@ -234,7 +234,8 @@ all(suite) -> server_no_wrap_sequence_number, extended_key_usage, validate_extensions_fun, no_authority_key_identifier, invalid_signature_client, invalid_signature_server, cert_expired, - client_with_cert_cipher_suites_handshake + client_with_cert_cipher_suites_handshake, unknown_server_ca_fail, + unknown_server_ca_accept ]. %% Test cases starts here. @@ -2613,12 +2614,13 @@ validate_extensions_fun(Config) when is_list(Config) -> %%-------------------------------------------------------------------- no_authority_key_identifier(doc) -> - ["Test cert that does not have authorityKeyIdentifier extension"]; + ["Test cert that does not have authorityKeyIdentifier extension" + " but are present in trusted certs db."]; no_authority_key_identifier(suite) -> []; no_authority_key_identifier(Config) when is_list(Config) -> - ClientOpts = ?config(client_opts, Config), + ClientOpts = ?config(client_verification_opts, Config), ServerOpts = ?config(server_opts, Config), PrivDir = ?config(priv_dir, Config), @@ -2676,7 +2678,7 @@ invalid_signature_server(suite) -> []; invalid_signature_server(Config) when is_list(Config) -> - ClientOpts = ?config(client_opts, Config), + ClientOpts = ?config(client_verification_opts, Config), ServerOpts = ?config(server_verification_opts, Config), PrivDir = ?config(priv_dir, Config), @@ -2793,7 +2795,7 @@ cert_expired(suite) -> []; cert_expired(Config) when is_list(Config) -> - ClientOpts = ?config(client_opts, Config), + ClientOpts = ?config(client_verification_opts, Config), ServerOpts = ?config(server_verification_opts, Config), PrivDir = ?config(priv_dir, Config), @@ -2882,6 +2884,59 @@ client_with_cert_cipher_suites_handshake(Config) when is_list(Config) -> ssl_test_lib:check_result(Server, ok, Client, ok), ssl_test_lib:close(Server), ssl_test_lib:close(Client). +%%-------------------------------------------------------------------- +unknown_server_ca_fail(doc) -> + ["Test that the client fails if the ca is unknown in verify_peer mode"]; +unknown_server_ca_fail(suite) -> + []; +unknown_server_ca_fail(Config) when is_list(Config) -> + ClientOpts = ?config(client_opts, Config), + ServerOpts = ?config(server_opts, Config), + {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), + Server = ssl_test_lib:start_server_error([{node, ServerNode}, {port, 0}, + {from, self()}, + {mfa, {ssl_test_lib, + no_result, []}}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + Client = ssl_test_lib:start_client_error([{node, ClientNode}, {port, Port}, + {host, Hostname}, + {from, self()}, + {mfa, {ssl_test_lib, + no_result, []}}, + {options, + [{verify, verify_peer}| ClientOpts]}]), + + ssl_test_lib:check_result(Server, {error,"unknown ca"}, Client, {error, "unknown ca"}), + ssl_test_lib:close(Server), + ssl_test_lib:close(Client). + +%%-------------------------------------------------------------------- +unknown_server_ca_accept(doc) -> + ["Test that the client succeds if the ca is unknown in verify_none mode"]; +unknown_server_ca_accept(suite) -> + []; +unknown_server_ca_accept(Config) when is_list(Config) -> + ClientOpts = ?config(client_opts, Config), + ServerOpts = ?config(server_opts, Config), + {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), + Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, + {from, self()}, + {mfa, {?MODULE, + send_recv_result_active, []}}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port}, + {host, Hostname}, + {from, self()}, + {mfa, {?MODULE, + send_recv_result_active, []}}, + {options, + [{verify, verify_none}| ClientOpts]}]), + + 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_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl index c35178460f..ce164f7e4c 100644 --- a/lib/ssl/test/ssl_test_lib.erl +++ b/lib/ssl/test/ssl_test_lib.erl @@ -332,7 +332,7 @@ make_dsa_cert(Config) -> {cacertfile, ServerCaCertFile}, {certfile, ServerCertFile}, {keyfile, ServerKeyFile}]}, {server_dsa_verify_opts, [{ssl_imp, new},{reuseaddr, true}, - {cacertfile, ServerCaCertFile}, + {cacertfile, ClientCaCertFile}, {certfile, ServerCertFile}, {keyfile, ServerKeyFile}, {verify, verify_peer}]}, {client_dsa_opts, [{ssl_imp, new},{reuseaddr, true}, -- cgit v1.2.3