Skip to content

Commit

Permalink
Support for mojom RuntimeFeature on interfaces
Browse files Browse the repository at this point in the history
This CL adds support for the `RuntimeFeature` attribute on mojom
interfaces in C++. See doc[0] for details.

```
module foo.mojom;

feature kTastyFeature {
  const string name = "TastyFeature";
  const bool default_state = false;
};

[RuntimeFeature=kTastyFeature]
interface GatedInterface {
  // GatedInterface cannot be bound to a remote<> or receiver<>.
};

interface OtherInterface {
  // Sending disabled interfaces will result in bad messages.
  PassInterface(pending_remote<GatedInterface>? iface);
}
```

Developers should test the runtime feature to avoid binding or
calling a disabled interface:

```
 remote<OtherInterface> remote = ...;

 if (base::FeatureList::IsEnabled(kTastyFeature)) {
  PendingRemote<GatedInterface> pending_guarded;
  PendingReceiver<GatedInterface> pending_recv =
      pending_guarded.InitWithNewEndpointAndPassReceiver();
  auto impl = std::make_unique<GatedImpl>();
  auto weak_ref =
      MakeSelfOwnedReceiver(std::move(impl),
                           std::move(pending_recv));

    remote->PassInterface(std::move(pending_guarded));
 }
```

The annotation exists to allow
feature-flags to be enforced by the IPC system - but attempting to
use a disabled interface may lead to safe crashes in production
builds - it is up to developers to consult the exposed underlying
feature before attempting to use a disabled feature. DCHECKS in
test builds should validate this by crashing early.

In the example above - if kTastyFeature is not enabled and
the if() statement omitted:-

* pending_guarded & pending_recv will never bind.
* MakeSelfOwnedReceiver will return a null weak_ref.
* remote->PassInterface() will not serialize or deserialize the gated
interface.

Details

Feature tests happen via a templated function defined in
runtime_features.h and specialized in mojom-module.h files for
interfaces that define a RuntimeEnabled attribute. Interfaces
not guarded with a feature will work as before - checks for these are
emitted as constexpr templates so should not incur a runtime cost.

In release (non-DCHECK) builds mojo C++ bindings will (often
silently) not bind mojo message pipes to concrete receiver<T>,
remote<T> or SelfOwnedReceiver<T>, and will refuse to serialize
pending disabled interfaces (resulting in a bad message).

In debug (DCHECK) builds mojo C++ bindings will crash if an
attempt is made to use a concrete remote<T> or receiver<T> for
an interface annotated with a disabled feature.

Compromised processes can still request or serialize disabled
interfaces, but the non-compromised end will consult its view
of the feature state and refuse to concretely bind such faked
requests.

Remote & Receiver sets gain Add() methods which return
an optional<Id> - these can replace uses of .Add() in contexts
where generic code might host feature guarded interfaces in future.

"casting" generic receivers via the .As<Interface>() method
will return null receivers or DCHECK.

ServiceFactory will safely skip .Add()ing binders for disabled
interfaces.

RuntimeFeature is not yet implemented for methods, and non-C++
bindings are not yet aware of mojo-hosted features.

[0] https://docs.google.com/document/d/1kKFHu73SNwpBfPJzrVH2vLbpu9_1WuZhngFBqtJhwqw/edit?pli=1#heading=h.2s5rrpm5k56k

Tests: mojo_unittests --gtest_filter=FeatureBindingsTest.*
Bug: 1278253
Change-Id: I2cace494dd766bdd59f4cf781c8432ee7ec00ca2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4779888
Reviewed-by: Daniel Cheng <[email protected]>
Commit-Queue: Alex Gough <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1230224}
  • Loading branch information
quidity authored and Chromium LUCI CQ committed Nov 28, 2023
1 parent 44e2910 commit 221f4e9
Show file tree
Hide file tree
Showing 33 changed files with 1,276 additions and 53 deletions.
1 change: 1 addition & 0 deletions mojo/public/cpp/bindings/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ component("bindings") {
"receiver_set.h",
"remote.h",
"remote_set.h",
"runtime_features.h",
"self_owned_associated_receiver.h",
"self_owned_receiver.h",
"sequence_local_sync_event_watcher.h",
Expand Down
46 changes: 46 additions & 0 deletions mojo/public/cpp/bindings/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,52 @@ TableListenerImpl impl(listener.InitWithNewPipeAndPassReceiver());
table->AddListener(std::move(listener));
```

### RuntimeFeature on interfaces

If an interface is marked with a `RuntimeFeature` attribute, and the associated
feature is disabled, then it is not possible to bind the interface to a
receiver, and not possible to create a remote to call methods on. Attempts to
bind remotes or receivers will result in the underlying pipe being `reset()`.
`SelfOwnedReceivers` will not be created. A compromised process can override
these checks and might falsely request a disabled interface but a trustworthy
process will not bind a concrete endpoint to interact with the disabled
interface.

Note that it remains possible to create and transport generic wrapper
objects to disabled interfaces - security decisions should be made based on a
test of the generated feature - or the bound state of a Remote or Receiver.

```mojom
// Feature controls runtime availability of interface.
[RuntimeFeature=kUseElevator]
interface DefaultDenied {
GetInt() => (int32 ret);
};
interface PassesInterfaces {
BindPendingRemoteDisabled(pending_remote<DefaultDenied> iface);
BindPendingReceiverDisabled(pending_receiver<DefaultDenied> iface);
};
```

```C++
void BindPendingRemoteDisabled(
mojo::PendingRemote<mojom::DefaultDenied> iface) override {
mojo::Remote<mojom::DefaultDenied> denied_remote;
// Remote will not bind:
denied_remote.Bind(std::move(iface));
ASSERT_FALSE(denied_remote);
}
void BindPendingReceiverDisabled(
mojo::PendingReceiver<mojom::DefaultDenied> iface) override {
std::unique_ptr<DefaultDeniedImpl> denied_impl;
// Object can still be created:
denied_impl = std::make_unique<DefaultDeniedImpl>(std::move(iface));
// But its internal receiver_ will not bind or receive remote calls.
ASSERT_FALSE(denied_impl->receiver().is_bound());
}
```
## Other Interface Binding Types
The [Interfaces](#Interfaces) section above covers basic usage of the most
Expand Down
31 changes: 19 additions & 12 deletions mojo/public/cpp/bindings/associated_receiver.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "mojo/public/cpp/bindings/raw_ptr_impl_ref_traits.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/runtime_features.h"

namespace mojo {

Expand Down Expand Up @@ -189,8 +190,10 @@ class AssociatedReceiver : public internal::AssociatedReceiverBase {
scoped_refptr<base::SequencedTaskRunner> task_runner = nullptr) {
DCHECK(!is_bound()) << "AssociatedReceiver for " << Interface::Name_
<< " is already bound";

PendingAssociatedRemote<Interface> remote;
if (!internal::GetRuntimeFeature_ExpectEnabled<Interface>()) {
return remote;
}
Bind(remote.InitWithNewEndpointAndPassReceiver(), std::move(task_runner));
return remote;
}
Expand All @@ -204,17 +207,20 @@ class AssociatedReceiver : public internal::AssociatedReceiverBase {
scoped_refptr<base::SequencedTaskRunner> task_runner = nullptr) {
DCHECK(!is_bound()) << "AssociatedReceiver for " << Interface::Name_
<< " is already bound";

if (pending_receiver) {
BindImpl(pending_receiver.PassHandle(), &stub_,
base::WrapUnique(new typename Interface::RequestValidator_()),
internal::SyncMethodTraits<Interface>::GetOrdinals(),
std::move(task_runner), Interface::Version_, Interface::Name_,
Interface::MessageToMethodInfo_,
Interface::MessageToMethodName_);
} else {
if (!pending_receiver) {
reset();
return;
}
if (!internal::GetRuntimeFeature_ExpectEnabled<Interface>()) {
reset();
return;
}

BindImpl(pending_receiver.PassHandle(), &stub_,
base::WrapUnique(new typename Interface::RequestValidator_()),
internal::SyncMethodTraits<Interface>::GetOrdinals(),
std::move(task_runner), Interface::Version_, Interface::Name_,
Interface::MessageToMethodInfo_, Interface::MessageToMethodName_);
}

// Binds this AssociatedReceiver with the returned PendingAssociatedRemote
Expand All @@ -230,9 +236,10 @@ class AssociatedReceiver : public internal::AssociatedReceiverBase {
BindNewEndpointAndPassDedicatedRemote() {
DCHECK(!is_bound()) << "AssociatedReceiver for " << Interface::Name_
<< " is already bound";

PendingAssociatedRemote<Interface> remote = BindNewEndpointAndPassRemote();
remote.EnableUnassociatedUsage();
if (remote) {
remote.EnableUnassociatedUsage();
}
return remote;
}

Expand Down
13 changes: 11 additions & 2 deletions mojo/public/cpp/bindings/associated_remote.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h"
#include "mojo/public/cpp/bindings/pending_associated_receiver.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "mojo/public/cpp/bindings/runtime_features.h"
#include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h"

namespace mojo {
Expand Down Expand Up @@ -185,7 +186,9 @@ class AssociatedRemote {
scoped_refptr<base::SequencedTaskRunner> task_runner = nullptr) {
DCHECK(!is_bound()) << "AssociatedRemote for " << Interface::Name_
<< " is already bound";

if (!internal::GetRuntimeFeature_ExpectEnabled<Interface>()) {
return PendingAssociatedReceiver<Interface>();
}
ScopedInterfaceEndpointHandle remote_handle;
ScopedInterfaceEndpointHandle receiver_handle;
ScopedInterfaceEndpointHandle::CreatePairPendingAssociation(
Expand All @@ -209,6 +212,10 @@ class AssociatedRemote {
reset();
return;
}
if (!internal::GetRuntimeFeature_ExpectEnabled<Interface>()) {
reset();
return;
}

internal_state_.Bind(
AssociatedInterfacePtrInfo<Interface>(pending_remote.PassHandle(),
Expand Down Expand Up @@ -238,7 +245,9 @@ class AssociatedRemote {

PendingAssociatedReceiver<Interface> receiver =
BindNewEndpointAndPassReceiver();
receiver.EnableUnassociatedUsage();
if (receiver) {
receiver.EnableUnassociatedUsage();
}
return receiver;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/component_export.h"
#include "base/strings/string_piece.h"
#include "mojo/public/cpp/bindings/pending_associated_receiver.h"
#include "mojo/public/cpp/bindings/runtime_features.h"
#include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h"

namespace mojo {
Expand Down Expand Up @@ -65,6 +66,9 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) GenericPendingAssociatedReceiver {
// if and only if that interface's name matches the stored interface name.
template <typename Interface>
mojo::PendingAssociatedReceiver<Interface> As() {
if (!internal::GetRuntimeFeature_ExpectEnabled<Interface>()) {
return mojo::PendingAssociatedReceiver<Interface>();
}
return mojo::PendingAssociatedReceiver<Interface>(
PassHandleIfNameIs(Interface::Name_));
}
Expand Down
7 changes: 6 additions & 1 deletion mojo/public/cpp/bindings/generic_pending_receiver.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/component_export.h"
#include "base/strings/string_piece.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/runtime_features.h"
#include "mojo/public/cpp/system/message_pipe.h"
#include "third_party/perfetto/include/perfetto/tracing/traced_value_forward.h"

Expand Down Expand Up @@ -58,9 +59,13 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) GenericPendingReceiver {
mojo::ScopedMessagePipeHandle PassPipe();

// Takes ownership of the pipe, strongly typed as an |Interface| receiver, if
// and only if that interface's name matches the stored interface name.
// and only if that interface's name matches the stored interface name. May
// return an empty pending receiver for RuntimeFeature disabled Interfaces.
template <typename Interface>
mojo::PendingReceiver<Interface> As() {
if (!internal::GetRuntimeFeature_ExpectEnabled<Interface>()) {
return mojo::PendingReceiver<Interface>();
}
return mojo::PendingReceiver<Interface>(PassPipeIfNameIs(Interface::Name_));
}

Expand Down
31 changes: 31 additions & 0 deletions mojo/public/cpp/bindings/lib/interface_serialization.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/runtime_features.h"
#include "mojo/public/cpp/system/handle.h"
#include "mojo/public/cpp/system/message_pipe.h"

Expand All @@ -34,13 +35,19 @@ struct Serializer<AssociatedInterfacePtrInfoDataView<Base>,
AssociatedInterface_Data* output,
Message* message) {
DCHECK(!input.handle().is_valid() || input.handle().pending_association());
if (!GetRuntimeFeature_ExpectEnabled<T>()) {
input.reset();
}
SerializeAssociatedInterfaceInfo(input.PassHandle(), input.version(),
*message, *output);
}

static bool Deserialize(AssociatedInterface_Data* input,
PendingAssociatedRemote<T>* output,
Message* message) {
if (!GetRuntimeFeature_ExpectEnabled<T>()) {
return false;
}
auto handle = DeserializeAssociatedEndpointHandle(input->handle, *message);
if (!handle.is_valid()) {
*output = PendingAssociatedRemote<T>();
Expand All @@ -61,12 +68,18 @@ struct Serializer<AssociatedInterfaceRequestDataView<Base>,
AssociatedEndpointHandle_Data* output,
Message* message) {
DCHECK(!input.handle().is_valid() || input.handle().pending_association());
if (!GetRuntimeFeature_ExpectEnabled<T>()) {
input.reset();
}
SerializeAssociatedEndpoint(input.PassHandle(), *message, *output);
}

static bool Deserialize(AssociatedEndpointHandle_Data* input,
PendingAssociatedReceiver<T>* output,
Message* message) {
if (!GetRuntimeFeature_ExpectEnabled<T>()) {
return false;
}
auto handle = DeserializeAssociatedEndpointHandle(*input, *message);
if (!handle.is_valid())
*output = PendingAssociatedReceiver<T>();
Expand All @@ -83,12 +96,18 @@ struct Serializer<AssociatedInterfaceRequestDataView<T>,
AssociatedEndpointHandle_Data* output,
Message* message) {
DCHECK(!input.is_valid() || input.pending_association());
if (!GetRuntimeFeature_ExpectEnabled<T>()) {
input.reset();
}
SerializeAssociatedEndpoint(std::move(input), *message, *output);
}

static bool Deserialize(AssociatedEndpointHandle_Data* input,
ScopedInterfaceEndpointHandle* output,
Message* message) {
if (!GetRuntimeFeature_ExpectEnabled<T>()) {
return false;
}
*output = DeserializeAssociatedEndpointHandle(*input, *message);
return true;
}
Expand All @@ -101,6 +120,9 @@ struct Serializer<InterfacePtrDataView<Base>, PendingRemote<T>> {
static void Serialize(PendingRemote<T>& input,
Interface_Data* output,
Message* message) {
if (!GetRuntimeFeature_ExpectEnabled<T>()) {
input.reset();
}
// |PassPipe()| invalidates all state, so capture |version()| first.
uint32_t version = input.version();
SerializeInterfaceInfo(input.PassPipe(), version, *message, *output);
Expand All @@ -109,6 +131,9 @@ struct Serializer<InterfacePtrDataView<Base>, PendingRemote<T>> {
static bool Deserialize(Interface_Data* input,
PendingRemote<T>* output,
Message* message) {
if (!GetRuntimeFeature_ExpectEnabled<T>()) {
return false;
}
*output = PendingRemote<T>(
DeserializeHandleAs<MessagePipeHandle>(input->handle, *message),
input->version);
Expand All @@ -123,12 +148,18 @@ struct Serializer<InterfaceRequestDataView<Base>, PendingReceiver<T>> {
static void Serialize(PendingReceiver<T>& input,
Handle_Data* output,
Message* message) {
if (!GetRuntimeFeature_ExpectEnabled<T>()) {
input.reset();
}
SerializeHandle(ScopedHandle::From(input.PassPipe()), *message, *output);
}

static bool Deserialize(Handle_Data* input,
PendingReceiver<T>* output,
Message* message) {
if (!GetRuntimeFeature_ExpectEnabled<T>()) {
return false;
}
DeserializeHandleAsReceiver(*input, *message, *output->internal_state());
return true;
}
Expand Down
4 changes: 4 additions & 0 deletions mojo/public/cpp/bindings/pending_associated_receiver.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/task/sequenced_task_runner.h"
#include "build/build_config.h"
#include "mojo/public/cpp/bindings/lib/multiplex_router.h"
#include "mojo/public/cpp/bindings/runtime_features.h"
#include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h"
#include "mojo/public/cpp/system/message_pipe.h"

Expand Down Expand Up @@ -135,6 +136,9 @@ namespace mojo {
template <typename Interface>
PendingAssociatedRemote<Interface>
PendingAssociatedReceiver<Interface>::InitWithNewEndpointAndPassRemote() {
if (!internal::GetRuntimeFeature_ExpectEnabled<Interface>()) {
return PendingAssociatedRemote<Interface>();
}
ScopedInterfaceEndpointHandle remote_handle;
ScopedInterfaceEndpointHandle::CreatePairPendingAssociation(&handle_,
&remote_handle);
Expand Down
4 changes: 4 additions & 0 deletions mojo/public/cpp/bindings/pending_associated_remote.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "build/build_config.h"
#include "mojo/public/cpp/bindings/associated_interface_ptr_info.h"
#include "mojo/public/cpp/bindings/lib/multiplex_router.h"
#include "mojo/public/cpp/bindings/runtime_features.h"
#include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h"
#include "mojo/public/cpp/system/message_pipe.h"

Expand Down Expand Up @@ -132,6 +133,9 @@ namespace mojo {
template <typename Interface>
PendingAssociatedReceiver<Interface>
PendingAssociatedRemote<Interface>::InitWithNewEndpointAndPassReceiver() {
if (!internal::GetRuntimeFeature_ExpectEnabled<Interface>()) {
return PendingAssociatedReceiver<Interface>();
}
ScopedInterfaceEndpointHandle receiver_handle;
ScopedInterfaceEndpointHandle::CreatePairPendingAssociation(&handle_,
&receiver_handle);
Expand Down
4 changes: 4 additions & 0 deletions mojo/public/cpp/bindings/pending_receiver.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "mojo/public/cpp/bindings/lib/pending_receiver_state.h"
#include "mojo/public/cpp/bindings/message.h"
#include "mojo/public/cpp/bindings/pipe_control_message_proxy.h"
#include "mojo/public/cpp/bindings/runtime_features.h"
#include "mojo/public/cpp/system/message_pipe.h"

namespace mojo {
Expand Down Expand Up @@ -165,6 +166,9 @@ template <typename Interface>
PendingRemote<Interface>
PendingReceiver<Interface>::InitWithNewPipeAndPassRemote() {
DCHECK(!is_valid()) << "PendingReceiver already has a remote";
if (!internal::GetRuntimeFeature_ExpectEnabled<Interface>()) {
return PendingRemote<Interface>();
}
MessagePipe pipe;
state_.pipe = std::move(pipe.handle0);
return PendingRemote<Interface>(std::move(pipe.handle1), 0u);
Expand Down
4 changes: 4 additions & 0 deletions mojo/public/cpp/bindings/pending_remote.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "mojo/public/cpp/bindings/lib/pending_remote_state.h"
#include "mojo/public/cpp/bindings/message.h"
#include "mojo/public/cpp/bindings/pipe_control_message_proxy.h"
#include "mojo/public/cpp/bindings/runtime_features.h"
#include "mojo/public/cpp/system/message_pipe.h"

namespace mojo {
Expand Down Expand Up @@ -168,6 +169,9 @@ template <typename Interface>
PendingReceiver<Interface>
PendingRemote<Interface>::InitWithNewPipeAndPassReceiver() {
DCHECK(!is_valid()) << "PendingReceiver already has a remote";
if (!internal::GetRuntimeFeature_ExpectEnabled<Interface>()) {
return PendingReceiver<Interface>();
}
MessagePipe pipe;
state_.pipe = std::move(pipe.handle0);
return PendingReceiver<Interface>(std::move(pipe.handle1));
Expand Down
Loading

0 comments on commit 221f4e9

Please sign in to comment.