ssl: Correct key_usage check

The Key Usage extension is described in section 4.2.1.3 of X.509, with the following possible flags:

  KeyUsage ::= BIT STRING {
       digitalSignature        (0),
       nonRepudiation          (1), -- recent editions of X.509 have
                            -- renamed this bit to contentCommitment
       keyEncipherment         (2),
       dataEncipherment        (3),
       keyAgreement            (4),
       keyCertSign             (5),
       cRLSign                 (6),
       encipherOnly            (7),
       decipherOnly            (8) }
In SSL/TLS, when the server certificate contains a RSA key, then:

either a DHE or ECDHE cipher suite is used, in which case the RSA key
is used for a signature (see section 7.4.3 of RFC 5246: the "Server
Key Exchange" message); this exercises the digitalSignature key usage;

or "plain RSA" is used, with a random value (the 48-byte pre-master
secret) being encrypted by the client with the server's public key
(see section 7.4.7.1 of RFC 5246); this is right in the definition of
the keyEncipherment key usage flag.

dataEncipherment does not apply, because what is encrypted is not
directly meaningful data, but a value which is mostly generated
randomly and used to derive symmetric keys. keyAgreement does not
apply either, because that one is for key agreement algorithms which
are not a case of asymmetric encryption (e.g. Diffie-Hellman). The
keyAgreement usage flag would appear in a certificate which contains a
DH key, not a RSA key. nonRepudiation is not used, because whatever is
signed as part of a SSL/TLS key exchange cannot be used as proof for a
third party (there is nothing in a SSL/TLS tunnel that the client
could record and then use to convince a judge when tring to sue the
server itself; the data which is exchanged within the tunnel is not
signed by the server).

When a ECDSA key is used then "keyAgreement" flag is needed for beeing
ECDH "capable" (as opposed to ephemeral ECDHE)

3 files changed, 86 insertions, 47 deletions

diff --git a/lib/ssl/src/ssl_cipher.erl b/lib/ssl/src/ssl_cipher.erl
index d482b27a8f..76273390d0 100644
--- a/lib/ssl/src/ssl_cipher.erl
+++ b/lib/ssl/src/ssl_cipher.erl
@@ -2035,7 +2035,7 @@ filter(DerCert, Ciphers0, Version) ->
                 Ciphers0, Version, OtpCert),
     {_, Sign} = public_key:pkix_sign_types(SigAlg#'SignatureAlgorithm'.algorithm),
     filter_suites_signature(Sign, Ciphers, Version).
- 
+
 %%--------------------------------------------------------------------
 -spec filter_suites([erl_cipher_suite()] | [cipher_suite()], map()) ->
                            [erl_cipher_suite()] |  [cipher_suite()].
@@ -2466,29 +2466,33 @@ next_iv(Bin, IV) ->
     <<_:FirstPart/binary, NextIV:IVSz/binary>> = Bin,
     NextIV.
 
-
-filter_suites_pubkey(rsa, CiphersSuites0, Version, OtpCert) ->
+filter_suites_pubkey(rsa, CiphersSuites0, _Version, OtpCert) ->
     KeyUses = key_uses(OtpCert),
+    NotECDSAKeyed = (CiphersSuites0 -- ec_keyed_suites(CiphersSuites0)) 
+        -- dss_keyed_suites(CiphersSuites0),
     CiphersSuites = filter_keyuse_suites(keyEncipherment, KeyUses,
-                                         (CiphersSuites0 -- ec_keyed_suites(CiphersSuites0)) 
-                                         -- dss_keyed_suites(CiphersSuites0),
+                                         NotECDSAKeyed,
                                          rsa_suites_encipher(CiphersSuites0)),
     filter_keyuse_suites(digitalSignature, KeyUses, CiphersSuites,
-                         rsa_signed_suites(CiphersSuites, Version));
-filter_suites_pubkey(dsa, Ciphers, _, _OtpCert) ->  
-    (Ciphers -- rsa_keyed_suites(Ciphers)) -- ec_keyed_suites(Ciphers);
+                         rsa_ecdhe_dhe_suites(CiphersSuites));
+filter_suites_pubkey(dsa, Ciphers, _, OtpCert) ->  
+    KeyUses = key_uses(OtpCert),
+    NotECRSAKeyed =  (Ciphers -- rsa_keyed_suites(Ciphers)) -- ec_keyed_suites(Ciphers),
+    filter_keyuse_suites(digitalSignature, KeyUses, NotECRSAKeyed,
+                         dss_dhe_suites(Ciphers));
 filter_suites_pubkey(ec, Ciphers, _, OtpCert) ->
-    Uses = key_uses(OtpCert),  
-    filter_keyuse_suites(digitalSignature, Uses,
-                         (Ciphers -- rsa_keyed_suites(Ciphers)) -- dss_keyed_suites(Ciphers),
-                         ecdsa_sign_suites(Ciphers)).
+    Uses = key_uses(OtpCert),
+    NotRSADSAKeyed = (Ciphers -- rsa_keyed_suites(Ciphers)) -- dss_keyed_suites(Ciphers),
+    CiphersSuites = filter_keyuse_suites(digitalSignature, Uses, NotRSADSAKeyed,
+                                   ec_ecdhe_suites(Ciphers)),
+    filter_keyuse_suites(keyAgreement, Uses, CiphersSuites, ec_ecdh_suites(Ciphers)).
 
 filter_suites_signature(rsa, Ciphers, Version) ->
-    Ciphers -- ecdsa_signed_suites(Ciphers, Version) -- dsa_signed_suites(Ciphers, Version);
+    (Ciphers -- ecdsa_signed_suites(Ciphers, Version)) -- dsa_signed_suites(Ciphers, Version);
 filter_suites_signature(dsa, Ciphers, Version) ->
-    Ciphers -- ecdsa_signed_suites(Ciphers, Version) -- rsa_signed_suites(Ciphers, Version);
+    (Ciphers -- ecdsa_signed_suites(Ciphers, Version)) -- rsa_signed_suites(Ciphers, Version);
 filter_suites_signature(ecdsa, Ciphers, Version) ->
-    Ciphers -- rsa_signed_suites(Ciphers, Version) -- dsa_signed_suites(Ciphers, Version).
+    (Ciphers -- rsa_signed_suites(Ciphers, Version)) -- dsa_signed_suites(Ciphers, Version).
 
 
 %% From RFC 5246 - Section  7.4.2.  Server Certificate
@@ -2555,8 +2559,6 @@ rsa_keyed(rsa_psk) ->
     true;
 rsa_keyed(srp_rsa) -> 
     true;
-rsa_keyed(ecdhe_rsa) -> 
-    true;
 rsa_keyed(_) -> 
     false.
 
@@ -2597,24 +2599,22 @@ dsa_signed_suites(Ciphers, Version) ->
                              cipher_filters => [],
                              mac_filters => [],
                              prf_filters => []}).
-
-dsa_signed({3,N}) when N >= 3 ->
-    fun(dhe_dss) -> true;
-       (ecdhe_dss) -> true;
-       (_) -> false
-    end;
 dsa_signed(_) ->
     fun(dhe_dss) -> true;
-       (ecdh_dss) -> true;
-       (ecdhe_dss) -> true;
        (_) -> false
     end.
 
+dss_dhe_suites(Ciphers) ->
+    filter_suites(Ciphers, #{key_exchange_filters => [fun(dhe_dss) -> true;
+                                                         (_) -> false
+                                                      end],
+                             cipher_filters => [],
+                             mac_filters => [],
+                             prf_filters => []}).
+
 ec_keyed(ecdh_ecdsa) ->
     true;
-ec_keyed(ecdhe_ecdsa) -> 
-    true;
-ec_keyed(ecdh_rsa) -> 
+ec_keyed(ecdh_rsa) ->
     true;
 ec_keyed(_) -> 
     false.
@@ -2626,9 +2626,28 @@ ec_keyed_suites(Ciphers) ->
                              mac_filters => [],
                              prf_filters => []}).
 
-%% EC Certs key can be used for signing
-ecdsa_sign_suites(Ciphers)->
+%% EC Certs key usage keyAgreement
+ec_ecdh_suites(Ciphers)->
+    filter_suites(Ciphers, #{key_exchange_filters => [fun(ecdh_ecdsa) -> true;
+                                                         (_) -> false
+                                                      end],
+                             cipher_filters => [],
+                             mac_filters => [],
+                             prf_filters => []}).
+
+%% EC Certs key usage digitalSignature
+ec_ecdhe_suites(Ciphers) ->
     filter_suites(Ciphers, #{key_exchange_filters => [fun(ecdhe_ecdsa) -> true;
+                                                         (ecdhe_rsa) -> true;
+                                                         (_) -> false
+                                                      end],
+                             cipher_filters => [],
+                             mac_filters => [],
+                             prf_filters => []}).
+%% RSA Certs key usage digitalSignature
+rsa_ecdhe_dhe_suites(Ciphers) ->
+    filter_suites(Ciphers, #{key_exchange_filters => [fun(dhe_rsa) -> true;
+                                                         (ecdhe_rsa) -> true;
                                                          (_) -> false
                                                       end],
                              cipher_filters => [],
@@ -2641,11 +2660,14 @@ key_uses(OtpCert) ->
     Extensions = ssl_certificate:extensions_list(TBSExtensions),
     case ssl_certificate:select_extension(?'id-ce-keyUsage', Extensions) of
 	undefined ->
-	    undefined;
+	    [];
 	#'Extension'{extnValue = KeyUses} ->
             KeyUses
     end.
 
+%% If no key-usage extension is defined all key-usages are allowed
+filter_keyuse_suites(_, [], CiphersSuites, _) ->
+    CiphersSuites;
 filter_keyuse_suites(Use, KeyUse, CipherSuits, Suites) ->
     case ssl_certificate:is_valid_key_usage(KeyUse, Use) of
 	true ->
diff --git a/lib/ssl/src/ssl_handshake.erl b/lib/ssl/src/ssl_handshake.erl
index 35cd50a9a7..43ae312483 100644
--- a/lib/ssl/src/ssl_handshake.erl
+++ b/lib/ssl/src/ssl_handshake.erl
@@ -2150,13 +2150,12 @@ sign_algo(Alg) ->
 is_acceptable_hash_sign(Algos, _, _, KeyExAlgo, SupportedHashSigns) when 
       KeyExAlgo == dh_dss;
       KeyExAlgo == dh_rsa;
-      KeyExAlgo == ecdh_ecdsa;
       KeyExAlgo == ecdh_rsa;
       KeyExAlgo == ecdh_ecdsa
       ->
     %% *dh_* could be called only *dh in TLS-1.2
     is_acceptable_hash_sign(Algos, SupportedHashSigns); 
-is_acceptable_hash_sign(Algos, rsa, ecdsa, ecdh_rsa, SupportedHashSigns) ->
+is_acceptable_hash_sign(Algos, rsa, ecdsa, ecdhe_rsa, SupportedHashSigns) ->
     is_acceptable_hash_sign(Algos, SupportedHashSigns); 
 is_acceptable_hash_sign({_, rsa} = Algos, rsa, _, dhe_rsa, SupportedHashSigns) ->
     is_acceptable_hash_sign(Algos, SupportedHashSigns); 
@@ -2186,7 +2185,7 @@ is_acceptable_hash_sign(_, _, _, KeyExAlgo, _) when
       KeyExAlgo == ecdhe_anon     
       ->
     true; 
-is_acceptable_hash_sign(_,_, _,_,_) ->
+is_acceptable_hash_sign(_,_,_,_,_) ->
     false.					
 is_acceptable_hash_sign(Algos, SupportedHashSigns) ->
     lists:member(Algos, SupportedHashSigns).
diff --git a/lib/ssl/test/ssl_ECC.erl b/lib/ssl/test/ssl_ECC.erl
index df61ce4964..6422cd75e9 100644
--- a/lib/ssl/test/ssl_ECC.erl
+++ b/lib/ssl/test/ssl_ECC.erl
@@ -33,53 +33,65 @@
 
 %% ECDH_RSA 
 client_ecdh_rsa_server_ecdh_rsa(Config) when is_list(Config) ->
+    Ext = x509_test:extensions([{key_usage, [keyAgreement]}]),
     Suites = all_rsa_suites(Config),
     Default = ssl_test_lib:default_cert_chain_conf(),
-    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain, Default}, 
+    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain,
+                                                        [[], [], [{extensions, Ext}]]},
                                                        {client_chain, Default}],
                                                       ecdh_rsa, ecdh_rsa, Config),
     ssl_test_lib:basic_test(ssl_test_lib:ssl_options(COpts, Config), 
                ssl_test_lib:ssl_options(SOpts, Config),
                [{check_keyex, ecdh_rsa}, {ciphers, Suites} | proplists:delete(check_keyex, Config)]).
 client_ecdhe_rsa_server_ecdh_rsa(Config)  when is_list(Config) ->
+    Ext = x509_test:extensions([{key_usage, [keyAgreement]}]),
     Suites = all_rsa_suites(Config),
     Default = ssl_test_lib:default_cert_chain_conf(),
-    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain, Default}, 
+    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain,
+                                                        [[], [], [{extensions, Ext}]]},
                                                        {client_chain, Default}], 
                                                       ecdhe_rsa, ecdh_rsa, Config),
     ssl_test_lib:basic_test(ssl_test_lib:ssl_options(COpts, Config), 
                ssl_test_lib:ssl_options(SOpts, Config),  
                [{check_keyex, ecdh_rsa}, {ciphers, Suites} | proplists:delete(check_keyex, Config)]).
 client_ecdhe_ecdsa_server_ecdh_rsa(Config)  when is_list(Config) ->
+    Ext = x509_test:extensions([{key_usage, [keyAgreement]}]),
     Suites = all_rsa_suites(Config),
     Default = ssl_test_lib:default_cert_chain_conf(),
-    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain, Default}, 
+    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain,
+                                                        [[], [], [{extensions, Ext}]]},
                                                        {client_chain, Default}],
                                                       ecdhe_ecdsa, ecdh_rsa, Config),
     ssl_test_lib:basic_test(ssl_test_lib:ssl_options(COpts, Config), 
-               ssl_test_lib:ssl_options(SOpts, Config),
-               [{check_keyex, ecdh_rsa}, {ciphers, Suites} | proplists:delete(check_keyex, Config)]).
+                            ssl_test_lib:ssl_options(SOpts, Config),
+                            [{check_keyex, ecdh_rsa}, {ciphers, Suites} | proplists:delete(check_keyex, Config)]).
 
 %% ECDHE_RSA    
 client_ecdh_rsa_server_ecdhe_rsa(Config)  when is_list(Config) ->
+    Ext = x509_test:extensions([{key_usage, [digitalSignature]}]),
     Default = ssl_test_lib:default_cert_chain_conf(),
-    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain, Default}, 
+    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain,
+                                                        [[], [], [{extensions, Ext}]]},
                                                        {client_chain, Default}], 
                                                       ecdh_rsa, ecdhe_rsa, Config),
     ssl_test_lib:basic_test(ssl_test_lib:ssl_options(COpts, Config), 
                ssl_test_lib:ssl_options(SOpts, Config), 
                [{check_keyex, ecdhe_rsa} | proplists:delete(check_keyex, Config)]).
 client_ecdhe_rsa_server_ecdhe_rsa(Config)  when is_list(Config) ->
+    Ext = x509_test:extensions([{key_usage, [digitalSignature]}]),
     Default = ssl_test_lib:default_cert_chain_conf(),
-    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain, Default}, 
+    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain,
+                                                        [[], [], [{extensions, Ext}]]},
                                                        {client_chain, Default}], 
                                                       ecdhe_rsa, ecdhe_rsa, Config),
     ssl_test_lib:basic_test(ssl_test_lib:ssl_options(COpts, Config), 
-               ssl_test_lib:ssl_options(SOpts, Config),  
+                            ssl_test_lib:ssl_options(SOpts, Config),
                [{check_keyex, ecdhe_rsa} | proplists:delete(check_keyex, Config)]).
 client_ecdhe_ecdsa_server_ecdhe_rsa(Config)  when is_list(Config) ->
+    Ext = x509_test:extensions([{key_usage, [digitalSignature]}]),
     Default = ssl_test_lib:default_cert_chain_conf(),
-    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain, Default}, 
+    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain,
+                                                        [[], [], [{extensions, Ext}]]},
                                                        {client_chain, Default}],
                                                       ecdh_ecdsa, ecdhe_rsa, Config),
     ssl_test_lib:basic_test(ssl_test_lib:ssl_options(COpts, Config), 
@@ -121,24 +133,30 @@ client_ecdhe_ecdsa_server_ecdh_ecdsa(Config)  when is_list(Config) ->
 
 %% ECDHE_ECDSA
 client_ecdh_rsa_server_ecdhe_ecdsa(Config)  when is_list(Config) ->
-     Default = ssl_test_lib:default_cert_chain_conf(),
-    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain, Default}, 
+    Ext = x509_test:extensions([{key_usage, [digitalSignature]}]),
+    Default = ssl_test_lib:default_cert_chain_conf(),
+    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain,
+                                                        [[], [], [{extensions, Ext}]]},
                                                        {client_chain, Default}], 
                                                       ecdh_rsa, ecdhe_ecdsa, Config), 
     ssl_test_lib:basic_test(ssl_test_lib:ssl_options(COpts, Config), 
                ssl_test_lib:ssl_options(SOpts, Config), 
                [{check_keyex, ecdhe_ecdsa} | proplists:delete(check_keyex, Config)]).
 client_ecdh_ecdsa_server_ecdhe_ecdsa(Config)  when is_list(Config) ->
+    Ext = x509_test:extensions([{key_usage, [digitalSignature]}]),
     Default = ssl_test_lib:default_cert_chain_conf(),
-    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain, Default}, 
+    {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain,
+                                                        [[], [], [{extensions, Ext}]]},
                                                        {client_chain, Default}], 
                                                       ecdh_ecdsa, ecdhe_ecdsa, Config), 
     ssl_test_lib:basic_test(ssl_test_lib:ssl_options(COpts, Config), 
                ssl_test_lib:ssl_options(SOpts, Config),
                [{check_keyex, ecdhe_ecdsa} | proplists:delete(check_keyex, Config)]).
 client_ecdhe_ecdsa_server_ecdhe_ecdsa(Config)  when is_list(Config) ->
+    Ext = x509_test:extensions([{key_usage, [digitalSignature]}]),
     Default = ssl_test_lib:default_cert_chain_conf(),
-     {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain, Default}, 
+     {COpts, SOpts} = ssl_test_lib:make_ec_cert_chains([{server_chain,
+                                                         [[], [], [{extensions, Ext}]]},
                                                         {client_chain, Default}], 
                                                        ecdhe_ecdsa, ecdhe_ecdsa, Config),
     ssl_test_lib:basic_test(ssl_test_lib:ssl_options(COpts, Config),