This change loads opencryptoki and uses the TPM for keygen tags.

base/nss_util.cc

Index: base/nss_util.cc
diff --git a/base/nss_util.cc b/base/nss_util.cc
index fe78fe0c218bfb37162702f2cf6b5815b52cbe41..92c43a909e759fc9870c2a5b4628a98c482c6dc0 100644
--- a/base/nss_util.cc
+++ b/base/nss_util.cc
@@ -18,6 +18,7 @@
 #include <sys/vfs.h>
 #endif
 
+#include "base/crypto/scoped_nss_types.h"
 #include "base/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
@@ -38,6 +39,32 @@
 namespace base {
 
 namespace {

[wtc, 2011/03/16 18:41:44]

Nit: please add a blank line after this line.

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done.

+#if defined(OS_CHROMEOS)
+static const char kNssDatabaseName[] = "Real NSS database";

[wtc, 2011/03/16 18:41:44]

Why do you need "Real"?  Is there a "Fake NSS database" too?

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

This is what it was always called, I just moved it here.  In any case, the
reason it's called that is because before the cryptohome is mounted, we have to
open a read-only non-persistent database.  Once it is mounted, we open the r/w
database that we actually use (which is this one).  I think the name is fairly
appropriate, but I didn't name it.

+
+// TODO(gspencer): Get these values from cryptohomed's dbus API when
+// we ask if it has initialized the TPM yet.  These should not be
+// hard-coded here.
+static const char kHardwareTokenName[] = "Initialized by CrOS";

[kmixter1, 2011/03/16 06:33:18]

If we support real hardware tokens this may seem confusing.  We seem to call it
the TPM-backed or TPM-wrapped token.  opencryptoki and TrouSers refer to it as
just the "TPM token".  Maybe kTpmTokenName?

[rginda, 2011/03/16 17:09:37]

s/Name/Label/ would be more PKCS11'y.

[wtc, 2011/03/16 18:41:44]

I agree with kmixter1 that these should be named "kTPMTokenName", kTPMUserPIN,
kTPMSecurityOfficerPIN.

On 2011/03/16 17:09:37, rginda wrote:
Nit: the NSS function is named PK11_GetTokenName, so it'd be
better to use "Name" here for consistency.

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Changed the names to the ones wtc suggests.

+static const char kHardwareTokenUserPin[] = "111111";
+static const char kHardwareTokenSecurityOfficerPin[] = "000000";

[wtc, 2011/03/16 18:41:44]

Nit: in the crypto and net code we usually use all caps for
abbreviations, so please try to use "NSS" and "PIN" in
identifiers.

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done: changed to mirror other crypto code.  Note that this isn't typical for the
rest of Google: even acronyms use camel case (because of names like
"GetDNSSSLURI()": "GetDnsSslUri()" is clearer).

+
+// Fake certificate authority database used for testing.
+static const FilePath::CharType kReadOnlyCertDB[] =

[kmixter1, 2011/03/16 06:33:18]

I don't understand what this does - is this only for testing?  And if so should
it be submitted?

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Yes, it's only for testing, it's the location where we look for the fake root
for testing.  I didn't actually add this at all: I just moved it from down below
because it made sense to collect all the constants into one location.  It's been
checked in for a while.

+    FILE_PATH_LITERAL("/etc/fake_root_ca/nssdb");
+#endif  // defined(OS_CHROMEOS)
+
+std::string GetNssError() {

[wtc, 2011/03/16 18:41:44]

Nit: please rename this function GetNSSErrorMessage or
GetNSSErrorString.  "Error" alone usually means "error code".

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done.

+  std::string result;
+  if (PR_GetErrorTextLength()) {
+    scoped_ptr<char> error_text(new char[PR_GetErrorTextLength() + 1]);
+    PRInt32 copied = PR_GetErrorText(error_text.get());
+    result = std::string(error_text.get(), copied);
+  } else {
+    result = StringPrintf("NSS Error code: %d", PR_GetError());
+  }
+  return result;
+}
 
 #if defined(USE_NSS)
 FilePath GetDefaultConfigDirectory() {
@@ -62,8 +89,6 @@ FilePath GetDefaultConfigDirectory() {
 // caller to failover to NSS_NoDB_Init() at that point.
 FilePath GetInitialConfigDirectory() {
 #if defined(OS_CHROMEOS)
-  static const FilePath::CharType kReadOnlyCertDB[] =
-      FILE_PATH_LITERAL("/etc/fake_root_ca/nssdb");
   return FilePath(kReadOnlyCertDB);
 #else
   return GetDefaultConfigDirectory();
@@ -73,6 +98,12 @@ FilePath GetInitialConfigDirectory() {
 // This callback for NSS forwards all requests to a caller-specified
 // CryptoModuleBlockingPasswordDelegate object.
 char* PKCS11PasswordFunc(PK11SlotInfo* slot, PRBool retry, void* arg) {
+#if defined(OS_CHROMEOS)
+  // If we get asked for a password for the hardware token, then

[wtc, 2011/03/16 18:41:44]

Nit: hardware token => TPM

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done.

+  // return the static password we use.
+  if (std::string(PK11_GetTokenName(slot)) == std::string(kHardwareTokenName))

[kmixter1, 2011/03/16 06:33:18]

nit: constructing to std::string on rhs isn't necessary.

[wtc, 2011/03/16 18:41:44]

Just use strcmp() to compare the two C strings here and at
line 185 below.

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

I'm going to continue to use std::string operator== here because GetTPMTokenName
returns one, and because in the future when we get the name from the dbus API to
cryptohomed, it will continue to return one (i.e. I don't want to use the
constant here because it's just one more place I'll have to get rid of it anyhow
when the dbus API comes in).

+    return PORT_Strdup(kHardwareTokenUserPin);
+#endif
   base::CryptoModuleBlockingPasswordDelegate* delegate =
       reinterpret_cast<base::CryptoModuleBlockingPasswordDelegate*>(arg);
   if (delegate) {
@@ -116,22 +147,48 @@ void UseLocalCacheOfNSSDatabaseIfNFS(const FilePath& database_dir) {
 #endif  // defined(OS_LINUX)
 }
 
-// Load nss's built-in root certs.
-SECMODModule *InitDefaultRootCerts() {
-  const char* kModulePath = "libnssckbi.so";
-  char modparams[1024];
-  snprintf(modparams, sizeof(modparams),
-           "name=\"Root Certs\" library=\"%s\"", kModulePath);
-  SECMODModule *root = SECMOD_LoadUserModule(modparams, NULL, PR_FALSE);
-  if (root)
-    return root;
-
-  // Aw, snap.  Can't find/load root cert shared library.
-  // This will make it hard to talk to anybody via https.
-  NOTREACHED();
+#endif  // defined(USE_NSS)

[wtc, 2011/03/16 18:41:44]

Please move this #endif to include AutoSECMODListLock and
FindTokenByName.

The USE_NSS macro is defined for Linux and Chrome OS, which
uses the complete NSS, but undefined for Mac and Windows,
which uses a stripped-down subset of NSS.  FindTokenByNam
won't be used by Mac and Windows.

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done.

+
+// A helper class that acquires the SECMOD list read lock while the
+// AutoSECMODListLock is in scope.

[kmixter1, 2011/03/16 06:33:18]

Should the name of the class contain "read" since it's a read lock specifically
we're getting?

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done.

+class AutoSECMODListLock {
+ public:
+  AutoSECMODListLock()
+      : lock_(SECMOD_GetDefaultModuleListLock()) {
+    if (!lock_) {
+      LOG(ERROR) << "AutoSECMODListLock: Unable to obtain lock.";

[wtc, 2011/03/16 18:41:44]

Nit: "obtain lock" could be misinterpreted to mean "acquire lock".

I suggest "lock is not initialized".

+      return;
+    }
+    SECMOD_GetReadLock(lock_);
+  }
+
+  ~AutoSECMODListLock() {
+    if (lock_)
+      SECMOD_ReleaseReadLock(lock_);
+  }
+
+  bool has_lock() { return lock_ != NULL; }

[kmixter1, 2011/03/16 06:33:18]

nit: const function

[wtc, 2011/03/16 18:41:44]

Is the has_lock function useful?  "lock_initialized" would
be a better name.  ("has lock" could also mean "owns lock".)

It may be better to remove the lock_ null pointer check
and just let the AutoSECMODListLock constructor crash
inside the SECMOD_GetReadLock call.

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done.

+ private:

[kmixter1, 2011/03/16 06:33:18]

nit: One line above private according to style guide?

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done.

+  SECMODListLock* lock_;
+  DISALLOW_COPY_AND_ASSIGN(AutoSECMODListLock);
+};
+
+PK11SlotInfo* FindTokenByName(const std::string& token_name) {
+  AutoSECMODListLock auto_lock;
+  if (!auto_lock.has_lock())
+    return NULL;
+  SECMODModuleList* head = SECMOD_GetDefaultModuleList();
+  for(SECMODModuleList* item = head; item != NULL; item = item->next) {
+    int slot_count = item->module->loaded ? item->module->slotCount : 0;

[kmixter1, 2011/03/16 06:33:18]

My brief reading of NSS says that it internally will crash if item->module is
NULL, so I assume it's ok to also make the non-NULL assumption here.  May be
worth noting the assumption is valid or DCHECKing.

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done.

+    for (int i = 0; i < slot_count; i++) {
+      PK11SlotInfo* slot = item->module->slots[i];
+      if (std::string(PK11_GetTokenName(slot)) == token_name) {
+        return PK11_ReferenceSlot(slot);
+      }
+    }
+  }
   return NULL;
 }
-#endif  // defined(USE_NSS)
 
 // A singleton to initialize/deinitialize NSPR.
 // Separate from the NSS singleton because we initialize NSPR on the UI thread.
@@ -166,15 +223,45 @@ class NSSInitSingleton {
   void OpenPersistentNSSDB() {
     if (!chromeos_user_logged_in_) {
       // GetDefaultConfigDirectory causes us to do blocking IO on UI thread.
-      // Temporarily allow it until we fix http://crbug.com.70119
+      // Temporarily allow it until we fix http://crbug.com/70119
       ThreadRestrictions::ScopedAllowIO allow_io;
       chromeos_user_logged_in_ = true;
-      real_db_slot_ = OpenUserDB(GetDefaultConfigDirectory(),
-                                 "Real NSS database");
+
+      // This creates a new DB slot in NSS that is read/write, unlike
+      // the cert DB and the "default" crypto key provider, which are
+      // still read-only (because we initialized NSS before we had a
+      // cryptohome mounted).
+      real_db_slot_ = OpenUserDB(GetDefaultConfigDirectory(), kNssDatabaseName);
+
+      // This loads the opencryptoki module so we can talk to the
+      // hardware TPM.
+      opencryptoki_module_ = LoadModule(
+          "opencryptoki",
+          "/usr/lib/opencryptoki/libopencryptoki.so",
+          // trustOrder=100 -- means it'll select this as the most
+          //   trusted slot for the mechanisms it provides.
+          // slotParams=... -- selects RSA as only mechanism, and only
+          //   asks for the password when necessary (instead of every
+          //   time, or after a timeout).
+          "trustOrder=100 slotParams=(1={slotFlags=[RSA] askpw=only})");
+      if (opencryptoki_module_) {
+        // We shouldn't need to initialize the RSA PIN here because
+        // it'll be taken care of by cryptohomed, but we have to make
+        // sure that it is initialized.
+
+        // TODO(gspencer): replace this with a dbus call to check and
+        // see that cryptohomed has initialized the PINs already.
+        EnsureRsaPinInit();

[wtc, 2011/03/16 18:41:44]

This function should be renamed EnsureTPMPINInit, and
GetRsaSlot should be renamed GetTPMSlot.

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done.

+      }
     }
   }
+
+  PK11SlotInfo* GetRsaSlot() {

[kmixter1, 2011/03/16 06:33:18]

Seems like "RSA" applies to a software slot as well.  GetHardwareSlot or
GetTpmBackedSlot?

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Renamed to GetTPMSlot

+    return FindTokenByName(kHardwareTokenName);
+  }
 #endif  // defined(OS_CHROMEOS)
 
+
   bool OpenTestNSSDB(const FilePath& path, const char* description) {
     test_db_slot_ = OpenUserDB(path, description);
     return !!test_db_slot_;
@@ -211,6 +298,7 @@ class NSSInitSingleton {
       : real_db_slot_(NULL),
         test_db_slot_(NULL),
         root_(NULL),
+        opencryptoki_module_(NULL),
         chromeos_user_logged_in_(false) {
     EnsureNSPRInit();
 
@@ -241,7 +329,7 @@ class NSSInitSingleton {
     status = NSS_NoDB_Init(NULL);
     if (status != SECSuccess) {
       LOG(ERROR) << "Error initializing NSS without a persistent "
-                    "database: NSS error code " << PR_GetError();
+                    "database: " << GetNssError();

[kmixter1, 2011/03/16 06:33:18]

nice

     }
 #else
     FilePath database_dir = GetInitialConfigDirectory();
@@ -263,16 +351,15 @@ class NSSInitSingleton {
       if (status != SECSuccess) {
         LOG(ERROR) << "Error initializing NSS with a persistent "
                       "database (" << nss_config_dir
-                   << "): NSS error code " << PR_GetError();
+                   << "): " << GetNssError();
       }
     }
     if (status != SECSuccess) {
-      LOG(WARNING) << "Initialize NSS without a persistent database "
-                      "(~/.pki/nssdb).";
+      VLOG(1) << "Initializing NSS without a persistent database.";
       status = NSS_NoDB_Init(NULL);
       if (status != SECSuccess) {
         LOG(ERROR) << "Error initializing NSS without a persistent "
-                      "database: NSS error code " << PR_GetError();
+                      "database: " << GetNssError();
         return;
       }
     }
@@ -310,16 +397,68 @@ class NSSInitSingleton {
       SECMOD_DestroyModule(root_);
       root_ = NULL;
     }
+    if (opencryptoki_module_) {
+      SECMOD_UnloadUserModule(opencryptoki_module_);
+      SECMOD_DestroyModule(opencryptoki_module_);
+      opencryptoki_module_ = NULL;
+    }
 
     SECStatus status = NSS_Shutdown();
     if (status != SECSuccess) {
       // We VLOG(1) because this failure is relatively harmless (leaking, but
       // we're shutting down anyway).
-      VLOG(1) << "NSS_Shutdown failed; see "
-                 "http://code.google.com/p/chromium/issues/detail?id=4609";
+      VLOG(1) << "NSS_Shutdown failed; see http://crosbug.com/4609";

[kmixter1, 2011/03/16 06:33:18]

crbug.com/4609

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Whoops! Nice catch.  Done.

     }
   }
 
+#if defined(USE_NSS)
+  // Load nss's built-in root certs.
+  SECMODModule *InitDefaultRootCerts() {

[kmixter1, 2011/03/16 06:33:18]

nit: star in wrong place (even though you just moved this function)

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done.

+    SECMODModule *root = LoadModule("Root Certs", "libnssckbi.so", NULL);
+    if (root)
+      return root;
+
+    // Aw, snap.  Can't find/load root cert shared library.
+    // This will make it hard to talk to anybody via https.
+    NOTREACHED();
+    return NULL;
+  }
+#endif
+
+  // Load the given module for this NSS session.
+  SECMODModule* LoadModule(const char* name,

[wtc, 2011/03/16 18:41:44]

Put the LoadModule function inside the USE_NSS block
above.

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Done.

+                           const char* library_path,
+                           const char* params) {
+    std::string modparams = StringPrintf(
+        "name=\"%s\" library=\"%s\" %s",
+        name, library_path, params ? params : "");
+
+    // Shouldn't need to const_cast here, but SECMOD doesn't believe
+    // in const interfaces.
+    SECMODModule* module = SECMOD_LoadUserModule(
+        const_cast<char*>(modparams.c_str()), NULL, PR_FALSE);
+    if (!module) {
+      LOG(ERROR) << "Error loading " << name << " module into NSS: "
+                 << GetNssError();
+      return NULL;
+    }
+    return module;
+  }
+
+#if defined(OS_CHROMEOS)
+  void EnsureRsaPinInit() {
+    base::ScopedPK11Slot rsa_slot(GetRsaSlot());
+    if (rsa_slot.get()) {
+      if (PK11_NeedUserInit(rsa_slot.get())) {
+        AutoNSSWriteLock lock;
+        PK11_InitPin(rsa_slot.get(),

[kmixter1, 2011/03/16 06:33:18]

If we incorrectly get here, executing this means we will clear the user's TPM
slot.  Seems like this code may be helpful for testing but should never be hit
in production.  Remove or only run for DEBUG builds?

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

So, this will be removed as soon as we can ask cryptohomed if it has initialized
it.  It also won't clear the TPM because it won't happen if the TPM is already
initialized.  I'll add a TODO, however.

+                     kHardwareTokenSecurityOfficerPin,
+                     kHardwareTokenUserPin);
+      }
+    }
+  }
+#endif
+
   static PK11SlotInfo* OpenUserDB(const FilePath& path,
                                   const char* description) {
     const std::string modspec =
@@ -332,14 +471,15 @@ class NSSInitSingleton {
     }
     else {
       LOG(ERROR) << "Error opening persistent database (" << modspec
-                 << "): NSS error code " << PR_GetError();
+                 << "): " << GetNssError();
     }
     return db_slot;
   }
 
   PK11SlotInfo* real_db_slot_;  // Overrides internal key slot if non-NULL.
   PK11SlotInfo* test_db_slot_;  // Overrides internal key slot and real_db_slot_
-  SECMODModule *root_;
+  SECMODModule* root_;
+  SECMODModule* opencryptoki_module_;
   bool chromeos_user_logged_in_;
 #if defined(USE_NSS)
   // TODO(davidben): When https://bugzilla.mozilla.org/show_bug.cgi?id=564011
@@ -408,7 +548,7 @@ AutoNSSWriteLock::~AutoNSSWriteLock() {
 void OpenPersistentNSSDB() {
   g_nss_singleton.Get().OpenPersistentNSSDB();
 }
-#endif
+#endif  // defined(OS_CHROMEOS)
 
 // TODO(port): Implement this more simply.  We can convert by subtracting an
 // offset (the difference between NSPR's and base::Time's epochs).
@@ -433,4 +573,12 @@ PK11SlotInfo* GetDefaultNSSKeySlot() {
   return g_nss_singleton.Get().GetDefaultKeySlot();
 }
 
+PK11SlotInfo* GetRsaKeySlot() {

[wtc, 2011/03/16 18:41:44]

This function should be named GetPreferredKeySlot or
GetPermanentKeySlot.

I wonder if we can just change GetDefaultKeySlot() to
use the TPM slot if it exists.

[Greg Spencer (Chromium), 2011/03/16 21:37:51]

Yeah, I used RSA because I was trying to imply that it was the preferred slot on
the given platform for RSA keys.  I like GetPreferredKeySlot better.
No, because in some cases (inserting CA certs, for instance), we don't actually
want to use the TPM (mainly for performance).

+#if defined(OS_CHROMEOS)
+  return g_nss_singleton.Get().GetRsaSlot();
+#else
+  return g_nss_singleton.Get().GetDefaultKeySlot();
+#endif
+}
+
 }  // namespace base