Added policy to disable/enable a system log upload.

BUG=471888

Review URL: https://codereview.chromium.org/1280003004

Cr-Commit-Position: refs/heads/master@{#343861}

chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc

@@ -410,6 +410,16 @@ void DecodeReportingPolicies(const em::ChromeDeviceSettingsProto& policy,
                     NULL);
     }
   }
+
+  if (policy.has_device_log_upload_settings()) {
+    const em::DeviceLogUploadSettingsProto& container(
+        policy.device_log_upload_settings());
+    if (container.has_log_upload_enabled()) {
+      policies->Set(
+          key::kLogUploadEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
+          new base::FundamentalValue(container.log_upload_enabled()), NULL);
+    }
+  }
 }
 
 void DecodeAutoUpdatePolicies(const em::ChromeDeviceSettingsProto& policy,

chrome/browser/chromeos/policy/proto/chrome_device_policy.proto

@@ -625,6 +625,12 @@ message LoginScreenDomainAutoCompleteProto {
   optional string login_screen_domain_auto_complete = 1;
 }
 
+// Settings that control whether a device would send system logs to the server.
+message DeviceLogUploadSettingsProto {
+  // Whether the device should send system logs. The default is false.
+  optional bool log_upload_enabled = 1 [default = false];
+}
+
 message ChromeDeviceSettingsProto {
   optional DevicePolicyRefreshRateProto device_policy_refresh_rate = 1;
   optional UserWhitelistProto user_whitelist = 2;
@@ -667,4 +673,5 @@ message ChromeDeviceSettingsProto {
   optional ExtensionCacheSizeProto extension_cache_size = 36;
   optional LoginScreenDomainAutoCompleteProto
       login_screen_domain_auto_complete = 37;
+  optional DeviceLogUploadSettingsProto device_log_upload_settings = 38;
 }

chrome/browser/chromeos/policy/system_log_uploader.cc

@@ -151,10 +151,21 @@ SystemLogUploader::SystemLogUploader(
       upload_frequency_(GetUploadFrequency()),
       task_runner_(task_runner),
       syslog_delegate_(syslog_delegate.Pass()),
+      upload_enabled_(false),
       weak_factory_(this) {
   if (!syslog_delegate_)
     syslog_delegate_.reset(new SystemLogDelegate());
   DCHECK(syslog_delegate_);
+
+  // Watch for policy changes.
+  upload_enabled_observer_ = chromeos::CrosSettings::Get()->AddSettingsObserver(
+      chromeos::kLogUploadEnabled,
+      base::Bind(&SystemLogUploader::RefreshUploadSettings,
+                 base::Unretained(this)));
+
+  // Fetch the current value of the policy.
+  RefreshUploadSettings();
+
   // Immediately schedule the next system log upload (last_upload_attempt_ is
   // set to the start of the epoch, so this will trigger an update upload in the
   // immediate future).
@@ -190,6 +201,24 @@ void SystemLogUploader::OnFailure(UploadJob::ErrorCode error_code) {
   }
 }
 
+void SystemLogUploader::RefreshUploadSettings() {
+  // Attempt to fetch the current value of the reporting settings.
+  // If trusted values are not available, register this function to be called
+  // back when they are available.
+  chromeos::CrosSettings* settings = chromeos::CrosSettings::Get();
+  if (chromeos::CrosSettingsProvider::TRUSTED !=
+      settings->PrepareTrustedValues(
+          base::Bind(&SystemLogUploader::RefreshUploadSettings,
+                     weak_factory_.GetWeakPtr()))) {
+    return;
+  }
+
+  // CrosSettings are trusted - we want to use the last trusted values, by
+  // default do not upload system logs.
+  if (!settings->GetBoolean(chromeos::kLogUploadEnabled, &upload_enabled_))
+    upload_enabled_ = false;
+}
+
 void SystemLogUploader::UploadSystemLogs(scoped_ptr<SystemLogs> system_logs) {
   // Must be called on the main thread.
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -220,8 +249,15 @@ void SystemLogUploader::StartLogUpload() {
   // Must be called on the main thread.
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  syslog_delegate_->LoadSystemLogs(base::Bind(
-      &SystemLogUploader::UploadSystemLogs, weak_factory_.GetWeakPtr()));
+  if (upload_enabled_) {
+    syslog_delegate_->LoadSystemLogs(base::Bind(
+        &SystemLogUploader::UploadSystemLogs, weak_factory_.GetWeakPtr()));
+  } else {
+    // If upload is disabled, schedule the next attempt after 12h.
+    retry_count_ = 0;
+    last_upload_attempt_ = base::Time::NowFromSystemTime();
+    ScheduleNextSystemLogUpload(upload_frequency_);
+  }
 }
 
 void SystemLogUploader::ScheduleNextSystemLogUpload(base::TimeDelta frequency) {

chrome/browser/chromeos/policy/system_log_uploader.h

@@ -13,6 +13,7 @@
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
 #include "chrome/browser/chromeos/policy/upload_job.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
 
 namespace base {
 class SequencedTaskRunner;
@@ -76,6 +77,9 @@ class SystemLogUploader : public UploadJob::Delegate {
   void OnFailure(UploadJob::ErrorCode error_code) override;
 
  private:
+  // Updates the system log upload enabled field from settings.
+  void RefreshUploadSettings();
+
   // Starts the system log loading process.
   void StartLogUpload();
 
@@ -105,6 +109,15 @@ class SystemLogUploader : public UploadJob::Delegate {
   // The Delegate is used to load system logs and create UploadJobs.
   scoped_ptr<Delegate> syslog_delegate_;
 
+  // True if system log upload is enabled. Kept cached in this object because
+  // CrosSettings can switch to an unstrusted state temporarily, and we want to
+  // use the last-known trusted values.
+  bool upload_enabled_;
+
+  // Observer to changes in system log upload settings.
+  scoped_ptr<chromeos::CrosSettings::ObserverSubscription>
+      upload_enabled_observer_;
+
   base::ThreadChecker thread_checker_;
 
   // Note: This should remain the last member so it'll be destroyed and

chrome/browser/chromeos/policy/system_log_uploader_unittest.cc

@@ -6,6 +6,9 @@
 #include "base/test/test_simple_task_runner.h"
 #include "base/time/time.h"
 #include "chrome/browser/chromeos/policy/system_log_uploader.h"
+#include "chrome/browser/chromeos/settings/scoped_cros_settings_test_helper.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_utils.h"
 #include "net/http/http_request_headers.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -110,6 +113,7 @@ class MockSystemLogDelegate : public SystemLogUploader::Delegate {
   ~MockSystemLogDelegate() override {}
 
   void LoadSystemLogs(const LogUploadCallback& upload_callback) override {
+    EXPECT_TRUE(is_upload_allowed_);
     upload_callback.Run(
         make_scoped_ptr(new SystemLogUploader::SystemLogs(system_logs_)));
   }
@@ -121,7 +125,12 @@ class MockSystemLogDelegate : public SystemLogUploader::Delegate {
                                              system_logs_.size()));
   }
 
+  void set_upload_allowed(bool is_upload_allowed) {
+    is_upload_allowed_ = is_upload_allowed;
+  }
+
  private:
+  bool is_upload_allowed_;
   bool is_upload_error_;
   SystemLogUploader::SystemLogs system_logs_;
 };
@@ -132,6 +141,15 @@ class SystemLogUploaderTest : public testing::Test {
  public:
   SystemLogUploaderTest() : task_runner_(new base::TestSimpleTaskRunner()) {}
 
+  void SetUp() override {
+    settings_helper_.ReplaceProvider(chromeos::kLogUploadEnabled);
+  }
+
+  void TearDown() override {
+    settings_helper_.RestoreProvider();
+    content::RunAllBlockingPoolTasksUntilIdle();
+  }
+
   // Given a pending task to upload system logs.
   void RunPendingUploadTaskAndCheckNext(const SystemLogUploader& uploader,
                                         base::TimeDelta expected_delay) {
@@ -156,16 +174,33 @@ class SystemLogUploaderTest : public testing::Test {
     EXPECT_GE(next_task, uploader.last_upload_attempt() + expected_delay);
   }
 
+ protected:
+  content::TestBrowserThreadBundle thread_bundle_;
+  chromeos::ScopedCrosSettingsTestHelper settings_helper_;
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
 };
 
+// Check disabled system log uploads by default.
+TEST_F(SystemLogUploaderTest, Basic) {
+  EXPECT_TRUE(task_runner_->GetPendingTasks().empty());
+
+  scoped_ptr<MockSystemLogDelegate> syslog_delegate(
+      new MockSystemLogDelegate(false, SystemLogUploader::SystemLogs()));
+  syslog_delegate->set_upload_allowed(false);
+  SystemLogUploader uploader(syslog_delegate.Pass(), task_runner_);
+
+  task_runner_->RunPendingTasks();
+}
+
 // One success task pending.
 TEST_F(SystemLogUploaderTest, SuccessTest) {
   EXPECT_TRUE(task_runner_->GetPendingTasks().empty());
 
-  SystemLogUploader uploader(make_scoped_ptr(new MockSystemLogDelegate(
-                                 false, SystemLogUploader::SystemLogs())),
-                             task_runner_);
+  scoped_ptr<MockSystemLogDelegate> syslog_delegate(
+      new MockSystemLogDelegate(false, SystemLogUploader::SystemLogs()));
+  syslog_delegate->set_upload_allowed(true);
+  settings_helper_.SetBoolean(chromeos::kLogUploadEnabled, true);
+  SystemLogUploader uploader(syslog_delegate.Pass(), task_runner_);
 
   EXPECT_EQ(1U, task_runner_->GetPendingTasks().size());
 
@@ -178,9 +213,11 @@ TEST_F(SystemLogUploaderTest, SuccessTest) {
 TEST_F(SystemLogUploaderTest, ThreeFailureTest) {
   EXPECT_TRUE(task_runner_->GetPendingTasks().empty());
 
-  SystemLogUploader uploader(make_scoped_ptr(new MockSystemLogDelegate(
-                                 true, SystemLogUploader::SystemLogs())),
-                             task_runner_);
+  scoped_ptr<MockSystemLogDelegate> syslog_delegate(
+      new MockSystemLogDelegate(true, SystemLogUploader::SystemLogs()));
+  syslog_delegate->set_upload_allowed(true);
+  settings_helper_.SetBoolean(chromeos::kLogUploadEnabled, true);
+  SystemLogUploader uploader(syslog_delegate.Pass(), task_runner_);
 
   EXPECT_EQ(1U, task_runner_->GetPendingTasks().size());
 
@@ -203,9 +240,11 @@ TEST_F(SystemLogUploaderTest, CheckHeaders) {
   EXPECT_TRUE(task_runner_->GetPendingTasks().empty());
 
   SystemLogUploader::SystemLogs system_logs = GenerateTestSystemLogFiles();
-  SystemLogUploader uploader(
-      make_scoped_ptr(new MockSystemLogDelegate(false, system_logs)),
-      task_runner_);
+  scoped_ptr<MockSystemLogDelegate> syslog_delegate(
+      new MockSystemLogDelegate(false, system_logs));
+  syslog_delegate->set_upload_allowed(true);
+  settings_helper_.SetBoolean(chromeos::kLogUploadEnabled, true);
+  SystemLogUploader uploader(syslog_delegate.Pass(), task_runner_);
 
   EXPECT_EQ(1U, task_runner_->GetPendingTasks().size());
 
@@ -214,4 +253,34 @@ TEST_F(SystemLogUploaderTest, CheckHeaders) {
                     SystemLogUploader::kDefaultUploadDelayMs));
 }
 
+// Disable system log uploads after one failed log upload.
+TEST_F(SystemLogUploaderTest, DisableLogUpload) {
+  EXPECT_TRUE(task_runner_->GetPendingTasks().empty());
+
+  scoped_ptr<MockSystemLogDelegate> syslog_delegate(
+      new MockSystemLogDelegate(true, SystemLogUploader::SystemLogs()));
+  MockSystemLogDelegate* mock_delegate = syslog_delegate.get();
+  settings_helper_.SetBoolean(chromeos::kLogUploadEnabled, true);
+  mock_delegate->set_upload_allowed(true);
+  SystemLogUploader uploader(syslog_delegate.Pass(), task_runner_);
+
+  EXPECT_EQ(1U, task_runner_->GetPendingTasks().size());
+  RunPendingUploadTaskAndCheckNext(uploader,
+                                   base::TimeDelta::FromMilliseconds(
+                                       SystemLogUploader::kErrorUploadDelayMs));
+
+  // Disable log upload and check that frequency is usual, because there is no
+  // errors, we should not upload logs.
+  settings_helper_.SetBoolean(chromeos::kLogUploadEnabled, false);
+  mock_delegate->set_upload_allowed(false);
+  task_runner_->RunPendingTasks();
+
+  RunPendingUploadTaskAndCheckNext(
+      uploader, base::TimeDelta::FromMilliseconds(
+                    SystemLogUploader::kDefaultUploadDelayMs));
+  RunPendingUploadTaskAndCheckNext(
+      uploader, base::TimeDelta::FromMilliseconds(
+                    SystemLogUploader::kDefaultUploadDelayMs));
+}
+
 }  // namespace policy

chrome/browser/chromeos/settings/device_settings_provider.cc

@@ -62,6 +62,7 @@ const char* const kKnownSettings[] = {
     kExtensionCacheSize,
     kHeartbeatEnabled,
     kHeartbeatFrequency,
+    kLogUploadEnabled,
     kPolicyMissingMitigationMode,
     kRebootOnShutdown,
     kReleaseChannel,
@@ -441,6 +442,19 @@ void DecodeGenericPolicies(
   }
 }
 
+void DecodeLogUploadPolicies(const em::ChromeDeviceSettingsProto& policy,
+                             PrefValueMap* new_values_cache) {
+  if (!policy.has_device_log_upload_settings())
+    return;
+
+  const em::DeviceLogUploadSettingsProto& log_upload_policy =
+      policy.device_log_upload_settings();
+  if (log_upload_policy.has_log_upload_enabled()) {
+    new_values_cache->SetBoolean(kLogUploadEnabled,
+                                 log_upload_policy.log_upload_enabled());
+  }
+}
+
 void DecodeDeviceState(const em::PolicyData& policy_data,
                        PrefValueMap* new_values_cache) {
   if (!policy_data.has_device_state())
@@ -634,6 +648,7 @@ void DeviceSettingsProvider::UpdateValuesCache(
   DecodeReportingPolicies(settings, &new_values_cache);
   DecodeHeartbeatPolicies(settings, &new_values_cache);
   DecodeGenericPolicies(settings, &new_values_cache);
+  DecodeLogUploadPolicies(settings, &new_values_cache);
   DecodeDeviceState(policy_data, &new_values_cache);
 
   // Collect all notifications but send them only after we have swapped the

chrome/browser/chromeos/settings/device_settings_provider_unittest.cc

@@ -99,6 +99,18 @@ class DeviceSettingsProviderTest : public DeviceSettingsTestBase {
     Mock::VerifyAndClearExpectations(this);
   }
 
+  // Helper routine to enable/disable log upload settings in policy.
+  void SetLogUploadSettings(bool enable_log_upload) {
+    EXPECT_CALL(*this, SettingChanged(_)).Times(AtLeast(1));
+    em::DeviceLogUploadSettingsProto* proto =
+        device_policy_.payload().mutable_device_log_upload_settings();
+    proto->set_log_upload_enabled(enable_log_upload);
+    device_policy_.Build();
+    device_settings_test_helper_.set_policy_blob(device_policy_.GetBlob());
+    ReloadDeviceSettings();
+    Mock::VerifyAndClearExpectations(this);
+  }
+
   // Helper routine to ensure all heartbeat policies have been correctly
   // decoded.
   void VerifyHeartbeatSettings(bool expected_enable_state,
@@ -140,6 +152,14 @@ class DeviceSettingsProviderTest : public DeviceSettingsTestBase {
                                     &expected_frequency_value));
   }
 
+  // Helper routine to ensure log upload policy has been correctly
+  // decoded.
+  void VerifyLogUploadSettings(bool expected_enable_state) {
+    const base::FundamentalValue expected_enabled_value(expected_enable_state);
+    EXPECT_TRUE(base::Value::Equals(provider_->Get(kLogUploadEnabled),
+                                    &expected_enabled_value));
+  }
+
   // Helper routine to set LoginScreenDomainAutoComplete policy.
   void SetDomainAutoComplete(const std::string& domain) {
     EXPECT_CALL(*this, SettingChanged(_)).Times(AtLeast(1));
@@ -478,4 +498,11 @@ TEST_F(DeviceSettingsProviderTest, DecodeDomainAutoComplete) {
   VerifyDomainAutoComplete(&domain_value);
 }
 
+TEST_F(DeviceSettingsProviderTest, DecodeLogUploadSettings) {
+  SetLogUploadSettings(true);
+  VerifyLogUploadSettings(true);
+
+  SetLogUploadSettings(false);
+  VerifyLogUploadSettings(false);
+}
 } // namespace chromeos

chrome/test/data/policy/policy_test_cases.json

@@ -2363,6 +2363,9 @@
   "HeartbeatFrequency": {
   },
 
+  "LogUploadEnabled": {
+  },
+
   "ReportDeviceVersionInfo": {
   },
 

chromeos/settings/cros_settings_names.cc

@@ -121,6 +121,9 @@ const char kHeartbeatEnabled[] = "cros.device_status.heartbeat_enabled";
 // How frequently heartbeats are sent up, in milliseconds.
 const char kHeartbeatFrequency[] = "cros.device_status.heartbeat_frequency";
 
+// Determines whether system logs should be sent to the management server.
+const char kLogUploadEnabled[] = "cros.device_status.log_upload_enabled";
+
 // 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[] =

chromeos/settings/cros_settings_names.h

@@ -64,6 +64,8 @@ CHROMEOS_EXPORT extern const char kReportUploadFrequency[];
 CHROMEOS_EXPORT extern const char kHeartbeatEnabled[];
 CHROMEOS_EXPORT extern const char kHeartbeatFrequency[];
 
+CHROMEOS_EXPORT extern const char kLogUploadEnabled[];
+
 CHROMEOS_EXPORT extern const char kPolicyMissingMitigationMode[];
 
 CHROMEOS_EXPORT extern const char kAllowRedeemChromeOsRegistrationOffers[];

components/policy/resources/policy_templates.json

@@ -123,7 +123,7 @@
 #   persistent IDs for all fields (but not for groups!) are needed. These are
 #   specified by the 'id' keys of each policy. NEVER CHANGE EXISTING IDs,
 #   because doing so would break the deployed wire format!
-#   For your editing convenience: highest ID currently used: 305
+#   For your editing convenience: highest ID currently used: 306
 #
 # Placeholders:
 #   The following placeholder strings are automatically substituted:
@@ -4394,6 +4394,24 @@
       frequency is 30 seconds and the maximum frequency is 24 hours - values
       outside of this range will be clamped to this range.''',
     },
+    {
+      'name': 'LogUploadEnabled',
+      'type': 'main',
+      'schema': { 'type': 'boolean' },
+      'supported_on': ['chrome_os:46-'],
+      'device_only': True,
+      'features': {
+        'dynamic_refresh': True,
+      },
+      'example_value': False,
+      'id': 306,
+      'caption': '''Send system logs to the management server''',
+      'desc': '''Send system logs to the management server, to allow
+      admins to monitor system logs.
+
+      If this policy is set to true, system logs will be sent. If set
+      to false or unset, then no system logs will be sent.''',
+    },
     {
       'name': 'DeviceUserWhitelist',
       'type': 'list',

tools/metrics/histograms/histograms.xml

@@ -56022,6 +56022,7 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   <int value="303" label="Welcome page on OS upgrade enabled"/>
   <int value="304" label="Use hardware acceleration when available"/>
   <int value="305" label="Android Negotiate authenticator account type"/>
+  <int value="306" label="Send system logs to the management server"/>
 </enum>
 
 <enum name="EnterprisePolicyInvalidations" type="int">