From e56167dd6ca8d37d26ea7f19933691a3bda41113 Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Mon, 21 Jan 2013 10:54:53 +0100 Subject: ssl: Enhance error handling Remove filter mechanisms that made error messages backwards compatible with old ssl but hid information about what actually happened. This does not break the documented API however other reason terms may be returned, so code that matches on the reason part of {error, Reason} may fail. --- lib/ssl/src/ssl_alert.erl | 11 ++-- lib/ssl/src/ssl_connection.erl | 10 ++-- lib/ssl/test/ssl_basic_SUITE.erl | 4 +- lib/ssl/test/ssl_certificate_verify_SUITE.erl | 44 +++++++-------- lib/ssl/test/ssl_test_lib.erl | 77 +++++++++++++++++++++++++++ lib/ssl/test/ssl_to_openssl_SUITE.erl | 2 +- 6 files changed, 108 insertions(+), 40 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/ssl_alert.erl b/lib/ssl/src/ssl_alert.erl index 222b3f1ad7..f94a1136a0 100644 --- a/lib/ssl/src/ssl_alert.erl +++ b/lib/ssl/src/ssl_alert.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2007-2012. All Rights Reserved. +%% Copyright Ericsson AB 2007-2013. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -36,8 +36,7 @@ %% Internal application API %%==================================================================== %%-------------------------------------------------------------------- --spec reason_code(#alert{}, client | server) -> closed | esslconnect | - esslaccept | string(). +-spec reason_code(#alert{}, client | server) -> closed | {essl, string()}. %% %% Description: Returns the error reason that will be returned to the %% user. @@ -45,12 +44,8 @@ reason_code(#alert{description = ?CLOSE_NOTIFY}, _) -> closed; -reason_code(#alert{description = ?HANDSHAKE_FAILURE}, client) -> - esslconnect; -reason_code(#alert{description = ?HANDSHAKE_FAILURE}, server) -> - esslaccept; reason_code(#alert{description = Description}, _) -> - description_txt(Description). + {essl, description_txt(Description)}. %%-------------------------------------------------------------------- -spec alert_txt(#alert{}) -> string(). diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index cde13069b5..809dcc04e4 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2007-2012. All Rights Reserved. +%% Copyright Ericsson AB 2007-2013. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -1135,7 +1135,7 @@ init_certificates(#ssl_options{cacerts = CaCerts, {ok, _, _, _, _, _} = ssl_manager:connection_init(Certs, Role) catch Error:Reason -> - handle_file_error(?LINE, Error, Reason, CACertFile, ecacertfile, + handle_file_error(?LINE, Error, Reason, CACertFile, {ecacertfile, Reason}, erlang:get_stacktrace()) end, init_certificates(Cert, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHandle, CacheHandle, CertFile, Role). @@ -1157,7 +1157,7 @@ init_certificates(undefined, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHan {ok, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHandle, CacheRef, OwnCert} catch Error:Reason -> - handle_file_error(?LINE, Error, Reason, CertFile, ecertfile, + handle_file_error(?LINE, Error, Reason, CertFile, {ecertfile, Reason}, erlang:get_stacktrace()) end; init_certificates(Cert, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHandle, CacheRef, _, _) -> @@ -1176,7 +1176,7 @@ init_private_key(DbHandle, undefined, KeyFile, Password, _) -> private_key(public_key:pem_entry_decode(PemEntry, Password)) catch Error:Reason -> - handle_file_error(?LINE, Error, Reason, KeyFile, ekeyfile, + handle_file_error(?LINE, Error, Reason, KeyFile, {ekeyfile, Reason}, erlang:get_stacktrace()) end; @@ -1234,7 +1234,7 @@ init_diffie_hellman(DbHandle,_, DHParamFile, server) -> catch Error:Reason -> handle_file_error(?LINE, Error, Reason, - DHParamFile, edhfile, erlang:get_stacktrace()) + DHParamFile, {edhfile, Reason}, erlang:get_stacktrace()) end. sync_send_all_state_event(FsmPid, Event) -> diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl index 5ba71f9218..1cc1f09ad7 100644 --- a/lib/ssl/test/ssl_basic_SUITE.erl +++ b/lib/ssl/test/ssl_basic_SUITE.erl @@ -1567,8 +1567,8 @@ default_reject_anonymous(Config) when is_list(Config) -> [{ciphers,[Cipher]} | ClientOpts]}]), - ssl_test_lib:check_result(Server, {error, "insufficient security"}, - Client, {error, "insufficient security"}). + ssl_test_lib:check_result(Server, {error, {essl, "insufficient security"}}, + Client, {error, {essl, "insufficient security"}}). %%-------------------------------------------------------------------- reuse_session() -> diff --git a/lib/ssl/test/ssl_certificate_verify_SUITE.erl b/lib/ssl/test/ssl_certificate_verify_SUITE.erl index 9677d98c1b..86e1d47be7 100644 --- a/lib/ssl/test/ssl_certificate_verify_SUITE.erl +++ b/lib/ssl/test/ssl_certificate_verify_SUITE.erl @@ -252,8 +252,8 @@ server_require_peer_cert_fail(Config) when is_list(Config) -> {from, self()}, {options, [{active, false} | BadClientOpts]}]), - ssl_test_lib:check_result(Server, {error, esslaccept}, - Client, {error, esslconnect}). + ssl_test_lib:check_result(Server, {error, {essl, "handshake failure"}}, + Client, {error, {essl, "handshake failure"}}). %%-------------------------------------------------------------------- @@ -293,14 +293,14 @@ verify_fun_always_run_client(Config) when is_list(Config) -> [{verify, verify_peer}, {verify_fun, FunAndState} | ClientOpts]}]), - %% Server error may be esslaccept or closed depending on timing + %% Server error may be {essl,"handshake failure"} or closed depending on timing %% this is not a bug it is a circumstance of how tcp works! receive {Server, ServerError} -> ct:print("Server Error ~p~n", [ServerError]) end, - ssl_test_lib:check_result(Client, {error, esslconnect}). + ssl_test_lib:check_result(Client, {error, {essl, "handshake failure"}}). %%-------------------------------------------------------------------- verify_fun_always_run_server() -> @@ -342,14 +342,14 @@ verify_fun_always_run_server(Config) when is_list(Config) -> [{verify, verify_peer} | ClientOpts]}]), - %% Client error may be esslconnect or closed depending on timing + %% Client error may be {essl, "handshake failure" } or closed depending on timing %% this is not a bug it is a circumstance of how tcp works! receive {Client, ClientError} -> ct:print("Client Error ~p~n", [ClientError]) end, - ssl_test_lib:check_result(Server, {error, esslaccept}). + ssl_test_lib:check_result(Server, {error, {essl, "handshake failure"}}). %%-------------------------------------------------------------------- @@ -380,7 +380,7 @@ client_verify_none_passive(Config) when is_list(Config) -> ssl_test_lib:close(Client). %%-------------------------------------------------------------------- cert_expired() -> - [{doc,"Test server with invalid signature"}]. + [{doc,"Test server with expired certificate"}]. cert_expired(Config) when is_list(Config) -> ClientOpts = ?config(client_verification_opts, Config), @@ -432,8 +432,8 @@ cert_expired(Config) when is_list(Config) -> {from, self()}, {options, [{verify, verify_peer} | ClientOpts]}]), - ssl_test_lib:check_result(Server, {error, "certificate expired"}, - Client, {error, "certificate expired"}). + ssl_test_lib:check_result(Server, {error, {essl, "certificate expired"}}, + Client, {error, {essl, "certificate expired"}}). two_digits_str(N) when N < 10 -> lists:flatten(io_lib:format("0~p", [N])); @@ -679,7 +679,7 @@ delete_authority_key_extension([Head | Rest], Acc) -> %%-------------------------------------------------------------------- invalid_signature_server() -> - [{doc,"Test server with invalid signature"}]. + [{doc,"Test client with invalid signature"}]. invalid_signature_server(Config) when is_list(Config) -> ClientOpts = ?config(client_verification_opts, Config), @@ -710,8 +710,8 @@ invalid_signature_server(Config) when is_list(Config) -> {from, self()}, {options, [{verify, verify_peer} | ClientOpts]}]), - tcp_delivery_workaround(Server, {error, "bad certificate"}, - Client, {error,"bad certificate"}). + tcp_delivery_workaround(Server, {error, {essl, "bad certificate"}}, + Client, {error, {essl, "bad certificate"}}). %%-------------------------------------------------------------------- @@ -747,8 +747,8 @@ invalid_signature_client(Config) when is_list(Config) -> {from, self()}, {options, NewClientOpts}]), - tcp_delivery_workaround(Server, {error, "bad certificate"}, - Client, {error,"bad certificate"}). + tcp_delivery_workaround(Server, {error, {essl, "bad certificate"}}, + Client, {error, {essl, "bad certificate"}}). %%-------------------------------------------------------------------- @@ -829,8 +829,8 @@ unknown_server_ca_fail(Config) when is_list(Config) -> {verify_fun, FunAndState} | ClientOpts]}]), - ssl_test_lib:check_result(Server, {error,"unknown ca"}, - Client, {error, "unknown ca"}). + ssl_test_lib:check_result(Server, {error, {essl, "unknown ca"}}, + Client, {error, {essl, "unknown ca"}}). %%-------------------------------------------------------------------- unknown_server_ca_accept_verify_none() -> @@ -947,10 +947,6 @@ tcp_delivery_workaround(Server, ServerMsg, Client, ClientMsg) -> {Client, {error,closed}} -> server_msg(Server, ServerMsg); {Server, {error,closed}} -> - client_msg(Client, ClientMsg); - {Client, {error, esslconnect}} -> - server_msg(Server, ServerMsg); - {Server, {error, esslaccept}} -> client_msg(Client, ClientMsg) end. @@ -961,8 +957,8 @@ client_msg(Client, ClientMsg) -> {Client, {error,closed}} -> ct:print("client got close"), ok; - {Client, {error, esslconnect}} -> - ct:print("client got econnaborted"), + {Client, {error, Reason}} -> + ct:print("client got econnaborted: ~p", [Reason]), ok; Unexpected -> ct:fail(Unexpected) @@ -974,8 +970,8 @@ server_msg(Server, ServerMsg) -> {Server, {error,closed}} -> ct:print("server got close"), ok; - {Server, {error, esslaccept}} -> - ct:print("server got econnaborted"), + {Server, {error, Reason}} -> + ct:print("server got econnaborted: ~p", [Reason]), ok; Unexpected -> ct:fail(Unexpected) diff --git a/lib/ssl/test/ssl_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl index 76b302b1cb..8d96a70a6e 100644 --- a/lib/ssl/test/ssl_test_lib.erl +++ b/lib/ssl/test/ssl_test_lib.erl @@ -203,6 +203,67 @@ close(Pid) -> ct:print("Pid: ~p down due to:~p ~n", [Pid, Reason]) end. + +check_result(Server, {error, SReason} = ServerMsg, Client, {error, closed} = ClientMsg) -> + receive + {Server, {error, {SReason, _}}} -> + receive + {Client, ClientMsg} -> + ok; + Unexpected -> + Reason = {{expected, {Client, ClientMsg}}, + {got, Unexpected}}, + ct:fail(Reason) + end; + {Client, ClientMsg} -> + receive + {Server, {error, {SReason, _}}} -> + ok; + Unexpected -> + Reason = {{expected, {Server,{error, {SReason, 'term()'}}}, + {got, Unexpected}}}, + ct:fail(Reason) + end; + {Port, {data,Debug}} when is_port(Port) -> + io:format("openssl ~s~n",[Debug]), + check_result(Server, ServerMsg, Client, ClientMsg); + + Unexpected -> + Reason = {{expected, {Client, ClientMsg}}, + {expected, {Server, {error, {SReason, 'term()'}}}, {got, Unexpected}}}, + ct:fail(Reason) + end; + +check_result(Server, {error, closed} = ServerMsg, Client, {error, CReson} = ClientMsg) -> + receive + {Server, ServerMsg} -> + receive + {Client, {error, {CReson, _}}} -> + ok; + Unexpected -> + Reason = {{expected, {Client, {error, {CReson, 'term()'}}}, + {got, Unexpected}}}, + ct:fail(Reason) + end; + {Client, {error, {CReson, _}}} -> + receive + {Server, ServerMsg} -> + ok; + Unexpected -> + Reason = {{expected, {Server, ServerMsg}}, + {got, Unexpected}}, + ct:fail(Reason) + end; + {Port, {data,Debug}} when is_port(Port) -> + io:format("openssl ~s~n",[Debug]), + check_result(Server, ServerMsg, Client, ClientMsg); + + Unexpected -> + Reason = {{expected, {Client, {error, {CReson, 'term()'}}}, + {expected, {Server, ServerMsg}}, {got, Unexpected}}}, + ct:fail(Reason) + end; + check_result(Server, ServerMsg, Client, ClientMsg) -> receive {Server, ServerMsg} -> @@ -233,6 +294,22 @@ check_result(Server, ServerMsg, Client, ClientMsg) -> ct:fail(Reason) end. +check_result(Pid, {error, Reason} = Err) when Reason == ecertfile; + Reason == ecacertfile; + Reason == ekeyfile; + Reason == edhfile -> + receive + {Pid, {error, {Reason, Str}}} when is_list(Str) -> + ok; + {Port, {data,Debug}} when is_port(Port) -> + io:format("openssl ~s~n",[Debug]), + check_result(Pid, Err); + Unexpected -> + Reason = {{expected, {Pid, {error, {Reason, "'appropriate error string'"}}}}, + {got, Unexpected}}, + ct:fail(Reason) + end; + check_result(Pid, Msg) -> receive {Pid, Msg} -> diff --git a/lib/ssl/test/ssl_to_openssl_SUITE.erl b/lib/ssl/test/ssl_to_openssl_SUITE.erl index d5e7d515fd..7c0c00bf36 100644 --- a/lib/ssl/test/ssl_to_openssl_SUITE.erl +++ b/lib/ssl/test/ssl_to_openssl_SUITE.erl @@ -902,7 +902,7 @@ ssl2_erlang_server_openssl_client(Config) when is_list(Config) -> ok end, - ssl_test_lib:check_result(Server, {error,"protocol version"}), + ssl_test_lib:check_result(Server, {error, {essl, "protocol version"}}), process_flag(trap_exit, false). %%-------------------------------------------------------------------- -- cgit v1.2.3