Enable client cert auth for sign-in frame StoragePartition

Enable client certificate authentication in the sign-in profile for
the StoragePartition which is currently used by the sign-in frame.
Additionally, wire up the DeviceLoginScreenAutoSelectCertificateForUrls
policy to content settings on the sign-in screen.

BUG=723849
TEST=browser_tests --gtest_filter=WebViewClientCertsLoginTest.*

Change-Id: Ic5345bc3446c621008088909771c6eca445aa3f3
Reviewed-on: https://chromium-review.googlesource.com/790295
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521232}

chrome/browser/chrome_content_browser_client.cc

@@ -270,8 +270,10 @@
 #include "chrome/browser/chromeos/fileapi/mtp_file_system_backend_delegate.h"
 #include "chrome/browser/chromeos/login/signin/merge_session_navigation_throttle.h"
 #include "chrome/browser/chromeos/login/signin/merge_session_throttling_utils.h"
+#include "chrome/browser/chromeos/login/signin_partition_manager.h"
 #include "chrome/browser/chromeos/login/startup_utils.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/system/input_device_settings.h"
 #include "chrome/browser/metrics/leak_detector/leak_detector_remote_controller.h"
 #include "chrome/browser/ui/browser_dialogs.h"
@@ -2373,8 +2375,40 @@ void ChromeContentBrowserClient::SelectClientCertificate(
       << "Invalid URL string: https://"
       << cert_request_info->host_and_port.ToString();
 
+  bool may_show_cert_selection = true;
+
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
+#if defined(OS_CHROMEOS)
+  if (chromeos::ProfileHelper::IsSigninProfile(profile)) {
+    // TODO(pmarko): crbug.com/723849: Set |may_show_cert_selection| to false
+    // and remove the command-line flag after prototype phase when the
+    // DeviceLoginScreenAutoSelectCertificateForUrls policy is live.
+    may_show_cert_selection =
+        chromeos::switches::IsSigninFrameClientCertUserSelectionEnabled();
+
+    content::StoragePartition* storage_partition =
+        content::BrowserContext::GetStoragePartition(
+            profile, web_contents->GetSiteInstance());
+    chromeos::login::SigninPartitionManager* signin_partition_manager =
+        chromeos::login::SigninPartitionManager::Factory::GetForBrowserContext(
+            profile);
+
+    // On the sign-in profile, only allow client certs in the context of the
+    // sign-in frame.
+    if (!signin_partition_manager->IsCurrentSigninStoragePartition(
+            storage_partition)) {
+      LOG(WARNING)
+          << "Client cert requested in sign-in profile in wrong context.";
+      // Continue without client certificate. We do this to mimic the case of no
+      // client certificate being present in the profile's certificate store.
+      delegate->ContinueWithCertificate(nullptr, nullptr);
+      return;
+    }
+    VLOG(1) << "Client cert requested in sign-in profile.";
+  }
+#endif  // defined(OS_CHROMEOS)
+
   std::unique_ptr<base::Value> filter =
       HostContentSettingsMapFactory::GetForProfile(profile)->GetWebsiteSetting(
           requesting_url, requesting_url,
@@ -2406,6 +2440,15 @@ void ChromeContentBrowserClient::SelectClientCertificate(
     }
   }
 
+  if (!may_show_cert_selection) {
+    LOG(WARNING) << "No client cert matched by policy and user selection is "
+                    "not allowed.";
+    // Continue without client certificate. We do this to mimic the case of no
+    // client certificate being present in the profile's certificate store.
+    delegate->ContinueWithCertificate(nullptr, nullptr);
+    return;
+  }
+
   chrome::ShowSSLClientCertificateSelector(web_contents, cert_request_info,
                                            std::move(client_certs),
                                            std::move(delegate));

chrome/browser/chromeos/login/signin_partition_manager.cc

@@ -7,6 +7,7 @@
 #include "base/guid.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
+#include "chromeos/chromeos_switches.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/storage_partition.h"
@@ -70,13 +71,9 @@ void SigninPartitionManager::StartSigninSession(
   GURL guest_site = GetGuestSiteURL(storage_partition_domain_,
                                     current_storage_partition_name_);
 
-  // Prepare the StoragePartition
   current_storage_partition_ =
       content::BrowserContext::GetStoragePartitionForSite(browser_context_,
                                                           guest_site, true);
-
-  // TODO(pmarko): crbug.com/723849: Set UserData on |current_storage_partition|
-  // to allow client certificates.
 }
 
 void SigninPartitionManager::CloseCurrentSigninSession(
@@ -112,6 +109,11 @@ SigninPartitionManager::GetCurrentStoragePartition() {
   return current_storage_partition_;
 }
 
+bool SigninPartitionManager::IsCurrentSigninStoragePartition(
+    const content::StoragePartition* storage_partition) const {
+  return IsInSigninSession() && storage_partition == current_storage_partition_;
+}
+
 SigninPartitionManager::Factory::Factory()
     : BrowserContextKeyedServiceFactory(
           "SigninPartitionManager",

chrome/browser/chromeos/login/signin_partition_manager.h

@@ -54,12 +54,16 @@ class SigninPartitionManager : public KeyedService {
   // that is between StartSigninSession and CloseCurrentSigninSession calls.
   const std::string& GetCurrentStoragePartitionName() const;
 
-  // Returns the current StoragePartition. May only be called after
-  // StartSigninSession has been called. May only be called when a sign-in
+  // Returns the current StoragePartition. May only be called when a sign-in
   // session is active, that is between StartSigninSession and
   // CloseCurrentSigninSession calls.
   content::StoragePartition* GetCurrentStoragePartition();
 
+  // Returns true if |storage_partition| is the partition in use by the current
+  // sign-in session. Returns false if no sign-in session is active.
+  bool IsCurrentSigninStoragePartition(
+      const content::StoragePartition* storage_partition) const;
+
   void SetClearStoragePartitionTaskForTesting(
       ClearStoragePartitionTask clear_storage_partition_task);
 

chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc

@@ -377,12 +377,26 @@ void DecodeLoginPolicies(const em::ChromeDeviceSettingsProto& policy,
     const em::LoginScreenInputMethodsProto& login_screen_input_methods(
         policy.login_screen_input_methods());
     for (const auto& input_method :
-         login_screen_input_methods.login_screen_input_methods())
+         login_screen_input_methods.login_screen_input_methods()) {
       input_methods->AppendString(input_method);
+    }
     policies->Set(key::kDeviceLoginScreenInputMethods, POLICY_LEVEL_MANDATORY,
                   POLICY_SCOPE_MACHINE, POLICY_SOURCE_CLOUD,
                   std::move(input_methods), nullptr);
   }
+
+  if (policy.has_device_login_screen_auto_select_certificate_for_urls()) {
+    std::unique_ptr<base::ListValue> rules(new base::ListValue);
+    const em::DeviceLoginScreenAutoSelectCertificateForUrls& proto_rules(
+        policy.device_login_screen_auto_select_certificate_for_urls());
+    for (const auto& rule :
+         proto_rules.login_screen_auto_select_certificate_rules()) {
+      rules->AppendString(rule);
+    }
+    policies->Set(key::kDeviceLoginScreenAutoSelectCertificateForUrls,
+                  POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
+                  POLICY_SOURCE_CLOUD, std::move(rules), nullptr);
+  }
 }
 
 void DecodeNetworkPolicies(const em::ChromeDeviceSettingsProto& policy,

chrome/browser/chromeos/policy/login_profile_policy_provider.cc

@@ -79,6 +79,17 @@ void ApplyValueAsMandatoryPolicy(const base::Value* value,
   }
 }
 
+// Applies the value of |device_policy| in |device_policy_map| as the
+// mandatory value of |user_policy| in |user_policy_map|. If the value of
+// |device_policy| is unset, does nothing.
+void ApplyDevicePolicyAsMandatoryPolicy(const std::string& device_policy,
+                                        const std::string& user_policy,
+                                        const PolicyMap& device_policy_map,
+                                        PolicyMap* user_policy_map) {
+  const base::Value* value = device_policy_map.GetValue(device_policy);
+  ApplyValueAsMandatoryPolicy(value, user_policy, user_policy_map);
+}
+
 }  // namespace
 
 LoginProfilePolicyProvider::LoginProfilePolicyProvider(
@@ -162,6 +173,10 @@ void LoginProfilePolicyProvider::UpdateFromDevicePolicy() {
       key::kVirtualKeyboardEnabled,
       device_policy_map, &user_policy_map);
 
+  ApplyDevicePolicyAsMandatoryPolicy(
+      key::kDeviceLoginScreenAutoSelectCertificateForUrls,
+      key::kAutoSelectCertificateForUrls, device_policy_map, &user_policy_map);
+
   const base::Value* value =
       device_policy_map.GetValue(key::kDeviceLoginScreenPowerManagement);
   const base::DictionaryValue* dict = NULL;

chrome/browser/profiles/profile_io_data.cc

@@ -146,6 +146,7 @@
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/net/nss_context.h"
+#include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/cryptohome_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/settings/cros_settings_names.h"
@@ -463,6 +464,18 @@ void ProfileIOData::InitializeOnUIThread(Profile* profile) {
 #endif
 
 #if defined(OS_CHROMEOS)
+  // Enable client certificates for the Chrome OS sign-in frame, if this feature
+  // is not disabled by a flag.
+  // Note that while this applies to the whole sign-in profile, client
+  // certificates will only be selected for the StoragePartition currently used
+  // in the sign-in frame (see SigninPartitionManager).
+  if (chromeos::switches::IsSigninFrameClientCertsEnabled() &&
+      chromeos::ProfileHelper::IsSigninProfile(profile)) {
+    // We only need the system slot for client certificates, not in NSS context
+    // (the sign-in profile's NSS context is not initialized).
+    params->system_key_slot_use_type = SystemKeySlotUseType::kUseForClientAuth;
+  }
+
   user_manager::UserManager* user_manager = user_manager::UserManager::Get();
   if (user_manager) {
     const user_manager::User* user =
@@ -481,7 +494,10 @@ void ProfileIOData::InitializeOnUIThread(Profile* profile) {
 
       // Use the device-wide system key slot only if the user is affiliated on
       // the device.
-      params->use_system_key_slot = user->IsAffiliated();
+      if (user->IsAffiliated()) {
+        params->system_key_slot_use_type =
+            SystemKeySlotUseType::kUseForClientAuthAndCertManagement;
+      }
     }
   }
 
@@ -644,7 +660,7 @@ ProfileIOData::AppRequestContext::~AppRequestContext() {
 ProfileIOData::ProfileParams::ProfileParams()
     : io_thread(NULL),
 #if defined(OS_CHROMEOS)
-      use_system_key_slot(false),
+      system_key_slot_use_type(SystemKeySlotUseType::kNone),
 #endif
       profile(NULL) {
 }
@@ -654,7 +670,7 @@ ProfileIOData::ProfileParams::~ProfileParams() {}
 ProfileIOData::ProfileIOData(Profile::ProfileType profile_type)
     : initialized_(false),
 #if defined(OS_CHROMEOS)
-      use_system_key_slot_(false),
+      system_key_slot_use_type_(SystemKeySlotUseType::kNone),
 #endif
       main_request_context_(nullptr),
       resource_context_(new ResourceContext(this)),
@@ -974,11 +990,15 @@ std::unique_ptr<net::ClientCertStore> ProfileIOData::CreateClientCertStore() {
   if (!client_cert_store_factory_.is_null())
     return client_cert_store_factory_.Run();
 #if defined(OS_CHROMEOS)
+  bool use_system_key_slot =
+      system_key_slot_use_type_ == SystemKeySlotUseType::kUseForClientAuth ||
+      system_key_slot_use_type_ ==
+          SystemKeySlotUseType::kUseForClientAuthAndCertManagement;
   return std::unique_ptr<net::ClientCertStore>(
       new chromeos::ClientCertStoreChromeOS(
           certificate_provider_ ? certificate_provider_->Copy() : nullptr,
           base::MakeUnique<chromeos::ClientCertFilterChromeOS>(
-              use_system_key_slot_, username_hash_),
+              use_system_key_slot, username_hash_),
           base::Bind(&CreateCryptoModuleBlockingPasswordDelegate,
                      kCryptoModulePasswordClientAuth)));
 #elif defined(USE_NSS_CERTS)
@@ -1123,9 +1143,16 @@ void ProfileIOData::Init(
 
 #if defined(OS_CHROMEOS)
   username_hash_ = profile_params_->username_hash;
-  use_system_key_slot_ = profile_params_->use_system_key_slot;
-  if (use_system_key_slot_)
+  system_key_slot_use_type_ = profile_params_->system_key_slot_use_type;
+  // If we're using the system slot for certificate management, we also must
+  // have access to the user's slots.
+  DCHECK(!(username_hash_.empty() &&
+           system_key_slot_use_type_ ==
+               SystemKeySlotUseType::kUseForClientAuthAndCertManagement));
+  if (system_key_slot_use_type_ ==
+      SystemKeySlotUseType::kUseForClientAuthAndCertManagement) {
     EnableNSSSystemKeySlotForResourceContext(resource_context_.get());
+  }
 
   certificate_provider_ = std::move(profile_params_->certificate_provider);
 #endif

chrome/browser/profiles/profile_io_data.h

@@ -259,6 +259,20 @@ class ProfileIOData {
   std::unique_ptr<net::ClientCertStore> CreateClientCertStore();
 
  protected:
+#if defined(OS_CHROMEOS)
+  // Defines possible ways in which a profile may use the Chrome OS system
+  // token.
+  enum class SystemKeySlotUseType {
+    // This profile does not use the system key slot.
+    kNone,
+    // This profile only uses the system key slot for client certiticates.
+    kUseForClientAuth,
+    // This profile uses the system key slot for client certificates and for
+    // certificate management.
+    kUseForClientAuthAndCertManagement
+  };
+#endif
+
   // A URLRequestContext for media that owns its HTTP factory, to ensure
   // it is deleted.
   class MediaRequestContext : public net::URLRequestContext {
@@ -343,7 +357,7 @@ class ProfileIOData {
 #if defined(OS_CHROMEOS)
     std::unique_ptr<policy::PolicyCertVerifier> policy_cert_verifier;
     std::string username_hash;
-    bool use_system_key_slot;
+    SystemKeySlotUseType system_key_slot_use_type;
     std::unique_ptr<chromeos::CertificateProvider> certificate_provider;
 #endif
 
@@ -583,7 +597,7 @@ class ProfileIOData {
   mutable std::unique_ptr<ChromeExpectCTReporter> expect_ct_reporter_;
 #if defined(OS_CHROMEOS)
   mutable std::string username_hash_;
-  mutable bool use_system_key_slot_;
+  mutable SystemKeySlotUseType system_key_slot_use_type_;
   mutable std::unique_ptr<chromeos::CertificateProvider> certificate_provider_;
 #endif
 

chromeos/chromeos_switches.cc

@@ -532,6 +532,22 @@ const char kDisablePerUserTimezone[] = "disable-per-user-timezone";
 const char kDisableFineGrainedTimeZoneDetection[] =
     "disable-fine-grained-time-zone-detection";
 
+// Disables client certificate authentication on the sign-in frame on the Chrome
+// OS sign-in profile.
+// TODO(pmarko): Remove this flag in M-66 if no issues are found
+// (crbug.com/723849).
+const char kDisableSigninFrameClientCerts[] =
+    "disable-signin-frame-client-certs";
+
+// Disables user selection of client certificate on the sign-in frame on the
+// Chrome OS sign-in profile.
+// TODO(pmarko): Remove this flag in M-65 when the
+// DeviceLoginScreenAutoSelectCertificateForUrls policy is enabled on the server
+// side (crbug.com/723849) and completely disable user selection of certificates
+// on the sign-in frame.
+const char kDisableSigninFrameClientCertUserSelection[] =
+    "disable-signin-frame-client-cert-user-selection";
+
 bool WakeOnWifiEnabled() {
   return !base::CommandLine::ForCurrentProcess()->HasSwitch(kDisableWakeOnWifi);
 }
@@ -630,5 +646,15 @@ bool IsZipArchiverUnpackerEnabled() {
       kEnableZipArchiverUnpacker);
 }
 
+bool IsSigninFrameClientCertsEnabled() {
+  return !base::CommandLine::ForCurrentProcess()->HasSwitch(
+      kDisableSigninFrameClientCerts);
+}
+
+bool IsSigninFrameClientCertUserSelectionEnabled() {
+  return !base::CommandLine::ForCurrentProcess()->HasSwitch(
+      kDisableSigninFrameClientCertUserSelection);
+}
+
 }  // namespace switches
 }  // namespace chromeos

chromeos/chromeos_switches.h

@@ -99,6 +99,8 @@ CHROMEOS_EXPORT extern const char kEnableTouchpadThreeFingerClick[];
 CHROMEOS_EXPORT extern const char kEnableFileManagerTouchMode[];
 CHROMEOS_EXPORT extern const char kDisableFileManagerTouchMode[];
 CHROMEOS_EXPORT extern const char kDisableFineGrainedTimeZoneDetection[];
+CHROMEOS_EXPORT extern const char kDisableSigninFrameClientCerts[];
+CHROMEOS_EXPORT extern const char kDisableSigninFrameClientCertUserSelection[];
 CHROMEOS_EXPORT extern const char kEnableVideoPlayerChromecastSupport[];
 CHROMEOS_EXPORT extern const char kEnableVoiceInteraction[];
 CHROMEOS_EXPORT extern const char kEnableZipArchiverPacker[];
@@ -180,6 +182,14 @@ CHROMEOS_EXPORT bool IsVoiceInteractionEnabled();
 // Returns true if Zip Archiver is enabled for unpacking files.
 CHROMEOS_EXPORT bool IsZipArchiverUnpackerEnabled();
 
+// Returns true if client certificate authentication for the sign-in frame on
+// the Chrome OS sign-in screen is enabled.
+CHROMEOS_EXPORT bool IsSigninFrameClientCertsEnabled();
+
+// Returns true if user selection of certificates is enabled for the sign-in
+// frame on the Chrome OS sign-in screen.
+CHROMEOS_EXPORT bool IsSigninFrameClientCertUserSelectionEnabled();
+
 }  // namespace switches
 }  // namespace chromeos