Skip to content

Commit

Permalink
Merge branch 'ingela/ssl/ECC-select-hash-sign/OTP-13711' into maint
Browse files Browse the repository at this point in the history 
* ingela/ssl/ECC-select-hash-sign/OTP-13711:
  ssl: Correct handling of signature algorithm selection
  ssl: Simplify and refactor tests
  • Loading branch information
@IngelaAndin
IngelaAndin committed Jul 8, 2016
2 parents fcddab2 + d7dcfb2 commit 3873689
Show file tree
Hide file tree
Showing 6 changed files with 276 additions and 112 deletions.
7 changes: 3 additions & 4 deletions lib/ssl/src/ssl_connection.erl
View file
Expand Up @@ -528,13 +528,12 @@ certify(internal, #server_key_exchange{exchange_keys = Keys},
end
end;

certify(internal, #certificate_request{hashsign_algorithms = HashSigns},
certify(internal, #certificate_request{} = CertRequest,
#state{session = #session{own_certificate = Cert},
key_algorithm = KeyExAlg,
role = client,
ssl_options = #ssl_options{signature_algs = SupportedHashSigns},
negotiated_version = Version} = State0, Connection) ->

case ssl_handshake:select_hashsign(HashSigns, Cert, KeyExAlg, SupportedHashSigns, Version) of
case ssl_handshake:select_hashsign(CertRequest, Cert, SupportedHashSigns, Version) of
#alert {} = Alert ->
Connection:handle_own_alert(Alert, Version, certify, State0);
NegotiatedHashSign ->
Expand Down
139 changes: 117 additions & 22 deletions lib/ssl/src/ssl_handshake.erl
View file
Expand Up @@ -74,7 +74,7 @@
]).

%% MISC
-export([select_version/3, prf/6, select_hashsign/5,
-export([select_version/3, prf/6, select_hashsign/4, select_hashsign/5,
select_hashsign_algs/3,
premaster_secret/2, premaster_secret/3, premaster_secret/4]).

Expand Down Expand Up @@ -581,7 +581,7 @@ prf({3,_N}, PRFAlgo, Secret, Label, Seed, WantedLength) ->
{atom(), atom()} | undefined | #alert{}.

%%
%% Description: Handles signature_algorithms extension
%% Description: Handles signature_algorithms hello extension (server)
%%--------------------------------------------------------------------
select_hashsign(_, undefined, _, _, _Version) ->
{null, anon};
Expand All @@ -593,14 +593,17 @@ select_hashsign(HashSigns, Cert, KeyExAlgo,
select_hashsign(#hash_sign_algos{hash_sign_algos = HashSigns}, Cert, KeyExAlgo, SupportedHashSigns,
{Major, Minor}) when Major >= 3 andalso Minor >= 3 ->
#'OTPCertificate'{tbsCertificate = TBSCert} = public_key:pkix_decode_cert(Cert, otp),
#'OTPSubjectPublicKeyInfo'{algorithm = {_,Algo, _}} = TBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo,
Sign = cert_sign(Algo),
case lists:filter(fun({sha, dsa = S}) when S == Sign ->
true;
({_, dsa}) ->
false;
({_, _} = Algos) ->
is_acceptable_hash_sign(Algos, Sign, KeyExAlgo, SupportedHashSigns);
#'OTPCertificate'{tbsCertificate = TBSCert,
signatureAlgorithm = {_,SignAlgo, _}} = public_key:pkix_decode_cert(Cert, otp),
#'OTPSubjectPublicKeyInfo'{algorithm = {_, SubjAlgo, _}} =
TBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo,

Sign = sign_algo(SignAlgo),
SubSing = sign_algo(SubjAlgo),

case lists:filter(fun({_, S} = Algos) when S == Sign ->
is_acceptable_hash_sign(Algos, Sign,
SubSing, KeyExAlgo, SupportedHashSigns);
(_) ->
false
end, HashSigns) of
Expand All @@ -613,6 +616,49 @@ select_hashsign(_, Cert, _, _, Version) ->
#'OTPCertificate'{tbsCertificate = TBSCert} = public_key:pkix_decode_cert(Cert, otp),
#'OTPSubjectPublicKeyInfo'{algorithm = {_,Algo, _}} = TBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo,
select_hashsign_algs(undefined, Algo, Version).
%%--------------------------------------------------------------------
-spec select_hashsign(#certificate_request{}, binary(),
[atom()], ssl_record:ssl_version()) ->
{atom(), atom()} | #alert{}.

%%
%% Description: Handles signature algorithms selection for certificate requests (client)
%%--------------------------------------------------------------------
select_hashsign(#certificate_request{}, undefined, _, {Major, Minor}) when Major >= 3 andalso Minor >= 3->
%% There client does not have a certificate and will send an empty reply, the server may fail
%% or accept the connection by its own preference. No signature algorihms needed as there is
%% no certificate to verify.
{undefined, undefined};

select_hashsign(#certificate_request{hashsign_algorithms = #hash_sign_algos{hash_sign_algos = HashSigns},
certificate_types = Types}, Cert, SupportedHashSigns,
{Major, Minor}) when Major >= 3 andalso Minor >= 3->
#'OTPCertificate'{tbsCertificate = TBSCert} = public_key:pkix_decode_cert(Cert, otp),
#'OTPCertificate'{tbsCertificate = TBSCert,
signatureAlgorithm = {_,SignAlgo, _}} = public_key:pkix_decode_cert(Cert, otp),
#'OTPSubjectPublicKeyInfo'{algorithm = {_, SubjAlgo, _}} =
TBSCert#'OTPTBSCertificate'.subjectPublicKeyInfo,

Sign = sign_algo(SignAlgo),
SubSign = sign_algo(SubjAlgo),

case is_acceptable_cert_type(SubSign, HashSigns, Types) andalso is_supported_sign(Sign, HashSigns) of
true ->
case lists:filter(fun({_, S} = Algos) when S == SubSign ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
(_) ->
false
end, HashSigns) of
[] ->
?ALERT_REC(?FATAL, ?INSUFFICIENT_SECURITY, no_suitable_signature_algorithm);
[HashSign | _] ->
HashSign
end;
false ->
?ALERT_REC(?FATAL, ?INSUFFICIENT_SECURITY, no_suitable_signature_algorithm)
end;
select_hashsign(#certificate_request{}, Cert, _, Version) ->
select_hashsign(undefined, Cert, undefined, undefined, Version).

%%--------------------------------------------------------------------
-spec select_hashsign_algs({atom(), atom()}| undefined, oid(), ssl_record:ssl_version()) ->
Expand Down Expand Up @@ -648,6 +694,7 @@ select_hashsign_algs(undefined, ?rsaEncryption, _) ->
select_hashsign_algs(undefined, ?'id-dsa', _) ->
{sha, dsa}.


%%--------------------------------------------------------------------
-spec master_secret(atom(), ssl_record:ssl_version(), #session{} | binary(), #connection_states{},
client | server) -> {binary(), #connection_states{}} | #alert{}.
Expand Down Expand Up @@ -1143,11 +1190,13 @@ certificate_types(_, {N, M}) when N >= 3 andalso M >= 3 ->
end;

certificate_types({KeyExchange, _, _, _}, _) when KeyExchange == rsa;
KeyExchange == dh_rsa;
KeyExchange == dhe_rsa;
KeyExchange == ecdhe_rsa ->
<<?BYTE(?RSA_SIGN)>>;

certificate_types({KeyExchange, _, _, _}, _) when KeyExchange == dhe_dss;
certificate_types({KeyExchange, _, _, _}, _) when KeyExchange == dh_dss;
KeyExchange == dhe_dss;
KeyExchange == srp_dss ->
<<?BYTE(?DSS_SIGN)>>;

Expand Down Expand Up @@ -2164,27 +2213,73 @@ distpoints_lookup([DistPoint | Rest], Issuer, Callback, CRLDbHandle) ->
[{DistPoint, {CRL, public_key:der_decode('CertificateList', CRL)}} || CRL <- CRLs]
end.

cert_sign(?rsaEncryption) ->
sign_algo(?rsaEncryption) ->
rsa;
cert_sign(?'id-ecPublicKey') ->
sign_algo(?'id-ecPublicKey') ->
ecdsa;
cert_sign(?'id-dsa') ->
sign_algo(?'id-dsa') ->
dsa;
cert_sign(Alg) ->
sign_algo(Alg) ->
{_, Sign} =public_key:pkix_sign_types(Alg),
Sign.

is_acceptable_hash_sign({_, Sign} = Algos, Sign, _, SupportedHashSigns) ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign(Algos,_, KeyExAlgo, SupportedHashSigns) when KeyExAlgo == dh_ecdsa;
KeyExAlgo == ecdh_rsa;
KeyExAlgo == ecdh_ecdsa ->
is_acceptable_hash_sign(Algos, _, _, KeyExAlgo, SupportedHashSigns) when
KeyExAlgo == dh_dss;
KeyExAlgo == dh_rsa;
KeyExAlgo == dh_ecdsa ->
%% dh_* could be called only dh in TLS-1.2
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign(_,_,_,_) ->
false.
is_acceptable_hash_sign(Algos, rsa, ecdsa, ecdh_rsa, SupportedHashSigns) ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign({_, rsa} = Algos, rsa, _, dhe_rsa, SupportedHashSigns) ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign({_, rsa} = Algos, rsa, rsa, ecdhe_rsa, SupportedHashSigns) ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign({_, rsa} = Algos, rsa, rsa, rsa, SupportedHashSigns) ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign({_, rsa} = Algos, rsa, _, srp_rsa, SupportedHashSigns) ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign({_, rsa} = Algos, rsa, _, rsa_psk, SupportedHashSigns) ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign({_, dsa} = Algos, dsa, _, dhe_dss, SupportedHashSigns) ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign({_, dsa} = Algos, dsa, _, srp_dss, SupportedHashSigns) ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign({_, ecdsa} = Algos, ecdsa, _, dhe_ecdsa, SupportedHashSigns) ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign({_, ecdsa} = Algos, ecdsa, ecdsa, ecdhe_ecdsa, SupportedHashSigns) ->
is_acceptable_hash_sign(Algos, SupportedHashSigns);
is_acceptable_hash_sign(_, _, _, KeyExAlgo, _) when
KeyExAlgo == psk;
KeyExAlgo == dhe_psk;
KeyExAlgo == srp_anon;
KeyExAlgo == dh_anon;
KeyExAlgo == ecdhe_anon
->
true;
is_acceptable_hash_sign(_,_, _,_,_) ->
false.

is_acceptable_hash_sign(Algos, SupportedHashSigns) ->
lists:member(Algos, SupportedHashSigns).

is_acceptable_cert_type(Sign, _HashSigns, Types) ->
lists:member(sign_type(Sign), binary_to_list(Types)).

is_supported_sign(Sign, HashSigns) ->
[] =/= lists:dropwhile(fun({_, S}) when S =/= Sign ->
true;
(_)->
false
end, HashSigns).
sign_type(rsa) ->
?RSA_SIGN;
sign_type(dsa) ->
?DSS_SIGN;
sign_type(ecdsa) ->
?ECDSA_SIGN.


bad_key(#'DSAPrivateKey'{}) ->
unacceptable_dsa_key;
bad_key(#'RSAPrivateKey'{}) ->
Expand Down
60 changes: 35 additions & 25 deletions lib/ssl/test/ssl_ECC_SUITE.erl
View file
Expand Up @@ -159,42 +159,42 @@ end_per_testcase(_TestCase, Config) ->

client_ecdh_server_ecdh(Config) when is_list(Config) ->
COpts = proplists:get_value(client_ecdh_rsa_opts, Config),
SOpts = proplists:get_value(server_ecdh_rsa_verify_opts, Config),
SOpts = proplists:get_value(server_ecdh_rsa_opts, Config),
basic_test(COpts, SOpts, Config).

client_ecdh_server_rsa(Config) when is_list(Config) ->
COpts = proplists:get_value(client_ecdh_rsa_opts, Config),
SOpts = proplists:get_value(server_ecdh_rsa_verify_opts, Config),
SOpts = proplists:get_value(server_opts, Config),
basic_test(COpts, SOpts, Config).

client_rsa_server_ecdh(Config) when is_list(Config) ->
COpts = proplists:get_value(client_ecdh_rsa_opts, Config),
SOpts = proplists:get_value(server_ecdh_rsa_verify_opts, Config),
COpts = proplists:get_value(client_opts, Config),
SOpts = proplists:get_value(server_ecdh_rsa_opts, Config),
basic_test(COpts, SOpts, Config).

client_rsa_server_rsa(Config) when is_list(Config) ->
COpts = proplists:get_value(client_verification_opts, Config),
SOpts = proplists:get_value(server_verification_opts, Config),
COpts = proplists:get_value(client_opts, Config),
SOpts = proplists:get_value(server_opts, Config),
basic_test(COpts, SOpts, Config).

client_ecdsa_server_ecdsa(Config) when is_list(Config) ->
COpts = proplists:get_value(client_ecdsa_opts, Config),
SOpts = proplists:get_value(server_ecdsa_verify_opts, Config),
SOpts = proplists:get_value(server_ecdsa_opts, Config),
basic_test(COpts, SOpts, Config).

client_ecdsa_server_rsa(Config) when is_list(Config) ->
COpts = proplists:get_value(client_ecdsa_opts, Config),
SOpts = proplists:get_value(server_ecdsa_verify_opts, Config),
SOpts = proplists:get_value(server_opts, Config),
basic_test(COpts, SOpts, Config).

client_rsa_server_ecdsa(Config) when is_list(Config) ->
COpts = proplists:get_value(client_ecdsa_opts, Config),
SOpts = proplists:get_value(server_ecdsa_verify_opts, Config),
COpts = proplists:get_value(client_opts, Config),
SOpts = proplists:get_value(server_ecdsa_opts, Config),
basic_test(COpts, SOpts, Config).

client_ecdsa_server_ecdsa_with_raw_key(Config) when is_list(Config) ->
COpts = proplists:get_value(client_ecdsa_opts, Config),
SOpts = proplists:get_value(server_ecdsa_verify_opts, Config),
SOpts = proplists:get_value(server_ecdsa_opts, Config),
ServerCert = proplists:get_value(certfile, SOpts),
ServerKeyFile = proplists:get_value(keyfile, SOpts),
{ok, PemBin} = file:read_file(ServerKeyFile),
Expand Down Expand Up @@ -244,20 +244,20 @@ basic_test(ClientCert, ClientKey, ClientCA, ServerCert, ServerKey, ServerCA, Con
check_result(Server, SType, Client, CType),
close(Server, Client).

start_client(openssl, Port, CA, OwnCa, Cert, Key, Config) ->
PrivDir = proplists:get_value(priv_dir, Config),
NewCA = new_ca(filename:join(PrivDir, "new_ca.pem"), CA, OwnCa),
start_client(openssl, Port, PeerCA, OwnCa, Cert, Key, _Config) ->
CA = new_openssl_ca("openssl_client_ca", PeerCA, OwnCa),
Version = tls_record:protocol_version(tls_record:highest_protocol_version([])),
Exe = "openssl",
Args = ["s_client", "-verify", "2", "-port", integer_to_list(Port),
ssl_test_lib:version_flag(Version),
"-cert", Cert, "-CAfile", NewCA,
"-cert", Cert, "-CAfile", CA,
"-key", Key, "-host","localhost", "-msg", "-debug"],

OpenSslPort = ssl_test_lib:portable_open_port(Exe, Args),
true = port_command(OpenSslPort, "Hello world"),
OpenSslPort;
start_client(erlang, Port, CA, _, Cert, Key, Config) ->
start_client(erlang, Port, PeerCA, OwnCa, Cert, Key, Config) ->
CA = new_ca("erlang_client_ca", PeerCA, OwnCa),
{ClientNode, _, Hostname} = ssl_test_lib:run_where(Config),
ssl_test_lib:start_client([{node, ClientNode}, {port, Port},
{host, Hostname},
Expand All @@ -267,20 +267,19 @@ start_client(erlang, Port, CA, _, Cert, Key, Config) ->
{cacertfile, CA},
{certfile, Cert}, {keyfile, Key}]}]).

start_server(openssl, CA, OwnCa, Cert, Key, Config) ->
PrivDir = proplists:get_value(priv_dir, Config),
NewCA = new_ca(filename:join(PrivDir, "new_ca.pem"), CA, OwnCa),

start_server(openssl, PeerCA, OwnCa, Cert, Key, _Config) ->
CA = new_openssl_ca("openssl_server_ca", PeerCA, OwnCa),
Port = ssl_test_lib:inet_port(node()),
Version = tls_record:protocol_version(tls_record:highest_protocol_version([])),
Exe = "openssl",
Args = ["s_server", "-accept", integer_to_list(Port), ssl_test_lib:version_flag(Version),
"-verify", "2", "-cert", Cert, "-CAfile", NewCA,
"-verify", "2", "-cert", Cert, "-CAfile", CA,
"-key", Key, "-msg", "-debug"],
OpenSslPort = ssl_test_lib:portable_open_port(Exe, Args),
true = port_command(OpenSslPort, "Hello world"),
{OpenSslPort, Port};
start_server(erlang, CA, _, Cert, Key, Config) ->
start_server(erlang, PeerCA, OwnCa, Cert, Key, Config) ->
CA = new_ca("erlang_server_ca", PeerCA, OwnCa),
{_, ServerNode, _} = ssl_test_lib:run_where(Config),
Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
{from, self()},
Expand All @@ -291,7 +290,8 @@ start_server(erlang, CA, _, Cert, Key, Config) ->
[{verify, verify_peer}, {cacertfile, CA},
{certfile, Cert}, {keyfile, Key}]}]),
{Server, ssl_test_lib:inet_port(Server)}.
start_server_with_raw_key(erlang, CA, _, Cert, Key, Config) ->
start_server_with_raw_key(erlang, PeerCA, OwnCa, Cert, Key, Config) ->
CA = new_ca("erlang_server_ca", PeerCA, OwnCa),
{_, ServerNode, _} = ssl_test_lib:run_where(Config),
Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
{from, self()},
Expand Down Expand Up @@ -336,9 +336,16 @@ close(Client, Server) ->
ssl_test_lib:close(Server),
ssl_test_lib:close(Client).

%% Work around OpenSSL bug, apparently the same bug as we had fixed in
%% 11629690ba61f8e0c93ef9b2b6102fd279825977
new_ca(FileName, CA, OwnCa) ->
{ok, P1} = file:read_file(CA),
E1 = public_key:pem_decode(P1),
{ok, P2} = file:read_file(OwnCa),
E2 = public_key:pem_decode(P2),
Pem = public_key:pem_encode(E1 ++E2),
file:write_file(FileName, Pem),
FileName.

new_openssl_ca(FileName, CA, OwnCa) ->
{ok, P1} = file:read_file(CA),
E1 = public_key:pem_decode(P1),
{ok, P2} = file:read_file(OwnCa),
Expand All @@ -347,6 +354,9 @@ new_ca(FileName, CA, OwnCa) ->
"OpenSSL 1.0.1p-freebsd" ++ _ ->
Pem = public_key:pem_encode(E1 ++E2),
file:write_file(FileName, Pem);
"LibreSSL" ++ _ ->
Pem = public_key:pem_encode(E1 ++E2),
file:write_file(FileName, Pem);
_ ->
Pem = public_key:pem_encode(E2 ++E1),
file:write_file(FileName, Pem)
Expand Down

0 comments on commit 3873689

Please sign in to comment.