Handle Origin Bound Certificate expiration.

chrome/browser/net/sqlite_origin_bound_cert_store_unittest.cc

 // Copyright (c) 2011 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 "base/bind.h"
 #include "base/file_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
 #include "base/scoped_temp_dir.h"
 #include "base/stl_util.h"
 #include "base/test/thread_test_helper.h"
 #include "chrome/browser/net/sqlite_origin_bound_cert_store.h"
 #include "chrome/common/chrome_constants.h"
 #include "content/test/test_browser_thread.h"
+#include "net/base/cert_test_util.h"
 #include "sql/statement.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::BrowserThread;
 
 class SQLiteOriginBoundCertStoreTest : public testing::Test {
  public:
   SQLiteOriginBoundCertStoreTest()
       : db_thread_(BrowserThread::DB) {
   }
 
  protected:
+  static void ReadTestKeyAndCert(std::string* key, std::string* cert) {
+    FilePath key_path = net::GetTestCertsDirectory().AppendASCII(
+        "unittest.originbound.key.der");
+    FilePath cert_path = net::GetTestCertsDirectory().AppendASCII(
+        "unittest.originbound.der");
+    ASSERT_TRUE(file_util::ReadFileToString(key_path, key));
+    ASSERT_TRUE(file_util::ReadFileToString(cert_path, cert));
+  }
+
+  static base::Time GetTestCertExpirationTime() {
+    // Cert expiration time from 'dumpasn1 unittest.originbound.der':
+    // GeneralizedTime 19/11/2111 02:23:45 GMT
+    base::Time::Exploded exploded_time;
+    exploded_time.year = 2111;
+    exploded_time.month = 11;
+    exploded_time.day_of_week = 0;  // Unused.
+    exploded_time.day_of_month = 19;
+    exploded_time.hour = 2;
+    exploded_time.minute = 23;
+    exploded_time.second = 45;
+    exploded_time.millisecond = 0;
+    return base::Time::FromUTCExploded(exploded_time);
+  }
+
   virtual void SetUp() {
     db_thread_.Start();
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     store_ = new SQLiteOriginBoundCertStore(
         temp_dir_.path().Append(chrome::kOBCertFilename));
     std::vector<net::DefaultOriginBoundCertStore::OriginBoundCert*> certs;
     ASSERT_TRUE(store_->Load(&certs));
     ASSERT_EQ(0u, certs.size());
     // Make sure the store gets written at least once.
     store_->AddOriginBoundCert(
         net::DefaultOriginBoundCertStore::OriginBoundCert(
             "https://encrypted.google.com:8443",
-            net::CLIENT_CERT_RSA_SIGN, "a", "b"));
+            net::CLIENT_CERT_RSA_SIGN,
+            base::Time(),
+            "a", "b"));
   }
 
   content::TestBrowserThread db_thread_;
   ScopedTempDir temp_dir_;
   scoped_refptr<SQLiteOriginBoundCertStore> store_;
 };
 
 TEST_F(SQLiteOriginBoundCertStoreTest, KeepOnDestruction) {
   store_->SetClearLocalStateOnExit(false);
   store_ = NULL;
@@ skipping 22 matching lines @@
   ASSERT_TRUE(helper->Run());
 
   ASSERT_FALSE(file_util::PathExists(
       temp_dir_.path().Append(chrome::kOBCertFilename)));
 }
 
 // Test if data is stored as expected in the SQLite database.
 TEST_F(SQLiteOriginBoundCertStoreTest, TestPersistence) {
   store_->AddOriginBoundCert(
       net::DefaultOriginBoundCertStore::OriginBoundCert(
-          "https://www.google.com/", net::CLIENT_CERT_ECDSA_SIGN, "c", "d"));
+          "https://www.google.com/",
+          net::CLIENT_CERT_ECDSA_SIGN,
+          base::Time(),
+          "c", "d"));
 
   std::vector<net::DefaultOriginBoundCertStore::OriginBoundCert*> certs;
   // Replace the store effectively destroying the current one and forcing it
   // to write it's data to disk. Then we can see if after loading it again it
   // is still there.
   store_ = NULL;
   scoped_refptr<base::ThreadTestHelper> helper(
       new base::ThreadTestHelper(
           BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
   // Make sure we wait until the destructor has run.
@@ skipping 31 matching lines @@
   STLDeleteContainerPointers(certs.begin(), certs.end());
   certs.clear();
   store_ = new SQLiteOriginBoundCertStore(
       temp_dir_.path().Append(chrome::kOBCertFilename));
 
   // Reload and check if the cert has been removed.
   ASSERT_TRUE(store_->Load(&certs));
   ASSERT_EQ(0U, certs.size());
 }
 
 TEST_F(SQLiteOriginBoundCertStoreTest, TestUpgrade) {

[wtc, 2011/12/15 23:34:28]

Should we rename this test TestUpgradeV1?

[mattm, 2011/12/20 00:28:38]

Done.

   // Reset the store.  We'll be using a different database for this test.
   store_ = NULL;
 
   FilePath v1_db_path(temp_dir_.path().AppendASCII("v1db"));
 
+  std::string key_data;
+  std::string cert_data;
+  ReadTestKeyAndCert(&key_data, &cert_data);
+
   // Create a version 1 database.
   {
     sql::Connection db;
     ASSERT_TRUE(db.Open(v1_db_path));
     ASSERT_TRUE(db.Execute(
         "CREATE TABLE meta(key LONGVARCHAR NOT NULL UNIQUE PRIMARY KEY,"
             "value LONGVARCHAR);"
         "INSERT INTO \"meta\" VALUES('version','1');"
         "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);"
-        "INSERT INTO \"origin_bound_certs\" VALUES("
-            "'https://google.com',X'AA',X'BB');"
+            "private_key BLOB NOT NULL,cert BLOB NOT NULL);"));
+
+    sql::Statement add_smt(db.GetUniqueStatement(
+        "INSERT INTO origin_bound_certs (origin, private_key, cert) "
+        "VALUES (?,?,?)"));
+    add_smt.BindString(0, "https://www.google.com:443");

[wtc, 2011/12/15 23:34:28]

Nit: to be more realistic, should we omit the default port 443? in the origin?

NOTE: the changes I suggested for this test also apply to
the next test, TestUpgradeV2.

[mattm, 2011/12/20 00:28:38]

That's actually how the origin is stored by the code. 
(SSLClientSocketNSS::OriginBoundClientAuthHandler just does origin = "https://"
+ host_and_port_.ToString();)

I see this doesn't actually match the serialization rules of the weborigin spec,
but since we're already doing it this way we'd need to add more DB upgrade code
if we wanted to fix it.  As long as it's consistent I guess we can just leave
it.

+    add_smt.BindBlob(1, key_data.data(), key_data.size());
+    add_smt.BindBlob(2, cert_data.data(), cert_data.size());
+    ASSERT_TRUE(add_smt.Run());
+
+    ASSERT_TRUE(db.Execute(
         "INSERT INTO \"origin_bound_certs\" VALUES("
             "'https://foo.com',X'CC',X'DD');"

[wtc, 2011/12/15 23:34:28]

Nit: this cert can use X'AA' and X'BB' now.  This would
require also updating the ASSERT_STREQ statements on lines
225-226 below.

[mattm, 2011/12/20 00:28:38]

Done.

         ));
   }
 
-  std::vector<net::DefaultOriginBoundCertStore::OriginBoundCert*> certs;
-  store_ = new SQLiteOriginBoundCertStore(v1_db_path);
+  // Load and test the DB contents twice.  First time ensures that we can use
+  // the updated values immediately.  Second time ensures that the updated
+  // values are stored and read correctly on next load.
+  for (int i = 0; i < 2; ++i) {
+    SCOPED_TRACE(i);
 
-  // Load the database and ensure the certs can be read and are marked as RSA.
-  ASSERT_TRUE(store_->Load(&certs));
-  ASSERT_EQ(2U, certs.size());
-  ASSERT_STREQ("https://google.com", certs[0]->origin().c_str());
-  ASSERT_EQ(net::CLIENT_CERT_RSA_SIGN, certs[0]->type());
-  ASSERT_STREQ("\xaa", certs[0]->private_key().c_str());
-  ASSERT_STREQ("\xbb", certs[0]->cert().c_str());
-  ASSERT_STREQ("https://foo.com", certs[1]->origin().c_str());
-  ASSERT_EQ(net::CLIENT_CERT_RSA_SIGN, certs[1]->type());
-  ASSERT_STREQ("\xcc", certs[1]->private_key().c_str());
-  ASSERT_STREQ("\xdd", certs[1]->cert().c_str());
+    std::vector<net::DefaultOriginBoundCertStore::OriginBoundCert*> certs;
+    store_ = new SQLiteOriginBoundCertStore(v1_db_path);
 
-  STLDeleteContainerPointers(certs.begin(), certs.end());
-  certs.clear();
+    // Load the database and ensure the certs can be read and are marked as RSA.
+    ASSERT_TRUE(store_->Load(&certs));
+    ASSERT_EQ(2U, certs.size());
 
-  store_ = NULL;
-  // Make sure we wait until the destructor has run.
-  scoped_refptr<base::ThreadTestHelper> helper(
-      new base::ThreadTestHelper(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
-  ASSERT_TRUE(helper->Run());
+    ASSERT_STREQ("https://www.google.com:443", certs[0]->origin().c_str());
+    ASSERT_EQ(net::CLIENT_CERT_RSA_SIGN, certs[0]->type());
+    ASSERT_EQ(GetTestCertExpirationTime(),
+              certs[0]->not_valid_after());
+    ASSERT_EQ(key_data, certs[0]->private_key());
+    ASSERT_EQ(cert_data, certs[0]->cert());
 
-  // Verify the database version is updated.
-  {
-    sql::Connection db;
-    ASSERT_TRUE(db.Open(v1_db_path));
-    sql::Statement smt(db.GetUniqueStatement(
-        "SELECT value FROM meta WHERE key = \"version\""));
-    ASSERT_TRUE(smt);
-    ASSERT_TRUE(smt.Step());
-    EXPECT_EQ(2, smt.ColumnInt(0));
-    EXPECT_FALSE(smt.Step());
+    ASSERT_STREQ("https://foo.com", certs[1]->origin().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]->not_valid_after());
+    ASSERT_STREQ("\xcc", certs[1]->private_key().c_str());
+    ASSERT_STREQ("\xdd", certs[1]->cert().c_str());
+
+    STLDeleteContainerPointers(certs.begin(), certs.end());
+    certs.clear();

[wtc, 2011/12/15 23:34:28]

I believe these two lines are equivalent to:
    STLDeleteElements(&certs);

[mattm, 2011/12/20 00:28:38]

Done.

+
+    store_ = NULL;
+    // Make sure we wait until the destructor has run.
+    scoped_refptr<base::ThreadTestHelper> helper(
+        new base::ThreadTestHelper(
+            BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
+    ASSERT_TRUE(helper->Run());
+
+    // Verify the database version is updated.
+    {
+      sql::Connection db;
+      ASSERT_TRUE(db.Open(v1_db_path));
+      sql::Statement smt(db.GetUniqueStatement(
+          "SELECT value FROM meta WHERE key = \"version\""));
+      ASSERT_TRUE(smt);
+      ASSERT_TRUE(smt.Step());
+      EXPECT_EQ(3, smt.ColumnInt(0));
+      EXPECT_FALSE(smt.Step());
+    }
   }
 }
 
+TEST_F(SQLiteOriginBoundCertStoreTest, TestUpgradeV2) {
+  // Reset the store.  We'll be using a different database for this test.
+  store_ = NULL;
+
+  FilePath v2_db_path(temp_dir_.path().AppendASCII("v2db"));
+
+  std::string key_data;
+  std::string cert_data;
+  ReadTestKeyAndCert(&key_data, &cert_data);
+
+  // Create a version 2 database.
+  {
+    sql::Connection db;
+    ASSERT_TRUE(db.Open(v2_db_path));
+    ASSERT_TRUE(db.Execute(
+        "CREATE TABLE meta(key LONGVARCHAR NOT NULL UNIQUE PRIMARY KEY,"
+            "value LONGVARCHAR);"
+        "INSERT INTO \"meta\" VALUES('version','2');"
+        "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);"
+        ));
+
+    sql::Statement add_smt(db.GetUniqueStatement(
+        "INSERT INTO origin_bound_certs (origin, private_key, cert, cert_type) "
+        "VALUES (?,?,?,?)"));
+    add_smt.BindString(0, "https://www.google.com:443");
+    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);
+    ASSERT_TRUE(add_smt.Run());
+
+    ASSERT_TRUE(db.Execute(
+        "INSERT INTO \"origin_bound_certs\" VALUES("
+            "'https://foo.com',X'CC',X'DD',64);"
+        ));
+  }
+
+

[wtc, 2011/12/15 23:34:28]

Nit: delete one blank line.

[mattm, 2011/12/20 00:28:38]

Done.

+  // Load and test the DB contents twice.  First time ensures that we can use
+  // the updated values immediately.  Second time ensures that the updated
+  // values are saved and read correctly on next load.
+  for (int i = 0; i < 2; ++i) {
+    SCOPED_TRACE(i);
+
+    std::vector<net::DefaultOriginBoundCertStore::OriginBoundCert*> certs;
+    store_ = new SQLiteOriginBoundCertStore(v2_db_path);
+
+    // Load the database and ensure the certs can be read and are marked as RSA.
+    ASSERT_TRUE(store_->Load(&certs));
+    ASSERT_EQ(2U, certs.size());
+
+    ASSERT_STREQ("https://www.google.com:443", certs[0]->origin().c_str());
+    ASSERT_EQ(net::CLIENT_CERT_RSA_SIGN, certs[0]->type());
+    ASSERT_EQ(GetTestCertExpirationTime(),
+              certs[0]->not_valid_after());
+    ASSERT_EQ(key_data, certs[0]->private_key());
+    ASSERT_EQ(cert_data, certs[0]->cert());
+
+    ASSERT_STREQ("https://foo.com", certs[1]->origin().c_str());
+    ASSERT_EQ(net::CLIENT_CERT_ECDSA_SIGN, certs[1]->type());
+    // Undecodable cert, expiration time will be uninitialized.
+    ASSERT_EQ(base::Time(), certs[1]->not_valid_after());
+    ASSERT_STREQ("\xcc", certs[1]->private_key().c_str());
+    ASSERT_STREQ("\xdd", certs[1]->cert().c_str());
+
+    STLDeleteContainerPointers(certs.begin(), certs.end());
+    certs.clear();
+
+    store_ = NULL;
+    // Make sure we wait until the destructor has run.
+    scoped_refptr<base::ThreadTestHelper> helper(
+        new base::ThreadTestHelper(
+            BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
+    ASSERT_TRUE(helper->Run());
+
+    // Verify the database version is updated.
+    {
+      sql::Connection db;
+      ASSERT_TRUE(db.Open(v2_db_path));
+      sql::Statement smt(db.GetUniqueStatement(
+          "SELECT value FROM meta WHERE key = \"version\""));
+      ASSERT_TRUE(smt);
+      ASSERT_TRUE(smt.Step());
+      EXPECT_EQ(3, smt.ColumnInt(0));
+      EXPECT_FALSE(smt.Step());
+    }
+  }
+}
+
 // Test that we can force the database to be written by calling Flush().
 TEST_F(SQLiteOriginBoundCertStoreTest, TestFlush) {
   // File timestamps don't work well on all platforms, so we'll determine
   // whether the DB file has been modified by checking its size.
   FilePath path = temp_dir_.path().Append(chrome::kOBCertFilename);
   base::PlatformFileInfo info;
   ASSERT_TRUE(file_util::GetFileInfo(path, &info));
   int64 base_size = info.size;
 
   // Write some certs, so the DB will have to expand by several KB.
   for (char c = 'a'; c < 'z'; ++c) {
     std::string origin(1, c);
     std::string private_key(1000, c);
     std::string cert(1000, c);
     store_->AddOriginBoundCert(
         net::DefaultOriginBoundCertStore::OriginBoundCert(
             origin,
             net::CLIENT_CERT_RSA_SIGN,
+            base::Time(),
             private_key,
             cert));
   }
 
   // Call Flush() and wait until the DB thread is idle.
   store_->Flush(base::Closure());
   scoped_refptr<base::ThreadTestHelper> helper(
       new base::ThreadTestHelper(
           BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
   ASSERT_TRUE(helper->Run());
@@ skipping 30 matching lines @@
 
   store_->Flush(base::Bind(&CallbackCounter::Callback, counter.get()));
 
   scoped_refptr<base::ThreadTestHelper> helper(
       new base::ThreadTestHelper(
           BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB)));
   ASSERT_TRUE(helper->Run());
 
   ASSERT_EQ(1, counter->callback_count());
 }