Migrate SQLiteChannelIDStore away from using deprecated ECPrivateKey functions

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 <memory>
 #include <set>
 #include <utility>
 #include <vector>
@@ skipping 15 matching lines @@
 #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 = 5;
-const int kCompatibleVersionNumber = 5;
+const int kCurrentVersionNumber = 6;
+const int kCompatibleVersionNumber = 6;
 
 }  // 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 157 matching lines @@
       KillDatabase();
     meta_table_.Reset();
     db_.reset();
     return;
   }
 
   db_->Preload();
 
   // Slurp all the certs into the out-vector.
   sql::Statement smt(db_->GetUniqueStatement(
-      "SELECT host, private_key, public_key, creation_time FROM channel_id"));
+      "SELECT host, private_key, creation_time FROM channel_id"));
   if (!smt.is_valid()) {
     if (corruption_detected_)
       KillDatabase();
     meta_table_.Reset();
     db_.reset();
     return;
   }
 
   while (smt.Step()) {
-    std::vector<uint8_t> private_key_from_db, public_key_from_db;
+    std::vector<uint8_t> private_key_from_db;
     smt.ColumnBlobAsVector(1, &private_key_from_db);
-    smt.ColumnBlobAsVector(2, &public_key_from_db);
     std::unique_ptr<crypto::ECPrivateKey> key(
-        crypto::ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
-            private_key_from_db, public_key_from_db));
+        crypto::ECPrivateKey::CreateFromPrivateKeyInfo(private_key_from_db));
     if (!key)
       continue;
     std::unique_ptr<DefaultChannelIDStore::ChannelID> channel_id(
         new DefaultChannelIDStore::ChannelID(
             smt.ColumnString(0),  // host
-            base::Time::FromInternalValue(smt.ColumnInt64(3)), std::move(key)));
+            base::Time::FromInternalValue(smt.ColumnInt64(2)), std::move(key)));
     channel_ids->push_back(std::move(channel_id));
   }
 
   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),
@@ skipping 32 matching lines @@
       return false;
     }
   }
 
   // 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 (host, private_key, public_key, creation_time) "
-        "VALUES (?, ?, ?, ?)"));
+        "VALUES (?, ?, \"\", ?)"));
     if (!statement.is_valid() || !insert_statement.is_valid()) {
       LOG(WARNING) << "Unable to update server bound cert database to "
-                   << "version 5.";
+                   << "version 6.";
       return false;
     }
 
     while (statement.Step()) {
       if (statement.ColumnInt64(3) != CLIENT_CERT_ECDSA_SIGN)
         continue;
       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);
+      std::vector<uint8_t> encrypted_private_key, private_key;
+      statement.ColumnBlobAsVector(2, &encrypted_private_key);
+      std::unique_ptr<crypto::ECPrivateKey> key(
+          crypto::ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
+              encrypted_private_key, std::vector<uint8_t>()));
+      if (!key || !key->ExportPrivateKey(&private_key)) {
+        LOG(WARNING) << "Unable to parse encrypted private key when migrating "
+                        "Channel ID database to version 6.";
+        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()) {
         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;
-        }
-        insert_statement.BindBlob(2, spki.data(),
-                                  static_cast<int>(spki.size()));
-        insert_statement.BindInt64(3, cert->valid_start().ToInternalValue());
+        insert_statement.BindInt64(2, cert->valid_start().ToInternalValue());
         if (!insert_statement.Run()) {
           LOG(WARNING) << "Unable to update channel id database to "
-                       << "version 5.";
+                       << "version 6.";
           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);
       }
     }
+  } else if (cur_version >= 2 && cur_version <= 5) {

[mattm, 2017/04/04 22:44:24]

Should be cur_version == 5? or maybe (cur_version > 4 && cur_version <= 5)?

(With the previous if condition it doesn't actually change behavior, but this is
a little confusing and could be error prone if refactored.)

[nharper, 2017/04/04 23:01:31]

Done.

+    sql::Statement select(
+        db_->GetUniqueStatement("SELECT host, private_key FROM channel_id"));
+    sql::Statement update(db_->GetUniqueStatement(
+        "UPDATE channel_id SET private_key = ? WHERE host = ?"));

[mattm, 2017/04/04 22:44:24]

Also set public_key to "" here?

[nharper, 2017/04/04 23:01:31]

Done.

+    if (!select.is_valid() || !update.is_valid()) {
+      LOG(WARNING) << "Invalid SQL statements to update Channel ID database to "
+                      "version 6.";
+      return false;
+    }
+
+    while (select.Step()) {
+      std::string host = select.ColumnString(0);
+      std::vector<uint8_t> encrypted_private_key, private_key;
+      select.ColumnBlobAsVector(1, &encrypted_private_key);
+      std::unique_ptr<crypto::ECPrivateKey> key(
+          crypto::ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
+              encrypted_private_key, std::vector<uint8_t>()));
+      if (!key || !key->ExportPrivateKey(&private_key)) {
+        LOG(WARNING) << "Unable to parse encrypted private key when migrating "
+                        "Channel ID database to version 6.";
+        continue;
+      }
+      update.Reset(true);
+      update.BindBlob(0, private_key.data(),
+                      static_cast<int>(private_key.size()));
+      update.BindString(1, host);
+      if (!update.Run()) {
+        LOG(WARNING) << "UPDATE statement failed when updating Channel ID "
+                        "database to version 6.";
+        return false;
+      }
+    }
   }
 
   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;
+    if (cur_version <= 4) {
+      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(kCompatibleVersionNumber);
   }
   transaction.Commit();
 
   // Put future migration cases here.
 
   return true;
 }
@@ skipping 116 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 channel_id (host, private_key, public_key, "
-      "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 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.
     std::unique_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());
-        std::vector<uint8_t> private_key, public_key;
-        if (!po->channel_id().key()->ExportEncryptedPrivateKey(&private_key))
-          continue;
-        if (!po->channel_id().key()->ExportPublicKey(&public_key))
+        std::vector<uint8_t> private_key;
+        if (!po->channel_id().key()->ExportPrivateKey(&private_key))
           continue;
         add_statement.BindBlob(
             1, private_key.data(), static_cast<int>(private_key.size()));
-        add_statement.BindBlob(2, public_key.data(),
-                               static_cast<int>(public_key.size()));
         add_statement.BindInt64(
-            3, po->channel_id().creation_time().ToInternalValue());
+            2, 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 91 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