diff options
author | mnissler@chromium.org <mnissler@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-14 12:46:06 +0000 |
---|---|---|
committer | mnissler@chromium.org <mnissler@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-14 12:46:06 +0000 |
commit | c6836eb3cbdeb61a3295350d52c2136b488e2f25 (patch) | |
tree | 1af60f2628dc7d4f76596181e9c4ef71dcd1a493 | |
parent | ed373bb1ff102c941cda58a0f01d198ff24c2dc5 (diff) | |
download | chromium_src-c6836eb3cbdeb61a3295350d52c2136b488e2f25.zip chromium_src-c6836eb3cbdeb61a3295350d52c2136b488e2f25.tar.gz chromium_src-c6836eb3cbdeb61a3295350d52c2136b488e2f25.tar.bz2 |
Re-upload the machine ID during policy fetch if it's missing on the server side.
BUG=chromium-os:22092
TEST=Enroll a device with a bogus serial number. In the policy response, the server should request a serial number update, which should be uploaded after next ChromiumOS boot.
Review URL: http://codereview.chromium.org/8933008
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114413 0039d316-1c4b-4281-b951-d872f2087c98
8 files changed, 185 insertions, 63 deletions
diff --git a/chrome/browser/policy/browser_policy_connector.cc b/chrome/browser/policy/browser_policy_connector.cc index 6d547a0..ced77c9 100644 --- a/chrome/browser/policy/browser_policy_connector.cc +++ b/chrome/browser/policy/browser_policy_connector.cc @@ -153,24 +153,6 @@ void BrowserPolicyConnector::RegisterForDevicePolicy( TokenType token_type) { #if defined(OS_CHROMEOS) if (device_data_store_.get()) { - if (device_data_store_->machine_id().empty() || - device_data_store_->machine_model().empty()) { - chromeos::system::StatisticsProvider* provider = - chromeos::system::StatisticsProvider::GetInstance(); - - std::string machine_model; - if (!provider->GetMachineStatistic(kMachineInfoSystemHwqual, - &machine_model)) { - LOG(ERROR) << "Failed to get machine model."; - } - - std::string machine_id = GetSerialNumber(); - if (machine_id.empty()) - LOG(ERROR) << "Failed to get machine serial number."; - - device_data_store_->set_machine_id(machine_id); - device_data_store_->set_machine_model(machine_model); - } device_data_store_->set_user_name(owner_email); switch (token_type) { case TOKEN_TYPE_OAUTH: @@ -443,6 +425,28 @@ void BrowserPolicyConnector::InitializeDevicePolicy() { void BrowserPolicyConnector::CompleteInitialization() { #if defined(OS_CHROMEOS) if (device_cloud_policy_subsystem_.get()) { + // Read serial number and machine model. This must be done before we call + // CompleteInitialization() below such that the serial number is available + // for re-submission in case we're doing serial number recovery. + if (device_data_store_->machine_id().empty() || + device_data_store_->machine_model().empty()) { + chromeos::system::StatisticsProvider* provider = + chromeos::system::StatisticsProvider::GetInstance(); + + std::string machine_model; + if (!provider->GetMachineStatistic(kMachineInfoSystemHwqual, + &machine_model)) { + LOG(ERROR) << "Failed to get machine model."; + } + + std::string machine_id = GetSerialNumber(); + if (machine_id.empty()) + LOG(ERROR) << "Failed to get machine serial number."; + + device_data_store_->set_machine_id(machine_id); + device_data_store_->set_machine_model(machine_model); + } + device_cloud_policy_subsystem_->CompleteInitialization( prefs::kDevicePolicyRefreshRate, kServiceInitializationStartupDelay); diff --git a/chrome/browser/policy/cloud_policy_cache_base.cc b/chrome/browser/policy/cloud_policy_cache_base.cc index 57118f4..f0d3dc5 100644 --- a/chrome/browser/policy/cloud_policy_cache_base.cc +++ b/chrome/browser/policy/cloud_policy_cache_base.cc @@ -16,7 +16,8 @@ namespace policy { CloudPolicyCacheBase::CloudPolicyCacheBase() : notifier_(NULL), initialization_complete_(false), - is_unmanaged_(false) { + is_unmanaged_(false), + machine_id_missing_(false) { public_key_version_.version = 0; public_key_version_.valid = false; } @@ -153,6 +154,7 @@ bool CloudPolicyCacheBase::DecodePolicyResponse( if (public_key_version->valid) public_key_version->version = policy_data.public_key_version(); } + machine_id_missing_ = policy_data.valid_serial_number_missing(); return DecodePolicyData(policy_data, mandatory, recommended); } diff --git a/chrome/browser/policy/cloud_policy_cache_base.h b/chrome/browser/policy/cloud_policy_cache_base.h index 330e9e7..78ef72d 100644 --- a/chrome/browser/policy/cloud_policy_cache_base.h +++ b/chrome/browser/policy/cloud_policy_cache_base.h @@ -68,6 +68,10 @@ class CloudPolicyCacheBase : public base::NonThreadSafe { return last_policy_refresh_time_; } + bool machine_id_missing() const { + return machine_id_missing_; + } + // Get the version of the encryption key currently used for decoding policy. // Returns true if the version is available, in which case |version| is filled // in. @@ -159,6 +163,10 @@ class CloudPolicyCacheBase : public base::NonThreadSafe { // Whether the the server has indicated this device is unmanaged. bool is_unmanaged_; + // Flag indicating whether the server claims that a valid machine identifier + // is missing on the server side. Read directly from the policy blob. + bool machine_id_missing_; + // Currently used public key version, if available. PublicKeyVersion public_key_version_; diff --git a/chrome/browser/policy/cloud_policy_controller.cc b/chrome/browser/policy/cloud_policy_controller.cc index 0e3559c..79bbb81 100644 --- a/chrome/browser/policy/cloud_policy_controller.cc +++ b/chrome/browser/policy/cloud_policy_controller.cc @@ -265,6 +265,8 @@ void CloudPolicyController::SendPolicyRequest() { em::DeviceStatusReportRequest device_status; fetch_request->set_signature_type(em::PolicyFetchRequest::SHA1_RSA); fetch_request->set_policy_type(data_store_->policy_type()); + if (cache_->machine_id_missing() && !data_store_->machine_id().empty()) + fetch_request->set_machine_id(data_store_->machine_id()); if (!cache_->is_unmanaged() && !cache_->last_policy_refresh_time().is_null()) { base::TimeDelta timestamp = diff --git a/chrome/browser/policy/cloud_policy_subsystem_unittest.cc b/chrome/browser/policy/cloud_policy_subsystem_unittest.cc index 173feb3..bcbc4c1 100644 --- a/chrome/browser/policy/cloud_policy_subsystem_unittest.cc +++ b/chrome/browser/policy/cloud_policy_subsystem_unittest.cc @@ -25,9 +25,9 @@ namespace policy { -using ::testing::_; using ::testing::AtMost; using ::testing::InSequence; +using ::testing::_; using content::BrowserThread; namespace em = enterprise_management; @@ -45,6 +45,7 @@ const char kDeviceManagementUrl[] = const char kUsername[] = "john@smith.com"; const char kAuthToken[] = "secret123"; const char kPolicyType[] = "google/chrome/test"; +const char kMachineId[] = "test-machine-id"; } // namespace @@ -53,8 +54,8 @@ ACTION_P(CreateFailedResponse, http_error_code) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); em::DeviceManagementResponse response_data; - arg2->response_data = response_data.SerializeAsString(); - arg2->response_code = http_error_code; + arg3->response_data = response_data.SerializeAsString(); + arg3->response_code = http_error_code; } // An action that returns an URLRequestJob with a successful device @@ -64,18 +65,23 @@ ACTION_P(CreateSuccessfulRegisterResponse, token) { em::DeviceManagementResponse response_data; response_data.mutable_register_response()->set_device_management_token(token); - arg2->response_data = response_data.SerializeAsString(); - arg2->response_code = 200; + arg3->response_data = response_data.SerializeAsString(); + arg3->response_code = 200; } // An action that returns an URLRequestJob with a successful policy response. -ACTION_P(CreateSuccessfulPolicyResponse, homepage_location) { +ACTION_P3(CreateSuccessfulPolicyResponse, + homepage_location, + set_serial_valid, + serial_valid) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); em::CloudPolicySettings settings; settings.mutable_homepagelocation()->set_homepagelocation(homepage_location); em::PolicyData policy_data; policy_data.set_policy_type(kPolicyType); policy_data.set_policy_value(settings.SerializeAsString()); + if (set_serial_valid) + policy_data.set_valid_serial_number_missing(serial_valid); em::DeviceManagementResponse response_data; em::DevicePolicyResponse* policy_response = @@ -84,15 +90,14 @@ ACTION_P(CreateSuccessfulPolicyResponse, homepage_location) { fetch_response->set_error_code(200); fetch_response->set_policy_data(policy_data.SerializeAsString()); - arg2->response_data = response_data.SerializeAsString(); - arg2->response_code = 200; + arg3->response_data = response_data.SerializeAsString(); + arg3->response_code = 200; } // Tests CloudPolicySubsystem by intercepting its network requests. // The requests are intercepted by PolicyRequestInterceptor and they are // logged by LoggingWorkScheduler for further examination. -template<typename TESTBASE> -class CloudPolicySubsystemTestBase : public TESTBASE { +class CloudPolicySubsystemTestBase : public testing::Test { public: CloudPolicySubsystemTestBase() : ui_thread_(BrowserThread::UI, &loop_), @@ -124,6 +129,12 @@ class CloudPolicySubsystemTestBase : public TESTBASE { kDeviceManagementUrl, logger_.get())); cloud_policy_subsystem_->CompleteInitialization( prefs::kDevicePolicyRefreshRate, 0); + + // Abort the test on unexpected requests. + ON_CALL(factory(), Intercept(_, _, _, _)) + .WillByDefault(InvokeWithoutArgs( + this, + &CloudPolicySubsystemTestBase::StopMessageLoop)); } virtual void TearDown() { @@ -137,15 +148,15 @@ class CloudPolicySubsystemTestBase : public TESTBASE { } void ExecuteTest() { + // Stop the test once all the expectations are met. This relies on a + // sequence being active (see tests below). + EXPECT_CALL(factory(), Intercept(_, _, _, _)) + .Times(AtMost(1)) + .WillRepeatedly( + InvokeWithoutArgs(this, + &CloudPolicySubsystemTestBase::StopMessageLoop)); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - // The first unexpected request of the policy subsystem will stop the - // message loop. - // This code relies on the fact that an InSequence object is active. - EXPECT_CALL(*(factory_.get()), Intercept(_, _, _)) - .Times(AtMost(1)) - .WillOnce(InvokeWithoutArgs( - this, - &CloudPolicySubsystemTestBase::StopMessageLoop)); data_store_->set_user_name(kUsername); data_store_->SetGaiaToken(kAuthToken); data_store_->SetDeviceToken("", true); @@ -165,28 +176,32 @@ class CloudPolicySubsystemTestBase : public TESTBASE { } void ExpectSuccessfulRegistration() { - EXPECT_CALL(*(factory_.get()), Intercept(kGaiaAuthHeader, "register", _)) + EXPECT_CALL(factory(), Intercept(kGaiaAuthHeader, "register", _, _)) .WillOnce(CreateSuccessfulRegisterResponse(kDMToken)); } void ExpectFailedRegistration(int n, int code) { - EXPECT_CALL(*(factory_.get()), Intercept(kGaiaAuthHeader, "register", _)) + EXPECT_CALL(factory(), Intercept(kGaiaAuthHeader, "register", _, _)) .Times(n) .WillRepeatedly(CreateFailedResponse(code)); } void ExpectFailedPolicy(int n, int code) { - EXPECT_CALL(*(factory_.get()), Intercept(kDMAuthHeader, "policy", _)) + EXPECT_CALL(factory(), Intercept(kDMAuthHeader, "policy", _, _)) .Times(n) .WillRepeatedly(CreateFailedResponse(code)); } - void ExpectSuccessfulPolicy(int n, const std::string& homepage) { - EXPECT_CALL(*(factory_.get()), Intercept(kDMAuthHeader, "policy", _)) + void ExpectSuccessfulPolicy(int n, + const std::string& homepage) { + EXPECT_CALL(factory(), Intercept(kDMAuthHeader, "policy", _, _)) .Times(n) - .WillRepeatedly(CreateSuccessfulPolicyResponse(homepage)); + .WillRepeatedly(CreateSuccessfulPolicyResponse(homepage, false, false)); } + TestingPolicyURLFetcherFactory& factory() { return *factory_; } + CloudPolicyDataStore* data_store() { return data_store_.get(); } + private: // Verifies for a given policy that it is provided by the subsystem. void VerifyPolicy(enum ConfigurationPolicyType type, Value* expected) { @@ -211,7 +226,7 @@ class CloudPolicySubsystemTestBase : public TESTBASE { int count = 0; // Length and max number of requests for the first interval. - int64 length = 10*60*1000; // 10 minutes + int64 length = 10 * 60 * 1000; // 10 minutes int64 limit = 10; // maximum nr of requests in the first 10 minutes while (cur <= events.back()) { @@ -221,7 +236,7 @@ class CloudPolicySubsystemTestBase : public TESTBASE { cur += length; // Length and max number of requests for the subsequent intervals. - length = 60*60*1000; // 60 minutes + length = 60 * 60 * 1000; // 60 minutes limit = 12; // maxminum nr of requests in the next 60 minutes } @@ -269,8 +284,8 @@ class CombinedTestDesc { }; class CloudPolicySubsystemCombinedTest - : public CloudPolicySubsystemTestBase< - testing::TestWithParam<CombinedTestDesc> > { + : public CloudPolicySubsystemTestBase, + public testing::WithParamInterface<CombinedTestDesc> { }; TEST_P(CloudPolicySubsystemCombinedTest, Combined) { @@ -309,7 +324,8 @@ INSTANTIATE_TEST_CASE_P( // the error code returned for registration attempts. class CloudPolicySubsystemRegistrationTest - : public CloudPolicySubsystemTestBase<testing::TestWithParam<int> > { + : public CloudPolicySubsystemTestBase, + public testing::WithParamInterface<int> { }; TEST_P(CloudPolicySubsystemRegistrationTest, Registration) { @@ -331,7 +347,7 @@ INSTANTIATE_TEST_CASE_P( // response from the server. class CloudPolicySubsystemRegistrationFailureTest - : public CloudPolicySubsystemTestBase<testing::Test> { + : public CloudPolicySubsystemTestBase { }; TEST_F(CloudPolicySubsystemRegistrationFailureTest, RegistrationFailure) { @@ -346,7 +362,8 @@ TEST_F(CloudPolicySubsystemRegistrationFailureTest, RegistrationFailure) { // the error code returned for failed policy attempts. class CloudPolicySubsystemPolicyTest - : public CloudPolicySubsystemTestBase<testing::TestWithParam<int> > { + : public CloudPolicySubsystemTestBase, + public testing::WithParamInterface<int> { }; TEST_P(CloudPolicySubsystemPolicyTest, Policy) { @@ -368,7 +385,8 @@ INSTANTIATE_TEST_CASE_P( // them. The parameter is the error code returned for registration attempts. class CloudPolicySubsystemPolicyReregisterTest - : public CloudPolicySubsystemTestBase<testing::TestWithParam<int> > { + : public CloudPolicySubsystemTestBase, + public testing::WithParamInterface<int> { }; TEST_P(CloudPolicySubsystemPolicyReregisterTest, Policy) { @@ -388,4 +406,56 @@ INSTANTIATE_TEST_CASE_P( CloudPolicySubsystemPolicyReregisterTest, testing::Values(401, 403, 410)); +MATCHER_P(PolicyWithSerial, expected_serial, "") { + return arg.policy_request().request(0).machine_id() == expected_serial; +} + +class CloudPolicySubsystemSerialNumberRecoveryTest + : public CloudPolicySubsystemTestBase { + protected: + virtual ~CloudPolicySubsystemSerialNumberRecoveryTest() {} + + virtual void SetUp() { + CloudPolicySubsystemTestBase::SetUp(); + data_store()->set_machine_id(kMachineId); + } + + void ExpectPolicyRequest(const std::string& serial, + bool set_serial_valid, + bool serial_valid) { + EXPECT_CALL(factory(), Intercept(kDMAuthHeader, "policy", + PolicyWithSerial(serial), _)) + .WillOnce(CreateSuccessfulPolicyResponse("", set_serial_valid, + serial_valid)); + } +}; + +// Tests that no serial is sent if the flag is not set. +TEST_F(CloudPolicySubsystemSerialNumberRecoveryTest, FlagNotSet) { + InSequence s; + ExpectSuccessfulRegistration(); + ExpectPolicyRequest("", false, false); + ExpectPolicyRequest("", false, false); + ExecuteTest(); +} + +// Tests that no serial is sent if the flag is set to false. +TEST_F(CloudPolicySubsystemSerialNumberRecoveryTest, FlagFalse) { + InSequence s; + ExpectSuccessfulRegistration(); + ExpectPolicyRequest("", true, false); + ExpectPolicyRequest("", false, false); + ExecuteTest(); +} + +// Tests that the serial is sent once if the server requests it. +TEST_F(CloudPolicySubsystemSerialNumberRecoveryTest, SerialRequested) { + InSequence s; + ExpectSuccessfulRegistration(); + ExpectPolicyRequest("", true, true); + ExpectPolicyRequest(kMachineId, false, false); + ExpectPolicyRequest("", false, false); + ExecuteTest(); +} + } // policy diff --git a/chrome/browser/policy/testing_policy_url_fetcher_factory.cc b/chrome/browser/policy/testing_policy_url_fetcher_factory.cc index 66a1835..76cf207 100644 --- a/chrome/browser/policy/testing_policy_url_fetcher_factory.cc +++ b/chrome/browser/policy/testing_policy_url_fetcher_factory.cc @@ -6,6 +6,7 @@ #include "base/bind.h" #include "chrome/browser/policy/logging_work_scheduler.h" +#include "chrome/browser/policy/proto/device_management_backend.pb.h" #include "googleurl/src/gurl.h" #include "googleurl/src/url_parse.h" #include "net/http/http_request_headers.h" @@ -76,11 +77,15 @@ void TestingPolicyURLFetcher::Start() { std::string auth_header; net::HttpRequestHeaders headers; - std::string request = GetRequestType(GetURL()); + std::string request_type = GetRequestType(GetURL()); GetExtraRequestHeaders(&headers); headers.GetHeader("Authorization", &auth_header); + + enterprise_management::DeviceManagementRequest request; + request.ParseFromString(upload_data()); + // The following method is mocked by the currently running test. - parent_->GetResponse(auth_header, request, &response_); + parent_->GetResponse(auth_header, request_type, request, &response_); // We need to channel this through the central event logger, so that ordering // with other logged tasks that have a delay is preserved. @@ -110,10 +115,11 @@ LoggingWorkScheduler* TestingPolicyURLFetcherFactory::scheduler() { void TestingPolicyURLFetcherFactory::GetResponse( const std::string& auth_header, - const std::string& request, + const std::string& request_type, + const enterprise_management::DeviceManagementRequest& request, TestURLResponse* response) { logger_->RegisterEvent(); - Intercept(auth_header, request, response); + Intercept(auth_header, request_type, request, response); } content::URLFetcher* TestingPolicyURLFetcherFactory::CreateURLFetcher( diff --git a/chrome/browser/policy/testing_policy_url_fetcher_factory.h b/chrome/browser/policy/testing_policy_url_fetcher_factory.h index b2a9c6b..ded4d9f 100644 --- a/chrome/browser/policy/testing_policy_url_fetcher_factory.h +++ b/chrome/browser/policy/testing_policy_url_fetcher_factory.h @@ -16,6 +16,10 @@ class GURL; +namespace enterprise_management { +class DeviceManagementRequest; +} + namespace policy { class EventLogger; @@ -45,15 +49,20 @@ class TestingPolicyURLFetcherFactory : public content::URLFetcherFactory, // Called back by TestingPolicyURLFetcher objects. Uses |Intercept| to get // the response and notifies |logger_| of a network request event. - void GetResponse(const std::string& auth_header, - const std::string& request_type, - TestURLResponse* response); + void GetResponse( + const std::string& auth_header, + const std::string& request_type, + const enterprise_management::DeviceManagementRequest& request, + TestURLResponse* response); // Place EXPECT_CALLs on this method to control the responses of the // produced URLFetchers. The response data should be copied into |response|. - MOCK_METHOD3(Intercept, void(const std::string& auth_header, - const std::string& request_type, - TestURLResponse* response)); + MOCK_METHOD4( + Intercept, + void(const std::string& auth_header, + const std::string& request_type, + const enterprise_management::DeviceManagementRequest& request, + TestURLResponse* response)); private: EventLogger* logger_; diff --git a/net/tools/testserver/device_management.py b/net/tools/testserver/device_management.py index 231669c..a12ccbc 100644 --- a/net/tools/testserver/device_management.py +++ b/net/tools/testserver/device_management.py @@ -75,6 +75,10 @@ PKCS1_RSA_OID = '\x2a\x86\x48\x86\xf7\x0d\x01\x01\x01' # SHA256 sum of "0". SHA256_0 = hashlib.sha256('0').digest() +# List of bad machine identifiers that trigger the |valid_serial_number_missing| +# flag to be set set in the policy fetch response. +BAD_MACHINE_IDS = [ '123490EN400015' ]; + class RequestHandler(object): """Decodes and handles device management requests from clients. @@ -228,7 +232,7 @@ class RequestHandler(object): return response # Unregister the device. - self._server.UnregisterDevice(token); + self._server.UnregisterDevice(token['device_token']); # Prepare and send the response. response = dm.DeviceManagementResponse() @@ -398,6 +402,9 @@ class RequestHandler(object): if not token_info: return error + if msg.machine_id: + self._server.UpdateMachineId(token_info['device_token'], msg.machine_id) + # Response is only given if the scope is specified in the config file. # Normally 'google/chromeos/device' and 'google/chromeos/user' should be # accepted. @@ -433,9 +440,12 @@ class RequestHandler(object): policy_data = dm.PolicyData() policy_data.policy_type = msg.policy_type policy_data.timestamp = int(time.time() * 1000) - policy_data.request_token = token_info['device_token']; + policy_data.request_token = token_info['device_token'] policy_data.policy_value = policy_value policy_data.machine_name = token_info['machine_name'] + policy_data.valid_serial_number_missing = ( + token_info['machine_id'] in BAD_MACHINE_IDS) + if signing_key: policy_data.public_key_version = key_version policy_data.username = self._server.username @@ -595,9 +605,20 @@ class TestServer(object): 'device_token': dmtoken, 'allowed_policy_types': allowed_policy_types[type], 'machine_name': 'chromeos-' + machine_id, + 'machine_id': machine_id, } return self._registered_tokens[dmtoken] + def UpdateMachineId(self, dmtoken, machine_id): + """Updates the machine identifier for a registered device. + + Args: + dmtoken: The device management token provided by the client. + machine_id: Updated hardware identifier value. + """ + if dmtoken in self._registered_tokens: + self._registered_tokens[dmtoken]['machine_id'] = machine_id + def LookupToken(self, dmtoken): """Looks up a device or a user by DM token. |