From d5a8beb25f23d3ade429725e3ed0fca4f2797a9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Wed, 27 Feb 2019 15:13:50 +0100 Subject: ssl: Fix ssl alerts Add missing alert to description_atom/1. Function clauses ordered by value of the alert. Change-Id: Ibb68ea261c42070c757b2815abd3f7b179880128 --- lib/ssl/src/ssl_alert.erl | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/ssl_alert.erl b/lib/ssl/src/ssl_alert.erl index e17476f33b..06b1b005a5 100644 --- a/lib/ssl/src/ssl_alert.erl +++ b/lib/ssl/src/ssl_alert.erl @@ -161,6 +161,8 @@ description_txt(?INSUFFICIENT_SECURITY) -> "Insufficient Security"; description_txt(?INTERNAL_ERROR) -> "Internal Error"; +description_txt(?INAPPROPRIATE_FALLBACK) -> + "Inappropriate Fallback"; description_txt(?USER_CANCELED) -> "User Canceled"; description_txt(?NO_RENEGOTIATION) -> @@ -179,8 +181,6 @@ description_txt(?BAD_CERTIFICATE_HASH_VALUE) -> "Bad Certificate Hash Value"; description_txt(?UNKNOWN_PSK_IDENTITY) -> "Unknown Psk Identity"; -description_txt(?INAPPROPRIATE_FALLBACK) -> - "Inappropriate Fallback"; description_txt(?CERTIFICATE_REQUIRED) -> "Certificate required"; description_txt(?NO_APPLICATION_PROTOCOL) -> @@ -232,10 +232,14 @@ description_atom(?INSUFFICIENT_SECURITY) -> insufficient_security; description_atom(?INTERNAL_ERROR) -> internal_error; +description_atom(?INAPPROPRIATE_FALLBACK) -> + inappropriate_fallback; description_atom(?USER_CANCELED) -> user_canceled; description_atom(?NO_RENEGOTIATION) -> no_renegotiation; +description_atom(?MISSING_EXTENSION) -> + missing_extension; description_atom(?UNSUPPORTED_EXTENSION) -> unsupported_extension; description_atom(?CERTIFICATE_UNOBTAINABLE) -> @@ -248,9 +252,9 @@ description_atom(?BAD_CERTIFICATE_HASH_VALUE) -> bad_certificate_hash_value; description_atom(?UNKNOWN_PSK_IDENTITY) -> unknown_psk_identity; -description_atom(?INAPPROPRIATE_FALLBACK) -> - inappropriate_fallback; +description_atom(?CERTIFICATE_REQUIRED) -> + certificate_required; description_atom(?NO_APPLICATION_PROTOCOL) -> no_application_protocol; description_atom(_) -> - 'unsupported/unkonwn_alert'. + 'unsupported/unknown_alert'. -- cgit v1.2.3 From 0651ad7a32d6dabbab22993d629e4170e9952167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Wed, 27 Feb 2019 15:18:03 +0100 Subject: ssl: Add ssl logger support for CertificateRequest Change-Id: I5fdade8474147d05bc12d28fec91a47d4fd6e73b --- lib/ssl/src/ssl_logger.erl | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'lib/ssl') diff --git a/lib/ssl/src/ssl_logger.erl b/lib/ssl/src/ssl_logger.erl index b82b3937a1..f497315235 100644 --- a/lib/ssl/src/ssl_logger.erl +++ b/lib/ssl/src/ssl_logger.erl @@ -181,6 +181,11 @@ parse_handshake(Direction, #hello_request{} = HelloRequest) -> [header_prefix(Direction)]), Message = io_lib:format("~p", [?rec_info(hello_request, HelloRequest)]), {Header, Message}; +parse_handshake(Direction, #certificate_request_1_3{} = CertificateRequest) -> + Header = io_lib:format("~s Handshake, CertificateRequest", + [header_prefix(Direction)]), + Message = io_lib:format("~p", [?rec_info(certificate_request_1_3, CertificateRequest)]), + {Header, Message}; parse_handshake(Direction, #certificate_1_3{} = Certificate) -> Header = io_lib:format("~s Handshake, Certificate", [header_prefix(Direction)]), -- cgit v1.2.3 From 85f04feeb89d12443d12c7e233712bf8c299e187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Wed, 27 Feb 2019 15:38:12 +0100 Subject: ssl: Implement state 'wait_cert' Implement state 'wait_cert' with its handler function do_wait_cert/2. Send CertificateRequest if peer verification is enabled. Send Alert 'certificate required' if client answers with empty Certificate and option 'fail_if_no_peer_cert' is set to true. Change-Id: I72c73bcb6bc68ea60e6fe41cdd29ccfe40d18322 --- lib/ssl/src/tls_connection_1_3.erl | 28 +++++++- lib/ssl/src/tls_handshake_1_3.erl | 132 +++++++++++++++++++++++++++++++++---- 2 files changed, 145 insertions(+), 15 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/tls_connection_1_3.erl b/lib/ssl/src/tls_connection_1_3.erl index 71ac6a9310..3c292a43b0 100644 --- a/lib/ssl/src/tls_connection_1_3.erl +++ b/lib/ssl/src/tls_connection_1_3.erl @@ -110,6 +110,7 @@ %% gen_statem helper functions -export([start/4, negotiated/4, + wait_cert/4, wait_finished/4 ]). @@ -140,12 +141,33 @@ negotiated(internal, Map, State0, _Module) -> case tls_handshake_1_3:do_negotiated(Map, State0) of #alert{} = Alert -> ssl_connection:handle_own_alert(Alert, {3,4}, negotiated, State0); - State -> - {next_state, wait_finished, State, []} - + {State, NextState} -> + {next_state, NextState, State, []} end. +wait_cert(internal, + #change_cipher_spec{} = ChangeCipherSpec, State0, _Module) -> + case tls_handshake_1_3:do_wait_cert(ChangeCipherSpec, State0) of + #alert{} = Alert -> + ssl_connection:handle_own_alert(Alert, {3,4}, wait_cert, State0); + {State1, NextState} -> + {Record, State} = tls_connection:next_record(State1), + tls_connection:next_event(NextState, Record, State) + end; +wait_cert(internal, + #certificate_1_3{} = Certificate, State0, _Module) -> + case tls_handshake_1_3:do_wait_cert(Certificate, State0) of + {#alert{} = Alert, State} -> + ssl_connection:handle_own_alert(Alert, {3,4}, wait_cert, State); + {State1, NextState} -> + {Record, State} = tls_connection:next_record(State1), + tls_connection:next_event(NextState, Record, State) + end; +wait_cert(Type, Msg, State, Connection) -> + ssl_connection:handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection). + + wait_finished(internal, #change_cipher_spec{} = ChangeCipherSpec, State0, _Module) -> case tls_handshake_1_3:do_wait_finished(ChangeCipherSpec, State0) of diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl index 3bc1290361..9c167e90d6 100644 --- a/lib/ssl/src/tls_handshake_1_3.erl +++ b/lib/ssl/src/tls_handshake_1_3.erl @@ -44,6 +44,7 @@ -export([do_start/2, do_negotiated/2, + do_wait_cert/2, do_wait_finished/2]). %%==================================================================== @@ -87,6 +88,36 @@ encrypted_extensions() -> }. +certificate_request(SignAlgs0, SignAlgsCert0) -> + %% Input arguments contain TLS 1.2 algorithms due to backward compatibility + %% reasons. These {Hash, Algo} tuples must be filtered before creating the + %% the extensions. + SignAlgs = filter_tls13_algs(SignAlgs0), + SignAlgsCert = filter_tls13_algs(SignAlgsCert0), + Extensions0 = add_signature_algorithms(#{}, SignAlgs), + Extensions = add_signature_algorithms_cert(Extensions0, SignAlgsCert), + #certificate_request_1_3{ + certificate_request_context = <<>>, + extensions = Extensions}. + + +add_signature_algorithms(Extensions, SignAlgs) -> + Extensions#{signature_algorithms => + #signature_algorithms{signature_scheme_list = SignAlgs}}. + + +add_signature_algorithms_cert(Extensions, undefined) -> + Extensions; +add_signature_algorithms_cert(Extensions, SignAlgsCert) -> + Extensions#{signature_algorithms_cert => + #signature_algorithms{signature_scheme_list = SignAlgsCert}}. + + +filter_tls13_algs(undefined) -> undefined; +filter_tls13_algs(Algo) -> + lists:filter(fun is_atom/1, Algo). + + %% TODO: use maybe monad for error handling! %% enum { %% X509(0), @@ -497,7 +528,7 @@ do_negotiated(#{client_share := ClientKey, #state{connection_states = ConnectionStates0, session = #session{session_id = SessionId, own_certificate = OwnCert}, - ssl_options = #ssl_options{} = _SslOpts, + ssl_options = #ssl_options{} = SslOpts, key_share = KeyShare, handshake_env = #handshake_env{tls_handshake_history = _HHistory0}, connection_env = #connection_env{private_key = CertPrivateKey}, @@ -527,28 +558,31 @@ do_negotiated(#{client_share := ClientKey, %% Encode EncryptedExtensions State4 = tls_connection:queue_handshake(EncryptedExtensions, State3), + %% Create and send CertificateRequest ({verify, verify_peer}) + {State5, NextState} = maybe_send_certificate_request(State4, SslOpts), + %% Create Certificate Certificate = certificate(OwnCert, CertDbHandle, CertDbRef, <<>>, server), %% Encode Certificate - State5 = tls_connection:queue_handshake(Certificate, State4), + State6 = tls_connection:queue_handshake(Certificate, State5), %% Create CertificateVerify CertificateVerify = Maybe(certificate_verify(CertPrivateKey, SignatureScheme, - State5, server)), + State6, server)), %% Encode CertificateVerify - State6 = tls_connection:queue_handshake(CertificateVerify, State5), + State7 = tls_connection:queue_handshake(CertificateVerify, State6), %% Create Finished - Finished = finished(State6), + Finished = finished(State7), %% Encode Finished - State7 = tls_connection:queue_handshake(Finished, State6), + State8 = tls_connection:queue_handshake(Finished, State7), %% Send first flight - {State8, _} = tls_connection:send_handshake_flight(State7), + {State9, _} = tls_connection:send_handshake_flight(State8), - State8 + {State9, NextState} catch {Ref, {state_not_implemented, State}} -> @@ -557,6 +591,20 @@ do_negotiated(#{client_share := ClientKey, end. +do_wait_cert(#change_cipher_spec{}, State0) -> + {State0, wait_cert}; +do_wait_cert(#certificate_1_3{} = Certificate, State0) -> + {Ref,Maybe} = maybe(), + try + NextState = Maybe(process_client_certificate(Certificate, State0)), + {State0, NextState} + + catch + {Ref, {certificate_required, State}} -> + {?ALERT_REC(?FATAL, ?CERTIFICATE_REQUIRED, certificate_required), State} + end. + + do_wait_finished(#change_cipher_spec{}, #state{connection_states = _ConnectionStates0, session = #session{session_id = _SessionId, @@ -659,6 +707,42 @@ send_hello_retry_request(State0, _, _, _) -> {ok, {State0, negotiated}}. +maybe_send_certificate_request(State, #ssl_options{verify = verify_none}) -> + {State, wait_finished}; +maybe_send_certificate_request(State, #ssl_options{ + verify = verify_peer, + signature_algs = SignAlgs, + signature_algs_cert = SignAlgsCert}) -> + CertificateRequest = certificate_request(SignAlgs, SignAlgsCert), + {tls_connection:queue_handshake(CertificateRequest, State), wait_cert}. + + +process_client_certificate(#certificate_1_3{ + certificate_request_context = <<>>, + certificate_list = []}, + #state{ssl_options = + #ssl_options{ + fail_if_no_peer_cert = false}}) -> + {ok, wait_finished}; +process_client_certificate(#certificate_1_3{ + certificate_request_context = <<>>, + certificate_list = []}, + #state{ssl_options = + #ssl_options{ + fail_if_no_peer_cert = true}} = State0) -> + + %% At this point the client believes that the connection is up and starts using + %% its traffic secrets. In order to be able send an proper Alert to the client + %% the server should also change its connection state and use the traffic + %% secrets. + State1 = calculate_traffic_secrets(State0), + State = ssl_record:step_encryption_state(State1), + {error, {certificate_required, State}}; +process_client_certificate(_, _) -> + %% TODO: validate cert + {ok, wait_cv}. + + %% 4.4.1. The Transcript Hash %% %% As an exception to this general rule, when the server responds to a @@ -746,10 +830,8 @@ calculate_traffic_secrets(#state{connection_states = ConnectionStates, MasterSecret = tls_v1:key_schedule(master_secret, HKDFAlgo, HandshakeSecret), - {Messages0, _} = HHistory, - - %% Drop Client Finish - [_|Messages] = Messages0, + %% Get the correct list messages for the handshake context. + Messages = get_handshake_context(HHistory), %% Calculate [sender]_application_traffic_secret_0 ClientAppTrafficSecret0 = @@ -845,6 +927,32 @@ cipher_init(Key, IV, FinishedKey) -> tag_len = 16}. +%% Handling the case when client is authenticated +get_handshake_context({[<<20,_/binary>>,<<11,_/binary>>|Messages], _}) -> + %% Handshake context messages: + %% ClientHello (client) (0) + %% ServerHello (server) (1), + %% EncryptedExtensions (server) (8) + %% CertificateRequest (server) (13), + %% Certificate (server) (11) + %% CertificateVerify (server) (15), + %% Finished (server) (20) + %% Certificate (client) (11) - Drop! Not included in calculations! + %% Finished (client) (20) - Drop! Not included in calculations! + Messages; +%% Normal case +get_handshake_context({[_| Messages], _}) -> + %% Handshake context messages: + %% ClientHello (client) (0) + %% ServerHello (server) (1), + %% EncryptedExtensions (server) (8) + %% Certificate (server) (11) + %% CertificateVerify (server) (15), + %% Finished (server) (20) + %% Finished (client) (20) - Drop! Not included in calculations! + Messages. + + %% If there is no overlap between the received %% "supported_groups" and the groups supported by the server, then the %% server MUST abort the handshake with a "handshake_failure" or an -- cgit v1.2.3 From 1e06a50821bff93643f342019840e8932e151686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Wed, 27 Feb 2019 16:15:50 +0100 Subject: ssl: Test client authentication (empty cert) Test client authentication when client responds with empty Certificate. Change-Id: I725ae60c6d097ca13c5f4354e35377ecacf98dea --- lib/ssl/test/ssl_basic_SUITE.erl | 64 +++++++++++++++++++++++++++++++++++++++- lib/ssl/test/ssl_test_lib.erl | 14 +++++++-- 2 files changed, 74 insertions(+), 4 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl index 292916692d..5d5c23ca2c 100644 --- a/lib/ssl/test/ssl_basic_SUITE.erl +++ b/lib/ssl/test/ssl_basic_SUITE.erl @@ -281,7 +281,9 @@ tls13_test_group() -> tls13_1_RTT_handshake, tls13_basic_ssl_server_openssl_client, tls13_custom_groups_ssl_server_openssl_client, - tls13_hello_retry_request_ssl_server_openssl_client]. + tls13_hello_retry_request_ssl_server_openssl_client, + tls13_client_auth_empty_cert_alert_ssl_server_openssl_client, + tls13_client_auth_empty_cert_ssl_server_openssl_client]. %%-------------------------------------------------------------------- init_per_suite(Config0) -> @@ -5588,6 +5590,66 @@ tls13_hello_retry_request_ssl_server_openssl_client(Config) -> ssl_test_lib:close(Server), ssl_test_lib:close_port(Client). +tls13_client_auth_empty_cert_alert_ssl_server_openssl_client() -> + [{doc,"TLS 1.3: Test client authentication when client sends an empty certificate and fail_if_no_peer_cert is set to true."}]. + +tls13_client_auth_empty_cert_alert_ssl_server_openssl_client(Config) -> + ClientOpts0 = ssl_test_lib:ssl_options(client_rsa_opts, Config), + %% Delete Client Cert and Key + ClientOpts1 = proplists:delete(certfile, ClientOpts0), + ClientOpts = proplists:delete(keyfile, ClientOpts1), + + ServerOpts0 = ssl_test_lib:ssl_options(server_rsa_opts, Config), + %% Set versions + ServerOpts = [{versions, ['tlsv1.2','tlsv1.3']}, + {verify, verify_peer}, + {fail_if_no_peer_cert, true}|ServerOpts0], + {_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, send_recv_result_active, []}}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + + Client = ssl_test_lib:start_basic_client(openssl, 'tlsv1.3', Port, ClientOpts), + + ssl_test_lib:check_result(Server, + {error, + {tls_alert, + {certificate_required, + "received SERVER ALERT: Fatal - Certificate required - certificate_required"}}}), + ssl_test_lib:close(Server), + ssl_test_lib:close_port(Client). + +tls13_client_auth_empty_cert_ssl_server_openssl_client() -> + [{doc,"TLS 1.3: Test client authentication when client sends an empty certificate and fail_if_no_peer_cert is set to false."}]. + +tls13_client_auth_empty_cert_ssl_server_openssl_client(Config) -> + ClientOpts0 = ssl_test_lib:ssl_options(client_rsa_opts, Config), + %% Delete Client Cert and Key + ClientOpts1 = proplists:delete(certfile, ClientOpts0), + ClientOpts = proplists:delete(keyfile, ClientOpts1), + + ServerOpts0 = ssl_test_lib:ssl_options(server_rsa_opts, Config), + %% Set versions + ServerOpts = [{versions, ['tlsv1.2','tlsv1.3']}, + {verify, verify_peer}, + {fail_if_no_peer_cert, false}|ServerOpts0], + {_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, send_recv_result_active, []}}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + + Client = ssl_test_lib:start_basic_client(openssl, 'tlsv1.3', Port, ClientOpts), + + ssl_test_lib:check_result(Server, ok), + ssl_test_lib:close(Server), + ssl_test_lib:close_port(Client). + %%-------------------------------------------------------------------- %% Internal functions ------------------------------------------------ diff --git a/lib/ssl/test/ssl_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl index f628b4e6d4..7d83dbd382 100644 --- a/lib/ssl/test/ssl_test_lib.erl +++ b/lib/ssl/test/ssl_test_lib.erl @@ -1114,15 +1114,23 @@ start_basic_client(openssl, Version, Port, ClientOpts) -> Exe = "openssl", Args0 = ["s_client", "-verify", "2", "-port", integer_to_list(Port), ssl_test_lib:version_flag(Version), - "-cert", Cert, "-CAfile", CA, - "-key", Key, "-host","localhost", "-msg", "-debug"], - Args = + "-CAfile", CA, "-host", "localhost", "-msg", "-debug"], + Args1 = case Groups0 of undefined -> Args0; G -> Args0 ++ ["-groups", G] end, + Args = + case {Cert, Key} of + {C, K} when C =:= undefined orelse + K =:= undefined -> + Args0; + {C, K} -> + Args1 ++ ["-cert", C, "-key", K] + end, + OpenSslPort = ssl_test_lib:portable_open_port(Exe, Args), true = port_command(OpenSslPort, "Hello world"), OpenSslPort. -- cgit v1.2.3 From 2daeee7aea9af5b403ac7557b8f20a4f34bc6b61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Wed, 27 Feb 2019 16:49:02 +0100 Subject: ssl: Validate client certificates (TLS 1.3) Implement validation of client certificates in state 'wait_cert'. Implement state 'wait_cv'. Clean up handler functions. Change-Id: I5c410bf7afe34632f27fabcd61670764fedb105d --- lib/ssl/src/ssl_handshake.erl | 5 +- lib/ssl/src/tls_connection_1_3.erl | 52 +++++----- lib/ssl/src/tls_handshake_1_3.erl | 195 ++++++++++++++++++++++++------------- 3 files changed, 158 insertions(+), 94 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index 260f603e90..6c95a7edf8 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -79,7 +79,10 @@ select_hashsign_algs/3, empty_extensions/2, add_server_share/3 ]). --export([get_cert_params/1]). +-export([get_cert_params/1, + server_name/3, + validation_fun_and_state/9, + handle_path_validation_error/7]). %%==================================================================== %% Create handshake messages diff --git a/lib/ssl/src/tls_connection_1_3.erl b/lib/ssl/src/tls_connection_1_3.erl index 3c292a43b0..436eca03f3 100644 --- a/lib/ssl/src/tls_connection_1_3.erl +++ b/lib/ssl/src/tls_connection_1_3.erl @@ -111,19 +111,14 @@ -export([start/4, negotiated/4, wait_cert/4, + wait_cv/4, wait_finished/4 ]). -start(internal, - #change_cipher_spec{} = ChangeCipherSpec, State0, _Module) -> - case tls_handshake_1_3:do_start(ChangeCipherSpec, State0) of - #alert{} = Alert -> - ssl_connection:handle_own_alert(Alert, {3,4}, start, State0); - State1 -> - {Record, State} = tls_connection:next_record(State1), - tls_connection:next_event(?FUNCTION_NAME, Record, State) - end; +start(internal, #change_cipher_spec{}, State0, _Module) -> + {Record, State} = tls_connection:next_record(State0), + tls_connection:next_event(?FUNCTION_NAME, Record, State); start(internal, #client_hello{} = Hello, State0, _Module) -> case tls_handshake_1_3:do_start(Hello, State0) of #alert{} = Alert -> @@ -137,6 +132,9 @@ start(Type, Msg, State, Connection) -> ssl_connection:handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection). +negotiated(internal, #change_cipher_spec{}, State0, _Module) -> + {Record, State} = tls_connection:next_record(State0), + tls_connection:next_event(?FUNCTION_NAME, Record, State); negotiated(internal, Map, State0, _Module) -> case tls_handshake_1_3:do_negotiated(Map, State0) of #alert{} = Alert -> @@ -146,15 +144,9 @@ negotiated(internal, Map, State0, _Module) -> end. -wait_cert(internal, - #change_cipher_spec{} = ChangeCipherSpec, State0, _Module) -> - case tls_handshake_1_3:do_wait_cert(ChangeCipherSpec, State0) of - #alert{} = Alert -> - ssl_connection:handle_own_alert(Alert, {3,4}, wait_cert, State0); - {State1, NextState} -> - {Record, State} = tls_connection:next_record(State1), - tls_connection:next_event(NextState, Record, State) - end; +wait_cert(internal, #change_cipher_spec{}, State0, _Module) -> + {Record, State} = tls_connection:next_record(State0), + tls_connection:next_event(?FUNCTION_NAME, Record, State); wait_cert(internal, #certificate_1_3{} = Certificate, State0, _Module) -> case tls_handshake_1_3:do_wait_cert(Certificate, State0) of @@ -168,15 +160,25 @@ wait_cert(Type, Msg, State, Connection) -> ssl_connection:handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection). -wait_finished(internal, - #change_cipher_spec{} = ChangeCipherSpec, State0, _Module) -> - case tls_handshake_1_3:do_wait_finished(ChangeCipherSpec, State0) of - #alert{} = Alert -> - ssl_connection:handle_own_alert(Alert, {3,4}, wait_finished, State0); - State1 -> +wait_cv(internal, #change_cipher_spec{}, State0, _Module) -> + {Record, State} = tls_connection:next_record(State0), + tls_connection:next_event(?FUNCTION_NAME, Record, State); +wait_cv(internal, + #certificate_verify_1_3{} = CertificateVerify, State0, _Module) -> + case tls_handshake_1_3:do_wait_cv(CertificateVerify, State0) of + {#alert{} = Alert, State} -> + ssl_connection:handle_own_alert(Alert, {3,4}, wait_cv, State); + {State1, NextState} -> {Record, State} = tls_connection:next_record(State1), - tls_connection:next_event(?FUNCTION_NAME, Record, State) + tls_connection:next_event(NextState, Record, State) end; +wait_cv(Type, Msg, State, Connection) -> + ssl_connection:handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection). + + +wait_finished(internal, #change_cipher_spec{}, State0, _Module) -> + {Record, State} = tls_connection:next_record(State0), + tls_connection:next_event(?FUNCTION_NAME, Record, State); wait_finished(internal, #finished{} = Finished, State0, Module) -> case tls_handshake_1_3:do_wait_finished(Finished, State0) of diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl index 9c167e90d6..7a7e3575d7 100644 --- a/lib/ssl/src/tls_handshake_1_3.erl +++ b/lib/ssl/src/tls_handshake_1_3.erl @@ -45,6 +45,7 @@ -export([do_start/2, do_negotiated/2, do_wait_cert/2, + do_wait_cv/2, do_wait_finished/2]). %%==================================================================== @@ -410,29 +411,6 @@ build_content(Context, THash) -> %%==================================================================== -do_start(#change_cipher_spec{}, - #state{connection_states = _ConnectionStates0, - session = #session{session_id = _SessionId, - own_certificate = _OwnCert}, - ssl_options = #ssl_options{} = _SslOpts, - key_share = _KeyShare, - handshake_env = #handshake_env{tls_handshake_history = _HHistory0}, - static_env = #static_env{ - cert_db = _CertDbHandle, - cert_db_ref = _CertDbRef, - socket = _Socket, - transport_cb = _Transport} - } = State0) -> - %% {Ref,Maybe} = maybe(), - - try - - State0 - - catch - {_Ref, {state_not_implemented, State}} -> - ?ALERT_REC(?FATAL, ?INTERNAL_ERROR, {state_not_implemented, State}) - end; do_start(#client_hello{cipher_suites = ClientCiphers, session_id = SessionId, extensions = Extensions} = _Hello, @@ -585,49 +563,37 @@ do_negotiated(#{client_share := ClientKey, {State9, NextState} catch - {Ref, {state_not_implemented, State}} -> - %% TODO - ?ALERT_REC(?FATAL, ?INTERNAL_ERROR, {state_not_implemented, State}) + {Ref, badarg} -> + ?ALERT_REC(?FATAL, ?INTERNAL_ERROR, {digitally_sign, badarg}) end. -do_wait_cert(#change_cipher_spec{}, State0) -> - {State0, wait_cert}; do_wait_cert(#certificate_1_3{} = Certificate, State0) -> {Ref,Maybe} = maybe(), try - NextState = Maybe(process_client_certificate(Certificate, State0)), - {State0, NextState} - + Maybe(process_client_certificate(Certificate, State0)) catch {Ref, {certificate_required, State}} -> - {?ALERT_REC(?FATAL, ?CERTIFICATE_REQUIRED, certificate_required), State} + {?ALERT_REC(?FATAL, ?CERTIFICATE_REQUIRED, certificate_required), State}; + {Ref, {{certificate_unknown, Reason}, State}} -> + {?ALERT_REC(?FATAL, ?CERTIFICATE_UNKNOWN, Reason), State}; + {Ref, {{internal_error, Reason}, State}} -> + {?ALERT_REC(?FATAL, ?INTERNAL_ERROR, Reason), State}; + {#alert{} = Alert, State} -> + {Alert, State} end. -do_wait_finished(#change_cipher_spec{}, - #state{connection_states = _ConnectionStates0, - session = #session{session_id = _SessionId, - own_certificate = _OwnCert}, - ssl_options = #ssl_options{} = _SslOpts, - key_share = _KeyShare, - handshake_env = #handshake_env{tls_handshake_history = _HHistory0}, - static_env = #static_env{ - cert_db = _CertDbHandle, - cert_db_ref = _CertDbRef, - socket = _Socket, - transport_cb = _Transport} - } = State0) -> - %% {Ref,Maybe} = maybe(), - +do_wait_cv(#certificate_verify_1_3{} = _CertificateVerify, State0) -> + {Ref,Maybe} = maybe(), try + Maybe(verify_certificate(State0)) + catch + {Ref, {bad_certificate, Reason}} -> + {?ALERT_REC(?FATAL, ?BAD_CERTIFICATE, {bad_certificate, Reason}), State0} + end. - State0 - catch - {_Ref, {state_not_implemented, State}} -> - ?ALERT_REC(?FATAL, ?INTERNAL_ERROR, {state_not_implemented, State}) - end; do_wait_finished(#finished{verify_data = VerifyData}, #state{connection_states = _ConnectionStates0, session = #session{session_id = _SessionId, @@ -655,16 +621,19 @@ do_wait_finished(#finished{verify_data = VerifyData}, catch {Ref, decrypt_error} -> - ?ALERT_REC(?FATAL, ?DECRYPT_ERROR, decrypt_error); - {_, {state_not_implemented, State}} -> - %% TODO - ?ALERT_REC(?FATAL, ?INTERNAL_ERROR, {state_not_implemented, State}) + ?ALERT_REC(?FATAL, ?DECRYPT_ERROR, decrypt_error) end. %% TODO: Remove this function! -%% not_implemented(State) -> -%% {error, {state_not_implemented, State}}. +%% not_implemented(State, Reason) -> +%% {error, {not_implemented, State, Reason}}. +%% +%% not_implemented(update_secrets, State0, Reason) -> +%% State1 = calculate_traffic_secrets(State0), +%% State = ssl_record:step_encryption_state(State1), +%% {error, {not_implemented, State, Reason}}. + %% Recipients of Finished messages MUST verify that the contents are @@ -722,8 +691,8 @@ process_client_certificate(#certificate_1_3{ certificate_list = []}, #state{ssl_options = #ssl_options{ - fail_if_no_peer_cert = false}}) -> - {ok, wait_finished}; + fail_if_no_peer_cert = false}} = State) -> + {ok, {State, wait_finished}}; process_client_certificate(#certificate_1_3{ certificate_request_context = <<>>, certificate_list = []}, @@ -738,9 +707,80 @@ process_client_certificate(#certificate_1_3{ State1 = calculate_traffic_secrets(State0), State = ssl_record:step_encryption_state(State1), {error, {certificate_required, State}}; -process_client_certificate(_, _) -> - %% TODO: validate cert - {ok, wait_cv}. +process_client_certificate(#certificate_1_3{certificate_list = Certs0}, + #state{ssl_options = SslOptions, + static_env = + #static_env{ + role = Role, + host = Host, + cert_db = CertDbHandle, + cert_db_ref = CertDbRef, + crl_db = CRLDbHandle}} = State0) -> + %% TODO: handle extensions! + + %% Remove extensions from list of certificates! + Certs = convert_certificate_chain(Certs0), + + case validate_certificate_chain(Certs, CertDbHandle, CertDbRef, + SslOptions, CRLDbHandle, Role, Host) of + {ok, PeerCert} -> + State = store_peer_cert(State0, PeerCert), + + {ok, {State, wait_cv}}; + {error, Reason} -> + State1 = calculate_traffic_secrets(State0), + State = ssl_record:step_encryption_state(State1), + {error, {Reason, State}}; + %% TODO: refactor!? + #alert{} = Alert -> + State1 = calculate_traffic_secrets(State0), + State = ssl_record:step_encryption_state(State1), + {Alert, State} + end. + + +validate_certificate_chain(Certs, CertDbHandle, CertDbRef, SslOptions, CRLDbHandle, Role, Host) -> + ServerName = ssl_handshake:server_name(SslOptions#ssl_options.server_name_indication, Host, Role), + [PeerCert | ChainCerts ] = Certs, + try + {TrustedCert, CertPath} = + ssl_certificate:trusted_cert_and_path(Certs, CertDbHandle, CertDbRef, + SslOptions#ssl_options.partial_chain), + ValidationFunAndState = + ssl_handshake:validation_fun_and_state(SslOptions#ssl_options.verify_fun, Role, + CertDbHandle, CertDbRef, ServerName, + SslOptions#ssl_options.customize_hostname_check, + SslOptions#ssl_options.crl_check, CRLDbHandle, CertPath), + Options = [{max_path_length, SslOptions#ssl_options.depth}, + {verify_fun, ValidationFunAndState}], + case public_key:pkix_path_validation(TrustedCert, CertPath, Options) of + {ok, {_PublicKeyInfo,_}} -> + {ok, PeerCert}; + {error, Reason} -> + ssl_handshake:handle_path_validation_error(Reason, PeerCert, ChainCerts, + SslOptions, Options, + CertDbHandle, CertDbRef) + end + catch + error:{badmatch,{asn1, Asn1Reason}} -> + %% ASN-1 decode of certificate somehow failed + {error, {certificate_unknown, {failed_to_decode_certificate, Asn1Reason}}}; + error:OtherReason -> + {error, {internal_error, {unexpected_error, OtherReason}}} + end. + + +store_peer_cert(#state{session = #session{} = Session} = State, PeerCert) -> + State#state{session = Session#session{peer_certificate = PeerCert}}. + + +convert_certificate_chain(Certs) -> + Fun = fun(#certificate_entry{data = Data}) -> + {true, Data}; + (_) -> + false + end, + lists:filtermap(Fun, Certs). %% 4.4.1. The Transcript Hash @@ -927,11 +967,25 @@ cipher_init(Key, IV, FinishedKey) -> tag_len = 16}. -%% Handling the case when client is authenticated +%% Client is authenticated +get_handshake_context({[<<20,_/binary>>,<<15,_/binary>>,<<11,_/binary>>|Messages], _}) -> + %% Handshake context messages: + %% ClientHello (client) (1) + %% ServerHello (server) (2), + %% EncryptedExtensions (server) (8) + %% CertificateRequest (server) (13), + %% Certificate (server) (11) + %% CertificateVerify (server) (15), + %% Finished (server) (20) + %% Certificate (client) (11) - Drop! Not included in calculations! + %% CertificateVerify (client) (15) - Drop! Not included in calculations! + %% Finished (client) (20) - Drop! Not included in calculations! + Messages; +%% Client is authenticated (empty certificate) get_handshake_context({[<<20,_/binary>>,<<11,_/binary>>|Messages], _}) -> %% Handshake context messages: - %% ClientHello (client) (0) - %% ServerHello (server) (1), + %% ClientHello (client) (1) + %% ServerHello (server) (2), %% EncryptedExtensions (server) (8) %% CertificateRequest (server) (13), %% Certificate (server) (11) @@ -943,8 +997,8 @@ get_handshake_context({[<<20,_/binary>>,<<11,_/binary>>|Messages], _}) -> %% Normal case get_handshake_context({[_| Messages], _}) -> %% Handshake context messages: - %% ClientHello (client) (0) - %% ServerHello (server) (1), + %% ClientHello (client) (1) + %% ServerHello (server) (2), %% EncryptedExtensions (server) (8) %% Certificate (server) (11) %% CertificateVerify (server) (15), @@ -953,6 +1007,11 @@ get_handshake_context({[_| Messages], _}) -> Messages. +verify_certificate(State) -> + %% TODO: implement + {ok, {State, wait_finished}}. + + %% If there is no overlap between the received %% "supported_groups" and the groups supported by the server, then the %% server MUST abort the handshake with a "handshake_failure" or an -- cgit v1.2.3 From 372ea8b13a9ccbd9833813838ad3cd1635d3fb5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Fri, 1 Mar 2019 15:40:01 +0100 Subject: ssl: Test client authentication with certificate Change-Id: I09c0501ea790941001b11a3f6d12a96f18da2bea --- lib/ssl/test/ssl_basic_SUITE.erl | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) (limited to 'lib/ssl') diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl index 5d5c23ca2c..630b935599 100644 --- a/lib/ssl/test/ssl_basic_SUITE.erl +++ b/lib/ssl/test/ssl_basic_SUITE.erl @@ -283,7 +283,8 @@ tls13_test_group() -> tls13_custom_groups_ssl_server_openssl_client, tls13_hello_retry_request_ssl_server_openssl_client, tls13_client_auth_empty_cert_alert_ssl_server_openssl_client, - tls13_client_auth_empty_cert_ssl_server_openssl_client]. + tls13_client_auth_empty_cert_ssl_server_openssl_client, + tls13_client_auth_ssl_server_openssl_client]. %%-------------------------------------------------------------------- init_per_suite(Config0) -> @@ -5651,6 +5652,32 @@ tls13_client_auth_empty_cert_ssl_server_openssl_client(Config) -> ssl_test_lib:close_port(Client). +tls13_client_auth_ssl_server_openssl_client() -> + [{doc,"TLS 1.3: Test client authentication."}]. + +tls13_client_auth_ssl_server_openssl_client(Config) -> + ClientOpts = ssl_test_lib:ssl_options(client_rsa_opts, Config), + + ServerOpts0 = ssl_test_lib:ssl_options(server_rsa_opts, Config), + %% Set versions + ServerOpts = [{versions, ['tlsv1.2','tlsv1.3']}, + {verify, verify_peer}, + {fail_if_no_peer_cert, true}|ServerOpts0], + {_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, send_recv_result_active, []}}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + + Client = ssl_test_lib:start_basic_client(openssl, 'tlsv1.3', Port, ClientOpts), + + ssl_test_lib:check_result(Server, ok), + ssl_test_lib:close(Server), + ssl_test_lib:close_port(Client). + + %%-------------------------------------------------------------------- %% Internal functions ------------------------------------------------ %%-------------------------------------------------------------------- -- cgit v1.2.3 From 839207c411f8cb09347f9497e5db931d7a30d5da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Wed, 6 Mar 2019 09:34:31 +0100 Subject: ssl: Verify CertificateVerify Verify CertificateVerify message against the handshake context and the public key provided by the Certificate message. Remove 'Context' argument from state handler functions and store data in the state variable. Refactor get_handshake_context/1 to cover all implemented cases. Change-Id: If803e05009331d1ec7e0ba2ea2b81d917a0add6d --- lib/ssl/src/ssl_handshake.hrl | 4 +- lib/ssl/src/tls_connection_1_3.erl | 10 +- lib/ssl/src/tls_handshake_1_3.erl | 245 ++++++++++++++++++++++++------------- 3 files changed, 171 insertions(+), 88 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/ssl_handshake.hrl b/lib/ssl/src/ssl_handshake.hrl index d4233bea9b..b248edcaa9 100644 --- a/lib/ssl/src/ssl_handshake.hrl +++ b/lib/ssl/src/ssl_handshake.hrl @@ -47,7 +47,9 @@ srp_username, is_resumable, time_stamp, - ecc + ecc, %% TLS 1.3 Group + sign_alg, %% TLS 1.3 Signature Algorithm + dh_public_value %% TLS 1.3 DH Public Value from peer }). -define(NUM_OF_SESSION_ID_BYTES, 32). % TSL 1.1 & SSL 3 diff --git a/lib/ssl/src/tls_connection_1_3.erl b/lib/ssl/src/tls_connection_1_3.erl index 436eca03f3..701a5860c2 100644 --- a/lib/ssl/src/tls_connection_1_3.erl +++ b/lib/ssl/src/tls_connection_1_3.erl @@ -123,10 +123,10 @@ start(internal, #client_hello{} = Hello, State0, _Module) -> case tls_handshake_1_3:do_start(Hello, State0) of #alert{} = Alert -> ssl_connection:handle_own_alert(Alert, {3,4}, start, State0); - {State, _, start} -> + {State, start} -> {next_state, start, State, []}; - {State, Context, negotiated} -> - {next_state, negotiated, State, [{next_event, internal, Context}]} + {State, negotiated} -> + {next_state, negotiated, State, [{next_event, internal, start_handshake}]} end; start(Type, Msg, State, Connection) -> ssl_connection:handle_common_event(Type, Msg, ?FUNCTION_NAME, State, Connection). @@ -135,8 +135,8 @@ start(Type, Msg, State, Connection) -> negotiated(internal, #change_cipher_spec{}, State0, _Module) -> {Record, State} = tls_connection:next_record(State0), tls_connection:next_event(?FUNCTION_NAME, Record, State); -negotiated(internal, Map, State0, _Module) -> - case tls_handshake_1_3:do_negotiated(Map, State0) of +negotiated(internal, Message, State0, _Module) -> + case tls_handshake_1_3:do_negotiated(Message, State0) of #alert{} = Alert -> ssl_connection:handle_own_alert(Alert, {3,4}, negotiated, State0); {State, NextState} -> diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl index 7a7e3575d7..858a1acdac 100644 --- a/lib/ssl/src/tls_handshake_1_3.erl +++ b/lib/ssl/src/tls_handshake_1_3.erl @@ -176,7 +176,7 @@ certificate_verify(PrivateKey, SignatureScheme, %% Digital signatures use the hash function defined by the selected signature %% scheme. - case digitally_sign(THash, <<"TLS 1.3, server CertificateVerify">>, + case sign(THash, <<"TLS 1.3, server CertificateVerify">>, HashAlgo, PrivateKey) of {ok, Signature} -> {ok, #certificate_verify_1_3{ @@ -384,7 +384,7 @@ certificate_entry(DER) -> %% 79 %% 00 %% 0101010101010101010101010101010101010101010101010101010101010101 -digitally_sign(THash, Context, HashAlgo, PrivateKey) -> +sign(THash, Context, HashAlgo, PrivateKey) -> Content = build_content(Context, THash), %% The length of the Salt MUST be equal to the length of the output @@ -401,6 +401,23 @@ digitally_sign(THash, Context, HashAlgo, PrivateKey) -> end. +verify(THash, Context, HashAlgo, Signature, PublicKey) -> + Content = build_content(Context, THash), + + %% The length of the Salt MUST be equal to the length of the output + %% of the digest algorithm: rsa_pss_saltlen = -1 + try public_key:verify(Content, HashAlgo, Signature, PublicKey, + [{rsa_padding, rsa_pkcs1_pss_padding}, + {rsa_pss_saltlen, -1}, + {rsa_mgf1_md, HashAlgo}]) of + Result -> + {ok, Result} + catch + error:badarg -> + {error, badarg} + end. + + build_content(Context, THash) -> Prefix = binary:copy(<<32>>, 64), <>. @@ -463,7 +480,8 @@ do_start(#client_hello{cipher_suites = ClientCiphers, %% Generate server_share KeyShare = ssl_cipher:generate_server_share(Group), - State1 = update_start_state(State0, Cipher, KeyShare, SessionId), + State1 = update_start_state(State0, Cipher, KeyShare, SessionId, + Group, SelectedSignAlg, ClientPubKey), %% 4.1.4. Hello Retry Request %% @@ -471,14 +489,7 @@ do_start(#client_hello{cipher_suites = ClientCiphers, %% message if it is able to find an acceptable set of parameters but the %% ClientHello does not contain sufficient information to proceed with %% the handshake. - {State2, NextState} = - Maybe(send_hello_retry_request(State1, ClientPubKey, KeyShare, SessionId)), - - %% TODO: Add Context to state? - Context = #{group => Group, - sign_alg => SelectedSignAlg, - client_share => ClientPubKey}, - {State2, Context, NextState} + Maybe(send_hello_retry_request(State1, ClientPubKey, KeyShare, SessionId)) %% TODO: %% - session handling @@ -499,23 +510,23 @@ do_start(#client_hello{cipher_suites = ClientCiphers, end. -do_negotiated(#{client_share := ClientKey, - group := SelectedGroup, - sign_alg := SignatureScheme - }, - #state{connection_states = ConnectionStates0, - session = #session{session_id = SessionId, - own_certificate = OwnCert}, - ssl_options = #ssl_options{} = SslOpts, - key_share = KeyShare, - handshake_env = #handshake_env{tls_handshake_history = _HHistory0}, - connection_env = #connection_env{private_key = CertPrivateKey}, - static_env = #static_env{ - cert_db = CertDbHandle, - cert_db_ref = CertDbRef, - socket = _Socket, - transport_cb = _Transport} - } = State0) -> +do_negotiated(start_handshake, + #state{connection_states = ConnectionStates0, + session = #session{session_id = SessionId, + own_certificate = OwnCert, + ecc = SelectedGroup, + sign_alg = SignatureScheme, + dh_public_value = ClientKey}, + ssl_options = #ssl_options{} = SslOpts, + key_share = KeyShare, + handshake_env = #handshake_env{tls_handshake_history = _HHistory0}, + connection_env = #connection_env{private_key = CertPrivateKey}, + static_env = #static_env{ + cert_db = CertDbHandle, + cert_db_ref = CertDbRef, + socket = _Socket, + transport_cb = _Transport} + } = State0) -> {Ref,Maybe} = maybe(), try @@ -584,13 +595,18 @@ do_wait_cert(#certificate_1_3{} = Certificate, State0) -> end. -do_wait_cv(#certificate_verify_1_3{} = _CertificateVerify, State0) -> +do_wait_cv(#certificate_verify_1_3{} = CertificateVerify, State0) -> {Ref,Maybe} = maybe(), try - Maybe(verify_certificate(State0)) + Maybe(verify_signature_algorithm(State0, CertificateVerify)), + Maybe(verify_certificate_verify(State0, CertificateVerify)) catch - {Ref, {bad_certificate, Reason}} -> - {?ALERT_REC(?FATAL, ?BAD_CERTIFICATE, {bad_certificate, Reason}), State0} + {Ref, {{bad_certificate, Reason}, State}} -> + {?ALERT_REC(?FATAL, ?BAD_CERTIFICATE, {bad_certificate, Reason}), State}; + {Ref, {badarg, State}} -> + {?ALERT_REC(?FATAL, ?INTERNAL_ERROR, {verify, badarg}), State}; + {Ref, {{handshake_failure, Reason}, State}} -> + {?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, {handshake_failure, Reason}), State} end. @@ -723,9 +739,8 @@ process_client_certificate(#certificate_1_3{certificate_list = Certs0}, case validate_certificate_chain(Certs, CertDbHandle, CertDbRef, SslOptions, CRLDbHandle, Role, Host) of - {ok, PeerCert} -> - State = store_peer_cert(State0, PeerCert), - + {ok, {PeerCert, PublicKeyInfo}} -> + State = store_peer_cert(State0, PeerCert, PublicKeyInfo), {ok, {State, wait_cv}}; {error, Reason} -> State1 = calculate_traffic_secrets(State0), @@ -754,8 +769,8 @@ validate_certificate_chain(Certs, CertDbHandle, CertDbRef, SslOptions, CRLDbHand Options = [{max_path_length, SslOptions#ssl_options.depth}, {verify_fun, ValidationFunAndState}], case public_key:pkix_path_validation(TrustedCert, CertPath, Options) of - {ok, {_PublicKeyInfo,_}} -> - {ok, PeerCert}; + {ok, {PublicKeyInfo,_}} -> + {ok, {PeerCert, PublicKeyInfo}}; {error, Reason} -> ssl_handshake:handle_path_validation_error(Reason, PeerCert, ChainCerts, SslOptions, Options, @@ -770,8 +785,10 @@ validate_certificate_chain(Certs, CertDbHandle, CertDbRef, SslOptions, CRLDbHand end. -store_peer_cert(#state{session = #session{} = Session} = State, PeerCert) -> - State#state{session = Session#session{peer_certificate = PeerCert}}. +store_peer_cert(#state{session = Session, + handshake_env = HsEnv} = State, PeerCert, PublicKeyInfo) -> + State#state{session = Session#session{peer_certificate = PeerCert}, + handshake_env = HsEnv#handshake_env{public_key_info = PublicKeyInfo}}. convert_certificate_chain(Certs) -> @@ -900,6 +917,11 @@ get_private_key(#key_share_entry{ {_, PrivateKey}}) -> PrivateKey. +%% TODO: implement EC keys +get_public_key({?'rsaEncryption', PublicKey, _}) -> + PublicKey. + + %% X25519, X448 calculate_shared_secret(OthersKey, MyKey, Group) when is_binary(OthersKey) andalso is_binary(MyKey) andalso @@ -944,7 +966,8 @@ update_connection_state(ConnectionState = #{security_parameters := SecurityParam update_start_state(#state{connection_states = ConnectionStates0, connection_env = CEnv, session = Session} = State, - Cipher, KeyShare, SessionId) -> + Cipher, KeyShare, SessionId, + Group, SelectedSignAlg, ClientPubKey) -> #{security_parameters := SecParamsR0} = PendingRead = maps:get(pending_read, ConnectionStates0), #{security_parameters := SecParamsW0} = PendingWrite = @@ -956,7 +979,10 @@ update_start_state(#state{connection_states = ConnectionStates0, pending_write => PendingWrite#{security_parameters => SecParamsW}}, State#state{connection_states = ConnectionStates, key_share = KeyShare, - session = Session#session{session_id = SessionId}, + session = Session#session{session_id = SessionId, + ecc = Group, + sign_alg = SelectedSignAlg, + dh_public_value = ClientPubKey}, connection_env = CEnv#connection_env{negotiated_version = {3,4}}}. @@ -967,49 +993,105 @@ cipher_init(Key, IV, FinishedKey) -> tag_len = 16}. -%% Client is authenticated -get_handshake_context({[<<20,_/binary>>,<<15,_/binary>>,<<11,_/binary>>|Messages], _}) -> - %% Handshake context messages: - %% ClientHello (client) (1) - %% ServerHello (server) (2), - %% EncryptedExtensions (server) (8) - %% CertificateRequest (server) (13), - %% Certificate (server) (11) - %% CertificateVerify (server) (15), - %% Finished (server) (20) - %% Certificate (client) (11) - Drop! Not included in calculations! - %% CertificateVerify (client) (15) - Drop! Not included in calculations! - %% Finished (client) (20) - Drop! Not included in calculations! - Messages; -%% Client is authenticated (empty certificate) -get_handshake_context({[<<20,_/binary>>,<<11,_/binary>>|Messages], _}) -> - %% Handshake context messages: - %% ClientHello (client) (1) - %% ServerHello (server) (2), - %% EncryptedExtensions (server) (8) - %% CertificateRequest (server) (13), - %% Certificate (server) (11) - %% CertificateVerify (server) (15), - %% Finished (server) (20) - %% Certificate (client) (11) - Drop! Not included in calculations! - %% Finished (client) (20) - Drop! Not included in calculations! +%% Verify CertificateVerify: +%% ClientHello (client) (1) +%% ServerHello (server) (2) +%% EncryptedExtensions (server) (8) +%% CertificateRequest (server) (13) +%% Certificate (server) (11) +%% CertificateVerify (server) (15) +%% Finished (server) (20) +%% Certificate (client) (11) +%% CertificateVerify (client) (15) - Drop! Not included in calculations! +get_handshake_context({[<<15,_/binary>>|Messages], _}) -> Messages; -%% Normal case -get_handshake_context({[_| Messages], _}) -> - %% Handshake context messages: - %% ClientHello (client) (1) - %% ServerHello (server) (2), - %% EncryptedExtensions (server) (8) - %% Certificate (server) (11) - %% CertificateVerify (server) (15), - %% Finished (server) (20) - %% Finished (client) (20) - Drop! Not included in calculations! - Messages. +%% Client is authenticated with certificate: +%% ClientHello (client) (1) +%% ServerHello (server) (2) +%% EncryptedExtensions (server) (8) +%% CertificateRequest (server) (13) +%% Certificate (server) (11) +%% CertificateVerify (server) (15) +%% Finished (server) (20) +%% Certificate (client) (11) - Drop! Not included in calculations! +%% CertificateVerify (client) (15) - Drop! Not included in calculations! +%% Finished (client) (20) - Drop! Not included in calculations! +%% +%% Client is authenticated but sends empty certificate: +%% ClientHello (client) (1) +%% ServerHello (server) (2) +%% EncryptedExtensions (server) (8) +%% CertificateRequest (server) (13) +%% Certificate (server) (11) +%% CertificateVerify (server) (15) +%% Finished (server) (20) +%% Certificate (client) (11) - Drop! Not included in calculations! +%% Finished (client) (20) - Drop! Not included in calculations! +%% +%% Client is not authenticated: +%% ClientHello (client) (1) +%% ServerHello (server) (2) +%% EncryptedExtensions (server) (8) +%% Certificate (server) (11) +%% CertificateVerify (server) (15) +%% Finished (server) (20) +%% Finished (client) (20) - Drop! Not included in calculations! +%% +%% Drop all client messages from the front of the iolist using the property that +%% incoming messages are binaries. +get_handshake_context({Messages, _}) -> + get_handshake_context(Messages); +get_handshake_context([H|T]) when is_binary(H) -> + get_handshake_context(T); +get_handshake_context(L) -> + L. + +verify_signature_algorithm(#state{session = #session{sign_alg = SignatureScheme}} = State, + #certificate_verify_1_3{algorithm = SignatureScheme2}) -> + %% TODO + ok. -verify_certificate(State) -> - %% TODO: implement - {ok, {State, wait_finished}}. + +verify_certificate_verify(#state{connection_states = ConnectionStates, + handshake_env = + #handshake_env{ + public_key_info = PublicKeyInfo, + tls_handshake_history = HHistory}} = State0, + #certificate_verify_1_3{ + algorithm = SignatureScheme, + signature = Signature}) -> + #{security_parameters := SecParamsR} = + ssl_record:pending_connection_state(ConnectionStates, write), + #security_parameters{prf_algorithm = HKDFAlgo} = SecParamsR, + + {HashAlgo, _, _} = + ssl_cipher:scheme_to_components(SignatureScheme), + + Messages = get_handshake_context(HHistory), + + Context = lists:reverse(Messages), + + %% Transcript-Hash uses the HKDF hash function defined by the cipher suite. + THash = tls_v1:transcript_hash(Context, HKDFAlgo), + + PublicKey = get_public_key(PublicKeyInfo), + + %% Digital signatures use the hash function defined by the selected signature + %% scheme. + case verify(THash, <<"TLS 1.3, client CertificateVerify">>, + HashAlgo, Signature, PublicKey) of + {ok, true} -> + {ok, {State0, wait_finished}}; + {ok, false} -> + State1 = calculate_traffic_secrets(State0), + State = ssl_record:step_encryption_state(State1), + {error, {{handshake_failure, "Failed to verify CertificateVerify"}, State}}; + {error, badarg} -> + State1 = calculate_traffic_secrets(State0), + State = ssl_record:step_encryption_state(State1), + {error, {badarg, State}} + end. %% If there is no overlap between the received @@ -1028,7 +1110,6 @@ select_common_groups(ServerGroups, ClientGroups) -> end. - %% RFC 8446 - 4.2.8. Key Share %% This vector MAY be empty if the client is requesting a %% HelloRetryRequest. Each KeyShareEntry value MUST correspond to a -- cgit v1.2.3 From 36ddf40a42d028a8b99fd8994de64e29c2780f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Wed, 6 Mar 2019 16:50:00 +0100 Subject: ssl: Verify signature algorithm in CV Verify if the signature algorithm used in the signature of CertificateVerify is one of those present in the supported_signature_algorithms field of the "signature_algorithms" extension in the CertificateRequest message. Change-Id: I7d3b5f10e3205447fb9a9a7e59b93568d1696432 --- lib/ssl/src/tls_handshake_1_3.erl | 55 ++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 21 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl index 858a1acdac..c250e95029 100644 --- a/lib/ssl/src/tls_handshake_1_3.erl +++ b/lib/ssl/src/tls_handshake_1_3.erl @@ -434,7 +434,6 @@ do_start(#client_hello{cipher_suites = ClientCiphers, #state{connection_states = _ConnectionStates0, ssl_options = #ssl_options{ciphers = ServerCiphers, signature_algs = ServerSignAlgs, - signature_algs_cert = _SignatureSchemes, %% TODO: check! supported_groups = ServerGroups0}, session = #session{own_certificate = Cert}} = State0) -> @@ -746,7 +745,6 @@ process_client_certificate(#certificate_1_3{certificate_list = Certs0}, State1 = calculate_traffic_secrets(State0), State = ssl_record:step_encryption_state(State1), {error, {Reason, State}}; - %% TODO: refactor!? #alert{} = Alert -> State1 = calculate_traffic_secrets(State0), State = ssl_record:step_encryption_state(State1), @@ -768,17 +766,19 @@ validate_certificate_chain(Certs, CertDbHandle, CertDbRef, SslOptions, CRLDbHand SslOptions#ssl_options.crl_check, CRLDbHandle, CertPath), Options = [{max_path_length, SslOptions#ssl_options.depth}, {verify_fun, ValidationFunAndState}], - case public_key:pkix_path_validation(TrustedCert, CertPath, Options) of - {ok, {PublicKeyInfo,_}} -> - {ok, {PeerCert, PublicKeyInfo}}; - {error, Reason} -> - ssl_handshake:handle_path_validation_error(Reason, PeerCert, ChainCerts, + %% TODO: Validate if Certificate is using a supported signature algorithm + %% (signature_algs_cert)! + case public_key:pkix_path_validation(TrustedCert, CertPath, Options) of + {ok, {PublicKeyInfo,_}} -> + {ok, {PeerCert, PublicKeyInfo}}; + {error, Reason} -> + ssl_handshake:handle_path_validation_error(Reason, PeerCert, ChainCerts, SslOptions, Options, CertDbHandle, CertDbRef) - end + end catch - error:{badmatch,{asn1, Asn1Reason}} -> - %% ASN-1 decode of certificate somehow failed + error:{badmatch,{asn1, Asn1Reason}} -> + %% ASN-1 decode of certificate somehow failed {error, {certificate_unknown, {failed_to_decode_certificate, Asn1Reason}}}; error:OtherReason -> {error, {internal_error, {unexpected_error, OtherReason}}} @@ -1047,20 +1047,33 @@ get_handshake_context(L) -> L. -verify_signature_algorithm(#state{session = #session{sign_alg = SignatureScheme}} = State, - #certificate_verify_1_3{algorithm = SignatureScheme2}) -> - %% TODO - ok. +%% If sent by a client, the signature algorithm used in the signature +%% MUST be one of those present in the supported_signature_algorithms +%% field of the "signature_algorithms" extension in the +%% CertificateRequest message. +verify_signature_algorithm(#state{ssl_options = + #ssl_options{ + signature_algs = ServerSignAlgs}} = State0, + #certificate_verify_1_3{algorithm = ClientSignAlg}) -> + case lists:member(ClientSignAlg, ServerSignAlgs) of + true -> + ok; + false -> + State1 = calculate_traffic_secrets(State0), + State = ssl_record:step_encryption_state(State1), + {error, {{handshake_failure, + "CertificateVerify has a not supported signature algorithm"}, State}} + end. verify_certificate_verify(#state{connection_states = ConnectionStates, - handshake_env = - #handshake_env{ - public_key_info = PublicKeyInfo, - tls_handshake_history = HHistory}} = State0, - #certificate_verify_1_3{ - algorithm = SignatureScheme, - signature = Signature}) -> + handshake_env = + #handshake_env{ + public_key_info = PublicKeyInfo, + tls_handshake_history = HHistory}} = State0, + #certificate_verify_1_3{ + algorithm = SignatureScheme, + signature = Signature}) -> #{security_parameters := SecParamsR} = ssl_record:pending_connection_state(ConnectionStates, write), #security_parameters{prf_algorithm = HKDFAlgo} = SecParamsR, -- cgit v1.2.3 From 0d37395176d63bb08c5cdbd46466630a132ea5b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Thu, 7 Mar 2019 13:02:54 +0100 Subject: ssl: Test HelloRetryRequest with client auth Change-Id: I6504d99a96ed6fc75dbdff78a6148ed39d3776c9 --- lib/ssl/test/ssl_basic_SUITE.erl | 99 +++++++++++++++++++++++++++++++++++++++- lib/ssl/test/ssl_test_lib.erl | 2 +- 2 files changed, 99 insertions(+), 2 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl index 630b935599..8fb2a014a4 100644 --- a/lib/ssl/test/ssl_basic_SUITE.erl +++ b/lib/ssl/test/ssl_basic_SUITE.erl @@ -284,7 +284,10 @@ tls13_test_group() -> tls13_hello_retry_request_ssl_server_openssl_client, tls13_client_auth_empty_cert_alert_ssl_server_openssl_client, tls13_client_auth_empty_cert_ssl_server_openssl_client, - tls13_client_auth_ssl_server_openssl_client]. + tls13_client_auth_ssl_server_openssl_client, + tls13_hrr_client_auth_empty_cert_alert_ssl_server_openssl_client, + tls13_hrr_client_auth_empty_cert_ssl_server_openssl_client, + tls13_hrr_client_auth_ssl_server_openssl_client]. %%-------------------------------------------------------------------- init_per_suite(Config0) -> @@ -5678,6 +5681,100 @@ tls13_client_auth_ssl_server_openssl_client(Config) -> ssl_test_lib:close_port(Client). +tls13_hrr_client_auth_empty_cert_alert_ssl_server_openssl_client() -> + [{doc,"TLS 1.3 (HelloRetryRequest): Test client authentication when client sends an empty certificate and fail_if_no_peer_cert is set to true."}]. + +tls13_hrr_client_auth_empty_cert_alert_ssl_server_openssl_client(Config) -> + ClientOpts0 = ssl_test_lib:ssl_options(client_rsa_opts, Config), + %% Delete Client Cert and Key + ClientOpts1 = proplists:delete(certfile, ClientOpts0), + ClientOpts2 = proplists:delete(keyfile, ClientOpts1), + ClientOpts = [{groups,"P-256:X25519"}|ClientOpts2], + + ServerOpts0 = ssl_test_lib:ssl_options(server_rsa_opts, Config), + %% Set versions + ServerOpts = [{versions, ['tlsv1.2','tlsv1.3']}, + {verify, verify_peer}, + {fail_if_no_peer_cert, true}, + {supported_groups, [x448, x25519]}|ServerOpts0], + {_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, send_recv_result_active, []}}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + + Client = ssl_test_lib:start_basic_client(openssl, 'tlsv1.3', Port, ClientOpts), + + ssl_test_lib:check_result(Server, + {error, + {tls_alert, + {certificate_required, + "received SERVER ALERT: Fatal - Certificate required - certificate_required"}}}), + ssl_test_lib:close(Server), + ssl_test_lib:close_port(Client). + + +tls13_hrr_client_auth_empty_cert_ssl_server_openssl_client() -> + [{doc,"TLS 1.3 (HelloRetryRequest): Test client authentication when client sends an empty certificate and fail_if_no_peer_cert is set to false."}]. + +tls13_hrr_client_auth_empty_cert_ssl_server_openssl_client(Config) -> + ClientOpts0 = ssl_test_lib:ssl_options(client_rsa_opts, Config), + %% Delete Client Cert and Key + ClientOpts1 = proplists:delete(certfile, ClientOpts0), + ClientOpts2 = proplists:delete(keyfile, ClientOpts1), + ClientOpts = [{groups,"P-256:X25519"}|ClientOpts2], + + ServerOpts0 = ssl_test_lib:ssl_options(server_rsa_opts, Config), + %% Set versions + ServerOpts = [{versions, ['tlsv1.2','tlsv1.3']}, + {verify, verify_peer}, + {fail_if_no_peer_cert, false}, + {supported_groups, [x448, x25519]}|ServerOpts0], + {_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, send_recv_result_active, []}}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + + Client = ssl_test_lib:start_basic_client(openssl, 'tlsv1.3', Port, ClientOpts), + + ssl_test_lib:check_result(Server, ok), + ssl_test_lib:close(Server), + ssl_test_lib:close_port(Client). + + +tls13_hrr_client_auth_ssl_server_openssl_client() -> + [{doc,"TLS 1.3 (HelloRetryRequest): Test client authentication."}]. + +tls13_hrr_client_auth_ssl_server_openssl_client(Config) -> + ClientOpts0 = ssl_test_lib:ssl_options(client_rsa_opts, Config), + ClientOpts = [{groups,"P-256:X25519"}|ClientOpts0], + + ServerOpts0 = ssl_test_lib:ssl_options(server_rsa_opts, Config), + %% Set versions + ServerOpts = [{versions, ['tlsv1.2','tlsv1.3']}, + {verify, verify_peer}, + {fail_if_no_peer_cert, true}, + {supported_groups, [x448, x25519]}|ServerOpts0], + {_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, send_recv_result_active, []}}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + + Client = ssl_test_lib:start_basic_client(openssl, 'tlsv1.3', Port, ClientOpts), + + ssl_test_lib:check_result(Server, ok), + ssl_test_lib:close(Server), + ssl_test_lib:close_port(Client). + + %%-------------------------------------------------------------------- %% Internal functions ------------------------------------------------ %%-------------------------------------------------------------------- diff --git a/lib/ssl/test/ssl_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl index 7d83dbd382..7f8e81dbd8 100644 --- a/lib/ssl/test/ssl_test_lib.erl +++ b/lib/ssl/test/ssl_test_lib.erl @@ -1126,7 +1126,7 @@ start_basic_client(openssl, Version, Port, ClientOpts) -> case {Cert, Key} of {C, K} when C =:= undefined orelse K =:= undefined -> - Args0; + Args1; {C, K} -> Args1 ++ ["-cert", C, "-key", K] end, -- cgit v1.2.3 From ecdfcfd6e85747b37881f873a64ee5ea068a94d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Thu, 7 Mar 2019 16:34:38 +0100 Subject: ssl: Fix get_handshake_context/2 Split get_handshake_context/2 into two functions. The new get_handshake_context_cv/2 returns the context for the verification of CertificateVerify. Change-Id: I461eb67bda1d9c1673e463d417c3e838fca6b40c --- lib/ssl/src/tls_handshake_1_3.erl | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl index c250e95029..9c6c9190a1 100644 --- a/lib/ssl/src/tls_handshake_1_3.erl +++ b/lib/ssl/src/tls_handshake_1_3.erl @@ -993,6 +993,8 @@ cipher_init(Key, IV, FinishedKey) -> tag_len = 16}. +%% Get handshake context for verification of CertificateVerify. +%% %% Verify CertificateVerify: %% ClientHello (client) (1) %% ServerHello (server) (2) @@ -1003,8 +1005,12 @@ cipher_init(Key, IV, FinishedKey) -> %% Finished (server) (20) %% Certificate (client) (11) %% CertificateVerify (client) (15) - Drop! Not included in calculations! -get_handshake_context({[<<15,_/binary>>|Messages], _}) -> - Messages; +get_handshake_context_cv({[<<15,_/binary>>|Messages], _}) -> + Messages. + + +%% Get handshake context for traffic key calculation. +%% %% Client is authenticated with certificate: %% ClientHello (client) (1) %% ServerHello (server) (2) @@ -1062,7 +1068,7 @@ verify_signature_algorithm(#state{ssl_options = State1 = calculate_traffic_secrets(State0), State = ssl_record:step_encryption_state(State1), {error, {{handshake_failure, - "CertificateVerify has a not supported signature algorithm"}, State}} + "CertificateVerify uses unsupported signature algorithm"}, State}} end. @@ -1081,7 +1087,7 @@ verify_certificate_verify(#state{connection_states = ConnectionStates, {HashAlgo, _, _} = ssl_cipher:scheme_to_components(SignatureScheme), - Messages = get_handshake_context(HHistory), + Messages = get_handshake_context_cv(HHistory), Context = lists:reverse(Messages), -- cgit v1.2.3 From 6608e84968c96366d76b6cc4a854d32a1c458fea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Mon, 11 Mar 2019 16:32:13 +0100 Subject: ssl: Fix Alert logging Report the role of the peer when logging incoming Alerts. Change-Id: I7eec46bc36f9080f5087b6a38e7f14ac628fe286 --- lib/ssl/src/ssl_connection.erl | 3 ++- lib/ssl/src/tls_connection.erl | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index 422c4c94ae..254b7f3f4d 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -385,7 +385,8 @@ handle_alert(#alert{level = ?FATAL} = Alert, StateName, log_alert(SslOpts#ssl_options.log_level, Role, Connection:protocol_name(), StateName, Alert#alert{role = opposite_role(Role)}), Pids = Connection:pids(State), - alert_user(Pids, Transport, Tracker, Socket, StateName, Opts, Pid, From, Alert, Role, Connection), + alert_user(Pids, Transport, Tracker, Socket, StateName, Opts, Pid, From, Alert, + opposite_role(Role), Connection), {stop, {shutdown, normal}, State}; handle_alert(#alert{level = ?WARNING, description = ?CLOSE_NOTIFY} = Alert, diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl index 8eb9e56375..39df2ad15b 100644 --- a/lib/ssl/src/tls_connection.erl +++ b/lib/ssl/src/tls_connection.erl @@ -308,9 +308,7 @@ handle_protocol_record(#ssl_tls{type = ?ALERT, fragment = EncAlerts}, StateName, handle_alerts(Alerts, {next_state, StateName, State}); [] -> ssl_connection:handle_own_alert(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, empty_alert), - Version, StateName, State); - #alert{} = Alert -> - ssl_connection:handle_own_alert(Alert, Version, StateName, State) + Version, StateName, State) catch _:_ -> ssl_connection:handle_own_alert(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, alert_decode_error), @@ -1135,6 +1133,7 @@ encode_handshake(Handshake, Version, ConnectionStates0, Hist0) -> encode_change_cipher(#change_cipher_spec{}, Version, ConnectionStates) -> tls_record:encode_change_cipher_spec(Version, ConnectionStates). +-spec decode_alerts(binary()) -> list(). decode_alerts(Bin) -> ssl_alert:decode(Bin). -- cgit v1.2.3 From 22ce783fe400ee5e3996802a634c8d5868edc82f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Mon, 11 Mar 2019 16:40:47 +0100 Subject: ssl: Improve verification of received Certificate Validate peer certificate against supported signature algorithms. Send 'Hanshake Failure' Alert if signature algorithm is not supported by the server. Change-Id: Iad428aad337f0f9764d23404c203f966664c4555 --- lib/ssl/src/ssl_cipher.erl | 14 ++++++++++- lib/ssl/src/tls_handshake_1_3.erl | 53 ++++++++++++++++++++++++++++----------- 2 files changed, 52 insertions(+), 15 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/ssl_cipher.erl b/lib/ssl/src/ssl_cipher.erl index 6e751f9ceb..fe8736d2df 100644 --- a/lib/ssl/src/ssl_cipher.erl +++ b/lib/ssl/src/ssl_cipher.erl @@ -45,7 +45,7 @@ random_bytes/1, calc_mac_hash/4, calc_mac_hash/6, is_stream_ciphersuite/1, signature_scheme/1, scheme_to_components/1, hash_size/1, effective_key_bits/1, - key_material/1]). + key_material/1, signature_algorithm_to_scheme/1]). %% RFC 8446 TLS 1.3 -export([generate_client_shares/1, generate_server_share/1, add_zero_padding/2]). @@ -900,6 +900,18 @@ scheme_to_components(rsa_pss_pss_sha512) -> {sha512, rsa_pss_pss, undefined}; scheme_to_components(rsa_pkcs1_sha1) -> {sha1, rsa_pkcs1, undefined}; scheme_to_components(ecdsa_sha1) -> {sha1, ecdsa, undefined}. + +%% TODO: Add support for EC and RSA-SSA signatures +signature_algorithm_to_scheme(#'SignatureAlgorithm'{algorithm = ?sha1WithRSAEncryption}) -> + rsa_pkcs1_sha1; +signature_algorithm_to_scheme(#'SignatureAlgorithm'{algorithm = ?sha256WithRSAEncryption}) -> + rsa_pkcs1_sha256; +signature_algorithm_to_scheme(#'SignatureAlgorithm'{algorithm = ?sha384WithRSAEncryption}) -> + rsa_pkcs1_sha384; +signature_algorithm_to_scheme(#'SignatureAlgorithm'{algorithm = ?sha512WithRSAEncryption}) -> + rsa_pkcs1_sha512. + + %% RFC 5246: 6.2.3.2. CBC Block Cipher %% %% Implementation note: Canvel et al. [CBCTIME] have demonstrated a diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl index 9c6c9190a1..1e8b046c1e 100644 --- a/lib/ssl/src/tls_handshake_1_3.erl +++ b/lib/ssl/src/tls_handshake_1_3.erl @@ -503,7 +503,7 @@ do_start(#client_hello{cipher_suites = ClientCiphers, {Ref, no_suitable_cipher} -> ?ALERT_REC(?FATAL, ?INSUFFICIENT_SECURITY, no_suitable_cipher); {Ref, {insufficient_security, no_suitable_signature_algorithm}} -> - ?ALERT_REC(?FATAL, ?INSUFFICIENT_SECURITY, no_suitable_signature_algorithm); + ?ALERT_REC(?FATAL, ?INSUFFICIENT_SECURITY, "No suitable signature algorithm"); {Ref, {insufficient_security, no_suitable_public_key}} -> ?ALERT_REC(?FATAL, ?INSUFFICIENT_SECURITY, no_suitable_public_key) end. @@ -589,6 +589,8 @@ do_wait_cert(#certificate_1_3{} = Certificate, State0) -> {?ALERT_REC(?FATAL, ?CERTIFICATE_UNKNOWN, Reason), State}; {Ref, {{internal_error, Reason}, State}} -> {?ALERT_REC(?FATAL, ?INTERNAL_ERROR, Reason), State}; + {Ref, {{handshake_failure, Reason}, State}} -> + {?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, Reason), State}; {#alert{} = Alert, State} -> {Alert, State} end. @@ -723,7 +725,9 @@ process_client_certificate(#certificate_1_3{ State = ssl_record:step_encryption_state(State1), {error, {certificate_required, State}}; process_client_certificate(#certificate_1_3{certificate_list = Certs0}, - #state{ssl_options = SslOptions, + #state{ssl_options = + #ssl_options{signature_algs = SignAlgs, + signature_algs_cert = SignAlgsCert} = SslOptions, static_env = #static_env{ role = Role, @@ -735,23 +739,44 @@ process_client_certificate(#certificate_1_3{certificate_list = Certs0}, %% Remove extensions from list of certificates! Certs = convert_certificate_chain(Certs0), - - case validate_certificate_chain(Certs, CertDbHandle, CertDbRef, - SslOptions, CRLDbHandle, Role, Host) of - {ok, {PeerCert, PublicKeyInfo}} -> - State = store_peer_cert(State0, PeerCert, PublicKeyInfo), - {ok, {State, wait_cv}}; - {error, Reason} -> - State1 = calculate_traffic_secrets(State0), - State = ssl_record:step_encryption_state(State1), - {error, {Reason, State}}; - #alert{} = Alert -> + case is_supported_signature_algorithm(Certs, SignAlgs, SignAlgsCert) of + true -> + case validate_certificate_chain(Certs, CertDbHandle, CertDbRef, + SslOptions, CRLDbHandle, Role, Host) of + {ok, {PeerCert, PublicKeyInfo}} -> + State = store_peer_cert(State0, PeerCert, PublicKeyInfo), + {ok, {State, wait_cv}}; + {error, Reason} -> + State1 = calculate_traffic_secrets(State0), + State = ssl_record:step_encryption_state(State1), + {error, {Reason, State}}; + #alert{} = Alert -> + State1 = calculate_traffic_secrets(State0), + State = ssl_record:step_encryption_state(State1), + {Alert, State} + end; + false -> State1 = calculate_traffic_secrets(State0), State = ssl_record:step_encryption_state(State1), - {Alert, State} + {error, {{handshake_failure, + "Client certificate uses unsupported signature algorithm"}, State}} end. +%% TODO: check whole chain! +is_supported_signature_algorithm(Certs, SignAlgs, undefined) -> + is_supported_signature_algorithm(Certs, SignAlgs); +is_supported_signature_algorithm(Certs, _, SignAlgsCert) -> + is_supported_signature_algorithm(Certs, SignAlgsCert). +%% +is_supported_signature_algorithm([BinCert|_], SignAlgs0) -> + #'OTPCertificate'{signatureAlgorithm = SignAlg} = + public_key:pkix_decode_cert(BinCert, otp), + SignAlgs = filter_tls13_algs(SignAlgs0), + Scheme = ssl_cipher:signature_algorithm_to_scheme(SignAlg), + lists:member(Scheme, SignAlgs). + + validate_certificate_chain(Certs, CertDbHandle, CertDbRef, SslOptions, CRLDbHandle, Role, Host) -> ServerName = ssl_handshake:server_name(SslOptions#ssl_options.server_name_indication, Host, Role), [PeerCert | ChainCerts ] = Certs, -- cgit v1.2.3 From 0597262c9a8f55fe72e6280ea8d0fc6bec1c39e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Mon, 11 Mar 2019 16:51:12 +0100 Subject: ssl: Handle unencrypted Alert (Illegal Parameter) Handle unencrypted 'Illegal Parameter' Alerts from openssl s_client when the server's connection states are already stepped into traffic encryption. Change-Id: I10951a9061e6f4b13d8ddb8ab99f8a812a483113 --- lib/ssl/src/tls_record_1_3.erl | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/tls_record_1_3.erl b/lib/ssl/src/tls_record_1_3.erl index 05acc08392..97331e1510 100644 --- a/lib/ssl/src/tls_record_1_3.erl +++ b/lib/ssl/src/tls_record_1_3.erl @@ -124,6 +124,20 @@ decode_cipher_text(#ssl_tls{type = ?OPAQUE_TYPE, {decode_inner_plaintext(PlainFragment), ConnectionStates} end; + +%% RFC8446 - TLS 1.3 (OpenSSL compatibility) +%% Handle unencrypted Alerts from openssl s_client when server's +%% connection states are already stepped into traffic encryption. +%% (E.g. openssl s_client receives a CertificateRequest with +%% a signature_algorithms_cert extension that does not contain +%% the signature algorithm of the client's certificate.) +decode_cipher_text(#ssl_tls{type = ?ALERT, + version = ?LEGACY_VERSION, + fragment = <<2,47>>}, + ConnectionStates0) -> + {#ssl_tls{type = ?ALERT, + version = {3,4}, %% Internally use real version + fragment = <<2,47>>}, ConnectionStates0}; %% RFC8446 - TLS 1.3 %% D.4. Middlebox Compatibility Mode %% - If not offering early data, the client sends a dummy @@ -139,7 +153,6 @@ decode_cipher_text(#ssl_tls{type = ?CHANGE_CIPHER_SPEC, {#ssl_tls{type = ?CHANGE_CIPHER_SPEC, version = {3,4}, %% Internally use real version fragment = <<1>>}, ConnectionStates0}; - decode_cipher_text(#ssl_tls{type = Type, version = ?LEGACY_VERSION, fragment = CipherFragment}, -- cgit v1.2.3 From 41ec369168bcf87efda39c8d1c9b471e867e3e78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Mon, 11 Mar 2019 16:55:10 +0100 Subject: ssl: Test handling of signature algorithms Change-Id: I433924f9c590efa94423db5df52dd3f5d53d9d20 --- lib/ssl/test/ssl_basic_SUITE.erl | 78 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) (limited to 'lib/ssl') diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl index 8fb2a014a4..6bf58766f8 100644 --- a/lib/ssl/test/ssl_basic_SUITE.erl +++ b/lib/ssl/test/ssl_basic_SUITE.erl @@ -287,7 +287,9 @@ tls13_test_group() -> tls13_client_auth_ssl_server_openssl_client, tls13_hrr_client_auth_empty_cert_alert_ssl_server_openssl_client, tls13_hrr_client_auth_empty_cert_ssl_server_openssl_client, - tls13_hrr_client_auth_ssl_server_openssl_client]. + tls13_hrr_client_auth_ssl_server_openssl_client, + tls13_unsupported_sign_algo_client_auth_ssl_server_openssl_client, + tls13_unsupported_sign_algo_cert_client_auth_ssl_server_openssl_client]. %%-------------------------------------------------------------------- init_per_suite(Config0) -> @@ -5775,6 +5777,80 @@ tls13_hrr_client_auth_ssl_server_openssl_client(Config) -> ssl_test_lib:close_port(Client). +tls13_unsupported_sign_algo_client_auth_ssl_server_openssl_client() -> + [{doc,"TLS 1.3: Test client authentication with unsupported signature_algorithm"}]. + +tls13_unsupported_sign_algo_client_auth_ssl_server_openssl_client(Config) -> + ClientOpts = ssl_test_lib:ssl_options(client_rsa_opts, Config), + + ServerOpts0 = ssl_test_lib:ssl_options(server_rsa_opts, Config), + %% Set versions + ServerOpts = [{versions, ['tlsv1.2','tlsv1.3']}, + {verify, verify_peer}, + %% Skip rsa_pkcs1_sha256! + {signature_algs, [rsa_pkcs1_sha384, rsa_pkcs1_sha512]}, + {fail_if_no_peer_cert, true}|ServerOpts0], + {_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, send_recv_result_active, []}}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + + Client = ssl_test_lib:start_basic_client(openssl, 'tlsv1.3', Port, ClientOpts), + + ssl_test_lib:check_result( + Server, + {error, + {tls_alert, + {insufficient_security, + "received SERVER ALERT: Fatal - Insufficient Security - " + "\"No suitable signature algorithm\""}}}), + ssl_test_lib:close(Server), + ssl_test_lib:close_port(Client). + + +%% Triggers Client Alert as openssl s_client does not have a certificate with a +%% signature algorithm supported by the server (signature_algorithms_cert extension +%% of CertificateRequest does not contain the algorithm of the client certificate). +tls13_unsupported_sign_algo_cert_client_auth_ssl_server_openssl_client() -> + [{doc,"TLS 1.3: Test client authentication with unsupported signature_algorithm_cert"}]. + +tls13_unsupported_sign_algo_cert_client_auth_ssl_server_openssl_client(Config) -> + ssl:set_log_level(debug), + + ClientOpts = ssl_test_lib:ssl_options(client_rsa_opts, Config), + + ServerOpts0 = ssl_test_lib:ssl_options(server_rsa_opts, Config), + %% Set versions + ServerOpts = [{versions, ['tlsv1.2','tlsv1.3']}, + {log_level, debug}, + {verify, verify_peer}, + {signature_algs, [rsa_pkcs1_sha256, rsa_pkcs1_sha384, rsa_pss_rsae_sha256]}, + %% Skip rsa_pkcs1_sha256! + {signature_algs_cert, [rsa_pkcs1_sha384, rsa_pkcs1_sha512]}, + {fail_if_no_peer_cert, true}|ServerOpts0], + {_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, send_recv_result_active, []}}, + {options, ServerOpts}]), + Port = ssl_test_lib:inet_port(Server), + + Client = ssl_test_lib:start_basic_client(openssl, 'tlsv1.3', Port, ClientOpts), + + ssl_test_lib:check_result( + Server, + {error, + {tls_alert, + {illegal_parameter, + "received CLIENT ALERT: Fatal - Illegal Parameter"}}}), + ssl_test_lib:close(Server), + ssl_test_lib:close_port(Client). + + %%-------------------------------------------------------------------- %% Internal functions ------------------------------------------------ %%-------------------------------------------------------------------- -- cgit v1.2.3 From a2d6b9a11bb51f85848f59982277b16197f7e6c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Dimitrov?= Date: Tue, 12 Mar 2019 11:48:30 +0100 Subject: ssl: Improve ssl logging Remove function ssl:set_log_level/1. Its functionality is already implemented by logger:set_application_level/2. Set log level for ssl modules to debug at application start. Former implementation required an extra call to logger:set_application_level/2 (beside setting ssl option {log_level, debug}) to enable debug logging. Change-Id: Id21be7fd58915e11124cc136bb92d8a7526b8a74 --- lib/ssl/doc/src/ssl.xml | 11 ----------- lib/ssl/src/ssl.erl | 29 +---------------------------- lib/ssl/src/ssl_app.erl | 6 +++--- lib/ssl/test/ssl_basic_SUITE.erl | 2 -- 4 files changed, 4 insertions(+), 44 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/doc/src/ssl.xml b/lib/ssl/doc/src/ssl.xml index b145aac6ab..60fa70c90c 100644 --- a/lib/ssl/doc/src/ssl.xml +++ b/lib/ssl/doc/src/ssl.xml @@ -1657,17 +1657,6 @@ fun(srp, Username :: string(), UserState :: term()) -> - - set_log_level(Level) -> ok | {error, Reason} - Sets log level for the SSL application. - - Level = atom() - - -

Sets log level for the SSL application.

- - - shutdown(SslSocket, How) -> ok | {error, Reason} Immediately closes a socket. diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl index 5a2d31ffc2..f7500b6f5f 100644 --- a/lib/ssl/src/ssl.erl +++ b/lib/ssl/src/ssl.erl @@ -55,8 +55,7 @@ format_error/1, renegotiate/1, prf/5, negotiated_protocol/1, connection_information/1, connection_information/2]). %% Misc --export([handle_options/2, tls_version/1, new_ssl_options/3, suite_to_str/1, - set_log_level/1]). +-export([handle_options/2, tls_version/1, new_ssl_options/3, suite_to_str/1]). -deprecated({ssl_accept, 1, eventually}). -deprecated({ssl_accept, 2, eventually}). @@ -1167,32 +1166,6 @@ suite_to_str(Cipher) -> ssl_cipher_format:suite_to_str(Cipher). -%%-------------------------------------------------------------------- --spec set_log_level(atom()) -> ok | {error, term()}. -%% -%% Description: Set log level for the SSL application -%%-------------------------------------------------------------------- -set_log_level(Level) -> - case application:get_all_key(ssl) of - {ok, PropList} -> - Modules = proplists:get_value(modules, PropList), - set_module_level(Modules, Level); - undefined -> - {error, ssl_not_started} - end. - -set_module_level(Modules, Level) -> - Fun = fun (Module) -> - ok = logger:set_module_level(Module, Level) - end, - try lists:map(Fun, Modules) of - _ -> - ok - catch - error:{badmatch, Error} -> - Error - end. - %%%-------------------------------------------------------------- %%% Internal functions %%%-------------------------------------------------------------------- diff --git a/lib/ssl/src/ssl_app.erl b/lib/ssl/src/ssl_app.erl index 2a5047c75c..9e6d676bef 100644 --- a/lib/ssl/src/ssl_app.erl +++ b/lib/ssl/src/ssl_app.erl @@ -44,11 +44,11 @@ start_logger() -> formatter => {ssl_logger, #{}}}, Filter = {fun logger_filters:domain/2,{log,sub,[otp,ssl]}}, logger:add_handler(ssl_handler, logger_std_h, Config), - logger:add_handler_filter(ssl_handler, filter_non_ssl, Filter). + logger:add_handler_filter(ssl_handler, filter_non_ssl, Filter), + logger:set_application_level(ssl, debug). %% %% Description: Stop SSL logger stop_logger() -> + logger:unset_application_level(ssl), logger:remove_handler(ssl_handler). - - diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl index 6bf58766f8..b566e817f7 100644 --- a/lib/ssl/test/ssl_basic_SUITE.erl +++ b/lib/ssl/test/ssl_basic_SUITE.erl @@ -5818,8 +5818,6 @@ tls13_unsupported_sign_algo_cert_client_auth_ssl_server_openssl_client() -> [{doc,"TLS 1.3: Test client authentication with unsupported signature_algorithm_cert"}]. tls13_unsupported_sign_algo_cert_client_auth_ssl_server_openssl_client(Config) -> - ssl:set_log_level(debug), - ClientOpts = ssl_test_lib:ssl_options(client_rsa_opts, Config), ServerOpts0 = ssl_test_lib:ssl_options(server_rsa_opts, Config), -- cgit v1.2.3