From a7446cd75e2a48e810faef26d91e9d70247297d1 Mon Sep 17 00:00:00 2001 From: Danil Zagoskin Date: Mon, 21 Apr 2014 20:57:03 +0400 Subject: ssl: TLSv1.2: proper default sign algo for RSA --- lib/ssl/src/ssl_handshake.erl | 3 +++ lib/ssl/test/ssl_handshake_SUITE.erl | 10 +++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index 10dd830baf..771fa6f377 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -635,6 +635,9 @@ select_hashsign(#hash_sign_algos{hash_sign_algos = HashSigns}, Cert) -> select_cert_hashsign(HashSign, _, {Major, Minor}) when HashSign =/= undefined andalso Major >= 3 andalso Minor >= 3 -> HashSign; +select_cert_hashsign(undefined, ?rsaEncryption, {Major, Minor}) when + is_integer(Major) andalso Major >= 3 andalso is_integer(Minor) andalso Minor >= 3 -> + {sha, rsa}; select_cert_hashsign(undefined,?'id-ecPublicKey', _) -> {sha, ecdsa}; select_cert_hashsign(undefined, ?rsaEncryption, _) -> diff --git a/lib/ssl/test/ssl_handshake_SUITE.erl b/lib/ssl/test/ssl_handshake_SUITE.erl index 6d020c472b..4c4b8e5137 100644 --- a/lib/ssl/test/ssl_handshake_SUITE.erl +++ b/lib/ssl/test/ssl_handshake_SUITE.erl @@ -26,6 +26,7 @@ -include_lib("common_test/include/ct.hrl"). -include("ssl_internal.hrl"). -include("tls_handshake.hrl"). +-include_lib("public_key/include/public_key.hrl"). %%-------------------------------------------------------------------- %% Common Test interface functions ----------------------------------- @@ -36,7 +37,8 @@ all() -> [decode_hello_handshake, decode_single_hello_extension_correctly, decode_supported_elliptic_curves_hello_extension_correctly, decode_unknown_hello_extension_correctly, - encode_single_hello_sni_extension_correctly]. + encode_single_hello_sni_extension_correctly, + select_proper_tls_1_2_rsa_default_hashsign]. %%-------------------------------------------------------------------- %% Test Cases -------------------------------------------------------- @@ -95,3 +97,9 @@ encode_single_hello_sni_extension_correctly(_Config) -> HelloExt = <>, Encoded = ssl_handshake:encode_hello_extensions(Exts), HelloExt = Encoded. + +select_proper_tls_1_2_rsa_default_hashsign(_Config) -> + % RFC 5246 section 7.4.1.4.1 tells to use {sha1,rsa} as default signature_algorithm for RSA key exchanges + {sha, rsa} = ssl_handshake:select_cert_hashsign(undefined, ?rsaEncryption, {3,3}), + {md5sha, rsa} = ssl_handshake:select_cert_hashsign(undefined, ?rsaEncryption, {undefined,undefined}). + -- cgit v1.2.3 From b4fc84117b6bd1105ca8ccb91f699564ac400dff Mon Sep 17 00:00:00 2001 From: Danil Zagoskin Date: Mon, 21 Apr 2014 22:00:09 +0400 Subject: ssl: always pass negotiated version when selecting hashsign Negotiated version is now always passed to ssl_handshake:select_hashsign because ssl_handshake:select_cert_hashsign has different rsa defaults on tlsv1.2 and older versions. --- lib/ssl/src/dtls_connection.erl | 2 +- lib/ssl/src/ssl_connection.erl | 5 +++-- lib/ssl/src/ssl_handshake.erl | 19 +++++++++---------- lib/ssl/src/tls_connection.erl | 2 +- lib/ssl/test/ssl_handshake_SUITE.erl | 4 +++- 5 files changed, 17 insertions(+), 15 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/dtls_connection.erl b/lib/ssl/src/dtls_connection.erl index 57f8dd86d3..ec0f408f51 100644 --- a/lib/ssl/src/dtls_connection.erl +++ b/lib/ssl/src/dtls_connection.erl @@ -202,13 +202,13 @@ hello(Hello = #client_hello{client_version = ClientVersion, session_cache = Cache, session_cache_cb = CacheCb, ssl_options = SslOpts}) -> - HashSign = ssl_handshake:select_hashsign(HashSigns, Cert), case dtls_handshake:hello(Hello, SslOpts, {Port, Session0, Cache, CacheCb, ConnectionStates0, Cert}, Renegotiation) of {Version, {Type, Session}, ConnectionStates, #hello_extensions{ec_point_formats = EcPointFormats, elliptic_curves = EllipticCurves} = ServerHelloExt} -> + HashSign = ssl_handshake:select_hashsign(HashSigns, Cert, Version), ssl_connection:hello({common_client_hello, Type, ServerHelloExt, HashSign}, State#state{connection_states = ConnectionStates, negotiated_version = Version, diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index edf49a340b..75100864c8 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -441,8 +441,9 @@ certify(#server_key_exchange{} = Msg, Connection:handle_unexpected_message(Msg, certify_server_keyexchange, State); certify(#certificate_request{hashsign_algorithms = HashSigns}, - #state{session = #session{own_certificate = Cert}} = State0, Connection) -> - HashSign = ssl_handshake:select_hashsign(HashSigns, Cert), + #state{session = #session{own_certificate = Cert}, + negotiated_version = Version} = State0, Connection) -> + HashSign = ssl_handshake:select_hashsign(HashSigns, Cert, Version), {Record, State} = Connection:next_record(State0#state{client_certificate_requested = true}), Connection:next_state(certify, certify, Record, State#state{cert_hashsign_algorithm = HashSign}); diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index 771fa6f377..c990eeedb5 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -73,7 +73,7 @@ ]). %% MISC --export([select_version/3, prf/5, select_hashsign/2, select_cert_hashsign/3, +-export([select_version/3, prf/5, select_hashsign/3, select_cert_hashsign/3, premaster_secret/2, premaster_secret/3, premaster_secret/4]). %%==================================================================== @@ -591,22 +591,22 @@ prf({3,1}, Secret, Label, Seed, WantedLength) -> prf({3,_N}, Secret, Label, Seed, WantedLength) -> {ok, tls_v1:prf(?SHA256, Secret, Label, Seed, WantedLength)}. %%-------------------------------------------------------------------- --spec select_hashsign(#hash_sign_algos{}| undefined, undefined | binary()) -> +-spec select_hashsign(#hash_sign_algos{}| undefined, undefined | binary(), ssl_record:ssl_version()) -> [{atom(), atom()}] | undefined. %% %% Description: %%-------------------------------------------------------------------- -select_hashsign(_, undefined) -> +select_hashsign(_, undefined, _Version) -> {null, anon}; -select_hashsign(undefined, Cert) -> +select_hashsign(undefined, Cert, Version) -> #'OTPCertificate'{tbsCertificate = TBSCert} = public_key:pkix_decode_cert(Cert, otp), #'OTPSubjectPublicKeyInfo'{algorithm = {_,Algo, _}} = TBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo, - select_cert_hashsign(undefined, Algo, {undefined, undefined}); -select_hashsign(#hash_sign_algos{hash_sign_algos = HashSigns}, Cert) -> + select_cert_hashsign(undefined, Algo, Version); +select_hashsign(#hash_sign_algos{hash_sign_algos = HashSigns}, Cert, Version) -> #'OTPCertificate'{tbsCertificate = TBSCert} =public_key:pkix_decode_cert(Cert, otp), #'OTPSubjectPublicKeyInfo'{algorithm = {_,Algo, _}} = TBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo, - DefaultHashSign = {_, Sign} = select_cert_hashsign(undefined, Algo, {undefined, undefined}), + DefaultHashSign = {_, Sign} = select_cert_hashsign(undefined, Algo, Version), case lists:filter(fun({sha, dsa}) -> true; ({_, dsa}) -> @@ -623,7 +623,7 @@ select_hashsign(#hash_sign_algos{hash_sign_algos = HashSigns}, Cert) -> HashSign end. %%-------------------------------------------------------------------- --spec select_cert_hashsign(#hash_sign_algos{}| undefined, oid(), ssl_record:ssl_version() | {undefined, undefined}) -> +-spec select_cert_hashsign(#hash_sign_algos{}| undefined, oid(), ssl_record:ssl_version()) -> {atom(), atom()}. %% @@ -635,8 +635,7 @@ select_hashsign(#hash_sign_algos{hash_sign_algos = HashSigns}, Cert) -> select_cert_hashsign(HashSign, _, {Major, Minor}) when HashSign =/= undefined andalso Major >= 3 andalso Minor >= 3 -> HashSign; -select_cert_hashsign(undefined, ?rsaEncryption, {Major, Minor}) when - is_integer(Major) andalso Major >= 3 andalso is_integer(Minor) andalso Minor >= 3 -> +select_cert_hashsign(undefined, ?rsaEncryption, {Major, Minor}) when Major >= 3 andalso Minor >= 3 -> {sha, rsa}; select_cert_hashsign(undefined,?'id-ecPublicKey', _) -> {sha, ecdsa}; diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl index c3171da566..8142a18c37 100644 --- a/lib/ssl/src/tls_connection.erl +++ b/lib/ssl/src/tls_connection.erl @@ -208,11 +208,11 @@ hello(Hello = #client_hello{client_version = ClientVersion, session_cache = Cache, session_cache_cb = CacheCb, ssl_options = SslOpts}) -> - HashSign = ssl_handshake:select_hashsign(HashSigns, Cert), case tls_handshake:hello(Hello, SslOpts, {Port, Session0, Cache, CacheCb, ConnectionStates0, Cert}, Renegotiation) of {Version, {Type, Session}, ConnectionStates, ServerHelloExt} -> + HashSign = ssl_handshake:select_hashsign(HashSigns, Cert, Version), ssl_connection:hello({common_client_hello, Type, ServerHelloExt, HashSign}, State#state{connection_states = ConnectionStates, negotiated_version = Version, diff --git a/lib/ssl/test/ssl_handshake_SUITE.erl b/lib/ssl/test/ssl_handshake_SUITE.erl index 4c4b8e5137..b4be768b58 100644 --- a/lib/ssl/test/ssl_handshake_SUITE.erl +++ b/lib/ssl/test/ssl_handshake_SUITE.erl @@ -101,5 +101,7 @@ encode_single_hello_sni_extension_correctly(_Config) -> select_proper_tls_1_2_rsa_default_hashsign(_Config) -> % RFC 5246 section 7.4.1.4.1 tells to use {sha1,rsa} as default signature_algorithm for RSA key exchanges {sha, rsa} = ssl_handshake:select_cert_hashsign(undefined, ?rsaEncryption, {3,3}), - {md5sha, rsa} = ssl_handshake:select_cert_hashsign(undefined, ?rsaEncryption, {undefined,undefined}). + % Older versions use MD5/SHA1 combination + {md5sha, rsa} = ssl_handshake:select_cert_hashsign(undefined, ?rsaEncryption, {3,2}), + {md5sha, rsa} = ssl_handshake:select_cert_hashsign(undefined, ?rsaEncryption, {3,0}). -- cgit v1.2.3 From 081ee510f1fb1d821a524bd6b8efd20e520add3c Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Wed, 23 Apr 2014 09:35:55 +0200 Subject: ssl: Refactor so that there is only one source for the default hashsign values Also fix DTLS call to supply its corresponding TLS version --- lib/ssl/src/dtls_connection.erl | 3 +- lib/ssl/src/ssl_connection.erl | 80 +++++++++--------------------------- lib/ssl/src/ssl_handshake.erl | 62 +++++++++++++++++++++------- lib/ssl/test/ssl_handshake_SUITE.erl | 8 ++-- 4 files changed, 73 insertions(+), 80 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/dtls_connection.erl b/lib/ssl/src/dtls_connection.erl index ec0f408f51..508983ddac 100644 --- a/lib/ssl/src/dtls_connection.erl +++ b/lib/ssl/src/dtls_connection.erl @@ -208,7 +208,8 @@ hello(Hello = #client_hello{client_version = ClientVersion, ConnectionStates, #hello_extensions{ec_point_formats = EcPointFormats, elliptic_curves = EllipticCurves} = ServerHelloExt} -> - HashSign = ssl_handshake:select_hashsign(HashSigns, Cert, Version), + HashSign = ssl_handshake:select_hashsign(HashSigns, Cert, + dtls_v1:corresponding_tls_version(Version)), ssl_connection:hello({common_client_hello, Type, ServerHelloExt, HashSign}, State#state{connection_states = ConnectionStates, negotiated_version = Version, diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl index 75100864c8..1eda926bcb 100644 --- a/lib/ssl/src/ssl_connection.erl +++ b/lib/ssl/src/ssl_connection.erl @@ -290,12 +290,11 @@ hello(#hello_request{}, #state{role = client} = State0, Connection) -> {Record, State} = Connection:next_record(State0), Connection:next_state(hello, hello, Record, State); -hello({common_client_hello, Type, ServerHelloExt, HashSign}, - #state{session = #session{cipher_suite = CipherSuite}, - negotiated_version = Version} = State, Connection) -> - {KeyAlg, _, _, _} = ssl_cipher:suite_definition(CipherSuite), - NegotiatedHashSign = negotiated_hashsign(HashSign, KeyAlg, Version), +hello({common_client_hello, Type, ServerHelloExt, NegotiatedHashSign}, + State, Connection) -> do_server_hello(Type, ServerHelloExt, + %% Note NegotiatedHashSign is only negotiated for real if + %% if TLS version is at least TLS-1.2 State#state{hashsign_algorithm = NegotiatedHashSign}, Connection); hello(timeout, State, _) -> @@ -432,7 +431,8 @@ certify(#server_key_exchange{exchange_keys = Keys}, calculate_secret(Params#server_key_params.params, State#state{hashsign_algorithm = HashSign}, Connection); false -> - ?ALERT_REC(?FATAL, ?DECRYPT_ERROR) + Connection:handle_own_alert(?ALERT_REC(?FATAL, ?DECRYPT_ERROR), + Version, certify, State) end end; @@ -560,7 +560,7 @@ cipher(#certificate_verify{signature = Signature, hashsign_algorithm = CertHashS tls_handshake_history = Handshake } = State0, Connection) -> - HashSign = ssl_handshake:select_cert_hashsign(CertHashSign, Algo, Version), + HashSign = ssl_handshake:select_hashsign_algs(CertHashSign, Algo, Version), case ssl_handshake:certificate_verify(Signature, PublicKeyInfo, Version, HashSign, MasterSecret, Handshake) of valid -> @@ -1564,60 +1564,6 @@ cipher_role(server, Data, Session, #state{connection_states = ConnectionStates0 session = Session}, cipher, Connection), Connection:next_state_connection(cipher, ack_connection(State#state{session = Session})). -negotiated_hashsign(undefined, Algo, Version) -> - default_hashsign(Version, Algo); -negotiated_hashsign(HashSign = {_, _}, _, _) -> - HashSign. - -%% RFC 5246, Sect. 7.4.1.4.1. Signature Algorithms -%% If the client does not send the signature_algorithms extension, the -%% server MUST do the following: -%% -%% - If the negotiated key exchange algorithm is one of (RSA, DHE_RSA, -%% DH_RSA, RSA_PSK, ECDH_RSA, ECDHE_RSA), behave as if client had -%% sent the value {sha1,rsa}. -%% -%% - If the negotiated key exchange algorithm is one of (DHE_DSS, -%% DH_DSS), behave as if the client had sent the value {sha1,dsa}. -%% -%% - If the negotiated key exchange algorithm is one of (ECDH_ECDSA, -%% ECDHE_ECDSA), behave as if the client had sent value {sha1,ecdsa}. - -default_hashsign(_Version = {Major, Minor}, KeyExchange) - when Major >= 3 andalso Minor >= 3 andalso - (KeyExchange == rsa orelse - KeyExchange == dhe_rsa orelse - KeyExchange == dh_rsa orelse - KeyExchange == ecdhe_rsa orelse - KeyExchange == ecdh_rsa orelse - KeyExchange == srp_rsa) -> - {sha, rsa}; -default_hashsign(_Version, KeyExchange) - when KeyExchange == rsa; - KeyExchange == dhe_rsa; - KeyExchange == dh_rsa; - KeyExchange == ecdhe_rsa; - KeyExchange == ecdh_rsa; - KeyExchange == srp_rsa -> - {md5sha, rsa}; -default_hashsign(_Version, KeyExchange) - when KeyExchange == ecdhe_ecdsa; - KeyExchange == ecdh_ecdsa -> - {sha, ecdsa}; -default_hashsign(_Version, KeyExchange) - when KeyExchange == dhe_dss; - KeyExchange == dh_dss; - KeyExchange == srp_dss -> - {sha, dsa}; -default_hashsign(_Version, KeyExchange) - when KeyExchange == dh_anon; - KeyExchange == ecdh_anon; - KeyExchange == psk; - KeyExchange == dhe_psk; - KeyExchange == rsa_psk; - KeyExchange == srp_anon -> - {null, anon}. - select_curve(#state{client_ecc = {[Curve|_], _}}) -> {namedCurve, Curve}; select_curve(_) -> @@ -1889,3 +1835,15 @@ new_ssl_options([undefined | Rest0], [Head1| Rest1], Acc) -> new_ssl_options(Rest0, Rest1, [Head1 | Acc]); new_ssl_options([Head0 | Rest0], [_| Rest1], Acc) -> new_ssl_options(Rest0, Rest1, [Head0 | Acc]). + +negotiated_hashsign(undefined, Alg, Version) -> + %% Not negotiated choose default + case is_anonymous(Alg) of + true -> + {null, anon}; + false -> + ssl_handshake:select_hashsign_algs(Alg, Version) + end; +negotiated_hashsign(HashSign = {_, _}, _, _) -> + HashSign. + diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl index c990eeedb5..fc67d2c28d 100644 --- a/lib/ssl/src/ssl_handshake.erl +++ b/lib/ssl/src/ssl_handshake.erl @@ -73,7 +73,8 @@ ]). %% MISC --export([select_version/3, prf/5, select_hashsign/3, select_cert_hashsign/3, +-export([select_version/3, prf/5, select_hashsign/3, + select_hashsign_algs/2, select_hashsign_algs/3, premaster_secret/2, premaster_secret/3, premaster_secret/4]). %%==================================================================== @@ -590,9 +591,11 @@ prf({3,1}, Secret, Label, Seed, WantedLength) -> {ok, tls_v1:prf(?MD5SHA, Secret, Label, Seed, WantedLength)}; prf({3,_N}, Secret, Label, Seed, WantedLength) -> {ok, tls_v1:prf(?SHA256, Secret, Label, Seed, WantedLength)}. + + %%-------------------------------------------------------------------- -spec select_hashsign(#hash_sign_algos{}| undefined, undefined | binary(), ssl_record:ssl_version()) -> - [{atom(), atom()}] | undefined. + {atom(), atom()} | undefined. %% %% Description: @@ -602,11 +605,11 @@ select_hashsign(_, undefined, _Version) -> select_hashsign(undefined, Cert, Version) -> #'OTPCertificate'{tbsCertificate = TBSCert} = public_key:pkix_decode_cert(Cert, otp), #'OTPSubjectPublicKeyInfo'{algorithm = {_,Algo, _}} = TBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo, - select_cert_hashsign(undefined, Algo, Version); + select_hashsign_algs(undefined, Algo, Version); select_hashsign(#hash_sign_algos{hash_sign_algos = HashSigns}, Cert, Version) -> #'OTPCertificate'{tbsCertificate = TBSCert} =public_key:pkix_decode_cert(Cert, otp), #'OTPSubjectPublicKeyInfo'{algorithm = {_,Algo, _}} = TBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo, - DefaultHashSign = {_, Sign} = select_cert_hashsign(undefined, Algo, Version), + DefaultHashSign = {_, Sign} = select_hashsign_algs(undefined, Algo, Version), case lists:filter(fun({sha, dsa}) -> true; ({_, dsa}) -> @@ -622,28 +625,59 @@ select_hashsign(#hash_sign_algos{hash_sign_algos = HashSigns}, Cert, Version) -> [HashSign| _] -> HashSign end. + %%-------------------------------------------------------------------- --spec select_cert_hashsign(#hash_sign_algos{}| undefined, oid(), ssl_record:ssl_version()) -> +-spec select_hashsign_algs(#hash_sign_algos{}| undefined, oid(), ssl_record:ssl_version()) -> {atom(), atom()}. +%% Description: For TLS 1.2 hash function and signature algorithm pairs can be +%% negotiated with the signature_algorithms extension, +%% for previous versions always use appropriate defaults. +%% RFC 5246, Sect. 7.4.1.4.1. Signature Algorithms +%% If the client does not send the signature_algorithms extension, the +%% server MUST do the following: (e.i defaults for TLS 1.2) +%% +%% - If the negotiated key exchange algorithm is one of (RSA, DHE_RSA, +%% DH_RSA, RSA_PSK, ECDH_RSA, ECDHE_RSA), behave as if client had +%% sent the value {sha1,rsa}. %% -%% Description: For TLS 1.2 selected cert_hash_sign will be recived -%% in the handshake message, for previous versions use appropriate defaults. -%% This function is also used by select_hashsign to extract -%% the alogrithm of the server cert key. +%% - If the negotiated key exchange algorithm is one of (DHE_DSS, +%% DH_DSS), behave as if the client had sent the value {sha1,dsa}. +%% +%% - If the negotiated key exchange algorithm is one of (ECDH_ECDSA, +%% ECDHE_ECDSA), behave as if the client had sent value {sha1,ecdsa}. + %%-------------------------------------------------------------------- -select_cert_hashsign(HashSign, _, {Major, Minor}) when HashSign =/= undefined andalso +select_hashsign_algs(HashSign, _, {Major, Minor}) when HashSign =/= undefined andalso Major >= 3 andalso Minor >= 3 -> HashSign; -select_cert_hashsign(undefined, ?rsaEncryption, {Major, Minor}) when Major >= 3 andalso Minor >= 3 -> +select_hashsign_algs(undefined, ?rsaEncryption, {Major, Minor}) when Major >= 3 andalso Minor >= 3 -> {sha, rsa}; -select_cert_hashsign(undefined,?'id-ecPublicKey', _) -> +select_hashsign_algs(undefined,?'id-ecPublicKey', _) -> {sha, ecdsa}; -select_cert_hashsign(undefined, ?rsaEncryption, _) -> +select_hashsign_algs(undefined, ?rsaEncryption, _) -> {md5sha, rsa}; -select_cert_hashsign(undefined, ?'id-dsa', _) -> +select_hashsign_algs(undefined, ?'id-dsa', _) -> {sha, dsa}. +-spec select_hashsign_algs(atom(), ssl_record:ssl_version()) -> {atom(), atom()}. +%% Wrap function to keep the knowledge of the default values in +%% one place only +select_hashsign_algs(Alg, Version) when (Alg == rsa orelse + Alg == dhe_rsa orelse + Alg == dh_rsa orelse + Alg == ecdhe_rsa orelse + Alg == ecdh_rsa orelse + Alg == srp_rsa) -> + select_hashsign_algs(undefined, ?rsaEncryption, Version); +select_hashsign_algs(Alg, Version) when (Alg == dhe_dss orelse + Alg == dh_dss orelse + Alg == srp_dss) -> + select_hashsign_algs(undefined, ?'id-dsa', Version); +select_hashsign_algs(Alg, Version) when (Alg == ecdhe_ecdsa orelse + Alg == ecdh_ecdsa) -> + select_hashsign_algs(undefined, ?'id-ecPublicKey', Version). + %%-------------------------------------------------------------------- -spec master_secret(atom(), ssl_record:ssl_version(), #session{} | binary(), #connection_states{}, client | server) -> {binary(), #connection_states{}} | #alert{}. diff --git a/lib/ssl/test/ssl_handshake_SUITE.erl b/lib/ssl/test/ssl_handshake_SUITE.erl index b4be768b58..5f36842f9e 100644 --- a/lib/ssl/test/ssl_handshake_SUITE.erl +++ b/lib/ssl/test/ssl_handshake_SUITE.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2008-2013. All Rights Reserved. +%% Copyright Ericsson AB 2008-2014. 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 @@ -100,8 +100,8 @@ encode_single_hello_sni_extension_correctly(_Config) -> select_proper_tls_1_2_rsa_default_hashsign(_Config) -> % RFC 5246 section 7.4.1.4.1 tells to use {sha1,rsa} as default signature_algorithm for RSA key exchanges - {sha, rsa} = ssl_handshake:select_cert_hashsign(undefined, ?rsaEncryption, {3,3}), + {sha, rsa} = ssl_handshake:select_hashsign_algs(undefined, ?rsaEncryption, {3,3}), % Older versions use MD5/SHA1 combination - {md5sha, rsa} = ssl_handshake:select_cert_hashsign(undefined, ?rsaEncryption, {3,2}), - {md5sha, rsa} = ssl_handshake:select_cert_hashsign(undefined, ?rsaEncryption, {3,0}). + {md5sha, rsa} = ssl_handshake:select_hashsign_algs(undefined, ?rsaEncryption, {3,2}), + {md5sha, rsa} = ssl_handshake:select_hashsign_algs(undefined, ?rsaEncryption, {3,0}). -- cgit v1.2.3 From abb5c21e25343139e47559dbf9a22d099f97154f Mon Sep 17 00:00:00 2001 From: Danil Zagoskin Date: Sun, 20 Apr 2014 02:25:42 +0400 Subject: ssl: Fix crash on garbage during handshake If a client sends some garbage in ssl record instead of valid fragment, server crashes with function_clause while receiving next record from client. This patch makes server raise handshake failure instead of crashing and exposing internal state to user code. --- lib/ssl/src/tls_connection.erl | 6 +++++- lib/ssl/test/ssl_basic_SUITE.erl | 46 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) (limited to 'lib/ssl') diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl index 8142a18c37..930706cde6 100644 --- a/lib/ssl/src/tls_connection.erl +++ b/lib/ssl/src/tls_connection.erl @@ -751,7 +751,11 @@ handle_tls_handshake(Handle, StateName, handle_tls_handshake(Handle, NextStateName, State); {stop, _,_} = Stop -> Stop - end. + end; + +handle_tls_handshake(_Handle, _StateName, #state{}) -> + throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE)). + write_application_data(Data0, From, #state{socket = Socket, negotiated_version = Version, diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl index 3d711021f3..406be65c3b 100644 --- a/lib/ssl/test/ssl_basic_SUITE.erl +++ b/lib/ssl/test/ssl_basic_SUITE.erl @@ -190,7 +190,8 @@ error_handling_tests()-> tcp_connect_big, close_transport_accept, recv_active, - recv_active_once + recv_active_once, + dont_crash_on_handshake_garbage ]. rizzo_tests() -> @@ -2646,6 +2647,49 @@ ciphersuite_vs_version(Config) when is_list(Config) -> %%-------------------------------------------------------------------- +dont_crash_on_handshake_garbage() -> + [{doc, "Ensure SSL server worker thows an alert on garbage during handshake " + "instead of crashing and exposing state to user code"}]. + +dont_crash_on_handshake_garbage(Config) -> + ServerOpts = ?config(server_opts, Config), + + {_ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config), + + Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0}, + {from, self()}, + {mfa, {ssl_test_lib, send_recv_result_active, []}}, + {options, ServerOpts}]), + unlink(Server), monitor(process, Server), + Port = ssl_test_lib:inet_port(Server), + + {ok, Socket} = gen_tcp:connect(Hostname, Port, [binary, {active, false}]), + + % Send hello and garbage record + ok = gen_tcp:send(Socket, + [<<22, 3,3, 49:16, 1, 45:24, 3,3, % client_hello + 16#deadbeef:256, % 32 'random' bytes = 256 bits + 0, 6:16, 0,255, 0,61, 0,57, 1, 0 >>, % some hello values + + <<22, 3,3, 5:16, 92,64,37,228,209>> % garbage + ]), + % Send unexpected change_cipher_spec + ok = gen_tcp:send(Socket, <<20, 0,0,12, 111,40,244,7,137,224,16,109,197,110,249,152>>), + + % Ensure we receive an alert, not sudden disconnect + {ok, <<21, _/binary>>} = drop_handshakes(Socket, 1000). + +drop_handshakes(Socket, Timeout) -> + {ok, <> = Header} = gen_tcp:recv(Socket, 5, Timeout), + {ok, <>} = gen_tcp:recv(Socket, RecLen, Timeout), + case RecType of + 22 -> drop_handshakes(Socket, Timeout); + _ -> {ok, <
>} + end. + + +%%-------------------------------------------------------------------- + hibernate() -> [{doc,"Check that an SSL connection that is started with option " "{hibernate_after, 1000} indeed hibernates after 1000ms of " -- cgit v1.2.3