Kerberos: Remember active account

Stores the active Kerberos account in a user pref variable. Restores
Kerberos credentials on startup. In case there is no KerberosAccounts
policy and the daemon still sleeps, the call to restore credentials also
wakes it up, which might trigger an update of credentials.

BUG=chromium:952244
TEST=Manually tested on device

Change-Id: I08d0fc84646313851db5325ae59c7654416a5a11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660856
Commit-Queue: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Kush Sinha <sinhak@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672031}

chrome/browser/chromeos/kerberos/kerberos_credentials_manager.cc

@@ -265,17 +265,17 @@ KerberosCredentialsManager::Observer::Observer() = default;
 
 KerberosCredentialsManager::Observer::~Observer() = default;
 
-KerberosCredentialsManager::KerberosCredentialsManager(
-    PrefService* local_state,
-    const Profile* primary_profile)
+KerberosCredentialsManager::KerberosCredentialsManager(PrefService* local_state,
+                                                       Profile* primary_profile)
     : local_state_(local_state),
+      primary_profile_(primary_profile),
       kerberos_files_handler_(
           base::BindRepeating(&KerberosCredentialsManager::GetKerberosFiles,
                               base::Unretained(this))) {
   DCHECK(!g_instance);
   g_instance = this;
 
-  DCHECK(primary_profile);
+  DCHECK(primary_profile_);
   const user_manager::User* primary_user =
       chromeos::ProfileHelper::Get()->GetUserByProfile(primary_profile);
   DCHECK(primary_user);
@@ -328,6 +328,14 @@ KerberosCredentialsManager::KerberosCredentialsManager(
     UpdateAccountsFromPref();
   else
     policy_service_->AddObserver(policy::POLICY_DOMAIN_CHROME, this);
+
+  // Get Kerberos files if there is an active principal. This also wakes up the
+  // daemon, which is important as it starts background renewal processes.
+  if (!GetActivePrincipalName().empty()) {
+    VLOG(1) << "Waking up Kerberos (the daemon, not the 3-headed dog) and "
+               "refreshing credentials.";
+    GetKerberosFiles();
+  }
 }
 
 KerberosCredentialsManager::~KerberosCredentialsManager() {
@@ -351,6 +359,12 @@ void KerberosCredentialsManager::RegisterLocalStatePrefs(
   registry->RegisterListPref(prefs::kKerberosAccounts);
 }
 
+void KerberosCredentialsManager::RegisterProfilePrefs(
+    PrefRegistrySimple* registry) {
+  registry->RegisterStringPref(prefs::kKerberosActivePrincipalName,
+                               std::string());
+}
+
 // static
 KerberosCredentialsManager::ResultCallback
 KerberosCredentialsManager::EmptyResultCallback() {
@@ -428,13 +442,13 @@ void KerberosCredentialsManager::OnAddAccountRunnerDone(
   if (Succeeded(error)) {
     // Don't change the active account if an account is added by policy.
     if (!is_managed)
-      active_principal_name_ = updated_principal;
+      SetActivePrincipalName(updated_principal);
 
     // Set active account.
     // TODO(https://crbug.com/948121): Wait until the files have been saved.
     // This is important when this code is triggered directly through a page
     // that requires Kerberos auth.
-    if (active_principal_name_ == updated_principal)
+    if (GetActivePrincipalName() == updated_principal)
       GetKerberosFiles();
 
     // Bring the merry news to the observers, but only if there is no
@@ -466,9 +480,9 @@ void KerberosCredentialsManager::OnRemoveAccount(
   LogError("RemoveAccount", response.error());
   if (Succeeded(response.error())) {
     // Clear out active credentials.
-    if (active_principal_name_ == principal_name) {
+    if (GetActivePrincipalName() == principal_name) {
       kerberos_files_handler_.DeleteFiles();
-      active_principal_name_.clear();
+      ClearActivePrincipalName();
     }
 
     // Express our condolence to the observers.
@@ -492,7 +506,7 @@ void KerberosCredentialsManager::OnClearAccounts(
   if (Succeeded(response.error())) {
     // Clear out active credentials.
     kerberos_files_handler_.DeleteFiles();
-    active_principal_name_.clear();
+    ClearActivePrincipalName();
 
     // Tattle on the lost accounts to the observers.
     NotifyAccountsChanged();
@@ -512,6 +526,8 @@ void KerberosCredentialsManager::OnListAccounts(
     ListAccountsCallback callback,
     const kerberos::ListAccountsResponse& response) {
   LogError("ListAccounts", response.error());
+  // Lazily validate principal here.
+  ValidateActivePrincipal(response);
   std::move(callback).Run(response);
 }
 
@@ -522,7 +538,7 @@ kerberos::ErrorType KerberosCredentialsManager::SetActiveAccount(
 
   // Don't early out if names are equal, this might be required to bootstrap
   // Kerberos credentials.
-  active_principal_name_ = principal_name;
+  SetActivePrincipalName(principal_name);
   GetKerberosFiles();
   NotifyAccountsChanged();
   return kerberos::ERROR_NONE;
@@ -577,11 +593,13 @@ void KerberosCredentialsManager::OnAcquireKerberosTgt(
 }
 
 void KerberosCredentialsManager::GetKerberosFiles() {
-  if (active_principal_name_.empty())
+  if (GetActivePrincipalName().empty())
     return;
 
+  VLOG(1) << "Refreshing credentials for " << GetActivePrincipalName();
+
   kerberos::GetKerberosFilesRequest request;
-  request.set_principal_name(active_principal_name_);
+  request.set_principal_name(GetActivePrincipalName());
   KerberosClient::Get()->GetKerberosFiles(
       request,
       base::BindOnce(&KerberosCredentialsManager::OnGetKerberosFiles,
@@ -596,22 +614,28 @@ void KerberosCredentialsManager::OnGetKerberosFiles(
     return;
 
   // Ignore if the principal changed in the meantime.
-  if (active_principal_name_ != principal_name) {
+  if (GetActivePrincipalName() != principal_name) {
     VLOG(1) << "Ignoring Kerberos files. Active principal changed from "
-            << principal_name << " to " << active_principal_name_;
+            << principal_name << " to " << GetActivePrincipalName();
     return;
   }
 
-  auto nullstr = base::Optional<std::string>();
-  kerberos_files_handler_.SetFiles(
-      response.files().has_krb5cc() ? response.files().krb5cc() : nullstr,
-      response.files().has_krb5conf() ? response.files().krb5conf() : nullstr);
+  // In case the credential cache is missing, remove the files. This could
+  // happen when switching from an account with ticket to an account without
+  // ticket. In that case, the files must go.
+  if (response.files().has_krb5cc()) {
+    DCHECK(response.files().has_krb5conf());
+    kerberos_files_handler_.SetFiles(response.files().krb5cc(),
+                                     response.files().krb5conf());
+  } else {
+    kerberos_files_handler_.DeleteFiles();
+  }
 }
 
 void KerberosCredentialsManager::OnKerberosFilesChanged(
     const std::string& principal_name) {
   // Only listen to the active account.
-  if (principal_name == active_principal_name_)
+  if (principal_name == GetActivePrincipalName())
     GetKerberosFiles();
 }
 
@@ -620,6 +644,39 @@ void KerberosCredentialsManager::NotifyAccountsChanged() {
     observer.OnAccountsChanged();
 }
 
+const std::string& KerberosCredentialsManager::GetActivePrincipalName() const {
+  // Using Get()->GetString() instead of GetString() directly to prevent a
+  // string copy.
+  return primary_profile_->GetPrefs()
+      ->Get(prefs::kKerberosActivePrincipalName)
+      ->GetString();
+}
+
+void KerberosCredentialsManager::SetActivePrincipalName(
+    const std::string& principal_name) {
+  primary_profile_->GetPrefs()->SetString(prefs::kKerberosActivePrincipalName,
+                                          principal_name);
+}
+
+void KerberosCredentialsManager::ClearActivePrincipalName() {
+  primary_profile_->GetPrefs()->ClearPref(prefs::kKerberosActivePrincipalName);
+}
+
+void KerberosCredentialsManager::ValidateActivePrincipal(
+    const kerberos::ListAccountsResponse& response) {
+  const std::string& active_principal = GetActivePrincipalName();
+  bool found = false;
+  for (int n = 0; n < response.accounts_size() && !found; ++n)
+    found |= response.accounts(n).principal_name() == active_principal;
+  if (!found) {
+    LOG(ERROR) << "Active principal does not exist. Restoring.";
+    if (response.accounts_size() > 0)
+      SetActivePrincipalName(response.accounts(0).principal_name());
+    else
+      ClearActivePrincipalName();
+  }
+}
+
 void KerberosCredentialsManager::UpdateEnabledFromPref() {
   const bool enabled = local_state_->GetBoolean(prefs::kKerberosEnabled);
   if (!enabled) {
@@ -668,8 +725,8 @@ void KerberosCredentialsManager::UpdateAccountsFromPref() {
     }
 
     // Kickstart active principal if it's not set yet.
-    if (active_principal_name_.empty())
-      active_principal_name_ = principal;
+    if (GetActivePrincipalName().empty())
+      SetActivePrincipalName(principal);
 
     // Get the password, default to not set.
     const std::string* password_str = account.FindStringKey(kPassword);

chrome/browser/chromeos/kerberos/kerberos_credentials_manager.h

@@ -52,7 +52,7 @@ class KerberosCredentialsManager : public policy::PolicyService::Observer {
   };
 
   KerberosCredentialsManager(PrefService* local_state,
-                             const Profile* primary_profile);
+                             Profile* primary_profile);
   ~KerberosCredentialsManager() override;
 
   // Singleton accessor. Available once the primary profile is available.
@@ -62,6 +62,9 @@ class KerberosCredentialsManager : public policy::PolicyService::Observer {
   // Registers prefs stored in local state.
   static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
 
+  // Registers prefs stored in user profiles.
+  static void RegisterProfilePrefs(PrefRegistrySimple* registry);
+
   // Helper method for ignoring the results of method calls.
   static ResultCallback EmptyResultCallback();
 
@@ -128,7 +131,9 @@ class KerberosCredentialsManager : public policy::PolicyService::Observer {
   kerberos::ErrorType SetActiveAccount(std::string principal_name);
 
   // Returns the currently active account or an empty string if there is none.
-  const std::string& GetActiveAccount() { return active_principal_name_; }
+  const std::string& GetActiveAccount() const {
+    return GetActivePrincipalName();
+  }
 
  private:
   friend class KerberosAddAccountRunner;
@@ -175,6 +180,17 @@ class KerberosCredentialsManager : public policy::PolicyService::Observer {
   // Calls OnAccountsChanged() on all observers.
   void NotifyAccountsChanged();
 
+  // Accessors for active principal (stored in user pref).
+  const std::string& GetActivePrincipalName() const;
+  void SetActivePrincipalName(const std::string& principal_name);
+  void ClearActivePrincipalName();
+
+  // Checks whether the active principal is contained in the given |response|.
+  // If not, resets it to the first principal or clears it if the list is empty.
+  // It's not expected that this ever triggers, but it provides a fail safe if
+  // the active principal should ever break for whatever reason.
+  void ValidateActivePrincipal(const kerberos::ListAccountsResponse& response);
+
   // Pref change handlers.
   void UpdateEnabledFromPref();
   void UpdateRememberPasswordEnabledFromPref();
@@ -190,6 +206,9 @@ class KerberosCredentialsManager : public policy::PolicyService::Observer {
   // Local state prefs, not owned.
   PrefService* local_state_ = nullptr;
 
+  // Primary profile, not owned.
+  Profile* primary_profile_ = nullptr;
+
   // Policy service of the primary profile, not owned.
   policy::PolicyService* policy_service_ = nullptr;
 
@@ -202,9 +221,6 @@ class KerberosCredentialsManager : public policy::PolicyService::Observer {
   // Keeps track of accounts currently being added.
   std::vector<std::unique_ptr<KerberosAddAccountRunner>> add_account_runners_;
 
-  // Currently active principal.
-  std::string active_principal_name_;
-
   // Variable expander for the principal name (replaces ${LOGIN_ID} etc.).
   std::unique_ptr<VariableExpander> principal_expander_;
 

chrome/browser/prefs/browser_prefs.cc

@@ -845,6 +845,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry,
   chromeos::CupsPrintersManager::RegisterProfilePrefs(registry);
   chromeos::first_run::RegisterProfilePrefs(registry);
   chromeos::file_system_provider::RegisterProfilePrefs(registry);
+  chromeos::KerberosCredentialsManager::RegisterProfilePrefs(registry);
   chromeos::KeyPermissions::RegisterProfilePrefs(registry);
   chromeos::multidevice_setup::MultiDeviceSetupService::RegisterProfilePrefs(
       registry);

chrome/common/pref_names.cc

@@ -936,12 +936,19 @@ const char kDeviceWallpaperImageFilePath[] =
     "policy.device_wallpaper_image_file_path";
 
 // Boolean whether Kerberos daemon supports remembering passwords.
+// Tied to KerberosRememberPasswordEnabled policy.
 const char kKerberosRememberPasswordEnabled[] =
     "kerberos.remember_password_enabled";
 // Boolean whether users may add new Kerberos accounts.
+// Tied to KerberosAddAccountsAllowed policy.
 const char kKerberosAddAccountsAllowed[] = "kerberos.add_accounts_allowed";
 // Dictionary specifying a pre-set list of Kerberos accounts.
+// Tied to KerberosAccounts policy.
 const char kKerberosAccounts[] = "kerberos.accounts";
+// Used by KerberosCredentialsManager to remember which account is currently
+// active (empty if none) and to determine whether to wake up the Kerberos
+// daemon on session startup.
+const char kKerberosActivePrincipalName[] = "kerberos.active_principal_name";
 
 // A boolean pref for enabling/disabling App reinstall recommendations in Zero
 // State Launcher by policy.

chrome/common/pref_names.h

@@ -305,6 +305,7 @@ extern const char kDeviceWallpaperImageFilePath[];
 extern const char kKerberosRememberPasswordEnabled[];
 extern const char kKerberosAddAccountsAllowed[];
 extern const char kKerberosAccounts[];
+extern const char kKerberosActivePrincipalName[];
 extern const char kAppReinstallRecommendationEnabled[];
 extern const char kStartupBrowserWindowLaunchSuppressed[];
 extern const char kDeviceWebUsbAllowDevicesForUrls[];