Handle Origin Bound Certificate expiration.

chrome/browser/net/sqlite_origin_bound_cert_store_unittest.cc

Index: chrome/browser/net/sqlite_origin_bound_cert_store_unittest.cc
diff --git a/chrome/browser/net/sqlite_origin_bound_cert_store_unittest.cc b/chrome/browser/net/sqlite_origin_bound_cert_store_unittest.cc
index cc3130d7b1959a89175ad7c7d686598ad5df5312..c5382e5e7ab2f893398e4152540e6eae15ac4fc1 100644
--- a/chrome/browser/net/sqlite_origin_bound_cert_store_unittest.cc
+++ b/chrome/browser/net/sqlite_origin_bound_cert_store_unittest.cc
@@ -12,6 +12,7 @@
 #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"
 
@@ -24,6 +25,30 @@ class SQLiteOriginBoundCertStoreTest : public testing::Test {
   }
 
  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());
@@ -36,7 +61,9 @@ class SQLiteOriginBoundCertStoreTest : public testing::Test {
     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_;
@@ -79,7 +106,10 @@ TEST_F(SQLiteOriginBoundCertStoreTest, RemoveOnDestruction) {
 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
@@ -137,6 +167,10 @@ TEST_F(SQLiteOriginBoundCertStoreTest, TestUpgrade) {
 
   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;
@@ -148,49 +182,163 @@ TEST_F(SQLiteOriginBoundCertStoreTest, TestUpgrade) {
         "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());
+
+    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_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;
-  // 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.
+  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(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_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());
+    }
   }
 }
 
@@ -212,6 +360,7 @@ TEST_F(SQLiteOriginBoundCertStoreTest, TestFlush) {
         net::DefaultOriginBoundCertStore::OriginBoundCert(
             origin,
             net::CLIENT_CERT_RSA_SIGN,
+            base::Time(),
             private_key,
             cert));
   }