Remove certificates from Channel ID

net/extras/sqlite/sqlite_channel_id_store.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/extras/sqlite/sqlite_channel_id_store.h"
 
 #include <set>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/metrics/histogram.h"
 #include "base/sequenced_task_runner.h"
 #include "base/strings/string_util.h"
+#include "net/cert/asn1_util.h"
 #include "net/cert/x509_certificate.h"
 #include "net/cookies/cookie_util.h"
 #include "net/ssl/ssl_client_cert_type.h"
 #include "sql/error_delegate_util.h"
 #include "sql/meta_table.h"
 #include "sql/statement.h"
 #include "sql/transaction.h"
 #include "url/gurl.h"
 
 namespace {
 
 // Version number of the database.
-const int kCurrentVersionNumber = 4;
+const int kCurrentVersionNumber = 5;
 const int kCompatibleVersionNumber = 1;

[mattm, 2015/04/10 01:00:27]

Should be 5 also?

[nharper, 2015/04/25 02:59:18]

I need it to be both 1 and 5, depending on the context. In EnsureDatabaseVersion
(when migrating from a previous version to the current version), the compatible
version number is 1 - we can migrate from version 1. However, when updating the
current and compatible versions once the schema is updated, they are both 5.

[mattm, 2015/04/27 20:55:53]

Hm, it looks to me that in this CL kCompatibleVersionNumber is only currently
used in the meta_table_.Init statement, where does it need to be 1?

[nharper, 2015/04/29 22:07:15]

Now that I re-read the code and what I wrote, I realized my mistake: The second
check in EnsureDatabaseVersion (meta_table_.GetCompatibleVersionNumber() >
kCurrentVersionNumber) is checking that the table (on disk) isn't newer than the
code - I thought it was doing kCompatibleVersionNumber >
meta_table_.GetVersionNumber() to check that the table (on disk) isn't too old
(which is where I'd want to check against 1).

I've updated kCompatibleVersionNumber to be 5.

 
-// Initializes the certs table, returning true on success.
+// Creates the new channel_id table (if it doesn't exist) and drops the old
+// origin_bound_certs table (if it exists), returning true on success. Success
+// means that the new table exists and the old table doesn't exist at the
+// end of the function, regardless of what sql commands are executed.
+/*
 bool InitTable(sql::Connection* db) {
-  // The table is named "origin_bound_certs" for backwards compatability before
-  // we renamed this class to SQLiteChannelIDStore.  Likewise, the primary
-  // key is "origin", but now can be other things like a plain domain.
-  if (!db->DoesTableExist("origin_bound_certs")) {
+  if (!db->DoesTableExist("channel_id")) {
     if (!db->Execute(
-            "CREATE TABLE origin_bound_certs ("
-            "origin TEXT NOT NULL UNIQUE PRIMARY KEY,"
+            "CREATE TABLE channel_id ("
+            "host TEXT NOT NULL UNIQUE PRIMARY KEY,"
             "private_key BLOB NOT NULL,"
-            "cert BLOB NOT NULL,"
-            "cert_type INTEGER,"
-            "expiration_time INTEGER,"
+            "public_key BLOB NOT NULL,"
             "creation_time INTEGER)")) {
       return false;
     }
   }
 
   return true;
 }
+*/

[Ryan Sleevi, 2015/04/09 22:40:09]

This code is commented out.

[nharper, 2015/04/10 00:32:08]

Sorry - removed.

 
 }  // namespace
 
 namespace net {
 
 // This class is designed to be shared between any calling threads and the
 // background task runner. It batches operations and commits them on a timer.
 class SQLiteChannelIDStore::Backend
     : public base::RefCountedThreadSafe<SQLiteChannelIDStore::Backend> {
  public:
@@ skipping 136 matching lines @@
                  base::Unretained(this)));
 
   if (!db_->Open(path_)) {
     NOTREACHED() << "Unable to open cert DB.";
     if (corruption_detected_)
       KillDatabase();
     db_.reset();
     return;
   }
 
-  if (!EnsureDatabaseVersion() || !InitTable(db_.get())) {
+  if (!EnsureDatabaseVersion()) {

[Ryan Sleevi, 2015/04/09 22:40:09]

Why no InitTable() call?

[nharper, 2015/04/10 00:32:08]

It didn't make sense as its own function - I rolled its functionality into
EnsureDatabaseVersion. With this schema change, I'm actually creating a new
table (sqlite won't let you drop columns), migrating, and dropping the old one,
so it didn't fit the old pattern of InitTable() very well.

     NOTREACHED() << "Unable to open cert DB.";
     if (corruption_detected_)
       KillDatabase();
     meta_table_.Reset();
     db_.reset();
     return;
   }
 
   db_->Preload();
 
   // Slurp all the certs into the out-vector.
   sql::Statement smt(db_->GetUniqueStatement(
-      "SELECT origin, private_key, cert, cert_type, expiration_time, "
-      "creation_time FROM origin_bound_certs"));
+      "SELECT  host, private_key, public_key, creation_time FROM channel_id"));
   if (!smt.is_valid()) {
     if (corruption_detected_)
       KillDatabase();
     meta_table_.Reset();
     db_.reset();
     return;

[Ryan Sleevi, 2015/04/09 22:40:09]

Doesn't this cause things to explode if no channel_ids exist (e.g. a new
profile)?

[nharper, 2015/04/10 00:32:08]

I didn't see any explosions.

I deleted my chromium profile and started up a build of this CL and there were
no explosions. I also navigated to a google site and checked the table in the
sqlite database to verify that the table was created, and that it had entries
(since going to google.com ends up creating channel ids for 7 different google
domains).

Maybe I should try putting the db in a bad state (where it still exists) so that
it makes EnsureDatabaseVersion fail, and see what happens?

   }
 
   while (smt.Step()) {
-    SSLClientCertType type = static_cast<SSLClientCertType>(smt.ColumnInt(3));
-    if (type != CLIENT_CERT_ECDSA_SIGN)
-      continue;
-    std::string private_key_from_db, cert_from_db;
+    std::string private_key_from_db, public_key_from_db;
     smt.ColumnBlobAsString(1, &private_key_from_db);
-    smt.ColumnBlobAsString(2, &cert_from_db);
+    smt.ColumnBlobAsString(2, &public_key_from_db);
     scoped_ptr<DefaultChannelIDStore::ChannelID> channel_id(
         new DefaultChannelIDStore::ChannelID(
-            smt.ColumnString(0),  // origin
-            base::Time::FromInternalValue(smt.ColumnInt64(5)),
-            base::Time::FromInternalValue(smt.ColumnInt64(4)),
-            private_key_from_db,
-            cert_from_db));
+            smt.ColumnString(0),  // host
+            base::Time::FromInternalValue(smt.ColumnInt64(3)),
+            private_key_from_db, public_key_from_db));
     channel_ids->push_back(channel_id.release());
   }
 
   UMA_HISTOGRAM_COUNTS_10000(
       "DomainBoundCerts.DBLoadedCount",
       static_cast<base::HistogramBase::Sample>(channel_ids->size()));
   base::TimeDelta load_time = base::TimeTicks::Now() - start;
   UMA_HISTOGRAM_CUSTOM_TIMES("DomainBoundCerts.DBLoadTime",
                              load_time,
                              base::TimeDelta::FromMilliseconds(1),
                              base::TimeDelta::FromMinutes(1),
                              50);
   DVLOG(1) << "loaded " << channel_ids->size() << " in "
            << load_time.InMilliseconds() << " ms";
 }
 
 bool SQLiteChannelIDStore::Backend::EnsureDatabaseVersion() {
   // Version check.
   if (!meta_table_.Init(
           db_.get(), kCurrentVersionNumber, kCompatibleVersionNumber)) {
     return false;
   }
 
   if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber) {
     LOG(WARNING) << "Server bound cert database is too new.";
     return false;
   }
 
   int cur_version = meta_table_.GetVersionNumber();
-  if (cur_version == 1) {
-    sql::Transaction transaction(db_.get());
-    if (!transaction.Begin())
-      return false;
+
+  sql::Transaction transaction(db_.get());
+  if (!transaction.Begin())
+    return false;
+
+  // Create new table if it doesn't already exist
+  if (!db_->DoesTableExist("channel_id")) {
     if (!db_->Execute(
-            "ALTER TABLE origin_bound_certs ADD COLUMN cert_type "
-            "INTEGER")) {
-      LOG(WARNING) << "Unable to update server bound cert database to "
-                   << "version 2.";
+            "CREATE TABLE channel_id ("
+            "host TEXT NOT NULL UNIQUE PRIMARY KEY,"
+            "private_key BLOB NOT NULL,"
+            "public_key BLOB NOT NULL,"
+            "creation_time INTEGER)")) {
       return false;
     }
-    // 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;
-    }
-    ++cur_version;
-    meta_table_.SetVersionNumber(cur_version);
-    meta_table_.SetCompatibleVersionNumber(
-        std::min(cur_version, kCompatibleVersionNumber));
-    transaction.Commit();
   }
 
-  if (cur_version <= 3) {
-    sql::Transaction transaction(db_.get());
-    if (!transaction.Begin())
-      return false;
-
-    if (cur_version == 2) {
-      if (!db_->Execute(
-              "ALTER TABLE origin_bound_certs ADD COLUMN "
-              "expiration_time INTEGER")) {
-        LOG(WARNING) << "Unable to update server bound cert database to "
-                     << "version 4.";
-        return false;
-      }
-    }
-
-    if (!db_->Execute(
-            "ALTER TABLE origin_bound_certs ADD COLUMN "
-            "creation_time INTEGER")) {
+  // Migrate from previous versions to new version if possible
+  if (cur_version >= 2 && cur_version <= 4) {
+    sql::Statement statement(db_->GetUniqueStatement(
+        "SELECT origin, cert, private_key, cert_type FROM origin_bound_certs"));
+    sql::Statement insert_statement(
+        db_->GetUniqueStatement("INSERT INTO channel_id VALUES (?, ?, ?, ?)"));
+    if (!statement.is_valid() || !insert_statement.is_valid()) {
       LOG(WARNING) << "Unable to update server bound cert database to "
-                   << "version 4.";
+                   << "version 5.";
       return false;
     }
 
-    sql::Statement statement(
-        db_->GetUniqueStatement("SELECT origin, cert FROM origin_bound_certs"));
-    sql::Statement update_expires_statement(db_->GetUniqueStatement(
-        "UPDATE origin_bound_certs SET expiration_time = ? WHERE origin = ?"));
-    sql::Statement update_creation_statement(db_->GetUniqueStatement(
-        "UPDATE origin_bound_certs SET creation_time = ? WHERE origin = ?"));
-    if (!statement.is_valid() || !update_expires_statement.is_valid() ||
-        !update_creation_statement.is_valid()) {
-      LOG(WARNING) << "Unable to update server bound cert database to "
-                   << "version 4.";
-      return false;
-    }
-
     while (statement.Step()) {
       std::string origin = statement.ColumnString(0);
       std::string cert_from_db;
       statement.ColumnBlobAsString(1, &cert_from_db);
+      std::string private_key;
+      statement.ColumnBlobAsString(2, &private_key);
+      if (statement.ColumnInt64(3) != CLIENT_CERT_ECDSA_SIGN) {
+        continue;
+      }
       // Parse the cert and extract the real value and then update the DB.
       scoped_refptr<X509Certificate> cert(X509Certificate::CreateFromBytes(
           cert_from_db.data(), static_cast<int>(cert_from_db.size())));
       if (cert.get()) {
-        if (cur_version == 2) {
-          update_expires_statement.Reset(true);
-          update_expires_statement.BindInt64(
-              0, cert->valid_expiry().ToInternalValue());
-          update_expires_statement.BindString(1, origin);
-          if (!update_expires_statement.Run()) {
-            LOG(WARNING) << "Unable to update server bound cert database to "
-                         << "version 4.";
-            return false;
-          }
+        insert_statement.Reset(true);
+        insert_statement.BindString(0, origin);
+        insert_statement.BindBlob(1, private_key.data(),
+                                  static_cast<int>(private_key.size()));
+        base::StringPiece spki;
+        if (!asn1::ExtractSPKIFromDERCert(cert_from_db, &spki)) {
+          LOG(WARNING) << "Unable to extract SPKI from cert when migrating "
+                          "channel id database to version 5.";
+          return false;
         }
-
-        update_creation_statement.Reset(true);
-        update_creation_statement.BindInt64(
-            0, cert->valid_start().ToInternalValue());
-        update_creation_statement.BindString(1, origin);
-        if (!update_creation_statement.Run()) {
-          LOG(WARNING) << "Unable to update server bound cert database to "
-                       << "version 4.";
+        insert_statement.BindBlob(2, spki.data(), spki.size());
+        insert_statement.BindInt64(3, cert->valid_start().ToInternalValue());
+        if (!insert_statement.Run()) {
+          LOG(WARNING) << "Unable to update channel id database to "
+                       << "version 5.";
           return false;
         }
       } else {
         // If there's a cert we can't parse, just leave it.  It'll get replaced
         // with a new one if we ever try to use it.
         LOG(WARNING) << "Error parsing cert for database upgrade for origin "
                      << statement.ColumnString(0);
       }
     }
-
-    cur_version = 4;
-    meta_table_.SetVersionNumber(cur_version);
-    meta_table_.SetCompatibleVersionNumber(
-        std::min(cur_version, kCompatibleVersionNumber));
-    transaction.Commit();
   }
 
+  if (cur_version < kCurrentVersionNumber) {
+    sql::Statement statement(
+        db_->GetUniqueStatement("DROP TABLE origin_bound_certs"));
+    if (!statement.Run()) {
+      LOG(WARNING) << "Error dropping old origin_bound_certs table";
+      return false;
+    }
+    meta_table_.SetVersionNumber(kCurrentVersionNumber);
+    meta_table_.SetCompatibleVersionNumber(kCurrentVersionNumber);

[mattm, 2015/04/10 01:00:27]

kCompatibleVersionNumber

[nharper, 2015/04/25 02:59:18]

The compatible version number here needs to be 5, not 1. Maybe I should change
kCompatibleVersionNumber to 2 constants?

const int kOldCompatibleVersionNumber = 1;
const int kNewCompatibleVersionNumber = 5;

[mattm, 2015/04/27 20:55:53]

Yeah, I think it should be 5 here also. I don't see where
kOldCompatibleVersionNumber is needed though.

[nharper, 2015/04/29 22:07:15]

I figured out my mistake and set it to 5.

+  }
+  transaction.Commit();
+
   // Put future migration cases here.
 
-  // When the version is too old, we just try to continue anyway, there should
-  // not be a released product that makes a database too old for us to handle.
-  LOG_IF(WARNING, cur_version < kCurrentVersionNumber)
-      << "Server bound cert database version " << cur_version
-      << " is too old to handle.";
-
   return true;
 }
 
 void SQLiteChannelIDStore::Backend::DatabaseErrorCallback(
     int error,
     sql::Statement* stmt) {
   DCHECK(background_task_runner_->RunsTasksOnCurrentThread());
 
   if (!sql::IsErrorCatastrophic(error))
     return;
@@ skipping 86 matching lines @@
     pending_.swap(ops);
     num_pending_ = 0;
   }
 
   // Maybe an old timer fired or we are already Close()'ed.
   if (!db_.get() || ops.empty())
     return;
 
   sql::Statement add_statement(db_->GetCachedStatement(
       SQL_FROM_HERE,
-      "INSERT INTO origin_bound_certs (origin, private_key, cert, cert_type, "
-      "expiration_time, creation_time) VALUES (?,?,?,?,?,?)"));
+      "INSERT INTO channel_id (host, private_key, public_key, "
+      "creation_time) VALUES (?,?,?,?)"));
   if (!add_statement.is_valid())
     return;
 
   sql::Statement del_statement(db_->GetCachedStatement(
-      SQL_FROM_HERE, "DELETE FROM origin_bound_certs WHERE origin=?"));
+      SQL_FROM_HERE, "DELETE FROM channel_id WHERE host=?"));
   if (!del_statement.is_valid())
     return;
 
   sql::Transaction transaction(db_.get());
   if (!transaction.Begin())
     return;
 
   for (PendingOperationsList::iterator it = ops.begin(); it != ops.end();
        ++it) {
     // Free the certs as we commit them to the database.
     scoped_ptr<PendingOperation> po(*it);
     switch (po->op()) {
       case PendingOperation::CHANNEL_ID_ADD: {
         add_statement.Reset(true);
         add_statement.BindString(0, po->channel_id().server_identifier());
         const std::string& private_key = po->channel_id().private_key();
         add_statement.BindBlob(
             1, private_key.data(), static_cast<int>(private_key.size()));
-        const std::string& cert = po->channel_id().cert();
-        add_statement.BindBlob(2, cert.data(), static_cast<int>(cert.size()));
-        add_statement.BindInt(3, CLIENT_CERT_ECDSA_SIGN);
+        const std::string& public_key = po->channel_id().public_key();
+        add_statement.BindBlob(2, public_key.data(),
+                               static_cast<int>(public_key.size()));
         add_statement.BindInt64(
-            4, po->channel_id().expiration_time().ToInternalValue());
-        add_statement.BindInt64(
-            5, po->channel_id().creation_time().ToInternalValue());
+            3, po->channel_id().creation_time().ToInternalValue());
         if (!add_statement.Run())
           NOTREACHED() << "Could not add a server bound cert to the DB.";
         break;
       }
       case PendingOperation::CHANNEL_ID_DELETE:
         del_statement.Reset(true);
         del_statement.BindString(0, po->channel_id().server_identifier());
         if (!del_statement.Run())
           NOTREACHED() << "Could not delete a server bound cert from the DB.";
         break;
@@ skipping 23 matching lines @@
 }
 
 void SQLiteChannelIDStore::Backend::BackgroundDeleteAllInList(
     const std::list<std::string>& server_identifiers) {
   DCHECK(background_task_runner_->RunsTasksOnCurrentThread());
 
   if (!db_.get())
     return;
 
   sql::Statement del_smt(db_->GetCachedStatement(
-      SQL_FROM_HERE, "DELETE FROM origin_bound_certs WHERE origin=?"));
+      SQL_FROM_HERE, "DELETE FROM channel_id WHERE host=?"));
   if (!del_smt.is_valid()) {
     LOG(WARNING) << "Unable to delete channel ids.";
     return;
   }
 
   sql::Transaction transaction(db_.get());
   if (!transaction.Begin()) {
     LOG(WARNING) << "Unable to delete channel ids.";
     return;
   }
@@ skipping 45 matching lines @@
   backend_->SetForceKeepSessionState();
 }
 
 SQLiteChannelIDStore::~SQLiteChannelIDStore() {
   backend_->Close();
   // We release our reference to the Backend, though it will probably still have
   // a reference if the background task runner has not run Close() yet.
 }
 
 }  // namespace net