Skip to content

Commit

Permalink
Update CRD host classes to support clipboard size limiting
Browse files Browse the repository at this point in the history
One enterprise use case that we don't currently support is the
ability to limit the size of the data that can be transferred
between client and host via ClipboardEvents (or the ability to
disable clipboard sync altogether).

This CL updates the Me2Me and It2Me hosts along with some related
classes to allow for limiting the data on the host side. A
subsequent CL will add the Chrome policy and init the host members
using it. I'm also thinking about whether to send a new event from
the host when the clipboard is disabled, but that too will be
implemented in a subsequent CL if we decide to go that direction.

Change-Id: I9c8a90428eee77d5d67ab3607ddd111f85cc4f62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3234582
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/main@{#934612}
  • Loading branch information
joedow-42 authored and Chromium LUCI CQ committed Oct 25, 2021
1 parent d1e5baa commit 498ef31
Show file tree
Hide file tree
Showing 10 changed files with 126 additions and 15 deletions.
16 changes: 12 additions & 4 deletions remoting/host/client_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ ClientSession::ClientSession(
mouse_clamping_filter_(&remote_input_filter_),
desktop_and_cursor_composer_notifier_(&mouse_clamping_filter_, this),
disable_input_filter_(&desktop_and_cursor_composer_notifier_),
disable_clipboard_filter_(clipboard_echo_filter_.host_filter()),
client_clipboard_factory_(clipboard_echo_filter_.client_filter()),
host_clipboard_filter_(clipboard_echo_filter_.host_filter()),
client_clipboard_filter_(clipboard_echo_filter_.client_filter()),
client_clipboard_factory_(&client_clipboard_filter_),
max_duration_(max_duration),
pairing_registry_(pairing_registry),
connection_(std::move(connection)),
Expand Down Expand Up @@ -421,8 +422,15 @@ void ClientSession::OnConnectionAuthenticated() {
connection_->set_input_stub(&disable_input_filter_);
host_input_filter_.set_input_stub(input_injector_.get());

if (desktop_environment_options_.clipboard_size().has_value()) {
int max_size = desktop_environment_options_.clipboard_size().value();

client_clipboard_filter_.set_max_size(max_size);
host_clipboard_filter_.set_max_size(max_size);
}

// Connect the clipboard stubs.
connection_->set_clipboard_stub(&disable_clipboard_filter_);
connection_->set_clipboard_stub(&host_clipboard_filter_);
clipboard_echo_filter_.set_host_stub(input_injector_.get());
clipboard_echo_filter_.set_client_stub(connection_->client_stub());
}
Expand Down Expand Up @@ -592,7 +600,7 @@ void ClientSession::SetDisableInputs(bool disable_inputs) {
input_tracker_.ReleaseAll();

disable_input_filter_.set_enabled(!disable_inputs);
disable_clipboard_filter_.set_enabled(!disable_inputs);
host_clipboard_filter_.set_enabled(!disable_inputs);
}

uint32_t ClientSession::desktop_session_id() const {
Expand Down
7 changes: 5 additions & 2 deletions remoting/host/client_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,12 @@ class ClientSession : public protocol::HostStub,
// pipeline.
protocol::ClipboardEchoFilter clipboard_echo_filter_;

// Filters used to manage enabling & disabling of input & clipboard.
// Filters used to manage enabling & disabling of input.
protocol::InputFilter disable_input_filter_;
protocol::ClipboardFilter disable_clipboard_filter_;

// Used to enable/disable clipboard sync and to restrict payload size.
protocol::ClipboardFilter host_clipboard_filter_;
protocol::ClipboardFilter client_clipboard_filter_;

// Factory for weak pointers to the client clipboard stub.
// This must appear after |clipboard_echo_filter_|, so that it won't outlive
Expand Down
9 changes: 9 additions & 0 deletions remoting/host/desktop_environment_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,15 @@ void DesktopEnvironmentOptions::set_enable_remote_webauthn(bool enabled) {
enable_remote_webauthn_ = enabled;
}

const absl::optional<size_t>& DesktopEnvironmentOptions::clipboard_size()
const {
return clipboard_size_;
}

void DesktopEnvironmentOptions::set_clipboard_size(size_t clipboard_size) {
clipboard_size_ = clipboard_size;
}

void DesktopEnvironmentOptions::ApplySessionOptions(
const SessionOptions& options) {
#if defined(OS_WIN)
Expand Down
9 changes: 9 additions & 0 deletions remoting/host/desktop_environment_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "base/memory/weak_ptr.h"
#include "remoting/base/session_options.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/webrtc/modules/desktop_capture/desktop_capture_options.h"

namespace remoting {
Expand Down Expand Up @@ -49,6 +50,9 @@ class DesktopEnvironmentOptions final {
bool enable_remote_webauthn() const;
void set_enable_remote_webauthn(bool enabled);

const absl::optional<size_t>& clipboard_size() const;
void set_clipboard_size(size_t clipboard_size);

const webrtc::DesktopCaptureOptions* desktop_capture_options() const;
webrtc::DesktopCaptureOptions* desktop_capture_options();

Expand Down Expand Up @@ -83,6 +87,11 @@ class DesktopEnvironmentOptions final {
// True if this host has the remote WebAuthn feature enabled.
bool enable_remote_webauthn_ = false;

// If set, this value is used to constrain the amount of data that can be
// transferred using ClipboardEvents. A value of 0 will effectively disable
// clipboard sharing.
absl::optional<size_t> clipboard_size_;

// The DesktopCaptureOptions to initialize DesktopCapturer.
webrtc::DesktopCaptureOptions desktop_capture_options_;
};
Expand Down
8 changes: 7 additions & 1 deletion remoting/host/it2me/it2me_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,17 @@ void It2MeHost::ConnectOnNetworkThread(
protocol_config->set_webrtc_supported(true);
session_manager->set_protocol_config(std::move(protocol_config));

// Create the host.
// Set up the desktop environment options.
DesktopEnvironmentOptions options(DesktopEnvironmentOptions::CreateDefault());
options.set_enable_user_interface(enable_dialogs_);
options.set_enable_notifications(enable_notifications_);
options.set_terminate_upon_input(terminate_upon_input_);
// TODO(joedow): Init |clipboard_size_| via a Chrome policy.
if (clipboard_size_.has_value()) {
options.set_clipboard_size(clipboard_size_.value());
}

// Create the host.
host_ = std::make_unique<ChromotingHost>(
desktop_environment_factory_.get(), std::move(session_manager),
transport_context, host_context_->audio_task_runner(),
Expand Down
4 changes: 4 additions & 0 deletions remoting/host/it2me/it2me_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "remoting/protocol/port_range.h"
#include "remoting/protocol/validating_authenticator.h"
#include "remoting/signaling/signal_strategy.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace base {
class DictionaryValue;
Expand Down Expand Up @@ -217,6 +218,9 @@ class It2MeHost : public base::RefCountedThreadSafe<It2MeHost>,
// The host port range policy setting.
PortRange udp_port_range_;

// Stores the clipboard size policy value.
absl::optional<size_t> clipboard_size_;

// Tracks the JID of the remote user when in a connecting state.
std::string connecting_jid_;

Expand Down
6 changes: 6 additions & 0 deletions remoting/host/remoting_me2me_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ class HostProcess : public ConfigWatcher::Delegate,
base::Value config_{base::Value::Type::DICTIONARY};
std::string host_owner_;
bool is_googler_ = false;
absl::optional<size_t> clipboard_size_;

std::unique_ptr<PolicyWatcher> policy_watcher_;
PolicyState policy_state_ = POLICY_INITIALIZING;
Expand Down Expand Up @@ -1646,6 +1647,11 @@ void HostProcess::StartHost() {
desktop_environment_options_.set_enable_remote_webauthn(true);
#endif

// TODO(joedow): Init |clipboard_size_| via a Chrome policy.
if (clipboard_size_.has_value()) {
desktop_environment_options_.set_clipboard_size(clipboard_size_.value());
}

host_ = std::make_unique<ChromotingHost>(
desktop_environment_factory_.get(), std::move(session_manager),
transport_context, context_->audio_task_runner(),
Expand Down
23 changes: 18 additions & 5 deletions remoting/protocol/clipboard_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@

#include "remoting/protocol/clipboard_filter.h"

#include "remoting/proto/internal.pb.h"

namespace remoting {
namespace protocol {

ClipboardFilter::ClipboardFilter() : clipboard_stub_(nullptr), enabled_(true) {
}
ClipboardFilter::ClipboardFilter() = default;

ClipboardFilter::ClipboardFilter(ClipboardStub* clipboard_stub)
: clipboard_stub_(clipboard_stub), enabled_(true) {
}
: clipboard_stub_(clipboard_stub) {}

ClipboardFilter::~ClipboardFilter() = default;

Expand All @@ -21,8 +21,21 @@ void ClipboardFilter::set_clipboard_stub(ClipboardStub* clipboard_stub) {
}

void ClipboardFilter::InjectClipboardEvent(const ClipboardEvent& event) {
if (enabled_ && clipboard_stub_ != nullptr)
if (!enabled_ || !clipboard_stub_) {
return;
}

if (max_size_.has_value() && max_size_.value() == 0) {
return;
}

if (!max_size_.has_value() || max_size_.value() >= event.data().size()) {
clipboard_stub_->InjectClipboardEvent(event);
} else {
ClipboardEvent resized_event(event);
resized_event.mutable_data()->resize(max_size_.value());
clipboard_stub_->InjectClipboardEvent(resized_event);
}
}

} // namespace protocol
Expand Down
12 changes: 9 additions & 3 deletions remoting/protocol/clipboard_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@
#include "base/compiler_specific.h"
#include "base/macros.h"
#include "remoting/protocol/clipboard_stub.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace remoting {
namespace protocol {

// Forwards clipboard events to |clipboard_stub|, if configured. Event
// forwarding may also be disabled independently of the configured
// |clipboard_stub|. ClipboardFilters initially have event forwarding enabled.
// |clipboard_stub|. ClipboardFilters initially have event forwarding enabled
// and no maximum size set. If |max_size| is configured, it will be used to
// limit the amount of data transferred when InjectClipboardEvent() is called.
class ClipboardFilter : public ClipboardStub {
public:
ClipboardFilter();
Expand All @@ -32,12 +35,15 @@ class ClipboardFilter : public ClipboardStub {
void set_enabled(bool enabled) { enabled_ = enabled; }
bool enabled() const { return enabled_; }

void set_max_size(size_t max_size) { max_size_ = max_size; }

// ClipboardStub interface.
void InjectClipboardEvent(const ClipboardEvent& event) override;

private:
ClipboardStub* clipboard_stub_;
bool enabled_;
ClipboardStub* clipboard_stub_ = nullptr;
bool enabled_ = true;
absl::optional<size_t> max_size_;
};

} // namespace protocol
Expand Down
47 changes: 47 additions & 0 deletions remoting/protocol/clipboard_filter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,53 @@ TEST(ClipboardFilterTest, EventsPassThroughFilter) {
clipboard_filter.InjectClipboardEvent(MakeClipboardEvent("text","foo"));
}

// Verify that the filter truncates payload if larger than |max_size|.
TEST(ClipboardFilterTest, PayloadLargerThanMaxSize) {
MockClipboardStub clipboard_stub;
ClipboardFilter clipboard_filter(&clipboard_stub);
clipboard_filter.set_max_size(2);

EXPECT_CALL(clipboard_stub,
InjectClipboardEvent(EqualsClipboardEvent("text", "fo")));

clipboard_filter.InjectClipboardEvent(MakeClipboardEvent("text", "foo"));
}

// Verify that the filter does not truncate payload if equal to |max_size|.
TEST(ClipboardFilterTest, PayloadEqualToMaxSize) {
MockClipboardStub clipboard_stub;
ClipboardFilter clipboard_filter(&clipboard_stub);
clipboard_filter.set_max_size(3);

EXPECT_CALL(clipboard_stub,
InjectClipboardEvent(EqualsClipboardEvent("text", "foo")));

clipboard_filter.InjectClipboardEvent(MakeClipboardEvent("text", "foo"));
}

// Verify that the filter does not truncate payload if smaller than |max_size|.
TEST(ClipboardFilterTest, PayloadSmallerThanMaxSize) {
MockClipboardStub clipboard_stub;
ClipboardFilter clipboard_filter(&clipboard_stub);
clipboard_filter.set_max_size(4);

EXPECT_CALL(clipboard_stub,
InjectClipboardEvent(EqualsClipboardEvent("text", "foo")));

clipboard_filter.InjectClipboardEvent(MakeClipboardEvent("text", "foo"));
}

// Verify that the filter ignores events if max_size is 0.
TEST(ClipboardFilterTest, MaxSizeSetToZero) {
MockClipboardStub clipboard_stub;
ClipboardFilter clipboard_filter(&clipboard_stub);
clipboard_filter.set_max_size(0);

EXPECT_CALL(clipboard_stub, InjectClipboardEvent(_)).Times(0);

clipboard_filter.InjectClipboardEvent(MakeClipboardEvent("text", "foo"));
}

// Verify that the filter ignores events if disabled.
TEST(ClipboardFilterTest, IgnoreEventsIfDisabled) {
MockClipboardStub clipboard_stub;
Expand Down

0 comments on commit 498ef31

Please sign in to comment.