diff options
author | Péter Dimitrov <[email protected]> | 2019-06-04 17:11:19 +0200 |
---|---|---|
committer | Péter Dimitrov <[email protected]> | 2019-06-07 14:26:41 +0200 |
commit | f79bea24bb252985c7abf18f4f03fcd604e9e512 (patch) | |
tree | e870be5bfe7f2a71ea7fe14a8e5aa159f07be711 /lib/ssl | |
parent | 83e0f5897ba6de0041819c0d7bdad9e856c73f6c (diff) | |
download | otp-f79bea24bb252985c7abf18f4f03fcd604e9e512.tar.gz otp-f79bea24bb252985c7abf18f4f03fcd604e9e512.tar.bz2 otp-f79bea24bb252985c7abf18f4f03fcd604e9e512.zip |
ssl: Fix alert handling (TLS 1.3)
Server and client use different secrets when sending certificate related
alerts. This is due to a change to the TLS protocol where clients send
their 'certificate' message after they have received the server's 'finished'
message.
Diffstat (limited to 'lib/ssl')
-rw-r--r-- | lib/ssl/src/tls_connection_1_3.erl | 4 | ||||
-rw-r--r-- | lib/ssl/src/tls_handshake_1_3.erl | 33 | ||||
-rw-r--r-- | lib/ssl/test/ssl_certificate_verify_SUITE.erl | 9 | ||||
-rw-r--r-- | lib/ssl/test/ssl_test_lib.erl | 39 |
4 files changed, 57 insertions, 28 deletions
diff --git a/lib/ssl/src/tls_connection_1_3.erl b/lib/ssl/src/tls_connection_1_3.erl index 821b7000cc..117e4f059d 100644 --- a/lib/ssl/src/tls_connection_1_3.erl +++ b/lib/ssl/src/tls_connection_1_3.erl @@ -228,8 +228,8 @@ wait_cert_cr(internal, #change_cipher_spec{}, State, _Module) -> tls_connection:next_event(?FUNCTION_NAME, no_record, State); wait_cert_cr(internal, #certificate_1_3{} = Certificate, State0, _Module) -> case tls_handshake_1_3:do_wait_cert_cr(Certificate, State0) of - #alert{} = Alert -> - ssl_connection:handle_own_alert(Alert, {3,4}, wait_cert_cr, State0); + {#alert{} = Alert, State} -> + ssl_connection:handle_own_alert(Alert, {3,4}, wait_cert_cr, State); {State1, NextState} -> tls_connection:next_event(NextState, no_record, State1) end; diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl index e16d0c761b..4de51c9a35 100644 --- a/lib/ssl/src/tls_handshake_1_3.erl +++ b/lib/ssl/src/tls_handshake_1_3.erl @@ -733,6 +733,8 @@ do_wait_cert(#certificate_1_3{} = Certificate, State0) -> {?ALERT_REC(?FATAL, ?INTERNAL_ERROR, Reason), State}; {Ref, {{handshake_failure, Reason}, State}} -> {?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, Reason), State}; + {Ref, {#alert{} = Alert, State}} -> + {Alert, State}; {#alert{} = Alert, State} -> {Alert, State} end. @@ -893,7 +895,9 @@ do_wait_cert_cr(#certificate_1_3{} = Certificate, State0) -> {Ref, {{internal_error, Reason}, _State}} -> ?ALERT_REC(?FATAL, ?INTERNAL_ERROR, Reason); {Ref, {{handshake_failure, Reason}, _State}} -> - ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, Reason) + ?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, Reason); + {Ref, {#alert{} = Alert, State}} -> + {Alert, State} end; do_wait_cert_cr(#certificate_request_1_3{} = CertificateRequest, State0) -> {Ref,Maybe} = maybe(), @@ -1112,13 +1116,11 @@ process_certificate(#certificate_1_3{certificate_list = Certs0}, 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), + State = update_encryption_state(Role, State0), {error, {Reason, State}}; - #alert{} = Alert -> - State1 = calculate_traffic_secrets(State0), - State = ssl_record:step_encryption_state(State1), - {Alert, State} + {ok, #alert{} = Alert} -> + State = update_encryption_state(Role, State0), + {error, {Alert, State}} end; false -> State1 = calculate_traffic_secrets(State0), @@ -1142,6 +1144,17 @@ is_supported_signature_algorithm([BinCert|_], SignAlgs0) -> lists:member(Scheme, SignAlgs). +%% Sets correct encryption state when sending Alerts in shared states that use different secrets. +%% - If client: use handshake secrets. +%% - If server: use traffic secrets as by this time the client's state machine +%% already stepped into the 'connection' state. +update_encryption_state(server, State0) -> + State1 = calculate_traffic_secrets(State0), + ssl_record:step_encryption_state(State1); +update_encryption_state(client, State) -> + State. + + 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, @@ -1162,9 +1175,9 @@ validate_certificate_chain(Certs, CertDbHandle, CertDbRef, SslOptions, CRLDbHand {ok, {PublicKeyInfo,_}} -> {ok, {PeerCert, PublicKeyInfo}}; {error, Reason} -> - ssl_handshake:handle_path_validation_error(Reason, PeerCert, ChainCerts, - SslOptions, Options, - CertDbHandle, CertDbRef) + {ok, ssl_handshake:handle_path_validation_error(Reason, PeerCert, ChainCerts, + SslOptions, Options, + CertDbHandle, CertDbRef)} end catch error:{badmatch,{asn1, Asn1Reason}} -> diff --git a/lib/ssl/test/ssl_certificate_verify_SUITE.erl b/lib/ssl/test/ssl_certificate_verify_SUITE.erl index 358e9f8f77..c6982bb928 100644 --- a/lib/ssl/test/ssl_certificate_verify_SUITE.erl +++ b/lib/ssl/test/ssl_certificate_verify_SUITE.erl @@ -302,7 +302,13 @@ server_require_peer_cert_fail(Config) when is_list(Config) -> {from, self()}, {options, [{active, Active} | BadClientOpts]}]), - ssl_test_lib:check_server_alert(Server, Client, handshake_failure). + Version = proplists:get_value(version,Config), + case Version of + 'tlsv1.3' -> + ssl_test_lib:check_server_alert(Server, Client, certificate_required); + _ -> + ssl_test_lib:check_server_alert(Server, Client, handshake_failure) + end. %%-------------------------------------------------------------------- server_require_peer_cert_empty_ok() -> @@ -855,6 +861,7 @@ invalid_signature_server(Config) when is_list(Config) -> {from, self()}, {options, [{verify, verify_peer} | ClientOpts]}]), ssl_test_lib:check_server_alert(Server, Client, unknown_ca). + %%-------------------------------------------------------------------- invalid_signature_client() -> diff --git a/lib/ssl/test/ssl_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl index c706c68d3a..0318cc81e3 100644 --- a/lib/ssl/test/ssl_test_lib.erl +++ b/lib/ssl/test/ssl_test_lib.erl @@ -428,41 +428,42 @@ check_result(Pid, Msg) -> {got, Unexpected}}, ct:fail(Reason) end. + check_server_alert(Pid, Alert) -> receive {Pid, {error, {tls_alert, {Alert, STxt}}}} -> check_server_txt(STxt), + ok; + {Pid, {error, closed}} -> ok end. check_server_alert(Server, Client, Alert) -> receive {Server, {error, {tls_alert, {Alert, STxt}}}} -> check_server_txt(STxt), - receive - {Client, {error, {tls_alert, {Alert, CTxt}}}} -> - check_client_txt(CTxt), - ok; - {Client, {error, closed}} -> - ok - end + check_client_alert(Client, Alert) end. check_client_alert(Pid, Alert) -> receive {Pid, {error, {tls_alert, {Alert, CTxt}}}} -> check_client_txt(CTxt), + ok; + {Pid, {ssl_error, _, {tls_alert, {Alert, CTxt}}}} -> + check_client_txt(CTxt), + ok; + {Pid, {error, closed}} -> ok end. check_client_alert(Server, Client, Alert) -> receive {Client, {error, {tls_alert, {Alert, CTxt}}}} -> check_client_txt(CTxt), - receive - {Server, {error, {tls_alert, {Alert, STxt}}}} -> - check_server_txt(STxt), - ok; - {Server, {error, closed}} -> - ok - end + check_server_alert(Server, Alert); + {Client, {ssl_error, _, {tls_alert, {Alert, CTxt}}}} -> + check_client_txt(CTxt), + ok; + {Client, {error, closed}} -> + ok end. check_server_txt("TLS server" ++ _) -> ok; @@ -1103,7 +1104,15 @@ run_client_error(Opts) -> Options = proplists:get_value(options, Opts), ct:log("~p:~p~nssl:connect(~p, ~p, ~p)~n", [?MODULE,?LINE, Host, Port, Options]), Error = Transport:connect(Host, Port, Options), - Pid ! {self(), Error}. + case Error of + {error, {tls_alert, _}} -> + Pid ! {self(), Error}; + {ok, _Socket} -> + receive + {ssl_error, _, {tls_alert, _}} = SslError -> + Pid ! {self(), SslError} + end + end. accepters(N) -> accepters([], N). |