Fix bug in ssl handshake protocol related to the choice of cipher suites

in client hello message when a client certificate is used

The client hello message now always include ALL available cipher suites
(or those specified by the ciphers option). Previous implementation would
filter them based on the client certificate key usage extension (such
filtering only makes sense for the server certificate).

4 files changed, 70 insertions, 7 deletions

diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index fcc30f6137..44e20fed30 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -64,7 +64,7 @@ client_hello(Host, Port, ConnectionStates, #ssl_options{versions = Versions,
     Version = ssl_record:highest_protocol_version(lists:map(Fun, Versions)),
     Pending = ssl_record:pending_connection_state(ConnectionStates, read),
     SecParams = Pending#connection_state.security_parameters,
-    Ciphers = available_suites(Cert, UserSuites, Version),
+    Ciphers = available_suites(UserSuites, Version),
 
     Id = ssl_manager:client_session_id(Host, Port, SslOpts),
 
@@ -524,13 +524,16 @@ select_session(Hello, Port, Session, Version,
 	    {resumed, CacheCb:lookup(Cache, {Port, SessionId})}
     end.
 
-available_suites(Cert, UserSuites, Version) ->
+available_suites(UserSuites, Version) ->
     case UserSuites of
 	[] ->
-	    ssl_cipher:filter(Cert, ssl_cipher:suites(Version));
+	    ssl_cipher:suites(Version);
 	_ ->
-	    ssl_cipher:filter(Cert, UserSuites)
+	    UserSuites
     end.
+
+available_suites(ServerCert, UserSuites, Version) ->
+    ssl_cipher:filter(ServerCert, available_suites(UserSuites, Version)).
  
 cipher_suites(Suites, false) ->
     [?TLS_EMPTY_RENEGOTIATION_INFO_SCSV | Suites];
diff --git a/lib/ssl/test/make_certs.erl b/lib/ssl/test/make_certs.erl
index 0cdf33c3e2..3c18a905b4 100644
--- a/lib/ssl/test/make_certs.erl
+++ b/lib/ssl/test/make_certs.erl
@@ -90,8 +90,10 @@ enduser(Root, OpenSSLCmd, CA, User) ->
     KeyFile = filename:join([UsrRoot, "key.pem"]), 
     ReqFile =  filename:join([UsrRoot, "req.pem"]), 
     create_req(Root, OpenSSLCmd, CnfFile, KeyFile, ReqFile),
-    CertFile =  filename:join([UsrRoot, "cert.pem"]), 
-    sign_req(Root, OpenSSLCmd, CA, "user_cert", ReqFile, CertFile).
+    CertFileAllUsage =  filename:join([UsrRoot, "cert.pem"]),
+    sign_req(Root, OpenSSLCmd, CA, "user_cert", ReqFile, CertFileAllUsage),
+    CertFileDigitalSigOnly =  filename:join([UsrRoot, "digital_signature_only_cert.pem"]),
+    sign_req(Root, OpenSSLCmd, CA, "user_cert_digital_signature_only", ReqFile, CertFileDigitalSigOnly).
 
 collect_certs(Root, CAs, Users) ->
     Bins = lists:foldr(
@@ -255,6 +257,7 @@ ca_cnf(CA) ->
      "RANDFILE	        = $dir/private/RAND\n"
      "\n"
      "x509_extensions   = user_cert\n"
+     "unique_subject  = no\n"
      "default_days	= 3600\n"
      "default_md	= sha1\n"
      "preserve	        = no\n"
@@ -279,6 +282,15 @@ ca_cnf(CA) ->
      "issuerAltName	= issuer:copy\n"
      "\n"
 
+     "[user_cert_digital_signature_only]\n"
+     "basicConstraints	= CA:false\n"
+     "keyUsage 		= digitalSignature\n"
+     "subjectKeyIdentifier = hash\n"
+     "authorityKeyIdentifier = keyid,issuer:always\n"
+     "subjectAltName	= email:copy\n"
+     "issuerAltName	= issuer:copy\n"
+     "\n"
+
      "[ca_cert]\n"
      "basicConstraints 	= critical,CA:true\n"
      "keyUsage 		= cRLSign, keyCertSign\n"
diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl
index 8a1b90ed98..c42a88b02f 100644
--- a/lib/ssl/test/ssl_basic_SUITE.erl
+++ b/lib/ssl/test/ssl_basic_SUITE.erl
@@ -233,7 +233,8 @@ all(suite) ->
      server_renegotiate_reused_session, client_no_wrap_sequence_number,
      server_no_wrap_sequence_number, extended_key_usage,
      validate_extensions_fun, no_authority_key_identifier,
-     invalid_signature_client, invalid_signature_server, cert_expired
+     invalid_signature_client, invalid_signature_server, cert_expired,
+     client_with_cert_cipher_suites_handshake
     ].
 
 %% Test cases starts here.
@@ -2849,6 +2850,39 @@ two_digits_str(N) ->
     lists:flatten(io_lib:format("~p", [N])).
 
 %%--------------------------------------------------------------------
+
+client_with_cert_cipher_suites_handshake(doc) ->
+    ["Test that client with a certificate without keyEncipherment usage "
+    " extension can connect to a server with restricted cipher suites "];
+
+client_with_cert_cipher_suites_handshake(suite) ->
+    [];
+
+client_with_cert_cipher_suites_handshake(Config) when is_list(Config) ->
+    ClientOpts =  ?config(client_verification_opts_digital_signature_only, Config),
+    ServerOpts =  ?config(server_verification_opts, Config),
+    {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+    Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+					{from, self()},
+					{mfa, {?MODULE,
+					       send_recv_result_active, []}},
+					{options, [{active, true},
+						   {ciphers, ssl_test_lib:rsa_non_signed_suites()}
+						   | ServerOpts]}]),
+    Port  = ssl_test_lib:inet_port(Server),
+    Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port},
+					{host, Hostname},
+					{from, self()},
+					{mfa, {?MODULE,
+					       send_recv_result_active, []}},
+					{options, [{active, true}
+						   | ClientOpts]}]),
+
+    ssl_test_lib:check_result(Server, ok, Client, ok),
+    ssl_test_lib:close(Server),
+    ssl_test_lib:close(Client).
+
+%%--------------------------------------------------------------------
 %%% Internal functions
 %%--------------------------------------------------------------------
 send_recv_result(Socket) ->
diff --git a/lib/ssl/test/ssl_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl
index dd0818827a..e34b6782a6 100644
--- a/lib/ssl/test/ssl_test_lib.erl
+++ b/lib/ssl/test/ssl_test_lib.erl
@@ -268,6 +268,8 @@ cert_options(Config) ->
 				      "client", "cacerts.pem"]),
     ClientCertFile = filename:join([?config(priv_dir, Config), 
 				    "client", "cert.pem"]),
+    ClientCertFileDigitalSignatureOnly = filename:join([?config(priv_dir, Config),
+				    "client", "digital_signature_only_cert.pem"]),
     ServerCaCertFile = filename:join([?config(priv_dir, Config), 
 				      "server", "cacerts.pem"]),
     ServerCertFile = filename:join([?config(priv_dir, Config), 
@@ -292,6 +294,10 @@ cert_options(Config) ->
 				{certfile, ClientCertFile},  
 				{keyfile, ClientKeyFile},
 				{ssl_imp, new}]}, 
+     {client_verification_opts_digital_signature_only, [{cacertfile, ClientCaCertFile},
+				{certfile, ClientCertFileDigitalSignatureOnly},
+				{keyfile, ClientKeyFile},
+				{ssl_imp, new}]},
      {server_opts, [{ssl_imp, new},{reuseaddr, true}, 
 		    {certfile, ServerCertFile}, {keyfile, ServerKeyFile}]},
      {server_verification_opts, [{ssl_imp, new},{reuseaddr, true}, 
@@ -571,6 +577,14 @@ rsa_suites() ->
 		 end,
 		 ssl:cipher_suites()).
 
+rsa_non_signed_suites() ->
+    lists:filter(fun({rsa, _, _}) ->
+			 true;
+		    (_) ->
+			 false
+		 end,
+		 ssl:cipher_suites()).
+
 dsa_suites() ->
      lists:filter(fun({dhe_dss, _, _}) ->
 			 true;