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).
pguyot · Aug 4, 2010 · 0975941 · 0975941
1 parent 0d553b4
commit 0975941
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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.
@@ -2848,6 +2849,39 @@ two_digits_str(N) when N < 10 ->
 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
 %%--------------------------------------------------------------------

diff --git 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;