Add hooks to content to request permission to connect to the PPAPI broker.

content/renderer/pepper/pepper_broker_impl.cc

 // Copyright (c) 2012 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 "content/renderer/pepper/pepper_broker_impl.h"
 
 #include "build/build_config.h"
 #include "content/public/renderer/renderer_restrict_dispatch_group.h"
 #include "content/renderer/pepper/pepper_plugin_delegate_impl.h"
 #include "content/renderer/pepper/pepper_proxy_channel_delegate_impl.h"
@@ skipping 98 matching lines @@
                                    PepperPluginDelegateImpl* delegate)
     : plugin_module_(plugin_module),
       delegate_(delegate->AsWeakPtr()) {
   DCHECK(plugin_module_);
   DCHECK(delegate_);
 
   plugin_module_->SetBroker(this);
 }
 
 PepperBrokerImpl::~PepperBrokerImpl() {
-  // Report failure to all clients that had pending operations.
-  for (ClientMap::iterator i = pending_connects_.begin();
-       i != pending_connects_.end(); ++i) {
-    base::WeakPtr<webkit::ppapi::PPB_Broker_Impl>& weak_ptr = i->second;
-    if (weak_ptr) {
-      weak_ptr->BrokerConnected(
-          ppapi::PlatformFileToInt(base::kInvalidPlatformFileValue),
-          PP_ERROR_ABORTED);
-    }
-  }
-  pending_connects_.clear();
-
+  ReportFailureToClients();
   plugin_module_->SetBroker(NULL);
   plugin_module_ = NULL;
 }
 
 // If the channel is not ready, queue the connection.
 void PepperBrokerImpl::Connect(webkit::ppapi::PPB_Broker_Impl* client) {
   DCHECK(pending_connects_.find(client) == pending_connects_.end())
       << "Connect was already called for this client";
 
   // Ensure this object and the associated broker exist as long as the
   // client exists. There is a corresponding Release() call in Disconnect(),
   // which is called when the PPB_Broker_Impl is destroyed. The only other
   // possible reference is in pending_connect_broker_, which only holds a
   // transient reference. This ensures the broker is available as long as the
   // plugin needs it and allows the plugin to release the broker when it is no
   // longer using it.
   AddRef();
 
-  if (!dispatcher_.get()) {
-    pending_connects_[client] = client->AsWeakPtr();
-    return;
-  }
-  DCHECK(pending_connects_.empty());
-
-  ConnectPluginToBroker(client);
+  pending_connects_[client].client = client->AsWeakPtr();
 }
 
 void PepperBrokerImpl::Disconnect(webkit::ppapi::PPB_Broker_Impl* client) {
   // Remove the pending connect if one exists. This class will not call client's
   // callback.
   pending_connects_.erase(client);
 
   // TODO(ddorwin): Send message disconnect message using dispatcher_.
 
   if (pending_connects_.empty()) {
     // There are no more clients of this broker. Ensure it will be deleted even
     // if the IPC response never comes and OnPepperBrokerChannelCreated is not
     // called to remove this object from pending_connect_broker_.
     // Before the broker is connected, clients must either be in
     // pending_connects_ or not yet associated with this object. Thus, if this
     // object is in pending_connect_broker_, there can be no associated clients
     // once pending_connects_ is empty and it is thus safe to remove this from
     // pending_connect_broker_. Doing so will cause this object to be deleted,
     // removing it from the PluginModule. Any new clients will create a new
     // instance of this object.
     // This doesn't solve all potential problems, but it helps with the ones
     // we can influence.
     if (delegate_) {
       bool stopped = delegate_->StopWaitingForBrokerConnection(this);
 
       // Verify the assumption that there are no references other than the one
-      // client holds, which will be released below.
+      // |client| holds, which will be released below.
       DCHECK(!stopped || HasOneRef());
     }
   }
 
   // Release the reference added in Connect().
   // This must be the last statement because it may delete this object.
   Release();
 }
 
 void PepperBrokerImpl::OnBrokerChannelConnected(
     const IPC::ChannelHandle& channel_handle) {
   scoped_ptr<PepperBrokerDispatcherWrapper> dispatcher(
       new PepperBrokerDispatcherWrapper);
-  if (dispatcher->Init(channel_handle)) {
-    dispatcher_.reset(dispatcher.release());
+  if (!dispatcher->Init(channel_handle)) {
+    ReportFailureToClients();
+    return;
+  }
 
-    // Process all pending channel requests from the plugins.
-    for (ClientMap::iterator i = pending_connects_.begin();
-         i != pending_connects_.end(); ++i) {
-      base::WeakPtr<webkit::ppapi::PPB_Broker_Impl>& weak_ptr = i->second;
-      if (weak_ptr)
-        ConnectPluginToBroker(weak_ptr);
+  dispatcher_.reset(dispatcher.release());
+
+  // Process all pending channel requests from the plugins.
+  for (ClientMap::iterator i = pending_connects_.begin();
+       i != pending_connects_.end();) {
+    base::WeakPtr<webkit::ppapi::PPB_Broker_Impl>& weak_ptr =
+        i->second.client;
+    LOG_IF(ERROR, !weak_ptr) << "Client has gone away";

[ddorwin, 2012/08/12 22:51:16]

Seems more like a warning.

[Bernhard Bauer, 2012/08/13 09:02:12]

Oops, that was for debugging. Removed.

+    LOG_IF(ERROR, !i->second.authorized) << "Client is waiting for permission";

[ddorwin, 2012/08/12 22:51:16]

Is this really an error that should always be logged?

+    if (!i->second.authorized) {

[ddorwin, 2012/08/12 22:51:16]

Is the renderer process responsible for enforcing the result? It seems the
browser should be - otherwise, a compromised renderer can allow the broker
regardless of the user's choice.
I think to fully secure this, the browser must create the socket pair (currently
in  PepperBrokerImpl::ConnectPluginToBroker) and pass it back (i.e. via
OnBrokerPermissionResult()).

[Bernhard Bauer, 2012/08/13 09:02:12]

Yeah, I talked about this with Justin. AFAIU, a compromised renderer could
always lie about its URL to bypass the content settings, so I'm not sure how
much we would gain from having the decision in the browser. In the light of
that, we decided to do the simpler thing and keep the decision in the renderer.
For other plug-in blocking mechanisms (content settings, outdated and infobarred
plug-ins) we also make the final decision in the renderer instead of the
browser.

+      ++i;
+      continue;
     }
-  } else {
-    // Report failure to all clients.
-    for (ClientMap::iterator i = pending_connects_.begin();
-         i != pending_connects_.end(); ++i) {
-      base::WeakPtr<webkit::ppapi::PPB_Broker_Impl>& weak_ptr = i->second;
-      if (weak_ptr) {
-        weak_ptr->BrokerConnected(
-            ppapi::PlatformFileToInt(base::kInvalidPlatformFileValue),
-            PP_ERROR_FAILED);
-      }
+
+    if (weak_ptr)
+      ConnectPluginToBroker(weak_ptr);
+
+    pending_connects_.erase(i++);
+  }
+}
+
+void PepperBrokerImpl::OnBrokerPermissionResult(
+    webkit::ppapi::PPB_Broker_Impl* client,
+    bool result) {
+  ClientMap::iterator entry = pending_connects_.find(client);
+  if (entry == pending_connects_.end())
+    return;
+
+  if (!entry->second.client) {
+    // Client has gone away.
+    pending_connects_.erase(entry);
+    return;
+  }
+
+  if (!result) {
+    // Report failure.
+    client->BrokerConnected(
+        ppapi::PlatformFileToInt(base::kInvalidPlatformFileValue),
+        PP_ERROR_NOACCESS);
+    pending_connects_.erase(entry);
+    return;
+  }
+
+  if (dispatcher_.get()) {
+    ConnectPluginToBroker(client);
+    pending_connects_.erase(entry);
+    return;
+  }
+
+  // Mark the request as authorized, continue waiting for the broker
+  // connection.
+  DCHECK(!entry->second.authorized);
+  entry->second.authorized = true;
+}
+
+PepperBrokerImpl::PendingConnection::PendingConnection() : authorized(false) {
+}
+
+PepperBrokerImpl::PendingConnection::~PendingConnection() {
+}
+
+void PepperBrokerImpl::ReportFailureToClients() {
+  for (ClientMap::iterator i = pending_connects_.begin();
+       i != pending_connects_.end(); ++i) {
+    base::WeakPtr<webkit::ppapi::PPB_Broker_Impl>& weak_ptr =
+        i->second.client;
+    if (weak_ptr) {
+      weak_ptr->BrokerConnected(
+          ppapi::PlatformFileToInt(base::kInvalidPlatformFileValue),
+          PP_ERROR_ABORTED);
     }
   }
   pending_connects_.clear();
 }
 
 void PepperBrokerImpl::ConnectPluginToBroker(
     webkit::ppapi::PPB_Broker_Impl* client) {
   base::SyncSocket::Handle plugin_handle = base::kInvalidPlatformFileValue;
   int32_t result = PP_OK;
 
@@ skipping 15 matching lines @@
 
   // TOOD(ddorwin): Change the IPC to asynchronous: Queue an object containing
   // client and plugin_socket.release(), then return.
   // That message handler will then call client->BrokerConnected() with the
   // saved pipe handle.
   // Temporarily, just call back.
   client->BrokerConnected(ppapi::PlatformFileToInt(plugin_handle), result);
 }
 
 }  // namespace content