Skip to content

Commit

Permalink
Show system tray privacy warning if XDR reporting is enabled
Browse files Browse the repository at this point in the history
If the DeviceReportXDREvents policy is enabled, show a privacy
warning notifying the user that network traffic may be monitored.

If DNS or the XDR policy is enabled the enterprise symbol will be used.
Merge the 2 cases into 1 bool "enterprise_monitored_web_requests".
Move from network state properties to global property.

and not shown when disabled.

Bug: b/283268889
Test: Verified that warning was shown when XDR policy is enabled
Change-Id: Iad1012388ef134d5a10cb50f7a9c47b1bdbf583e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4575915
Commit-Queue: Ryan Borzello <rborzello@google.com>
Auto-Submit: Ryan Borzello <rborzello@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1154001}
  • Loading branch information
Ryan Borzello authored and Chromium LUCI CQ committed Jun 6, 2023
1 parent 7d6e0c0 commit 5e476d2
Show file tree
Hide file tree
Showing 26 changed files with 318 additions and 43 deletions.
44 changes: 35 additions & 9 deletions ash/system/network/network_list_view_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ void NetworkListViewControllerImpl::NetworkListChanged() {

void NetworkListViewControllerImpl::GlobalPolicyChanged() {
UpdateMobileSection();
GetNetworkStateList();
}

void NetworkListViewControllerImpl::OnPropertiesUpdated(
Expand Down Expand Up @@ -399,17 +400,28 @@ void NetworkListViewControllerImpl::UpdateNetworkTypeExistence(
size_t NetworkListViewControllerImpl::ShowConnectionWarningIfNetworkMonitored(
size_t index) {
const NetworkStateProperties* default_network = model()->default_network();
const GlobalPolicy* global_policy = model()->global_policy();
bool using_proxy =
default_network && default_network->proxy_mode != ProxyMode::kDirect;
bool dns_queries_monitored =
default_network && default_network->dns_queries_monitored;
bool enterprise_monitored_web_requests =
global_policy && (global_policy->dns_queries_monitored ||
global_policy->report_xdr_events_enabled);

if (!connected_vpn_guid_.empty() || using_proxy || dns_queries_monitored) {
if (!connected_vpn_guid_.empty() || using_proxy ||
enterprise_monitored_web_requests) {
if (!connection_warning_) {
ShowConnectionWarning(/*show_managed_icon=*/dns_queries_monitored);
ShowConnectionWarning(
/*show_managed_icon=*/enterprise_monitored_web_requests);
}
// If the warning is already shown check if the label needs to be updated
// with a different message.
else if (connection_warning_label_->GetText() !=
GenerateLabelText(enterprise_monitored_web_requests)) {
connection_warning_label_->SetText(
GenerateLabelText(enterprise_monitored_web_requests));
}

if (!dns_queries_monitored) {
if (!enterprise_monitored_web_requests) {
MaybeShowConnectionWarningManagedIcon(using_proxy);
}

Expand Down Expand Up @@ -904,6 +916,23 @@ size_t NetworkListViewControllerImpl::CreateItemViewsIfMissingAndReorder(
return index;
}

std::u16string NetworkListViewControllerImpl::GenerateLabelText(
bool show_managed_icon) {
// If the device is not managed then the high risk disclosure should be shown.
if (!show_managed_icon) {
return l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_NETWORK_MONITORED_WARNING);
}

// If XDR reporting is enabled the high risk disclosure should be shown.
if (model()->global_policy()->report_xdr_events_enabled) {
return l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_NETWORK_MONITORED_WARNING);
}

return l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_MANAGED_WARNING);
}

void NetworkListViewControllerImpl::ShowConnectionWarning(
bool show_managed_icon) {
// Set up layout and apply sticky row property.
Expand All @@ -917,9 +946,7 @@ void NetworkListViewControllerImpl::ShowConnectionWarning(
// Set message label in middle of row.
std::unique_ptr<views::Label> label =
base::WrapUnique(TrayPopupUtils::CreateDefaultLabel());
label->SetText(l10n_util::GetStringUTF16(
show_managed_icon ? IDS_ASH_STATUS_TRAY_NETWORK_MANAGED_WARNING
: IDS_ASH_STATUS_TRAY_NETWORK_MONITORED_WARNING));
label->SetText(GenerateLabelText(show_managed_icon));
label->SetBackground(views::CreateSolidBackground(SK_ColorTRANSPARENT));
label->SetEnabledColor(AshColorProvider::Get()->GetContentLayerColor(
AshColorProvider::ContentLayerType::kTextColorPrimary));
Expand All @@ -936,7 +963,6 @@ void NetworkListViewControllerImpl::ShowConnectionWarning(

// Nothing to the right of the text.
connection_warning->SetContainerVisible(TriView::Container::END, false);

connection_warning->SetID(static_cast<int>(
NetworkListViewControllerViewChildId::kConnectionWarning));
// The warning messages are shown in the ethernet section.
Expand Down
13 changes: 9 additions & 4 deletions ash/system/network/network_list_view_controller_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ class ASH_EXPORT NetworkListViewControllerImpl
networks);

// Adds a warning indicator if connected to a VPN, if the default network
// has a proxy installed or if the secure DNS template URIs contain user or
// device identifiers.
// has a proxy installed, if the secure DNS template URIs contain user/device
// identifiers or if DeviceReportXDREvents is enabled.
size_t ShowConnectionWarningIfNetworkMonitored(size_t index);

// Returns true if mobile data section should be added to view.
Expand Down Expand Up @@ -153,9 +153,14 @@ class ASH_EXPORT NetworkListViewControllerImpl
networks,
NetworkIdToViewMap* previous_views);

// Generates the correct warning to display based on the enterprise status
// andn XDR reporting policy.
std::u16string GenerateLabelText(bool show_managed_icon);

// Creates a view that indicates connections might be monitored if
// connected to a VPN, if the default network has a proxy installed or if the
// secure DNS template URIs contain identifiers.
// connected to a VPN, if the default network has a proxy installed, if the
// secure DNS template URIs contain identifiers or if DeviceReportXDREvents is
// enabled.
void ShowConnectionWarning(bool show_managed_icon);

// Hides a connection warning, if visible.
Expand Down
51 changes: 47 additions & 4 deletions ash/system/network/network_list_view_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,9 @@ class NetworkListViewControllerTest : public AshTestBase,
->ConfigureRemoteForTesting(cros_network()->GetPendingRemote());
base::RunLoop().RunUntilIdle();
cros_network_->SetGlobalPolicy(
/*allow_only_policy_cellular_networks=*/false);
/*allow_only_policy_cellular_networks=*/false,
/*dns_queries_monitored=*/false,
/*report_xdr_events_enabled=*/false);

detailed_view_delegate_ =
std::make_unique<DetailedViewDelegate>(/*tray_controller=*/nullptr);
Expand Down Expand Up @@ -687,7 +689,10 @@ TEST_P(NetworkListViewControllerTest, MobileSectionHeaderAddEsimButtonStates) {
// When no Mobile networks are available and eSIM policy is set to allow only
// cellular devices which means adding a new eSIM is disallowed by enterprise
// policy, add eSIM button is not displayed.
cros_network()->SetGlobalPolicy(/*allow_only_policy_cellular_networks=*/true);
cros_network()->SetGlobalPolicy(
/*allow_only_policy_cellular_networks=*/true,
/*dns_queries_monitored=*/false,
/*report_xdr_events_enabled=*/false);

EXPECT_FALSE(GetAddEsimButton()->GetVisible());
}
Expand Down Expand Up @@ -1271,8 +1276,10 @@ TEST_P(NetworkListViewControllerTest,
EXPECT_THAT(GetConnectionWarning(), IsNull());
auto network = CrosNetworkConfigTestHelper::CreateStandaloneNetworkProperties(
kWifiName, NetworkType::kWiFi, ConnectionStateType::kConnected);
network->dns_queries_monitored = true;
cros_network()->AddNetworkAndDevice(std::move(network));
cros_network()->SetGlobalPolicy(
/*allow_only_policy_cellular_networks=*/false,
/*dns_queries_monitored=*/true,
/*report_xdr_events_enabled=*/false);

views::ImageView* icon = GetConnectionWarningIcon();
ASSERT_THAT(icon, NotNull());
Expand All @@ -1284,6 +1291,42 @@ TEST_P(NetworkListViewControllerTest,
GetConnectionLabelView()->GetText());
}

TEST_P(NetworkListViewControllerTest, ConnectionWarningDeviceReportXDREvents) {
EXPECT_THAT(GetConnectionWarning(), IsNull());
auto network = CrosNetworkConfigTestHelper::CreateStandaloneNetworkProperties(
kWifiName, NetworkType::kWiFi, ConnectionStateType::kConnected);

// First set XDR policy as false to check that managed warning is shown.
cros_network()->SetGlobalPolicy(
/*allow_only_policy_cellular_networks=*/false,
/*dns_queries_monitored=*/true,
/*report_xdr_events_enabled=*/false);

views::ImageView* icon = GetConnectionWarningIcon();
ASSERT_THAT(icon, NotNull());
EXPECT_TRUE(IsManagedIcon(icon));
ASSERT_THAT(GetConnectionWarning(), NotNull());
ASSERT_THAT(GetConnectionLabelView(), NotNull());
EXPECT_EQ(
l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_MANAGED_WARNING),
GetConnectionLabelView()->GetText());

// Update XDR policy to true and verify network monitored warning is shown.
cros_network()->SetGlobalPolicy(
/*allow_only_policy_cellular_networks=*/false,
/*dns_queries_monitored=*/true,
/*report_xdr_events_enabled=*/true);

icon = GetConnectionWarningIcon();
ASSERT_THAT(icon, NotNull());
EXPECT_TRUE(IsManagedIcon(icon));
ASSERT_THAT(GetConnectionWarning(), NotNull());
ASSERT_THAT(GetConnectionLabelView(), NotNull());
EXPECT_EQ(
l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_MONITORED_WARNING),
GetConnectionLabelView()->GetText());
}

TEST_P(NetworkListViewControllerTest, NetworkScanning) {
cros_network()->ClearNetworksAndDevices();
int initial_wifi_count = 0;
Expand Down
1 change: 0 additions & 1 deletion ash/webui/common/resources/network/onc_mojo.js
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,6 @@ export class OncMojo {
source: OncSource.kNone,
type: type,
typeState: {},
dnsQueriesMonitored: false,
};
switch (type) {
case NetworkType.kCellular:
Expand Down
1 change: 0 additions & 1 deletion ash/webui/eche_app_ui/system_info_provider_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,6 @@ class SystemInfoProviderTest : public testing::Test {
/*portal_probe_url=*/absl::nullopt,
/*priority=*/1,
/*proxy_mode=*/network_config::mojom::ProxyMode::kDirect,
/*dns_queries_monitored=*/false,
/*prohibited_by_policy=*/false,
/*source=*/
network_config::mojom::OncSource::kUser,
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2127,6 +2127,8 @@ source_set("ash") {
"net/system_proxy_manager.h",
"net/traffic_counters_handler.cc",
"net/traffic_counters_handler.h",
"net/xdr_manager.cc",
"net/xdr_manager.h",
"network_change_manager_client.cc",
"network_change_manager_client.h",
"night_light/night_light_client.cc",
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/ash/login/session/user_session_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1718,6 +1718,9 @@ void UserSessionManager::FinalizePrepareProfile(Profile* profile) {

secure_dns_manager_ =
std::make_unique<SecureDnsManager>(g_browser_process->local_state());

xdr_manager_ =
std::make_unique<XdrManager>(g_browser_process->policy_service());
}

// Save sync password hash and salt to profile prefs if they are available.
Expand Down Expand Up @@ -2451,6 +2454,7 @@ void UserSessionManager::Shutdown() {
password_service_voted_.reset();
password_was_saved_ = false;
secure_dns_manager_.reset();
xdr_manager_.reset();
}

void UserSessionManager::SetSwitchesForUser(
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ash/login/session/user_session_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "chrome/browser/ash/login/signin/oauth2_login_manager.h"
#include "chrome/browser/ash/login/signin/token_handle_util.h"
#include "chrome/browser/ash/net/secure_dns_manager.h"
#include "chrome/browser/ash/net/xdr_manager.h"
#include "chrome/browser/ash/release_notes/release_notes_notification.h"
#include "chrome/browser/ash/web_applications/help_app/help_app_notification_controller.h"
#include "chrome/browser/profiles/profile.h"
Expand Down Expand Up @@ -623,6 +624,8 @@ class UserSessionManager

std::unique_ptr<SecureDnsManager> secure_dns_manager_;

std::unique_ptr<XdrManager> xdr_manager_;

std::unique_ptr<ChildPolicyObserver> child_policy_observer_;

std::unique_ptr<U2FNotification> u2f_notification_;
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ash/net/secure_dns_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ void SecureDnsManager::OnPrefChanged() {

NetworkHandler::Get()
->network_metadata_store()
->set_secure_dns_templates_with_identifiers_active(
->SetSecureDnsTemplatesWithIdentifiersActive(
doh_templates_uri_resolver_->GetDohWithIdentifiersActive());
}

Expand Down
55 changes: 55 additions & 0 deletions chrome/browser/ash/net/xdr_manager.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/ash/net/xdr_manager.h"

#include <memory>

#include "base/logging.h"
#include "base/values.h"
#include "chromeos/ash/components/network/network_handler.h"
#include "chromeos/ash/components/network/network_metadata_store.h"
#include "components/policy/policy_constants.h"

namespace ash {

XdrManager::XdrManager(policy::PolicyService* policy_service) {
// Register callback for when DeviceReportXDREvents changes.
policy_registrar_ = std::make_unique<policy::PolicyChangeRegistrar>(
policy_service,
policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, std::string()));
policy_registrar_->Observe(policy::key::kDeviceReportXDREvents,
base::BindRepeating(&XdrManager::OnXdrPolicyChange,
base::Unretained(this)));
// Get and set initial XDR policy.
auto* report_xdr_events_value =
policy_service
->GetPolicies(policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME,
std::string()))
.GetValue(policy::key::kDeviceReportXDREvents,
base::Value::Type::BOOLEAN);
report_xdr_events_enabled_ =
report_xdr_events_value && report_xdr_events_value->GetBool();

SetNetworkMetadataStoreXdrValue();
}

XdrManager::~XdrManager() = default;

bool XdrManager::AreXdrPoliciesEnabled() {
return report_xdr_events_enabled_;
}

void XdrManager::OnXdrPolicyChange(const base::Value* previous,
const base::Value* current) {
report_xdr_events_enabled_ = current && current->GetBool();

SetNetworkMetadataStoreXdrValue();
}

void XdrManager::SetNetworkMetadataStoreXdrValue() {
NetworkHandler::Get()->network_metadata_store()->SetReportXdrEventsEnabled(
report_xdr_events_enabled_);
}
} // namespace ash
38 changes: 38 additions & 0 deletions chrome/browser/ash/net/xdr_manager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_ASH_NET_XDR_MANAGER_H_
#define CHROME_BROWSER_ASH_NET_XDR_MANAGER_H_

#include "components/policy/core/common/policy_namespace.h"
#include "components/policy/core/common/policy_service.h"

namespace ash {

// Responds to changes in the DeviceReportXDREvents policy and updates
// the network metadata to determine if connection warning should be shown.
class XdrManager {
public:
explicit XdrManager(policy::PolicyService* policy_service);
XdrManager(const XdrManager&) = delete;
XdrManager& operator=(const XdrManager&) = delete;
~XdrManager();

// Returns whether or not XDR events are being reported.
bool AreXdrPoliciesEnabled();

private:
// Updates the network metadata store value when the XDR policy changes.
void OnXdrPolicyChange(const base::Value* previous,
const base::Value* current);
// Sets the current value of the XDR policy in network metadata store.
void SetNetworkMetadataStoreXdrValue();

std::unique_ptr<policy::PolicyChangeRegistrar> policy_registrar_;
bool report_xdr_events_enabled_ = false;
};

} // namespace ash

#endif // CHROME_BROWSER_ASH_NET_XDR_MANAGER_H_
2 changes: 2 additions & 0 deletions chrome/test/data/webui/chromeos/fake_network_config_mojom.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ export class FakeNetworkConfig {
allow_only_policy_networks_to_autoconnect: false,
allow_only_policy_wifi_networks_to_connect: false,
allow_only_policy_wifi_networks_to_connect_if_available: false,
dns_queries_monitored: false,
report_xdr_events_enabled: false,
blocked_hex_ssids: [],
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,10 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) ManagedNetworkConfigurationHandler {
// Return the list of blocked WiFi networks (identified by HexSSIDs).
virtual std::vector<std::string> GetBlockedHexSSIDs() const = 0;

// Called after either secure DNS status or deviceReportXDREvents policy is
// updated.
virtual void OnEnterpriseMonitoredWebPoliciesApplied() const = 0;

// Called just before destruction to give observers a chance to remove
// themselves and disable any networking.
virtual void Shutdown() = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,13 @@ void ManagedNetworkConfigurationHandlerImpl::OnCellularPoliciesApplied(
}
}

void ManagedNetworkConfigurationHandlerImpl::
OnEnterpriseMonitoredWebPoliciesApplied() const {
for (auto& observer : observers_) {
observer.PoliciesApplied(std::string());
}
}

void ManagedNetworkConfigurationHandlerImpl::OnPoliciesApplied(
const NetworkProfile& profile,
const base::flat_set<std::string>& new_cellular_policy_guids) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) ManagedNetworkConfigurationHandlerImpl
const base::Value::Dict& new_properties,
base::OnceClosure callback) override;

void OnEnterpriseMonitoredWebPoliciesApplied() const override;

void OnPoliciesApplied(
const NetworkProfile& profile,
const base::flat_set<std::string>& new_cellular_policy_guids) override;
Expand Down

0 comments on commit 5e476d2

Please sign in to comment.