NamedMojoIpcServer: Allow per-connection implementations.

Each connection may have a different implementation to bind to,
and the implementation may depend upon the process ID of the
remote (although none of the current callers do).

Future callers will depends on the UID of the remote.

Bug: 1339108
Change-Id: Iaa4309c58c6831325494a589fb5713fdad881763
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4071784
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1080053}

chrome/updater/app/server/linux/update_service_stub.cc

@@ -95,11 +95,6 @@ MakeStateChangeObserverCallbacks(
       base::BindOnce(&StateChangeObserverWrapper::OnComplete, wrapper)};
 }
 
-// TODO(crbug.com/1378742): Implement some form of validation.
-static bool IsTrustedIPCEndpoint(base::ProcessId /*caller_pid*/) {
-  return true;
-}
-
 }  // namespace
 
 UpdateServiceStub::UpdateServiceStub(scoped_refptr<updater::UpdateService> impl,
@@ -107,8 +102,14 @@ UpdateServiceStub::UpdateServiceStub(scoped_refptr<updater::UpdateService> impl,
     : server_(
           GetActiveDutySocketPath(scope).MaybeAsASCII(),
           named_mojo_ipc_server::NamedMojoIpcServerBase::kUseIsolatedConnection,
-          this,
-          base::BindRepeating(&IsTrustedIPCEndpoint)),
+          base::BindRepeating(base::BindRepeating(
+              [](mojom::UpdateService* interface,
+                 base::ProcessId /* caller_pid */) {
+                // TODO(crbug.com/1378742): Implement some form of
+                // validation.
+                return interface;
+              },
+              this))),
       impl_(impl) {
   server_.set_disconnect_handler(base::BindRepeating(
       &UpdateServiceStub::OnClientDisconnected, base::Unretained(this)));

chrome/updater/app/server/posix/update_service_internal_stub.cc

@@ -28,12 +28,13 @@ UpdateServiceInternalStub::UpdateServiceInternalStub(
     : server_(
           GetUpdateServiceInternalServerName(scope),
           named_mojo_ipc_server::NamedMojoIpcServerBase::kUseIsolatedConnection,
-          this,
-          base::BindRepeating([](base::ProcessId pid) {
-            // TODO(crbug.com/1394773): Check the UID of the connecting process,
-            // not the PID.
-            return true;
-          })),
+          base::BindRepeating(
+              [](mojom::UpdateServiceInternal* interface, base::ProcessId pid) {
+                // TODO(crbug.com/1394773): Check the UID of the connecting
+                // process, not the PID.
+                return interface;
+              },
+              this)),
       impl_(impl),
       task_start_listener_(task_start_listener),
       task_end_listener_(task_end_listener) {

components/named_mojo_ipc_server/README.md

@@ -26,14 +26,12 @@ class MyInterfaceImpl: public mojom::MyInterface {
     server_.Close(server_.current_receiver());
   }
 
-  static bool IsTrustedMojoEndpoint(base::ProcessId caller_pid) {
-    // Verify the calling process...
-    return true;
-  }
-
   NamedMojoIpcServer<mojom::MyInterface> server_{"my_server_name",
       kMessagePipeId, this,
-      base::BindRepeating(&MyInterfaceImpl::IsTrustedMojoEndpoint)};
+      base::BindRepeating([](mojom::MyInterface* impl, base::ProcessId caller) {
+        // Verify the calling process, returning nullptr if unverified.
+        return impl; // impl must outlive NamedMojoIpcServer.
+      }, this)};
 };
 ```
 
@@ -66,3 +64,7 @@ additional connection brokerage steps which are abstracted by the former.
 
 On Windows, the server needs to have the following access rights on the client
 process: `PROCESS_DUP_HANDLE | PROCESS_QUERY_INFORMATION`.
+
+It is possible to bind a different implementation of the interface to each
+connection by returning different `mojom::MyInterface*` values rather than
+`this`. All implementations must outlive the NamedMojoIpcServer.

components/named_mojo_ipc_server/named_mojo_ipc_server.cc

@@ -60,10 +60,10 @@ void NamedMojoIpcServerBase::DelegateProxy::OnServerEndpointCreated() {
 NamedMojoIpcServerBase::NamedMojoIpcServerBase(
     const mojo::NamedPlatformChannel::ServerName& server_name,
     absl::optional<uint64_t> message_pipe_id,
-    IsTrustedMojoEndpointCallback is_trusted_endpoint_callback)
+    base::RepeatingCallback<void*(base::ProcessId)> impl_provider)
     : server_name_(server_name),
       message_pipe_id_(message_pipe_id),
-      is_trusted_endpoint_callback_(std::move(is_trusted_endpoint_callback)) {
+      impl_provider_(impl_provider) {
   io_sequence_ =
       base::ThreadPool::CreateSequencedTaskRunner({base::MayBlock()});
 }
@@ -118,7 +118,8 @@ void NamedMojoIpcServerBase::OnIpcDisconnected() {
 void NamedMojoIpcServerBase::OnClientConnected(
     mojo::PlatformChannelEndpoint endpoint,
     base::ProcessId peer_pid) {
-  if (!is_trusted_endpoint_callback_.Run(peer_pid)) {
+  void* impl = impl_provider_.Run(peer_pid);
+  if (!impl) {
     LOG(ERROR) << "Process " << peer_pid
                << " is not a trusted mojo endpoint. Connection refused.";
     return;
@@ -130,7 +131,7 @@ void NamedMojoIpcServerBase::OnClientConnected(
     mojo::ScopedMessagePipeHandle message_pipe =
         connection->Connect(std::move(endpoint));
     mojo::ReceiverId receiver_id =
-        TrackMessagePipe(std::move(message_pipe), peer_pid);
+        TrackMessagePipe(std::move(message_pipe), impl, peer_pid);
     active_connections_[receiver_id] = std::move(connection);
     return;
   }
@@ -158,7 +159,7 @@ void NamedMojoIpcServerBase::OnClientConnected(
 #undef INVALID_PROCESS_LOG
   mojo::OutgoingInvitation::Send(std::move(invitation), peer_process.Handle(),
                                  std::move(endpoint));
-  TrackMessagePipe(std::move(message_pipe), peer_pid);
+  TrackMessagePipe(std::move(message_pipe), impl, peer_pid);
 }
 
 void NamedMojoIpcServerBase::OnServerEndpointCreated() {

components/named_mojo_ipc_server/named_mojo_ipc_server.h

@@ -41,9 +41,6 @@ namespace named_mojo_ipc_server {
 // see MojoIpcServer.
 class NamedMojoIpcServerBase : public IpcServer {
  public:
-  using IsTrustedMojoEndpointCallback =
-      base::RepeatingCallback<bool(base::ProcessId)>;
-
   // DEPRECATED: New callers should not use an isolated connection. Pass a
   // valid message pipe ID instead.
   static constexpr absl::optional<uint64_t> kUseIsolatedConnection =
@@ -67,13 +64,14 @@ class NamedMojoIpcServerBase : public IpcServer {
   NamedMojoIpcServerBase(
       const mojo::NamedPlatformChannel::ServerName& server_name,
       absl::optional<uint64_t> message_pipe_id,
-      IsTrustedMojoEndpointCallback is_trusted_endpoint_callback);
+      base::RepeatingCallback<void*(base::ProcessId)> impl_provider);
   ~NamedMojoIpcServerBase() override;
 
   void OnIpcDisconnected();
 
   virtual mojo::ReceiverId TrackMessagePipe(
       mojo::ScopedMessagePipeHandle message_pipe,
+      void* impl,
       base::ProcessId peer_pid) = 0;
 
   virtual void UntrackMessagePipe(mojo::ReceiverId id) = 0;
@@ -114,7 +112,7 @@ class NamedMojoIpcServerBase : public IpcServer {
 
   mojo::NamedPlatformChannel::ServerName server_name_;
   absl::optional<uint64_t> message_pipe_id_;
-  IsTrustedMojoEndpointCallback is_trusted_endpoint_callback_;
+  base::RepeatingCallback<void*(base::ProcessId)> impl_provider_;
 
   bool server_started_ = false;
 
@@ -146,16 +144,20 @@ class NamedMojoIpcServer final : public NamedMojoIpcServerBase {
   //     kUseIsolatedConnection is used), the client must connect using an
   //     isolated connection. Note that using an isolated connection is
   //     DEPRECATED and new callers should always pass a valid message pipe ID.
-  // is_trusted_endpoint_callback: A predicate which returns true if the process
-  // referred to by the caller PID is a trusted mojo endpoint.
-  NamedMojoIpcServer(const mojo::NamedPlatformChannel::ServerName& server_name,
-                     absl::optional<uint64_t> message_pipe_id,
-                     Interface* interface_impl,
-                     IsTrustedMojoEndpointCallback is_trusted_endpoint_callback)
-      : NamedMojoIpcServerBase(server_name,
-                               message_pipe_id,
-                               std::move(is_trusted_endpoint_callback)),
-        interface_impl_(interface_impl) {
+  // impl_provider: A function that returns a pointer to an implementation,
+  //     or nullptr if the connecting endpoint should be rejected.
+  NamedMojoIpcServer(
+      const mojo::NamedPlatformChannel::ServerName& server_name,
+      absl::optional<uint64_t> message_pipe_id,
+      base::RepeatingCallback<Interface*(base::ProcessId)> impl_provider)
+      : NamedMojoIpcServerBase(
+            server_name,
+            message_pipe_id,
+            impl_provider.Then(base::BindRepeating([](Interface* impl) {
+              // Opacify the type for the base class, which takes no template
+              // parameters.
+              return reinterpret_cast<void*>(impl);
+            }))) {
     receiver_set_.set_disconnect_handler(base::BindRepeating(
         &NamedMojoIpcServer::OnIpcDisconnected, base::Unretained(this)));
   }
@@ -184,11 +186,12 @@ class NamedMojoIpcServer final : public NamedMojoIpcServerBase {
  private:
   // NamedMojoIpcServerBase implementation.
   mojo::ReceiverId TrackMessagePipe(mojo::ScopedMessagePipeHandle message_pipe,
+                                    void* impl,
                                     base::ProcessId peer_pid) override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
     return receiver_set_.Add(
-        interface_impl_,
+        reinterpret_cast<Interface*>(impl),
         mojo::PendingReceiver<Interface>(std::move(message_pipe)), peer_pid);
   }
 
@@ -200,7 +203,6 @@ class NamedMojoIpcServer final : public NamedMojoIpcServerBase {
 
   void UntrackAllMessagePipes() override { receiver_set_.Clear(); }
 
-  raw_ptr<Interface> interface_impl_;
   mojo::ReceiverSet<Interface, base::ProcessId> receiver_set_;
 };
 

components/named_mojo_ipc_server/named_mojo_ipc_server_unittest.cc

@@ -143,7 +143,8 @@ void NamedMojoIpcServerTest::CreateIpcServer() {
       test_server_name_,
       GetParam() ? NamedMojoIpcServerBase::kUseIsolatedConnection
                  : kTestMessagePipeId,
-      this, base::BindRepeating([](base::ProcessId) { return true; }));
+      base::BindRepeating(
+          [](test::mojom::Echo* impl, base::ProcessId) { return impl; }, this));
   ipc_server_->set_on_server_endpoint_created_callback_for_testing(
       base::BindRepeating(&NamedMojoIpcServerTest::OnServerEndpointCreated,
                           base::Unretained(this)));

remoting/host/chromoting_host.cc

@@ -136,8 +136,13 @@ void ChromotingHost::StartChromotingHostServices() {
 #endif
   ipc_server_ = std::make_unique<
       named_mojo_ipc_server::NamedMojoIpcServer<mojom::ChromotingHostServices>>(
-      GetChromotingHostServicesServerName(), message_pipe_id, this,
-      base::BindRepeating(&IsTrustedMojoEndpoint));
+      GetChromotingHostServicesServerName(), message_pipe_id,
+      base::BindRepeating(&IsTrustedMojoEndpoint)
+          .Then(base::BindRepeating(
+              [](mojom::ChromotingHostServices* impl, bool trusted) {
+                return trusted ? impl : nullptr;
+              },
+              base::Unretained(this))));
   ipc_server_->StartServer();
   HOST_LOG << "ChromotingHostServices IPC server has been started.";
 #else