Skip to content

Commit

Permalink
Make sure the device recovers from policy loss in the consumer case.
Browse files Browse the repository at this point in the history
Lock down login to the owner only and restore policy on successful owner login.

The machine is forced to restart if the owner didn't login because we have
no better way to clear the key store loaded for another user.

BUG=chromium-os:26793
TEST=TBA. Manually: Delete the policy blob and verify that policy is recovered correctly.

Review URL: https://chromiumcodereview.appspot.com/9466005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128877 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
pastarmovj@chromium.org committed Mar 26, 2012
1 parent 369ab5a commit 124766c
Show file tree
Hide file tree
Showing 18 changed files with 299 additions and 48 deletions.
3 changes: 3 additions & 0 deletions chrome/app/generated_resources.grd
Expand Up @@ -15104,6 +15104,9 @@ Battery full
<message name="IDS_LOGIN_ERROR_CAPTIVE_PORTAL" desc="An error message shown when we suggest that user may be behind the captive portal.">
Before signing in, please enter as Guest to activate the network <ph name="NETWORK_ID">$1<ex>Public Wifi</ex></ph>
</message>
<message name="IDS_LOGIN_ERROR_OWNER_REQUIRED" desc="Couldn't sign in because the machine is in policy safe mode and needs owner login">
Login has been resticted to the owner account only. Please reboot and sign in with the owner account. The machine will auto reboot in 30 seconds.
</message>
<message name="IDS_LOGIN_FIX_CAPTIVE_PORTAL" desc="An link text to fix Internet connection (captive portal) in guest mode.">
Enter as Guest
</message>
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/chromeos/cros_settings_names.cc
Expand Up @@ -68,4 +68,8 @@ const char kIdleLogoutWarningDuration[] = "cros.idle_logout.warning_duration";
// if the device is in KIOSK mode.
const char kStartUpUrls[] = "cros.start_up_urls";

// This policy should not appear in the protobuf ever but is used internally to
// signal that we are running in a "safe-mode" for policy recovery.
const char kPolicyMissingMitigationMode[] =
"cros.internal.policy_mitigation_mode";
} // namespace chromeos
2 changes: 2 additions & 0 deletions chrome/browser/chromeos/cros_settings_names.h
Expand Up @@ -43,6 +43,8 @@ extern const char kIdleLogoutWarningDuration[];

extern const char kStartUpUrls[];

extern const char kPolicyMissingMitigationMode[];

} // namespace chromeos

#endif // CHROME_BROWSER_CHROMEOS_CROS_SETTINGS_NAMES_H_
12 changes: 12 additions & 0 deletions chrome/browser/chromeos/dbus/cryptohome_client.cc
Expand Up @@ -98,6 +98,12 @@ class CryptohomeClientImpl : public CryptohomeClient {
return CallMethodAndBlock(&method_call, base::Bind(&PopBool, is_mounted));
}

// CryptohomeClient override.
virtual bool Unmount(bool *success) OVERRIDE {
INITIALIZE_METHOD_CALL(method_call, cryptohome::kCryptohomeUnmount);
return CallMethodAndBlock(&method_call, base::Bind(&PopBool, success));
}

// CryptohomeClient override.
virtual void AsyncCheckKey(const std::string& username,
const std::string& key,
Expand Down Expand Up @@ -464,6 +470,12 @@ class CryptohomeClientStubImpl : public CryptohomeClient {
return true;
}

// CryptohomeClient override.
virtual bool Unmount(bool* success) OVERRIDE {
*success = true;
return true;
}

// CryptohomeClient override.
virtual void AsyncCheckKey(const std::string& username,
const std::string& key,
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/chromeos/dbus/cryptohome_client.h
Expand Up @@ -61,6 +61,10 @@ class CryptohomeClient {
// This method blocks until the call returns.
virtual bool IsMounted(bool* is_mounted) = 0;

// Calls Unmount method and returns true when the call succeeds.
// This method blocks until the call returns.
virtual bool Unmount(bool* success) = 0;

// Calls AsyncCheckKey method. |callback| is called after the method call
// succeeds.
virtual void AsyncCheckKey(const std::string& username,
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/chromeos/dbus/mock_cryptohome_client.h
Expand Up @@ -21,6 +21,7 @@ class MockCryptohomeClient : public CryptohomeClient {
MOCK_METHOD1(SetAsyncCallStatusHandler, void(AsyncCallStatusHandler handler));
MOCK_METHOD0(ResetAsyncCallStatusHandler, void());
MOCK_METHOD1(IsMounted, bool(bool* is_mounted));
MOCK_METHOD1(Unmount, bool(bool* success));
MOCK_METHOD3(AsyncCheckKey,
void(const std::string& username,
const std::string& key,
Expand Down
69 changes: 34 additions & 35 deletions chrome/browser/chromeos/device_settings_provider.cc
Expand Up @@ -22,6 +22,8 @@
#include "chrome/browser/chromeos/login/signed_settings_helper.h"
#include "chrome/browser/chromeos/login/user_manager.h"
#include "chrome/browser/policy/app_pack_updater.h"
#include "chrome/browser/policy/browser_policy_connector.h"
#include "chrome/browser/policy/cloud_policy_constants.h"
#include "chrome/browser/ui/options/options_util.h"
#include "chrome/common/chrome_notification_types.h"
#include "chrome/installer/util/google_update_settings.h"
Expand All @@ -46,6 +48,7 @@ const char* kKnownSettings[] = {
kDeviceOwner,
kIdleLogoutTimeout,
kIdleLogoutWarningDuration,
kPolicyMissingMitigationMode,
kReleaseChannel,
kReleaseChannelDelegated,
kReportDeviceActivityTimes,
Expand Down Expand Up @@ -604,40 +607,38 @@ void DeviceSettingsProvider::ApplySideEffects() const {
}

bool DeviceSettingsProvider::MitigateMissingPolicy() {
// As this code runs only in exceptional cases it's fine to allow I/O here.
base::ThreadRestrictions::ScopedAllowIO allow_io;
FilePath legacy_policy_file(kLegacyPolicyFile);
// Check if legacy file exists but is not writable to avoid possible
// attack of creating this file through chronos (although this should be
// not possible in root owned location), but better be safe than sorry.
// TODO(pastarmovj): Remove this workaround once we have proper checking
// for policy corruption or when Cr48 is phased out the very latest.
// See: http://crosbug.com/24916.
if (file_util::PathExists(legacy_policy_file) &&
!file_util::PathIsWritable(legacy_policy_file)) {
// We are in pre 11 dev upgrading to post 17 version mode.
LOG(ERROR) << "Detected system upgraded from ChromeOS 11 or older with "
<< "missing policies. Switching to migration policy mode "
<< "until the owner logs in to regenerate the policy data.";
// In this situation we should pretend we have policy even though we
// don't until the owner logs in and restores the policy blob.
values_cache_.SetBoolean(kAccountsPrefAllowNewUser, true);
values_cache_.SetBoolean(kAccountsPrefAllowGuest, true);
trusted_ = true;
// Make sure we will recreate the policy once the owner logs in.
// Any value not in this list will be left to the default which is fine as
// we repopulate the whitelist with the owner and any other possible every
// time the user enables whitelist filtering on the UI.
migration_helper_->AddMigrationValue(
kAccountsPrefAllowNewUser, base::Value::CreateBooleanValue(true));
migration_helper_->MigrateValues();
// The last step is to pretend we loaded policy correctly and call everyone.
for (size_t i = 0; i < callbacks_.size(); ++i)
callbacks_[i].Run();
callbacks_.clear();
return true;
// First check if the device has been owned already and if not exit
// immediately.
if (g_browser_process->browser_policy_connector()->GetDeviceMode() !=
policy::DEVICE_MODE_CONSUMER) {
return false;
}
return false;

// If we are here the policy file were corrupted or missing. This can happen
// because we are migrating Pre R11 device to the new secure policies or there
// was an attempt to circumvent policy system. In this case we should populate
// the policy cache with "safe-mode" defaults which should allow the owner to
// log in but lock the device for anyone else until the policy blob has been
// recreated by the session manager.
LOG(ERROR) << "Corruption of the policy data has been detected."
<< "Switching to \"safe-mode\" policies until the owner logs in "
<< "to regenerate the policy data.";
values_cache_.SetBoolean(kAccountsPrefAllowNewUser, true);
values_cache_.SetBoolean(kAccountsPrefAllowGuest, true);
values_cache_.SetBoolean(kPolicyMissingMitigationMode, true);
trusted_ = true;
// Make sure we will recreate the policy once the owner logs in.
// Any value not in this list will be left to the default which is fine as
// we repopulate the whitelist with the owner and all other existing users
// every time the owner enables whitelist filtering on the UI.
migration_helper_->AddMigrationValue(
kAccountsPrefAllowNewUser, base::Value::CreateBooleanValue(true));
migration_helper_->MigrateValues();
// The last step is to pretend we loaded policy correctly and call everyone.
for (size_t i = 0; i < callbacks_.size(); ++i)
callbacks_[i].Run();
callbacks_.clear();
return true;
}

const base::Value* DeviceSettingsProvider::Get(const std::string& path) const {
Expand Down Expand Up @@ -708,8 +709,6 @@ void DeviceSettingsProvider::OnRetrievePolicyCompleted(
break;
}
case SignedSettings::NOT_FOUND:
// Verify if we don't have to mitigate pre Chrome 12 machine here and if
// needed do the magic.
if (MitigateMissingPolicy())
break;
case SignedSettings::KEY_UNAVAILABLE: {
Expand Down
13 changes: 12 additions & 1 deletion chrome/browser/chromeos/login/existing_user_controller.cc
Expand Up @@ -92,6 +92,9 @@ const char kCaptivePortalLaunchURL[] = "http://www.google.com/";
// Delay for transferring the auth cache to the system profile.
const long int kAuthCacheTransferDelayMs = 2000;

// Delay for restarting the ui if safe-mode login has failed.
const long int kSafeModeRestartUiDelayMs = 30000;

// Makes a call to the policy subsystem to reload the policy when we detect
// authentication change.
void RefreshPoliciesOnUIThread() {
Expand Down Expand Up @@ -472,7 +475,15 @@ void ExistingUserController::OnLoginFailure(const LoginFailure& failure) {
guest_mode_url_ = GURL::EmptyGURL();
std::string error = failure.GetErrorString();

if (!online_succeeded_for_.empty()) {
if (failure.reason() == LoginFailure::OWNER_REQUIRED) {
ShowError(IDS_LOGIN_ERROR_OWNER_REQUIRED, error);
content::BrowserThread::PostDelayedTask(
content::BrowserThread::UI, FROM_HERE,
base::Bind(&SessionManagerClient::StopSession,
base::Unretained(DBusThreadManager::Get()->
GetSessionManagerClient())),
kSafeModeRestartUiDelayMs);
} else if (!online_succeeded_for_.empty()) {
ShowGaiaPasswordChanged(online_succeeded_for_);
} else {
// Check networking after trying to login in case user is
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/chromeos/login/login_status_consumer.h
Expand Up @@ -29,6 +29,7 @@ class LoginFailure {
LOGIN_TIMED_OUT,
UNLOCK_FAILED,
NETWORK_AUTH_FAILED, // Could not authenticate against Google
OWNER_REQUIRED, // Only the device owner can log-in.
NUM_FAILURE_REASONS, // This has to be the last item.
};

Expand Down Expand Up @@ -76,6 +77,8 @@ class LoginFailure {
return net::ErrorToString(error_.network_error());
}
return "Google authentication failed.";
case OWNER_REQUIRED:
return "Login is restricted to the owner's account only.";
default:
NOTREACHED();
return std::string();
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/chromeos/login/mock_user_manager.h
Expand Up @@ -42,6 +42,7 @@ class MockUserManager : public UserManager {
const FilePath&));
MOCK_METHOD1(SaveUserImageFromProfileImage, void(const std::string&));
MOCK_METHOD1(DownloadProfileImage, void(const std::string&));
MOCK_METHOD0(LoadKeyStore, void(void));
MOCK_CONST_METHOD0(IsCurrentUserOwner, bool(void));
MOCK_CONST_METHOD0(IsCurrentUserNew, bool(void));
MOCK_CONST_METHOD0(IsCurrentUserEphemeral, bool(void));
Expand Down
79 changes: 77 additions & 2 deletions chrome/browser/chromeos/login/parallel_authenticator.cc
Expand Up @@ -14,7 +14,10 @@
#include "chrome/browser/chromeos/boot_times_loader.h"
#include "chrome/browser/chromeos/cros/cros_library.h"
#include "chrome/browser/chromeos/cros/cryptohome_library.h"
#include "chrome/browser/chromeos/cros_settings.h"
#include "chrome/browser/chromeos/cryptohome/async_method_caller.h"
#include "chrome/browser/chromeos/dbus/cryptohome_client.h"
#include "chrome/browser/chromeos/dbus/dbus_thread_manager.h"
#include "chrome/browser/chromeos/login/authentication_notification_details.h"
#include "chrome/browser/chromeos/login/login_status_consumer.h"
#include "chrome/browser/chromeos/login/ownership_service.h"
Expand Down Expand Up @@ -169,6 +172,8 @@ ParallelAuthenticator::ParallelAuthenticator(LoginStatusConsumer* consumer)
mount_guest_attempted_(false),
check_key_attempted_(false),
already_reported_success_(false),
owner_is_verified_(false),
user_can_login_(false),
using_oauth_(
!CommandLine::ForCurrentProcess()->HasSwitch(
switches::kSkipOAuthLogin)) {
Expand All @@ -195,14 +200,19 @@ void ParallelAuthenticator::AuthenticateToLogin(
login_token,
login_captcha,
!UserManager::Get()->IsKnownUser(canonicalized)));
{
// Reset the verified flag.
base::AutoLock for_this_block(owner_verified_lock_);
owner_is_verified_ = false;
}

const bool create_if_missing = false;
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::Bind(&Mount,
current_state_.get(),
static_cast<AuthAttemptStateResolver*>(this),
create_if_missing));

// ClientLogin authentication check should happen immediately here.
// We should not try OAuthLogin check until the profile loads.
if (!using_oauth_) {
Expand All @@ -225,6 +235,12 @@ void ParallelAuthenticator::CompleteLogin(Profile* profile,
password,
CrosLibrary::Get()->GetCryptohomeLibrary()->HashPassword(password),
!UserManager::Get()->IsKnownUser(canonicalized)));
{
// Reset the verified flag.
base::AutoLock for_this_block(owner_verified_lock_);
owner_is_verified_ = false;
}

const bool create_if_missing = false;
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
Expand Down Expand Up @@ -377,6 +393,39 @@ void ParallelAuthenticator::ResyncEncryptedData() {
static_cast<AuthAttemptStateResolver*>(this)));
}

bool ParallelAuthenticator::VerifyOwner() {
base::AutoLock for_this_block(owner_verified_lock_);
if (owner_is_verified_)
return true;
// Check if policy data is fine and continue in safe mode if needed.
bool is_safe_mode = false;
CrosSettings::Get()->GetBoolean(kPolicyMissingMitigationMode, &is_safe_mode);
if (!is_safe_mode) {
// Now we can continue with the login and report mount success.
user_can_login_ = true;
owner_is_verified_ = true;
return true;
}
// First we have to make sure the current user's cert store is available.
UserManager::Get()->LoadKeyStore();
// Now we can continue reading the private key.
BrowserThread::PostTask(
BrowserThread::FILE, FROM_HERE,
base::Bind(&ParallelAuthenticator::FinishVerifyOwnerOnFileThread, this));
return false;
}

void ParallelAuthenticator::FinishVerifyOwnerOnFileThread() {
base::AutoLock for_this_block(owner_verified_lock_);
// Now we can check if this user is the owner.
user_can_login_ =
OwnershipService::GetSharedInstance()->IsCurrentUserOwner();
owner_is_verified_ = true;
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::Bind(&ParallelAuthenticator::Resolve, this));
}

void ParallelAuthenticator::RetryAuth(Profile* profile,
const std::string& username,
const std::string& password,
Expand Down Expand Up @@ -546,6 +595,22 @@ void ParallelAuthenticator::Resolve() {
this,
current_state_->online_outcome()));
break;
case OWNER_REQUIRED: {
current_state_->ResetCryptohomeStatus();
bool success = false;
DBusThreadManager::Get()->GetCryptohomeClient()->Unmount(&success);
if (!success) {
// Maybe we should reboot immediately here?
LOG(ERROR) << "Couldn't unmount users home!";
}
BrowserThread::PostTask(BrowserThread::UI,
FROM_HERE,
base::Bind(
&ParallelAuthenticator::OnLoginFailure,
this,
LoginFailure(LoginFailure::OWNER_REQUIRED)));
break;
}
default:
NOTREACHED();
break;
Expand Down Expand Up @@ -665,7 +730,10 @@ ParallelAuthenticator::ResolveCryptohomeSuccessState() {
return RECOVER_MOUNT;
if (check_key_attempted_)
return UNLOCK;
return OFFLINE_LOGIN;

if (!VerifyOwner())
return CONTINUE;
return user_can_login_ ? OFFLINE_LOGIN : OWNER_REQUIRED;
}

ParallelAuthenticator::AuthState
Expand Down Expand Up @@ -710,4 +778,11 @@ void ParallelAuthenticator::ResolveLoginCompletionStatus() {
Resolve();
}

void ParallelAuthenticator::SetOwnerState(bool owner_check_finished,
bool check_result) {
base::AutoLock for_this_block(owner_verified_lock_);
owner_is_verified_ = owner_check_finished;
user_can_login_ = check_result;
}

} // namespace chromeos

0 comments on commit 124766c

Please sign in to comment.