Commit cbe2bc9f authored by Igor's avatar Igor Committed by Commit Bot

Fix the device disabled case at FrE check.

When the device is disabled, we receive from the server
RESTORE_MODE_DISABLED mode. The client has no information if the policy
forces to re-enroll or not. In the initial implementation it was treated
as device not being forced to re-enroll. This change fixes that case, so
that the flags from VPD don't get deleted for disabled device.

Also with a test image, the install_attributes file is deleted while
the checksum remain on disk. This has yet to be investigated, but to
avoid any problems when install_attributes file is corrupted, we set
the device as being in DEVICE_MODE_ENTERPRISE if auto_enrollment
detects the device is disabled.

BUG=chromium:996059
TEST=Manual tests and unit tests

Change-Id: I8bc30028af043fdb563e6374d7e5db4b30830218
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1771966Reviewed-by: default avatarIgor <igorcov@chromium.org>
Reviewed-by: default avatarBartosz Fabianowski <bartfab@chromium.org>
Reviewed-by: default avatarMaksim Ivanov <emaxx@chromium.org>
Commit-Queue: Igor <igorcov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692052}
parent 37ab9510
...@@ -214,6 +214,7 @@ bool AutoEnrollmentCheckScreen::UpdateAutoEnrollmentState( ...@@ -214,6 +214,7 @@ bool AutoEnrollmentCheckScreen::UpdateAutoEnrollmentState(
case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT: case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT:
case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH: case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH:
case policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT: case policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT:
case policy::AUTO_ENROLLMENT_STATE_DISABLED:
return false; return false;
case policy::AUTO_ENROLLMENT_STATE_SERVER_ERROR: case policy::AUTO_ENROLLMENT_STATE_SERVER_ERROR:
if (!ShouldBlockOnServerError()) if (!ShouldBlockOnServerError())
...@@ -282,6 +283,7 @@ bool AutoEnrollmentCheckScreen::IsCompleted() const { ...@@ -282,6 +283,7 @@ bool AutoEnrollmentCheckScreen::IsCompleted() const {
case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT: case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT:
case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH: case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH:
case policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT: case policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT:
case policy::AUTO_ENROLLMENT_STATE_DISABLED:
// Decision made, ready to proceed. // Decision made, ready to proceed.
return true; return true;
} }
......
...@@ -82,10 +82,10 @@ class AutoEnrollmentCheckScreen ...@@ -82,10 +82,10 @@ class AutoEnrollmentCheckScreen
bool UpdateCaptivePortalStatus( bool UpdateCaptivePortalStatus(
NetworkPortalDetector::CaptivePortalStatus new_captive_portal_status); NetworkPortalDetector::CaptivePortalStatus new_captive_portal_status);
// Configures the UI to reflect |auto_enrollment_state|. Returns true if and // Configures the UI to reflect |new_auto_enrollment_state|. Returns true if
// only if a UI change has been made. // and only if a UI change has been made.
bool UpdateAutoEnrollmentState( bool UpdateAutoEnrollmentState(
policy::AutoEnrollmentState auto_enrollment_state); policy::AutoEnrollmentState new_auto_enrollment_state);
// Configures the error screen. // Configures the error screen.
void ShowErrorScreen(NetworkError::ErrorState error_state); void ShowErrorScreen(NetworkError::ErrorState error_state);
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "chrome/browser/chromeos/policy/auto_enrollment_client_impl.h" #include "chrome/browser/chromeos/policy/auto_enrollment_client_impl.h"
#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h" #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
#include "chrome/browser/chromeos/policy/server_backed_state_keys_broker.h" #include "chrome/browser/chromeos/policy/server_backed_state_keys_broker.h"
#include "chrome/browser/chromeos/settings/device_settings_service.h"
#include "chrome/browser/net/system_network_context_manager.h" #include "chrome/browser/net/system_network_context_manager.h"
#include "chromeos/constants/chromeos_switches.h" #include "chromeos/constants/chromeos_switches.h"
#include "chromeos/dbus/cryptohome/cryptohome_client.h" #include "chromeos/dbus/cryptohome/cryptohome_client.h"
...@@ -24,6 +25,7 @@ ...@@ -24,6 +25,7 @@
#include "chromeos/dbus/system_clock/system_clock_client.h" #include "chromeos/dbus/system_clock/system_clock_client.h"
#include "chromeos/system/factory_ping_embargo_check.h" #include "chromeos/system/factory_ping_embargo_check.h"
#include "chromeos/system/statistics_provider.h" #include "chromeos/system/statistics_provider.h"
#include "chromeos/tpm/install_attributes.h"
#include "components/device_event_log/device_event_log.h" #include "components/device_event_log/device_event_log.h"
#include "components/policy/core/common/cloud/device_management_service.h" #include "components/policy/core/common/cloud/device_management_service.h"
#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
...@@ -130,7 +132,7 @@ std::string FRERequirementToString( ...@@ -130,7 +132,7 @@ std::string FRERequirementToString(
return std::string(); return std::string();
} }
std::string AutoenrollmentStateToString(policy::AutoEnrollmentState state) { std::string AutoEnrollmentStateToString(policy::AutoEnrollmentState state) {
switch (state) { switch (state) {
case policy::AutoEnrollmentState::AUTO_ENROLLMENT_STATE_IDLE: case policy::AutoEnrollmentState::AUTO_ENROLLMENT_STATE_IDLE:
return "Not started"; return "Not started";
...@@ -146,6 +148,8 @@ std::string AutoenrollmentStateToString(policy::AutoEnrollmentState state) { ...@@ -146,6 +148,8 @@ std::string AutoenrollmentStateToString(policy::AutoEnrollmentState state) {
return "No enrollment"; return "No enrollment";
case policy::AutoEnrollmentState::AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH: case policy::AutoEnrollmentState::AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH:
return "Zero-touch enrollment"; return "Zero-touch enrollment";
case policy::AutoEnrollmentState::AUTO_ENROLLMENT_STATE_DISABLED:
return "Device disabled";
} }
} }
...@@ -410,6 +414,7 @@ void AutoEnrollmentController::Start() { ...@@ -410,6 +414,7 @@ void AutoEnrollmentController::Start() {
case policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT: case policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT:
case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT: case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT:
case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH: case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH:
case policy::AUTO_ENROLLMENT_STATE_DISABLED:
// Abort re-start when there's already a final decision. // Abort re-start when there's already a final decision.
return; return;
...@@ -569,7 +574,7 @@ void AutoEnrollmentController::DetermineAutoEnrollmentCheckType() { ...@@ -569,7 +574,7 @@ void AutoEnrollmentController::DetermineAutoEnrollmentCheckType() {
// Skip everything if the device was in consumer mode previously. // Skip everything if the device was in consumer mode previously.
fre_requirement_ = GetFRERequirement(); fre_requirement_ = GetFRERequirement();
LOGIN_LOG(EVENT) << FRERequirementToString(fre_requirement_); VLOG(1) << FRERequirementToString(fre_requirement_);
if (fre_requirement_ == FRERequirement::kExplicitlyNotRequired) { if (fre_requirement_ == FRERequirement::kExplicitlyNotRequired) {
LOGIN_LOG(EVENT) << "Auto-enrollment disabled: VPD."; LOGIN_LOG(EVENT) << "Auto-enrollment disabled: VPD.";
auto_enrollment_check_type_ = AutoEnrollmentCheckType::kNone; auto_enrollment_check_type_ = AutoEnrollmentCheckType::kNone;
...@@ -765,7 +770,7 @@ void AutoEnrollmentController::StartClientForInitialEnrollment() { ...@@ -765,7 +770,7 @@ void AutoEnrollmentController::StartClientForInitialEnrollment() {
void AutoEnrollmentController::UpdateState( void AutoEnrollmentController::UpdateState(
policy::AutoEnrollmentState new_state) { policy::AutoEnrollmentState new_state) {
LOGIN_LOG(EVENT) << "New auto-enrollment state: " LOGIN_LOG(EVENT) << "New auto-enrollment state: "
<< AutoenrollmentStateToString(new_state); << AutoEnrollmentStateToString(new_state);
state_ = new_state; state_ = new_state;
// Stop the safeguard timer once a result comes in. // Stop the safeguard timer once a result comes in.
...@@ -778,10 +783,24 @@ void AutoEnrollmentController::UpdateState( ...@@ -778,10 +783,24 @@ void AutoEnrollmentController::UpdateState(
case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT: case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT:
case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH: case policy::AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH:
case policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT: case policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT:
case policy::AUTO_ENROLLMENT_STATE_DISABLED:
safeguard_timer_.Stop(); safeguard_timer_.Stop();
break; break;
} }
// Device disabling mode is relying on device state stored in install
// attributes. In case that file is corrupted, this should prevent device
// re-enabling.
if (state_ == policy::AUTO_ENROLLMENT_STATE_DISABLED) {
policy::DeviceMode device_mode =
chromeos::InstallAttributes::Get()->GetMode();
if (device_mode == policy::DeviceMode::DEVICE_MODE_PENDING ||
device_mode == policy::DeviceMode::DEVICE_MODE_NOT_SET) {
DeviceSettingsService::Get()->SetDeviceMode(
policy::DeviceMode::DEVICE_MODE_ENTERPRISE);
}
}
if (state_ == policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT) { if (state_ == policy::AUTO_ENROLLMENT_STATE_NO_ENROLLMENT) {
StartCleanupForcedReEnrollment(); StartCleanupForcedReEnrollment();
} else { } else {
......
...@@ -38,6 +38,8 @@ enum AutoEnrollmentState { ...@@ -38,6 +38,8 @@ enum AutoEnrollmentState {
AUTO_ENROLLMENT_STATE_NO_ENROLLMENT = 5, AUTO_ENROLLMENT_STATE_NO_ENROLLMENT = 5,
// Check completed successfully, zero-touch enrollment should be triggered. // Check completed successfully, zero-touch enrollment should be triggered.
AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH = 6, AUTO_ENROLLMENT_STATE_TRIGGER_ZERO_TOUCH = 6,
// Check completed successfully, device is disabled.
AUTO_ENROLLMENT_STATE_DISABLED = 7,
}; };
// Interacts with the device management service and determines whether this // Interacts with the device management service and determines whether this
......
...@@ -537,9 +537,11 @@ void AutoEnrollmentClientImpl::NextStep() { ...@@ -537,9 +537,11 @@ void AutoEnrollmentClientImpl::NextStep() {
const DeviceStateMode device_state_mode = GetDeviceStateMode(); const DeviceStateMode device_state_mode = GetDeviceStateMode();
switch (device_state_mode) { switch (device_state_mode) {
case RESTORE_MODE_NONE: case RESTORE_MODE_NONE:
case RESTORE_MODE_DISABLED:
ReportProgress(AUTO_ENROLLMENT_STATE_NO_ENROLLMENT); ReportProgress(AUTO_ENROLLMENT_STATE_NO_ENROLLMENT);
break; break;
case RESTORE_MODE_DISABLED:
ReportProgress(AUTO_ENROLLMENT_STATE_DISABLED);
break;
case RESTORE_MODE_REENROLLMENT_REQUESTED: case RESTORE_MODE_REENROLLMENT_REQUESTED:
case RESTORE_MODE_REENROLLMENT_ENFORCED: case RESTORE_MODE_REENROLLMENT_ENFORCED:
case INITIAL_MODE_ENROLLMENT_ENFORCED: case INITIAL_MODE_ENROLLMENT_ENFORCED:
......
...@@ -648,7 +648,7 @@ TEST_P(AutoEnrollmentClientImplTest, DeviceDisabled) { ...@@ -648,7 +648,7 @@ TEST_P(AutoEnrollmentClientImplTest, DeviceDisabled) {
EXPECT_EQ(DeviceManagementService::JobConfiguration::TYPE_AUTO_ENROLLMENT, EXPECT_EQ(DeviceManagementService::JobConfiguration::TYPE_AUTO_ENROLLMENT,
auto_enrollment_job_type_); auto_enrollment_job_type_);
EXPECT_EQ(GetExpectedStateRetrievalJobType(), state_retrieval_job_type_); EXPECT_EQ(GetExpectedStateRetrievalJobType(), state_retrieval_job_type_);
EXPECT_EQ(AUTO_ENROLLMENT_STATE_NO_ENROLLMENT, state_); EXPECT_EQ(AUTO_ENROLLMENT_STATE_DISABLED, state_);
VerifyCachedResult(true, 8); VerifyCachedResult(true, 8);
VerifyServerBackedState("example.com", kDeviceStateRestoreModeDisabled, VerifyServerBackedState("example.com", kDeviceStateRestoreModeDisabled,
kDisabledMessage); kDisabledMessage);
......
...@@ -122,8 +122,12 @@ void DeviceSettingsService::UnsetSessionManager() { ...@@ -122,8 +122,12 @@ void DeviceSettingsService::UnsetSessionManager() {
} }
void DeviceSettingsService::SetDeviceMode(policy::DeviceMode device_mode) { void DeviceSettingsService::SetDeviceMode(policy::DeviceMode device_mode) {
// Device mode can only change once. if (device_mode_ == device_mode)
DCHECK_EQ(policy::DEVICE_MODE_PENDING, device_mode_); return;
// Device mode can only change if was not set yet.
DCHECK(policy::DEVICE_MODE_PENDING == device_mode_ ||
policy::DEVICE_MODE_NOT_SET == device_mode_);
device_mode_ = device_mode; device_mode_ = device_mode;
if (GetOwnershipStatus() != OWNERSHIP_UNKNOWN) { if (GetOwnershipStatus() != OWNERSHIP_UNKNOWN) {
RunPendingOwnershipStatusCallbacks(); RunPendingOwnershipStatusCallbacks();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment