Skip to content

Commit

Permalink
Fix CRD remote command when RemoteAccessHostDomainList is set
Browse files Browse the repository at this point in the history
The CRD Remote Command uses a very specific, internal host domain
(chrome-enterprise-devices.gserviceaccount.com) to establish the
connection.

If the customer uses the RemoteAccessHostDomainList policy this will
cause the CRD Remote Command to be rejected, unless the customer adds
this internal host domain to their allowlist.

This CL fixes this by skipping the host domain check for the CRD Remote
Command enterprise connections.

(cherry picked from commit 9d3fc53)

Fixed: b/227767159
Test: new unittest in remoting_unittests
Test: manually deployed
Change-Id: I21952f132ce0b6a49fdb8012ab6cbb174007052e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3649602
Reviewed-by: Joe Downing <joedow@chromium.org>
Auto-Submit: Jeroen Dhollander <jeroendh@google.com>
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1004293}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3663223
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#398}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
Jeroen Dhollander authored and Chromium LUCI CQ committed May 31, 2022
1 parent 52a55c9 commit 1288dc3
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 1 deletion.
5 changes: 4 additions & 1 deletion remoting/host/it2me/it2me_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,10 @@ void It2MeHost::ConnectOnNetworkThread(
}

// Check the host domain policy.
if (!required_host_domain_list_.empty()) {
// Skip this check for enterprise sessions, as they use the device specific
// robot account as host, and we should not expect the customers to add this
// internal account to their host domain list.
if (!is_enterprise_session_ && !required_host_domain_list_.empty()) {
bool matched = false;
for (const auto& domain : required_host_domain_list_) {
if (base::EndsWith(username, std::string("@") + domain,
Expand Down
37 changes: 37 additions & 0 deletions remoting/host/it2me/it2me_host_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,30 @@ const char kTestStunServer[] = "test_relay_server.com";

} // namespace

// This is invoked automatically by the gtest framework, and improves the error
// messages when a test fails (by properly formatting the host state instead
// of printing their byte value).
void PrintTo(It2MeHostState state, std::ostream* os) {
#define CASE(_state) \
case It2MeHostState::_state: \
*os << #_state; \
return;

switch (state) {
CASE(kDisconnected);
CASE(kStarting);
CASE(kRequestedAccessCode);
CASE(kReceivedAccessCode);
CASE(kConnecting);
CASE(kConnected);
CASE(kError);
CASE(kInvalidDomainError);
}
NOTREACHED();
*os << "Unknown state " << static_cast<int>(state);
return;
}

class FakeIt2MeConfirmationDialog : public It2MeConfirmationDialog {
public:
FakeIt2MeConfirmationDialog(const std::string& remote_user_email,
Expand Down Expand Up @@ -812,6 +836,19 @@ TEST_F(It2MeHostTest,
ASSERT_EQ(It2MeHostState::kDisconnected, last_host_state_);
ASSERT_EQ(ErrorCode::OK, last_error_code_);
}

TEST_F(It2MeHostTest, EnterpriseSessionsShouldNotCheckHostDomain) {
SetPolicies({{policy::key::kRemoteAccessHostDomainList,
MakeList({"other-domain.com"})}});

is_enterprise_session_ = true;
StartHost();
ASSERT_EQ(It2MeHostState::kReceivedAccessCode, last_host_state_);

ShutdownHost();
ASSERT_EQ(It2MeHostState::kDisconnected, last_host_state_);
ASSERT_EQ(ErrorCode::OK, last_error_code_);
}
#endif

} // namespace remoting

0 comments on commit 1288dc3

Please sign in to comment.