Remove requested types from the server bound cert service: it only

supports a single type.

BUG=259097

Review URL: https://chromiumcodereview.appspot.com/20456002

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

chrome/browser/browsing_data/browsing_data_remover_unittest.cc

@@ -284,8 +284,10 @@ class RemoveServerBoundCertTester : public net::SSLConfigService::Observer {
                                    base::Time creation_time,
                                    base::Time expiration_time) {
     GetCertStore()->SetServerBoundCert(server_identifier,
-                                       net::CLIENT_CERT_RSA_SIGN, creation_time,
-                                       expiration_time, "a", "b");
+                                       creation_time,
+                                       expiration_time,
+                                       "a",
+                                       "b");
   }
 
   // Add a server bound cert for |server|, with the current time as the

chrome/browser/browsing_data/browsing_data_server_bound_cert_helper_unittest.cc

@@ -39,11 +39,9 @@ class BrowsingDataServerBoundCertHelperTest
     net::ServerBoundCertStore* cert_store =
         context->server_bound_cert_service()->GetCertStore();
     cert_store->SetServerBoundCert("https://www.google.com:443",
-                                   net::CLIENT_CERT_RSA_SIGN,
                                    base::Time(), base::Time(),
                                    "key", "cert");
     cert_store->SetServerBoundCert("https://www.youtube.com:443",
-                                   net::CLIENT_CERT_RSA_SIGN,
                                    base::Time(), base::Time(),
                                    "key", "cert");
   }
@@ -131,34 +129,6 @@ TEST_F(BrowsingDataServerBoundCertHelperTest, DeleteCert) {
   ASSERT_EQ(0UL, server_bound_cert_list_.size());
 }
 
-TEST_F(BrowsingDataServerBoundCertHelperTest, CannedUnique) {
-  std::string origin = "https://www.google.com:443";
-
-  scoped_refptr<CannedBrowsingDataServerBoundCertHelper> helper(
-      new CannedBrowsingDataServerBoundCertHelper());
-
-  ASSERT_TRUE(helper->empty());
-  helper->AddServerBoundCert(net::ServerBoundCertStore::ServerBoundCert(
-      origin, net::CLIENT_CERT_RSA_SIGN, base::Time(), base::Time(), "key",
-      "cert"));
-  helper->AddServerBoundCert(net::ServerBoundCertStore::ServerBoundCert(
-      origin, net::CLIENT_CERT_ECDSA_SIGN, base::Time(), base::Time(), "key",
-      "cert"));
-
-  helper->StartFetching(
-      base::Bind(&BrowsingDataServerBoundCertHelperTest::FetchCallback,
-                 base::Unretained(this)));
-  base::RunLoop().RunUntilIdle();
-
-  ASSERT_EQ(1UL, server_bound_cert_list_.size());
-  net::ServerBoundCertStore::ServerBoundCert& cert =
-      server_bound_cert_list_.front();
-
-  EXPECT_EQ("https://www.google.com:443", cert.server_identifier());
-  EXPECT_EQ(net::CLIENT_CERT_ECDSA_SIGN, cert.type());
-  EXPECT_EQ(0, ssl_config_changed_count_);
-}
-
 TEST_F(BrowsingDataServerBoundCertHelperTest, CannedEmpty) {
   std::string origin = "https://www.google.com";
 
@@ -167,8 +137,7 @@ TEST_F(BrowsingDataServerBoundCertHelperTest, CannedEmpty) {
 
   ASSERT_TRUE(helper->empty());
   helper->AddServerBoundCert(net::ServerBoundCertStore::ServerBoundCert(
-      origin, net::CLIENT_CERT_RSA_SIGN, base::Time(), base::Time(), "key",
-      "cert"));
+      origin, base::Time(), base::Time(), "key", "cert"));
   ASSERT_FALSE(helper->empty());
   helper->Reset();
   ASSERT_TRUE(helper->empty());

chrome/browser/browsing_data/mock_browsing_data_server_bound_cert_helper.cc

@@ -28,8 +28,7 @@ void MockBrowsingDataServerBoundCertHelper::AddServerBoundCertSample(
   DCHECK(server_bound_certs_.find(server_id) == server_bound_certs_.end());
   server_bound_cert_list_.push_back(
       net::ServerBoundCertStore::ServerBoundCert(
-          server_id, net::CLIENT_CERT_ECDSA_SIGN,
-          base::Time(), base::Time(), "key", "cert"));
+          server_id, base::Time(), base::Time(), "key", "cert"));
   server_bound_certs_[server_id] = true;
 }
 

chrome/browser/net/sqlite_server_bound_cert_store.cc

@@ -247,13 +247,16 @@ void SQLiteServerBoundCertStore::Backend::LoadOnDBThread(
   }
 
   while (smt.Step()) {
+    net::SSLClientCertType type =
+        static_cast<net::SSLClientCertType>(smt.ColumnInt(3));
+    if (type != net::CLIENT_CERT_ECDSA_SIGN)
+      continue;
     std::string private_key_from_db, cert_from_db;
     smt.ColumnBlobAsString(1, &private_key_from_db);
     smt.ColumnBlobAsString(2, &cert_from_db);
     scoped_ptr<net::DefaultServerBoundCertStore::ServerBoundCert> cert(
         new net::DefaultServerBoundCertStore::ServerBoundCert(
             smt.ColumnString(0),  // origin
-            static_cast<net::SSLClientCertType>(smt.ColumnInt(3)),
             base::Time::FromInternalValue(smt.ColumnInt64(5)),
             base::Time::FromInternalValue(smt.ColumnInt64(4)),
             private_key_from_db,
@@ -296,8 +299,9 @@ bool SQLiteServerBoundCertStore::Backend::EnsureDatabaseVersion() {
                    << "version 2.";
       return false;
     }
-    // All certs in version 1 database are rsa_sign, which has a value of 1.
-    if (!db_->Execute("UPDATE origin_bound_certs SET cert_type = 1")) {
+    // All certs in version 1 database are rsa_sign, which are unsupported.
+    // Just discard them all.
+    if (!db_->Execute("DELETE from origin_bound_certs")) {
       LOG(WARNING) << "Unable to update server bound cert database to "
                    << "version 2.";
       return false;
@@ -518,7 +522,7 @@ void SQLiteServerBoundCertStore::Backend::Commit() {
         add_smt.BindBlob(1, private_key.data(), private_key.size());
         const std::string& cert = po->cert().cert();
         add_smt.BindBlob(2, cert.data(), cert.size());
-        add_smt.BindInt(3, po->cert().type());
+        add_smt.BindInt(3, net::CLIENT_CERT_ECDSA_SIGN);
         add_smt.BindInt64(4, po->cert().expiration_time().ToInternalValue());
         add_smt.BindInt64(5, po->cert().creation_time().ToInternalValue());
         if (!add_smt.Run())

chrome/browser/net/sqlite_server_bound_cert_store_unittest.cc

@@ -15,6 +15,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/base/test_data_directory.h"
+#include "net/ssl/ssl_client_cert_type.h"
 #include "net/test/cert_test_util.h"
 #include "sql/statement.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -89,7 +90,6 @@ class SQLiteServerBoundCertStoreTest : public testing::Test {
     store_->AddServerBoundCert(
         net::DefaultServerBoundCertStore::ServerBoundCert(
             "google.com",
-            net::CLIENT_CERT_RSA_SIGN,
             base::Time::FromInternalValue(1),
             base::Time::FromInternalValue(2),
             "a", "b"));
@@ -106,7 +106,6 @@ TEST_F(SQLiteServerBoundCertStoreTest, TestPersistence) {
   store_->AddServerBoundCert(
       net::DefaultServerBoundCertStore::ServerBoundCert(
           "foo.com",
-          net::CLIENT_CERT_ECDSA_SIGN,
           base::Time::FromInternalValue(3),
           base::Time::FromInternalValue(4),
           "c", "d"));
@@ -124,27 +123,25 @@ TEST_F(SQLiteServerBoundCertStoreTest, TestPersistence) {
   // Reload and test for persistence
   Load(&certs);
   ASSERT_EQ(2U, certs.size());
-  net::DefaultServerBoundCertStore::ServerBoundCert* ec_cert;
-  net::DefaultServerBoundCertStore::ServerBoundCert* rsa_cert;
-  if (net::CLIENT_CERT_RSA_SIGN == certs[0]->type()) {
-    rsa_cert = certs[0];
-    ec_cert = certs[1];
+  net::DefaultServerBoundCertStore::ServerBoundCert* goog_cert;
+  net::DefaultServerBoundCertStore::ServerBoundCert* foo_cert;
+  if (certs[0]->server_identifier() == "google.com") {
+    goog_cert = certs[0];
+    foo_cert = certs[1];
   } else {
-    rsa_cert = certs[1];
-    ec_cert = certs[0];
+    goog_cert = certs[1];
+    foo_cert = certs[0];
   }
-  ASSERT_STREQ("google.com", rsa_cert->server_identifier().c_str());
-  ASSERT_EQ(net::CLIENT_CERT_RSA_SIGN, rsa_cert->type());
-  ASSERT_STREQ("a", rsa_cert->private_key().c_str());
-  ASSERT_STREQ("b", rsa_cert->cert().c_str());
-  ASSERT_EQ(1, rsa_cert->creation_time().ToInternalValue());
-  ASSERT_EQ(2, rsa_cert->expiration_time().ToInternalValue());
-  ASSERT_STREQ("foo.com", ec_cert->server_identifier().c_str());
-  ASSERT_EQ(net::CLIENT_CERT_ECDSA_SIGN, ec_cert->type());
-  ASSERT_STREQ("c", ec_cert->private_key().c_str());
-  ASSERT_STREQ("d", ec_cert->cert().c_str());
-  ASSERT_EQ(3, ec_cert->creation_time().ToInternalValue());
-  ASSERT_EQ(4, ec_cert->expiration_time().ToInternalValue());
+  ASSERT_EQ("google.com", goog_cert->server_identifier());
+  ASSERT_STREQ("a", goog_cert->private_key().c_str());
+  ASSERT_STREQ("b", goog_cert->cert().c_str());
+  ASSERT_EQ(1, goog_cert->creation_time().ToInternalValue());
+  ASSERT_EQ(2, goog_cert->expiration_time().ToInternalValue());
+  ASSERT_EQ("foo.com", foo_cert->server_identifier());
+  ASSERT_STREQ("c", foo_cert->private_key().c_str());
+  ASSERT_STREQ("d", foo_cert->cert().c_str());
+  ASSERT_EQ(3, foo_cert->creation_time().ToInternalValue());
+  ASSERT_EQ(4, foo_cert->expiration_time().ToInternalValue());
 
   // Now delete the cert and check persistence again.
   store_->DeleteServerBoundCert(*certs[0]);
@@ -207,23 +204,10 @@ TEST_F(SQLiteServerBoundCertStoreTest, TestUpgradeV1) {
     ScopedVector<net::DefaultServerBoundCertStore::ServerBoundCert> certs;
     store_ = new SQLiteServerBoundCertStore(v1_db_path, NULL);
 
-    // Load the database and ensure the certs can be read and are marked as RSA.
+    // Load the database. Because the existing v1 certs are implicitly of type
+    // RSA, which is unsupported, they're discarded.
     Load(&certs);
-    ASSERT_EQ(2U, certs.size());
-
-    ASSERT_STREQ("google.com", certs[0]->server_identifier().c_str());
-    ASSERT_EQ(net::CLIENT_CERT_RSA_SIGN, certs[0]->type());
-    ASSERT_EQ(GetTestCertExpirationTime(),
-              certs[0]->expiration_time());
-    ASSERT_EQ(key_data, certs[0]->private_key());
-    ASSERT_EQ(cert_data, certs[0]->cert());
-
-    ASSERT_STREQ("foo.com", certs[1]->server_identifier().c_str());
-    ASSERT_EQ(net::CLIENT_CERT_RSA_SIGN, certs[1]->type());
-    // Undecodable cert, expiration time will be uninitialized.
-    ASSERT_EQ(base::Time(), certs[1]->expiration_time());
-    ASSERT_STREQ("\xaa", certs[1]->private_key().c_str());
-    ASSERT_STREQ("\xbb", certs[1]->cert().c_str());
+    ASSERT_EQ(0U, certs.size());
 
     store_ = NULL;
     base::RunLoop().RunUntilIdle();
@@ -273,7 +257,7 @@ TEST_F(SQLiteServerBoundCertStoreTest, TestUpgradeV2) {
     add_smt.BindString(0, "google.com");
     add_smt.BindBlob(1, key_data.data(), key_data.size());
     add_smt.BindBlob(2, cert_data.data(), cert_data.size());
-    add_smt.BindInt64(3, 1);
+    add_smt.BindInt64(3, 64);
     ASSERT_TRUE(add_smt.Run());
 
     ASSERT_TRUE(db.Execute(
@@ -291,19 +275,17 @@ TEST_F(SQLiteServerBoundCertStoreTest, TestUpgradeV2) {
     ScopedVector<net::DefaultServerBoundCertStore::ServerBoundCert> certs;
     store_ = new SQLiteServerBoundCertStore(v2_db_path, NULL);
 
-    // Load the database and ensure the certs can be read and are marked as RSA.
+    // Load the database and ensure the certs can be read.
     Load(&certs);
     ASSERT_EQ(2U, certs.size());
 
-    ASSERT_STREQ("google.com", certs[0]->server_identifier().c_str());
-    ASSERT_EQ(net::CLIENT_CERT_RSA_SIGN, certs[0]->type());
+    ASSERT_EQ("google.com", certs[0]->server_identifier());
     ASSERT_EQ(GetTestCertExpirationTime(),
               certs[0]->expiration_time());
     ASSERT_EQ(key_data, certs[0]->private_key());
     ASSERT_EQ(cert_data, certs[0]->cert());
 
-    ASSERT_STREQ("foo.com", certs[1]->server_identifier().c_str());
-    ASSERT_EQ(net::CLIENT_CERT_ECDSA_SIGN, certs[1]->type());
+    ASSERT_EQ("foo.com", certs[1]->server_identifier());
     // Undecodable cert, expiration time will be uninitialized.
     ASSERT_EQ(base::Time(), certs[1]->expiration_time());
     ASSERT_STREQ("\xaa", certs[1]->private_key().c_str());
@@ -359,7 +341,7 @@ TEST_F(SQLiteServerBoundCertStoreTest, TestUpgradeV3) {
     add_smt.BindString(0, "google.com");
     add_smt.BindBlob(1, key_data.data(), key_data.size());
     add_smt.BindBlob(2, cert_data.data(), cert_data.size());
-    add_smt.BindInt64(3, 1);
+    add_smt.BindInt64(3, 64);
     add_smt.BindInt64(4, 1000);
     ASSERT_TRUE(add_smt.Run());
 
@@ -378,20 +360,18 @@ TEST_F(SQLiteServerBoundCertStoreTest, TestUpgradeV3) {
     ScopedVector<net::DefaultServerBoundCertStore::ServerBoundCert> certs;
     store_ = new SQLiteServerBoundCertStore(v3_db_path, NULL);
 
-    // Load the database and ensure the certs can be read and are marked as RSA.
+    // Load the database and ensure the certs can be read.
     Load(&certs);
     ASSERT_EQ(2U, certs.size());
 
-    ASSERT_STREQ("google.com", certs[0]->server_identifier().c_str());
-    ASSERT_EQ(net::CLIENT_CERT_RSA_SIGN, certs[0]->type());
+    ASSERT_EQ("google.com", certs[0]->server_identifier());
     ASSERT_EQ(1000, certs[0]->expiration_time().ToInternalValue());
     ASSERT_EQ(GetTestCertCreationTime(),
               certs[0]->creation_time());
     ASSERT_EQ(key_data, certs[0]->private_key());
     ASSERT_EQ(cert_data, certs[0]->cert());
 
-    ASSERT_STREQ("foo.com", certs[1]->server_identifier().c_str());
-    ASSERT_EQ(net::CLIENT_CERT_ECDSA_SIGN, certs[1]->type());
+    ASSERT_EQ("foo.com", certs[1]->server_identifier());
     ASSERT_EQ(2000, certs[1]->expiration_time().ToInternalValue());
     // Undecodable cert, creation time will be uninitialized.
     ASSERT_EQ(base::Time(), certs[1]->creation_time());
@@ -414,3 +394,76 @@ TEST_F(SQLiteServerBoundCertStoreTest, TestUpgradeV3) {
     }
   }
 }
+
+TEST_F(SQLiteServerBoundCertStoreTest, TestRSADiscarded) {
+  // Reset the store.  We'll be using a different database for this test.
+  store_ = NULL;
+
+  base::FilePath v4_db_path(temp_dir_.path().AppendASCII("v4dbrsa"));
+
+  std::string key_data;
+  std::string cert_data;
+  ReadTestKeyAndCert(&key_data, &cert_data);
+
+  // Create a version 4 database with a mix of RSA and ECDSA certs.
+  {
+    sql::Connection db;
+    ASSERT_TRUE(db.Open(v4_db_path));
+    ASSERT_TRUE(db.Execute(
+        "CREATE TABLE meta(key LONGVARCHAR NOT NULL UNIQUE PRIMARY KEY,"
+            "value LONGVARCHAR);"
+        "INSERT INTO \"meta\" VALUES('version','4');"
+        "INSERT INTO \"meta\" VALUES('last_compatible_version','1');"
+        "CREATE TABLE origin_bound_certs ("
+            "origin TEXT NOT NULL UNIQUE PRIMARY KEY,"
+            "private_key BLOB NOT NULL,"
+            "cert BLOB NOT NULL,"
+            "cert_type INTEGER,"
+            "expiration_time INTEGER,"
+            "creation_time INTEGER);"
+        ));
+
+    sql::Statement add_smt(db.GetUniqueStatement(
+        "INSERT INTO origin_bound_certs "
+        "(origin, private_key, cert, cert_type, expiration_time, creation_time)"
+        " VALUES (?,?,?,?,?,?)"));
+    add_smt.BindString(0, "google.com");
+    add_smt.BindBlob(1, key_data.data(), key_data.size());
+    add_smt.BindBlob(2, cert_data.data(), cert_data.size());
+    add_smt.BindInt64(3, 64);
+    add_smt.BindInt64(4, GetTestCertExpirationTime().ToInternalValue());
+    add_smt.BindInt64(5, base::Time::Now().ToInternalValue());
+    ASSERT_TRUE(add_smt.Run());
+
+    add_smt.Clear();
+    add_smt.Assign(db.GetUniqueStatement(
+        "INSERT INTO origin_bound_certs "
+        "(origin, private_key, cert, cert_type, expiration_time, creation_time)"
+        " VALUES (?,?,?,?,?,?)"));
+    add_smt.BindString(0, "foo.com");
+    add_smt.BindBlob(1, key_data.data(), key_data.size());
+    add_smt.BindBlob(2, cert_data.data(), cert_data.size());
+    add_smt.BindInt64(3, 1);
+    add_smt.BindInt64(4, GetTestCertExpirationTime().ToInternalValue());
+    add_smt.BindInt64(5, base::Time::Now().ToInternalValue());
+    ASSERT_TRUE(add_smt.Run());
+  }
+
+  ScopedVector<net::DefaultServerBoundCertStore::ServerBoundCert> certs;
+  store_ = new SQLiteServerBoundCertStore(v4_db_path, NULL);
+
+  // Load the database and ensure the certs can be read.
+  Load(&certs);
+  // Only the ECDSA cert (for google.com) is read, the RSA one is discarded.
+  ASSERT_EQ(1U, certs.size());
+
+  ASSERT_EQ("google.com", certs[0]->server_identifier());
+  ASSERT_EQ(GetTestCertExpirationTime(),
+            certs[0]->expiration_time());
+  ASSERT_EQ(key_data, certs[0]->private_key());
+  ASSERT_EQ(cert_data, certs[0]->cert());
+
+  store_ = NULL;
+  // Make sure we wait until the destructor has run.
+  base::RunLoop().RunUntilIdle();
+}

chrome/browser/ui/webui/cookies_tree_model_util.cc

@@ -17,6 +17,7 @@
 #include "content/public/browser/indexed_db_context.h"
 #include "grit/generated_resources.h"
 #include "net/cookies/canonical_cookie.h"
+#include "net/ssl/ssl_client_cert_type.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/text/bytes_formatting.h"
 #include "webkit/common/fileapi/file_system_types.h"
@@ -249,7 +250,7 @@ bool CookiesTreeModelUtil::GetCookieTreeNodeDictionary(
 
       dict->SetString(kKeyServerId, server_bound_cert.server_identifier());
       dict->SetString(kKeyCertType,
-                      ClientCertTypeToString(server_bound_cert.type()));
+                      ClientCertTypeToString(net::CLIENT_CERT_ECDSA_SIGN));
       dict->SetString(kKeyCreated, UTF16ToUTF8(
           base::TimeFormatFriendlyDateAndTime(
               server_bound_cert.creation_time())));