Use user specific NSSDatabase in CertLoader.

CertLoader is still global object, but it now loads only primary user's certificates. Loading only primary user's certificates is ok, since shill only uses primary user's network profile (and currently only network stack is interested in certificates from CertLoader). Added some tests for CertLoader and NetworkConnectionHandler. BUG=315343 Review URL: https://codereview.chromium.org/135193007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247414 0039d316-1c4b-4281-b951-d872f2087c98
hylschwy · Jan 28, 2014 · 69295ba · 69295ba
1 parent fb7b39f
commit 69295ba
Show file tree

Hide file tree

Showing 28 changed files with 735 additions and 206 deletions.
diff --git a/chrome/browser/chromeos/login/fake_login_utils.cc b/chrome/browser/chromeos/login/fake_login_utils.cc
@@ -109,6 +109,10 @@ void FakeLoginUtils::InitRlzDelayed(Profile* user_profile) {
   NOTREACHED() << "Method not implemented.";
 }
 
+void FakeLoginUtils::StartCertLoader(Profile* user_profile) {
+  NOTREACHED() << "Method not implemented.";
+}
+
 Profile* FakeLoginUtils::CreateProfile(const std::string& username_hash) {
   base::FilePath path;
   PathService::Get(chrome::DIR_USER_DATA, &path);

diff --git a/chrome/browser/chromeos/login/fake_login_utils.h b/chrome/browser/chromeos/login/fake_login_utils.h
@@ -34,6 +34,7 @@ class FakeLoginUtils : public LoginUtils {
       LoginStatusConsumer* consumer) OVERRIDE;
   virtual void RestoreAuthenticationSession(Profile* profile) OVERRIDE;
   virtual void InitRlzDelayed(Profile* user_profile) OVERRIDE;
+  virtual void StartCertLoader(Profile* user_profile) OVERRIDE;
 
   void SetExpectedCredentials(const std::string& username,
                               const std::string& password);

diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc
@@ -51,6 +51,7 @@
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/google/google_util_chromeos.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
+#include "chrome/browser/net/nss_context.h"
 #include "chrome/browser/pref_service_flags_storage.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -63,6 +64,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/pref_names.h"
+#include "chromeos/cert_loader.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/cryptohome/cryptohome_util.h"
 #include "chromeos/dbus/cryptohome_client.h"
@@ -81,6 +83,10 @@
 
 using content::BrowserThread;
 
+namespace net {
+class NSSCertDatabase;
+}
+
 namespace chromeos {
 
 namespace {
@@ -134,6 +140,7 @@ class LoginUtilsImpl
       LoginStatusConsumer* consumer) OVERRIDE;
   virtual void RestoreAuthenticationSession(Profile* profile) OVERRIDE;
   virtual void InitRlzDelayed(Profile* user_profile) OVERRIDE;
+  virtual void StartCertLoader(Profile* user_profile) OVERRIDE;
 
   // OAuth2LoginManager::Observer overrides.
   virtual void OnSessionRestoreStateChanged(
@@ -197,6 +204,10 @@ class LoginUtilsImpl
   // Initializes RLZ. If |disabled| is true, RLZ pings are disabled.
   void InitRlz(Profile* user_profile, bool disabled);
 
+  // Starts CertLoader with the provided NSS database. It must be called at most
+  // once, and with the primary user's database.
+  void StartCertLoaderWithNSSDB(net::NSSCertDatabase* database);
+
   // Attempts restarting the browser process and esures that this does
   // not happen while we are still fetching new OAuth refresh tokens.
   void AttemptRestart(Profile* profile);
@@ -584,10 +595,12 @@ void LoginUtilsImpl::FinalizePrepareProfile(Profile* user_profile) {
       content::NotificationService::AllSources(),
       content::Details<Profile>(user_profile));
 
-  // Initialize RLZ only for primary user.
+  // Initialize RLZ and CertLoader only for primary user.
   if (UserManager::Get()->GetPrimaryUser() ==
       UserManager::Get()->GetUserByProfile(user_profile)) {
     InitRlzDelayed(user_profile);
+    if (CertLoader::IsInitialized())
+      StartCertLoader(user_profile);
   }
   // TODO(altimofeev): This pointer should probably never be NULL, but it looks
   // like LoginUtilsImpl::OnProfileCreated() may be getting called before
@@ -640,6 +653,16 @@ void LoginUtilsImpl::InitRlz(Profile* user_profile, bool disabled) {
 #endif
 }
 
+void LoginUtilsImpl::StartCertLoader(Profile* user_profile) {
+  GetNSSCertDatabaseForProfile(
+      user_profile,
+      base::Bind(&LoginUtilsImpl::StartCertLoaderWithNSSDB, AsWeakPtr()));
+}
+
+void LoginUtilsImpl::StartCertLoaderWithNSSDB(net::NSSCertDatabase* database) {
+  CertLoader::Get()->StartWithNSSDB(database);
+}
+
 void LoginUtilsImpl::CompleteOffTheRecordLogin(const GURL& start_url) {
   VLOG(1) << "Completing incognito login";
 

diff --git a/chrome/browser/chromeos/login/login_utils.h b/chrome/browser/chromeos/login/login_utils.h
@@ -103,6 +103,9 @@ class LoginUtils {
 
   // Initialize RLZ.
   virtual void InitRlzDelayed(Profile* user_profile) = 0;
+
+  // Initiates process of starting CertLoader for the user_profile.
+  virtual void StartCertLoader(Profile* user_profile) = 0;
 };
 
 }  // namespace chromeos

diff --git a/chrome/browser/chromeos/login/mock_login_utils.h b/chrome/browser/chromeos/login/mock_login_utils.h
@@ -46,6 +46,7 @@ class MockLoginUtils : public LoginUtils {
   MOCK_METHOD2(TransferDefaultAuthCache, void(Profile*, Profile*));
   MOCK_METHOD0(StopBackgroundFetchers, void(void));
   MOCK_METHOD1(InitRlzDelayed, void(Profile*));
+  MOCK_METHOD1(StartCertLoader, void(Profile*));
 
   void DelegateToFake();
   FakeLoginUtils* GetFakeLoginUtils();

diff --git a/chrome/browser/chromeos/login/parallel_authenticator_unittest.cc b/chrome/browser/chromeos/login/parallel_authenticator_unittest.cc
@@ -321,10 +321,8 @@ TEST_F(ParallelAuthenticatorTest, ResolveOwnerNeededFailedMount) {
             SetAndResolveState(auth_.get(), state_.release()));
   EXPECT_TRUE(LoginState::Get()->IsInSafeMode());
 
-  // Simulate TPM token ready event. The tpm token parameters are not
-  // actually used by the DeviceSettingsService, so it is OK to pass arbitrary
-  // values.
-  DeviceSettingsService::Get()->OnTPMTokenReady("pin", "token_name", 0);
+  // Simulate TPM token ready event.
+  DeviceSettingsService::Get()->OnTPMTokenReady();
 
   // Flush all the pending operations. The operations should induce an owner
   // verification.

diff --git a/chrome/browser/chromeos/login/test_login_utils.cc b/chrome/browser/chromeos/login/test_login_utils.cc
@@ -41,4 +41,7 @@ scoped_refptr<Authenticator> TestLoginUtils::CreateAuthenticator(
 void TestLoginUtils::InitRlzDelayed(Profile* user_profile) {
 }
 
+void TestLoginUtils::StartCertLoader(Profile* user_profile) {
+}
+
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/login/test_login_utils.h b/chrome/browser/chromeos/login/test_login_utils.h
@@ -46,6 +46,8 @@ class TestLoginUtils : public LoginUtils {
 
   virtual void InitRlzDelayed(Profile* user_profile) OVERRIDE;
 
+  virtual void StartCertLoader(Profile* user_profile) OVERRIDE;
+
  private:
   std::string expected_username_;
   std::string expected_password_;

diff --git a/chrome/browser/chromeos/options/cert_library.cc b/chrome/browser/chromeos/options/cert_library.cc
@@ -36,7 +36,8 @@ namespace {
 // Root CA certificates that are built into Chrome use this token name.
 const char kRootCertificateTokenName[] = "Builtin Object Token";
 
-base::string16 GetDisplayString(net::X509Certificate* cert, bool hardware_backed) {
+base::string16 GetDisplayString(net::X509Certificate* cert,
+                                bool hardware_backed) {
   std::string org;
   if (!cert->subject().organization_names.empty())
     org = cert->subject().organization_names[0];
@@ -170,13 +171,8 @@ std::string CertLibrary::GetCertPkcs11IdAt(CertType type, int index) const {
 }
 
 bool CertLibrary::IsCertHardwareBackedAt(CertType type, int index) const {
-  if (!CertLoader::Get()->IsHardwareBacked())
-    return false;
   net::X509Certificate* cert = GetCertificateAt(type, index);
-  std::string cert_token_name =
-      x509_certificate_model::GetTokenName(cert->os_cert_handle());
-  return cert_token_name ==
-      CertLoader::Get()->tpm_token_name();
+  return CertLoader::Get()->IsCertificateHardwareBacked(cert);
 }
 
 int CertLibrary::GetCertIndexByPEM(CertType type,

diff --git a/chrome/browser/chromeos/settings/device_settings_service.cc b/chrome/browser/chromeos/settings/device_settings_service.cc
@@ -235,9 +235,7 @@ void DeviceSettingsService::PropertyChangeComplete(bool success) {
   EnsureReload(false);
 }
 
-void DeviceSettingsService::OnTPMTokenReady(const std::string& tpm_user_pin,
-                                            const std::string& tpm_token_name,
-                                            int tpm_token_slot_id) {
+void DeviceSettingsService::OnTPMTokenReady() {
   waiting_for_tpm_token_ = false;
 
   // TPMTokenLoader initializes the TPM and NSS database which is necessary to

diff --git a/chrome/browser/chromeos/settings/device_settings_service.h b/chrome/browser/chromeos/settings/device_settings_service.h
@@ -197,9 +197,7 @@ class DeviceSettingsService : public SessionManagerClient::Observer,
   virtual void PropertyChangeComplete(bool success) OVERRIDE;
 
   // TPMTokenLoader::Observer:
-  virtual void OnTPMTokenReady(const std::string& tpm_user_pin,
-                               const std::string& tpm_token_name,
-                               int tpm_token_slot_id) OVERRIDE;
+  virtual void OnTPMTokenReady() OVERRIDE;
 
  private:
   // Enqueues a new operation. Takes ownership of |operation| and starts it

diff --git a/chrome/browser/chromeos/settings/device_settings_service_unittest.cc b/chrome/browser/chromeos/settings/device_settings_service_unittest.cc
@@ -349,7 +349,7 @@ TEST_F(DeviceSettingsServiceTest, OnTPMTokenReadyForNonOwner) {
             device_settings_service_.GetOwnershipStatus());
   EXPECT_FALSE(is_owner_set_);
 
-  device_settings_service_.OnTPMTokenReady("tpm_pin", "tpm_token", 0);
+  device_settings_service_.OnTPMTokenReady();
   FlushDeviceSettings();
 
   EXPECT_FALSE(device_settings_service_.HasPrivateOwnerKey());
@@ -392,7 +392,7 @@ TEST_F(DeviceSettingsServiceTest, OnTPMTokenReadyForOwner) {
 
   owner_key_util_->SetPrivateKey(device_policy_.GetSigningKey());
   device_settings_service_.SetUsername(device_policy_.policy_data().username());
-  device_settings_service_.OnTPMTokenReady("tpm_pin", "tpm_token_name", 0);
+  device_settings_service_.OnTPMTokenReady();
   FlushDeviceSettings();
 
   EXPECT_TRUE(device_settings_service_.HasPrivateOwnerKey());
@@ -420,7 +420,7 @@ TEST_F(DeviceSettingsServiceTest, IsCurrentUserOwnerAsyncWithLoadedCerts) {
   device_settings_service_.SetUsername(device_policy_.policy_data().username());
   ReloadDeviceSettings();
 
-  device_settings_service_.OnTPMTokenReady("tpm_pin", "tpm_token_name", 0);
+  device_settings_service_.OnTPMTokenReady();
   FlushDeviceSettings();
 
   EXPECT_TRUE(device_settings_service_.HasPrivateOwnerKey());

diff --git a/chrome/browser/net/nss_context.cc b/chrome/browser/net/nss_context.cc
@@ -0,0 +1,57 @@
+// 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 "chrome/browser/net/nss_context.h"
+
+#include "base/message_loop/message_loop_proxy.h"
+#include "chrome/browser/profiles/profile.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/resource_context.h"
+
+using content::BrowserThread;
+
+namespace {
+
+// Relays callback to the right message loop.
+void DidGetCertDBOnIOThread(
+    scoped_refptr<base::MessageLoopProxy> response_message_loop,
+    const base::Callback<void(net::NSSCertDatabase*)>& callback,
+    net::NSSCertDatabase* cert_db) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  response_message_loop->PostTask(FROM_HERE, base::Bind(callback, cert_db));
+}
+
+// Gets NSSCertDatabase for the resource context.
+void GetCertDBOnIOThread(
+    content::ResourceContext* context,
+    scoped_refptr<base::MessageLoopProxy> response_message_loop,
+    const base::Callback<void(net::NSSCertDatabase*)>& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  // Note that the callback will be used only if the cert database hasn't yet
+  // been initialized.
+  net::NSSCertDatabase* cert_db = GetNSSCertDatabaseForResourceContext(
+      context,
+      base::Bind(&DidGetCertDBOnIOThread, response_message_loop, callback));
+
+  if (cert_db)
+    DidGetCertDBOnIOThread(response_message_loop, callback, cert_db);
+}
+
+}  // namespace
+
+void GetNSSCertDatabaseForProfile(
+    Profile* profile,
+    const base::Callback<void(net::NSSCertDatabase*)>& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  BrowserThread::PostTask(BrowserThread::IO,
+                          FROM_HERE,
+                          base::Bind(&GetCertDBOnIOThread,
+                                     profile->GetResourceContext(),
+                                     base::MessageLoopProxy::current(),
+                                     callback));
+}
+
diff --git a/chrome/browser/net/nss_context.h b/chrome/browser/net/nss_context.h
@@ -11,6 +11,8 @@
 #include "base/compiler_specific.h"
 #include "crypto/scoped_nss_types.h"
 
+class Profile;
+
 namespace net {
 class NSSCertDatabase;
 }
@@ -44,4 +46,13 @@ net::NSSCertDatabase* GetNSSCertDatabaseForResourceContext(
     const base::Callback<void(net::NSSCertDatabase*)>& callback)
     WARN_UNUSED_RESULT;
 
+// Gets a pointer to the NSSCertDatabase for the user associated with |context|.
+// It's a wrapper around |GetNSSCertDatabaseForResourceContext| which makes
+// sure it's called on IO thread (with |profile|'s resource context). The
+// callback will be called on the originating message loop.
+// It's accessing profile, so it should be called on the UI thread.
+void GetNSSCertDatabaseForProfile(
+    Profile* profile,
+    const base::Callback<void(net::NSSCertDatabase*)>& callback);
+
 #endif  // CHROME_BROWSER_NET_NSS_CONTEXT_H_
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
@@ -1280,6 +1280,7 @@
         'browser/net/network_stats.h',
         'browser/net/nss_context_chromeos.cc',
         'browser/net/nss_context_linux.cc',
+        'browser/net/nss_context.cc',
         'browser/net/nss_context.h',
         'browser/net/preconnect.cc',
         'browser/net/preconnect.h',
@@ -3114,6 +3115,7 @@
             'browser/certificate_manager_model.h',
             'browser/net/nss_context_chromeos.cc',
             'browser/net/nss_context_linux.cc',
+            'browser/net/nss_context.cc',
             'browser/net/nss_context.h',
           ],
         }],