The SSL server's RSA private key must be imported with the

KU_KEY_ENCIPHERMENT key usage to support the RSA key exchange algorithm.

Remove the incorrect workarounds for this bug.  In the
SSLServerSocketTest.DataTransfer unit test, do not proceed to data
transfer if the SSL connection cannot be established.

Not required for fixing this bug: create an RSA private key with all
applicable key usage bits to be future-proof.

R=hclam
BUG=67928
TEST=net_unittests --gtest_filter=SSLServerSocketTest.*
Review URL: http://codereview.chromium.org/6297008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@71739 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 18 insertions, 14 deletions

diff --git a/base/crypto/rsa_private_key_nss.cc b/base/crypto/rsa_private_key_nss.cc
index 3084636..202aa1d 100644
--- a/base/crypto/rsa_private_key_nss.cc
+++ b/base/crypto/rsa_private_key_nss.cc
@@ -223,9 +223,13 @@ RSAPrivateKey* RSAPrivateKey::CreateFromPrivateKeyInfoWithParams(
   SECItem der_private_key_info;
   der_private_key_info.data = const_cast<unsigned char*>(&input.front());
   der_private_key_info.len = input.size();
-  SECStatus rv =  PK11_ImportDERPrivateKeyInfoAndReturnKey(slot,
-      &der_private_key_info, NULL, NULL, permanent, sensitive,
-      KU_DIGITAL_SIGNATURE, &result->key_, NULL);
+  // Allow the private key to be used for key unwrapping, data decryption,
+  // and signature generation.
+  const unsigned int key_usage = KU_KEY_ENCIPHERMENT | KU_DATA_ENCIPHERMENT |
+                                 KU_DIGITAL_SIGNATURE;
+  SECStatus rv =  PK11_ImportDERPrivateKeyInfoAndReturnKey(
+      slot, &der_private_key_info, NULL, NULL, permanent, sensitive,
+      key_usage, &result->key_, NULL);
   PK11_FreeSlot(slot);
   if (rv != SECSuccess) {
     NOTREACHED();
diff --git a/net/socket/ssl_server_socket_nss.cc b/net/socket/ssl_server_socket_nss.cc
index 2e47fb8..270aff0 100644
--- a/net/socket/ssl_server_socket_nss.cc
+++ b/net/socket/ssl_server_socket_nss.cc
@@ -349,9 +349,15 @@ int SSLServerSocketNSS::InitializeSSLOptions() {
   der_private_key_info.data =
       const_cast<unsigned char*>(&key_vector.front());
   der_private_key_info.len = key_vector.size();
+  // The server's RSA private key must be imported into NSS with the
+  // following key usage bits:
+  // - KU_KEY_ENCIPHERMENT, required for the RSA key exchange algorithm.
+  // - KU_DIGITAL_SIGNATURE, required for the DHE_RSA and ECDHE_RSA key
+  //   exchange algorithms.
+  const unsigned int key_usage = KU_KEY_ENCIPHERMENT | KU_DIGITAL_SIGNATURE;
   rv =  PK11_ImportDERPrivateKeyInfoAndReturnKey(
       slot, &der_private_key_info, NULL, NULL, PR_FALSE, PR_FALSE,
-      KU_DIGITAL_SIGNATURE, &private_key, NULL);
+      key_usage, &private_key, NULL);
   PK11_FreeSlot(slot);
   if (rv != SECSuccess) {
     CERT_DestroyCertificate(cert);
diff --git a/net/socket/ssl_server_socket_unittest.cc b/net/socket/ssl_server_socket_unittest.cc
index 781a3f4..ca2c884 100644
--- a/net/socket/ssl_server_socket_unittest.cc
+++ b/net/socket/ssl_server_socket_unittest.cc
@@ -283,9 +283,6 @@ TEST_F(SSLServerSocketTest, Initialize) {
 TEST_F(SSLServerSocketTest, Handshake) {
   Initialize();
 
-  if (!base::CheckNSSVersion("3.12.8"))
-    return;
-
   TestCompletionCallback connect_callback;
   TestCompletionCallback accept_callback;
 
@@ -306,24 +303,21 @@ TEST_F(SSLServerSocketTest, Handshake) {
 TEST_F(SSLServerSocketTest, DataTransfer) {
   Initialize();
 
-  if (!base::CheckNSSVersion("3.12.8"))
-    return;
-
   TestCompletionCallback connect_callback;
   TestCompletionCallback accept_callback;
 
   // Establish connection.
   int client_ret = client_socket_->Connect(&connect_callback);
-  EXPECT_TRUE(client_ret == net::OK || client_ret == net::ERR_IO_PENDING);
+  ASSERT_TRUE(client_ret == net::OK || client_ret == net::ERR_IO_PENDING);
 
   int server_ret = server_socket_->Accept(&accept_callback);
-  EXPECT_TRUE(server_ret == net::OK || server_ret == net::ERR_IO_PENDING);
+  ASSERT_TRUE(server_ret == net::OK || server_ret == net::ERR_IO_PENDING);
 
   if (client_ret == net::ERR_IO_PENDING) {
-    EXPECT_EQ(net::OK, connect_callback.WaitForResult());
+    ASSERT_EQ(net::OK, connect_callback.WaitForResult());
   }
   if (server_ret == net::ERR_IO_PENDING) {
-    EXPECT_EQ(net::OK, accept_callback.WaitForResult());
+    ASSERT_EQ(net::OK, accept_callback.WaitForResult());
   }
 
   const int kReadBufSize = 1024;