Perform InterfaceProviderSpec intersection in the ServiceManager

services/service_manager/service_manager.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "services/service_manager/service_manager.h"
 
 #include <stdint.h>
 
 #include <utility>
 
@@ skipping 29 matching lines @@
 namespace {
 
 const char kCapability_UserID[] = "service_manager:user_id";
 const char kCapability_ClientProcess[] = "service_manager:client_process";
 const char kCapability_InstanceName[] = "service_manager:instance_name";
 const char kCapability_AllUsers[] = "service_manager:all_users";
 const char kCapability_InstancePerChild[] =
     "service_manager:instance_per_child";
 const char kCapability_ServiceManager[] = "service_manager:service_manager";
 
+bool Succeeded(mojom::ConnectResult result) {
+  return result == mojom::ConnectResult::SUCCEEDED;
+}
+
+void RunCallback(ConnectParams* params,
+                 mojom::ConnectResult result,
+                 const std::string& user_id) {
+  if (!params->connect_callback().is_null()) {
+    params->connect_callback().Run(result, user_id);
+    return;
+  }
+  if (!params->bind_interface_callback().is_null())
+    params->bind_interface_callback().Run(result, user_id);
+}
+
 }  // namespace
 
 Identity CreateServiceManagerIdentity() {
   return Identity(service_manager::mojom::kServiceName, mojom::kRootUserID);
 }
 
 Identity CreateCatalogIdentity() {
   return Identity(catalog::mojom::kServiceName, mojom::kRootUserID);
 }
 
@@ skipping 58 matching lines @@
     } else {
       // Notify the ServiceManager that this Instance is really going away.
       service_manager_->OnInstanceStopped(identity_);
     }
   }
 
   ~Instance() override {
     Stop();
   }
 
-  bool ConnectToService(std::unique_ptr<ConnectParams>* connect_params) {
+  bool OnConnect(std::unique_ptr<ConnectParams>* in_params) {
     if (!service_.is_bound())
       return false;
 
-    std::unique_ptr<ConnectParams> params(std::move(*connect_params));
+    std::unique_ptr<ConnectParams> params(std::move(*in_params));
     if (!params->connect_callback().is_null()) {
       params->connect_callback().Run(mojom::ConnectResult::SUCCEEDED,
                                      identity_.user_id());
     }
 
     InterfaceProviderSpecMap specs;
-    Instance* source = service_manager_->GetExistingInstance(params->source());
+    Instance* source =
+        service_manager_->GetExistingInstance(params->source());
     if (source)
       specs = source->interface_provider_specs_;
 
     pending_service_connections_++;
     service_->OnConnect(ServiceInfo(params->source(), specs),
                         params->TakeRemoteInterfaces(),
                         base::Bind(&Instance::OnConnectComplete,
                                    base::Unretained(this)));
     return true;
   }
 
+  bool OnBindInterface(std::unique_ptr<ConnectParams>* in_params) {
+    if (!service_.is_bound())

[Ken Rockot(use gerrit already), 2017/01/12 23:05:00]

We need to run the response callback in this case. I suppose we just might never
(or rarely) hit this branch in OnConnect and hence that hasn't been triggering a
DCHECK similar to http://crbug.com/680700).

+      return false;
+
+    std::unique_ptr<ConnectParams> params(std::move(*in_params));
+    InterfaceProviderSpecMap source_specs;
+    InterfaceProviderSpec source_connection_spec;
+    Instance* source =
+        service_manager_->GetExistingInstance(params->source());
+    if (source) {
+      source_specs = source->interface_provider_specs_;
+      source_connection_spec = source->GetConnectionSpec();
+    }
+
+    InterfaceSet exposed = GetInterfacesToExpose(source_connection_spec,
+                                                 identity_,
+                                                 GetConnectionSpec());
+    bool allowed = (exposed.size() == 1 && exposed.count("*") == 1) ||
+        exposed.count(params->interface_name()) > 0;
+    if (!allowed) {
+      std::stringstream ss;
+      ss << "Connection InterfaceProviderSpec prevented service: "
+         << params->source().name() << " from binding interface: "
+         << params->interface_name() << " exposed by: " << identity_.name();
+      LOG(ERROR) << ss.str();
+      params->bind_interface_callback().Run(mojom::ConnectResult::ACCESS_DENIED,
+                                            identity_.user_id());
+      return false;
+    }
+
+    params->bind_interface_callback().Run(mojom::ConnectResult::SUCCEEDED,
+                                          identity_.user_id());
+
+    pending_service_connections_++;
+    service_->OnBindInterface(
+        ServiceInfo(params->source(), source_specs),
+        params->interface_name(),
+        params->TakeInterfaceRequestPipe(),
+        base::Bind(&Instance::OnConnectComplete, base::Unretained(this)));
+    return true;
+  }
+
   void OnConnectComplete() {
     DCHECK_GT(pending_service_connections_, 0);
     pending_service_connections_--;
   }
 
   void StartWithService(mojom::ServicePtr service) {
     CHECK(!service_);
     state_ = State::STARTING;
     service_ = std::move(service);
     service_.set_connection_error_handler(
@@ skipping 61 matching lines @@
     // The service was started but the service manager hasn't received the
     // initial response from it yet.
     STARTING,
 
     // The service was started successfully.
     STARTED
   };
 
   // mojom::Connector implementation:
   void StartService(
-      const Identity& target,
+      const Identity& in_target,
       mojo::ScopedMessagePipeHandle service_handle,
       mojom::PIDReceiverRequest pid_receiver_request) override {
-    mojom::ServicePtr service;
-    service.Bind(mojom::ServicePtrInfo(std::move(service_handle), 0));
-    ConnectImpl(
-        target,
-        mojom::InterfaceProviderRequest(),
-        std::move(service),
-        std::move(pid_receiver_request),
-        base::Bind(
-            &service_manager::ServiceManager::Instance::EmptyConnectCallback,
-            weak_factory_.GetWeakPtr()));
-  }
-
-  void Connect(const service_manager::Identity& target,
-               mojom::InterfaceProviderRequest remote_interfaces,
-               const ConnectCallback& callback) override {
-    ConnectImpl(target, std::move(remote_interfaces), mojom::ServicePtr(),
-                mojom::PIDReceiverRequest(), callback);
-  }
-
-  void BindInterface(const service_manager::Identity& target,
-                     const std::string& interface_name,
-                     mojo::ScopedMessagePipeHandle interface_pipe,
-                     const BindInterfaceCallback& callback) override {
-    mojom::InterfaceProviderPtr remote_interfaces;
-    ConnectImpl(
-        target,
-        MakeRequest(&remote_interfaces),
-        mojom::ServicePtr(),
-        mojom::PIDReceiverRequest(),
-        base::Bind(
-            &service_manager::ServiceManager::Instance::BindCallbackWrapper,
-            weak_factory_.GetWeakPtr(),
-            callback));
-    remote_interfaces->GetInterface(interface_name, std::move(interface_pipe));
-    // TODO(beng): Rather than just forwarding thru to InterfaceProvider, do
-    //             manifest policy intersection here.
-  }
-
-  void ConnectImpl(const service_manager::Identity& in_target,
-                   mojom::InterfaceProviderRequest remote_interfaces,
-                   mojom::ServicePtr service,
-                   mojom::PIDReceiverRequest pid_receiver_request,
-                   const ConnectCallback& callback) {
     Identity target = in_target;
-    if (target.user_id() == mojom::kInheritUserID)
-      target.set_user_id(identity_.user_id());
-
-    if (!ValidateIdentity(target, callback))
-      return;
-    if (!ValidateClientProcessInfo(&service, &pid_receiver_request, target,
-                                   callback)) {
-      return;
-    }
-    if (!ValidateConnectionSpec(target, callback))
+    mojom::ConnectResult result =
+        ValidateConnectParams(&target, nullptr, nullptr);
+    if (!Succeeded(result))
       return;
 
     std::unique_ptr<ConnectParams> params(new ConnectParams);
     params->set_source(identity_);
     params->set_target(target);
-    params->set_remote_interfaces(std::move(remote_interfaces));
+
+    mojom::ServicePtr service;
+    service.Bind(mojom::ServicePtrInfo(std::move(service_handle), 0));
     params->set_client_process_info(std::move(service),
                                     std::move(pid_receiver_request));
+    service_manager_->Connect(
+        std::move(params), nullptr, weak_factory_.GetWeakPtr());
+  }
+
+  void Connect(const service_manager::Identity& in_target,
+               mojom::InterfaceProviderRequest remote_interfaces,
+               const ConnectCallback& callback) override {
+    Identity target = in_target;
+    mojom::ConnectResult result =
+        ValidateConnectParams(&target, nullptr, nullptr);
+    if (!Succeeded(result)) {
+      callback.Run(result, mojom::kInheritUserID);
+      return;
+    }
+
+    std::unique_ptr<ConnectParams> params(new ConnectParams);
+    params->set_source(identity_);
+    params->set_target(target);
+    params->set_remote_interfaces(std::move(remote_interfaces));
     params->set_connect_callback(callback);
     service_manager_->Connect(
         std::move(params), nullptr, weak_factory_.GetWeakPtr());
   }
+
+  void BindInterface(const service_manager::Identity& in_target,
+                     const std::string& interface_name,
+                     mojo::ScopedMessagePipeHandle interface_pipe,
+                     const BindInterfaceCallback& callback) override {
+    Identity target = in_target;
+    mojom::ConnectResult result =
+        ValidateConnectParams(&target, nullptr, nullptr);
+    if (!Succeeded(result)) {
+      callback.Run(result, mojom::kInheritUserID);
+      return;
+    }
+
+    std::unique_ptr<ConnectParams> params(new ConnectParams);
+    params->set_source(identity_);
+    params->set_target(target);
+    params->set_interface_request_info(interface_name,
+                                       std::move(interface_pipe));
+    params->set_bind_interface_callback(callback);
+    service_manager_->Connect(
+        std::move(params), nullptr, weak_factory_.GetWeakPtr());
+  }
 
   void Clone(mojom::ConnectorRequest request) override {
     connectors_.AddBinding(this, std::move(request));
   }
 
   // mojom::PIDReceiver:
   void SetPID(uint32_t pid) override {
     PIDAvailable(pid);
   }
 
   // InterfaceFactory<mojom::ServiceManager>:
   void Create(const Identity& remote_identity,
               mojom::ServiceManagerRequest request) override {
     service_manager_bindings_.AddBinding(this, std::move(request));
   }
 
   // mojom::ServiceManager implementation:
   void AddListener(mojom::ServiceManagerListenerPtr listener) override {
     // TODO(beng): this should only track the instances matching this user, and
     // root.
     service_manager_->AddListener(std::move(listener));
   }
 
-  bool ValidateIdentity(const Identity& identity,
-                        const ConnectCallback& callback) {
+  mojom::ConnectResult ValidateConnectParams(
+      Identity* target,
+      mojom::ServicePtr* service,
+      mojom::PIDReceiverRequest* pid_receiver_request) {
+    if (target->user_id() == mojom::kInheritUserID)
+      target->set_user_id(identity_.user_id());
+
+    mojom::ConnectResult result = ValidateIdentity(*target);
+    if (!Succeeded(result))
+      return result;
+
+    result = ValidateClientProcessInfo(service, pid_receiver_request, *target);
+    if (!Succeeded(result))
+      return result;
+    return ValidateConnectionSpec(*target);
+  }
+
+  mojom::ConnectResult ValidateIdentity(const Identity& identity) {
     if (identity.name().empty()) {
       LOG(ERROR) << "Error: empty service name.";
-      callback.Run(mojom::ConnectResult::INVALID_ARGUMENT,
-                   mojom::kInheritUserID);
-      return false;
+      return mojom::ConnectResult::INVALID_ARGUMENT;
     }
     if (!base::IsValidGUID(identity.user_id())) {
       LOG(ERROR) << "Error: invalid user_id: " << identity.user_id();
-      callback.Run(mojom::ConnectResult::INVALID_ARGUMENT,
-                   mojom::kInheritUserID);
-      return false;
+      return mojom::ConnectResult::INVALID_ARGUMENT;
     }
-    return true;
+    return mojom::ConnectResult::SUCCEEDED;
   }
 
-  bool ValidateClientProcessInfo(
+  mojom::ConnectResult ValidateClientProcessInfo(
       mojom::ServicePtr* service,
       mojom::PIDReceiverRequest* pid_receiver_request,
-      const Identity& target,
-      const ConnectCallback& callback) {
-    if (service->is_bound() || pid_receiver_request->is_pending()) {
+      const Identity& target) {
+    if (service && pid_receiver_request &&
+        (service->is_bound() || pid_receiver_request->is_pending())) {
       if (!HasCapability(GetConnectionSpec(), kCapability_ClientProcess)) {
         LOG(ERROR) << "Instance: " << identity_.name() << " attempting "
                    << "to register an instance for a process it created for "
                    << "target: " << target.name() << " without the "
                    << "service_manager{client_process} capability "
                    << "class.";
-        callback.Run(mojom::ConnectResult::ACCESS_DENIED,
-                     mojom::kInheritUserID);
-        return false;
+        return mojom::ConnectResult::ACCESS_DENIED;
       }
 
       if (!service->is_bound() || !pid_receiver_request->is_pending()) {
         LOG(ERROR) << "Must supply both service AND "
                    << "pid_receiver_request when sending client process info";
-        callback.Run(mojom::ConnectResult::INVALID_ARGUMENT,
-                     mojom::kInheritUserID);
-        return false;
+        return mojom::ConnectResult::INVALID_ARGUMENT;
       }
       if (service_manager_->GetExistingInstance(target)) {
         LOG(ERROR) << "Cannot client process matching existing identity:"
                    << "Name: " << target.name() << " User: "
                    << target.user_id() << " Instance: " << target.instance();
-        callback.Run(mojom::ConnectResult::INVALID_ARGUMENT,
-                     mojom::kInheritUserID);
-        return false;
+        return mojom::ConnectResult::INVALID_ARGUMENT;
       }
     }
-    return true;
+    return mojom::ConnectResult::SUCCEEDED;
   }
 
-  bool ValidateConnectionSpec(const Identity& target,
-                              const ConnectCallback& callback) {
+  mojom::ConnectResult ValidateConnectionSpec(const Identity& target) {
     InterfaceProviderSpec connection_spec = GetConnectionSpec();
     // TODO(beng): Need to do the following additional policy validation of
     // whether this instance is allowed to connect using:
     // - non-null client process info.
     if (target.user_id() != identity_.user_id() &&
         target.user_id() != mojom::kRootUserID &&
         !HasCapability(connection_spec, kCapability_UserID)) {
       LOG(ERROR) << "Instance: " << identity_.name()
                  << " running as: " << identity_.user_id()
                  << " attempting to connect to: " << target.name()
                  << " as: " << target.user_id() << " without "
                  << " the service:service_manager{user_id} capability.";
-      callback.Run(mojom::ConnectResult::ACCESS_DENIED,
-                   mojom::kInheritUserID);
-      return false;
+      return mojom::ConnectResult::ACCESS_DENIED;
     }
     if (!target.instance().empty() &&
         target.instance() != target.name() &&
         !HasCapability(connection_spec, kCapability_InstanceName)) {
       LOG(ERROR) << "Instance: " << identity_.name() << " attempting to "
                  << "connect to " << target.name()
                  << " using Instance name: " << target.instance()
                  << " without the "
                  << "service_manager{instance_name} capability.";
-      callback.Run(mojom::ConnectResult::ACCESS_DENIED, mojom::kInheritUserID);
-      return false;
+      return mojom::ConnectResult::ACCESS_DENIED;
     }
 
     if (allow_any_application_ ||
         connection_spec.requires.find(target.name()) !=
             connection_spec.requires.end()) {
-      return true;
+      return mojom::ConnectResult::SUCCEEDED;
     }
     LOG(ERROR) << "InterfaceProviderSpec prevented connection from: "
                << identity_.name() << " to: " << target.name();
-    callback.Run(mojom::ConnectResult::ACCESS_DENIED, mojom::kInheritUserID);
-    return false;
+    return mojom::ConnectResult::ACCESS_DENIED;
   }
 
   uint32_t GenerateUniqueID() const {
     static uint32_t id = mojom::kInvalidInstanceID;
     ++id;
     CHECK_NE(mojom::kInvalidInstanceID, id);
     return id;
   }
 
   void PIDAvailable(base::ProcessId pid) {
@@ skipping 349 matching lines @@
 void ServiceManager::NotifyServiceFailedToStart(const Identity& identity) {
   listeners_.ForAllPtrs(
       [identity](mojom::ServiceManagerListener* listener) {
         listener->OnServiceFailedToStart(identity);
       });
 }
 
 bool ServiceManager::ConnectToExistingInstance(
     std::unique_ptr<ConnectParams>* params) {
   Instance* instance = GetExistingInstance((*params)->target());
-  return instance && instance->ConnectToService(params);
+  if (instance) {
+    if ((*params)->HasInterfaceRequestInfo()) {
+      instance->OnBindInterface(params);
+      return true;
+    }
+    return instance->OnConnect(params);
+  }
+  return false;
 }
 
 ServiceManager::Instance* ServiceManager::CreateInstance(
     const Identity& source,
     const Identity& target,
     const InterfaceProviderSpecMap& specs) {
   CHECK(target.user_id() != mojom::kInheritUserID);
 
   auto instance = base::MakeUnique<Instance>(this, target, specs);
   Instance* raw_instance = instance.get();
@@ skipping 65 matching lines @@
                                        mojom::ResolveResultPtr result,
                                        mojom::ResolveResultPtr parent) {
   // If this request was originated by a specific Instance and that Instance is
   // no longer around, we ignore this response.
   if (has_source_instance && !source_instance)
     return;
 
   // If name resolution failed, we drop the connection.
   if (!result) {
     LOG(ERROR) << "Failed to resolve service name: " << params->target().name();
-    if (!params->connect_callback().is_null()) {
-      params->connect_callback().Run(
-          mojom::ConnectResult::INVALID_ARGUMENT, "");
-    }
+    RunCallback(params.get(), mojom::ConnectResult::INVALID_ARGUMENT, "");
     return;
   }
 
   std::string instance_name = params->target().instance();
 
   // |result->interface_provider_specs| can be empty when there is no manifest.
   InterfaceProviderSpec connection_spec = GetPermissiveInterfaceProviderSpec();
   auto it = result->interface_provider_specs.find(
       mojom::kServiceManager_ConnectorSpec);
   if (it != result->interface_provider_specs.end())
@@ skipping 49 matching lines @@
     mojom::ServiceRequest request(&service);
 
     // The catalog was unable to read a manifest for this service. We can't do
     // anything more.
     // TODO(beng): There may be some cases where it's valid to have an empty
     // spec, so we should probably include a return value in |result|.
     if (result->interface_provider_specs.empty()) {
       LOG(ERROR)
           << "Error: The catalog was unable to read a manifest for service \""
           << result->name << "\".";
-      if (!params->connect_callback().is_null())
-        params->connect_callback().Run(mojom::ConnectResult::ACCESS_DENIED, "");
+      RunCallback(params.get(), mojom::ConnectResult::ACCESS_DENIED, "");
       return;
     }
 
     if (parent) {
       // This service is provided by another service via a ServiceFactory.
       std::string target_user_id = target.user_id();
       std::string factory_instance_name = instance_name;
 
       auto spec_iter = parent->interface_provider_specs.find(
           mojom::kServiceManager_ConnectorSpec);
@@ skipping 17 matching lines @@
     } else {
       base::FilePath package_path;
       if (!service_overrides_ || !service_overrides_->GetExecutablePathOverride(
             target.name(), &package_path)) {
         package_path = result->package_path;
       }
       DCHECK(!package_path.empty());
 
       if (!instance->StartWithFilePath(package_path)) {
         OnInstanceError(instance);
-        if (!params->connect_callback().is_null()) {
-          params->connect_callback().Run(
-              mojom::ConnectResult::INVALID_ARGUMENT, "");
-        }
+        RunCallback(params.get(), mojom::ConnectResult::INVALID_ARGUMENT, "");
         return;
       }
     }
   }
 
   // Now that the instance has a Service, we can connect to it.
-  bool connected = instance->ConnectToService(&params);
-  DCHECK(connected);
+  if (params->HasInterfaceRequestInfo()) {
+    instance->OnBindInterface(&params);
+  } else {
+    bool connected = instance->OnConnect(&params);
+    DCHECK(connected);
+  }
 }
 
 base::WeakPtr<ServiceManager> ServiceManager::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
 }  // namespace service_manager