Allow MessageFilters to restrict listening to specific message classes

ipc/ipc_channel_proxy.cc

Index: ipc/ipc_channel_proxy.cc
diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
index 3a0cf307b78e5fd2e35c57ba14de4cec6ed6d4f7..3939166cb23ceb83028c222a37abb52a6cb4ec5c 100644
--- a/ipc/ipc_channel_proxy.cc
+++ b/ipc/ipc_channel_proxy.cc
@@ -17,6 +17,43 @@
 #include "ipc/ipc_message_utils.h"
 
 namespace IPC {
+namespace {
+
+bool ValidMessageClass(int message_class) {
+  return message_class >= 0 && message_class < LastIPCMsgStart;
+}
+
+bool EraseFilter(const ChannelProxy::MessageFilter* filter,
+                 std::vector<ChannelProxy::MessageFilter*>& filters) {
+  std::vector<ChannelProxy::MessageFilter*>::iterator it =
+      std::find(filters.begin(), filters.end(), filter);
+  if (it == filters.end())
+    return false;
+
+  filters.erase(it);
+  return true;
+}
+
+bool ApplyFilters(std::vector<ChannelProxy::MessageFilter*>& filters,
+                  const IPC::Message& message,
+                  const std::string& channel_id) {
+#ifdef IPC_MESSAGE_LOG_ENABLED
+  Logging* logger = Logging::GetInstance();
+#endif
+
+  for (size_t i = 0; i < filters.size(); ++i) {
+    if (filters[i]->OnMessageReceived(message)) {
+#ifdef IPC_MESSAGE_LOG_ENABLED
+      if (logger->Enabled())
+        logger->OnPostDispatchMessage(message, channel_id);
+#endif
+      return true;
+    }
+  }
+  return false;
+}
+
+}  // namespace
 
 //------------------------------------------------------------------------------
 
@@ -36,6 +73,11 @@ bool ChannelProxy::MessageFilter::OnMessageReceived(const Message& message) {
   return false;
 }
 
+bool ChannelProxy::MessageFilter::GetSupportedMessageClasses(
+    std::vector<uint32>* /*supported_message_classes*/) const {
+  return false;
+}
+
 ChannelProxy::MessageFilter::~MessageFilter() {}
 
 //------------------------------------------------------------------------------
@@ -71,16 +113,19 @@ bool ChannelProxy::Context::TryFilters(const Message& message) {
     logger->OnPreDispatchMessage(message);
 #endif
 
-  for (size_t i = 0; i < filters_.size(); ++i) {
-    if (filters_[i]->OnMessageReceived(message)) {
-#ifdef IPC_MESSAGE_LOG_ENABLED
-      if (logger->Enabled())
-        logger->OnPostDispatchMessage(message, channel_id_);
-#endif
-      return true;
-    }
+  if (ApplyFilters(message_global_filters_, message, channel_id_))
+    return true;
+
+  const int message_class = IPC_MESSAGE_CLASS(message);
+  if (!ValidMessageClass(message_class)) {
+    DCHECK(message.type() == IPC_REPLY_ID || message.type() == IPC_LOGGING_ID)
+        << "Invalid message class: " << message_class;

[piman, 2014/02/12 02:37:29]

You can DLOG_IF(ERROR), but don't DCHECK.
The path can occur in real life, and we can't assume it doesn't - we need to
handle it correctly. The DCHECK gives the wrong idea here.

(the rationale is that we should have a unit test to verify that we behave
correctly when an untrusted peer sends bogus data, and the DCHECK prevents one
from writing it).

In particular, you wouldn't want someone changing the code into

if (message.type() == IPC_REPLY_ID || message.type() == IPC_LOGGING_ID)
  return false;
DCHECK(ValidMessageClass(message_class));

Which would be equivalent semantically, but would be wrong in prod.

[jdduke (slow), 2014/02/13 19:55:26]

Right, makes perfect sense.

+    return false;
   }
-  return false;
+
+  return ApplyFilters(message_class_filters_[message_class],
+                      message,
+                      channel_id_);
 }
 
 // Called on the IPC::Channel thread
@@ -108,8 +153,8 @@ void ChannelProxy::Context::OnChannelConnected(int32 peer_pid) {
 
   // We cache off the peer_pid so it can be safely accessed from both threads.
   peer_pid_ = channel_->peer_pid();
-  for (size_t i = 0; i < filters_.size(); ++i)
-    filters_[i]->OnChannelConnected(peer_pid);
+  for (size_t i = 0; i < all_filters_.size(); ++i)
+    all_filters_[i]->OnChannelConnected(peer_pid);
 
   // See above comment about using listener_task_runner_ here.
   listener_task_runner_->PostTask(
@@ -118,8 +163,8 @@ void ChannelProxy::Context::OnChannelConnected(int32 peer_pid) {
 
 // Called on the IPC::Channel thread
 void ChannelProxy::Context::OnChannelError() {
-  for (size_t i = 0; i < filters_.size(); ++i)
-    filters_[i]->OnChannelError();
+  for (size_t i = 0; i < all_filters_.size(); ++i)
+    all_filters_[i]->OnChannelError();
 
   // See above comment about using listener_task_runner_ here.
   listener_task_runner_->PostTask(
@@ -139,8 +184,8 @@ void ChannelProxy::Context::OnChannelOpened() {
     return;
   }
 
-  for (size_t i = 0; i < filters_.size(); ++i)
-    filters_[i]->OnFilterAdded(channel_.get());
+  for (size_t i = 0; i < all_filters_.size(); ++i)
+    all_filters_[i]->OnFilterAdded(channel_.get());
 }
 
 // Called on the IPC::Channel thread
@@ -150,13 +195,16 @@ void ChannelProxy::Context::OnChannelClosed() {
   if (!channel_.get())
     return;
 
-  for (size_t i = 0; i < filters_.size(); ++i) {
-    filters_[i]->OnChannelClosing();
-    filters_[i]->OnFilterRemoved();
+  for (size_t i = 0; i < all_filters_.size(); ++i) {
+    all_filters_[i]->OnChannelClosing();
+    all_filters_[i]->OnFilterRemoved();
   }
 
   // We don't need the filters anymore.
-  filters_.clear();
+  all_filters_.clear();
+  message_global_filters_.clear();
+  for (size_t i = 0; i < LastIPCMsgStart; ++i)
+    message_class_filters_[i].clear();
 
   channel_.reset();
 
@@ -187,16 +235,31 @@ void ChannelProxy::Context::OnAddFilter() {
     new_filters.swap(pending_filters_);
   }
 
+  std::vector<uint32> supported_message_classes;
   for (size_t i = 0; i < new_filters.size(); ++i) {
-    filters_.push_back(new_filters[i]);
+    all_filters_.push_back(new_filters[i]);
+
+    // Determine if the filter should be applied to all messages, or only
+    // messages of a certain class.
+    MessageFilter* filter = new_filters[i].get();
+    if (filter->GetSupportedMessageClasses(&supported_message_classes)) {
+      DCHECK(!supported_message_classes.empty());
+      for (size_t i = 0; i < supported_message_classes.size(); ++i) {
+        DCHECK(ValidMessageClass(supported_message_classes[i]));
+        message_class_filters_[supported_message_classes[i]].push_back(filter);
+      }
+    } else {
+      message_global_filters_.push_back(filter);
+    }
+    supported_message_classes.clear();
 
     // If the channel has already been created, then we need to send this
     // message so that the filter gets access to the Channel.
     if (channel_.get())
-      new_filters[i]->OnFilterAdded(channel_.get());
+      filter->OnFilterAdded(channel_.get());
     // Ditto for if the channel has been connected.
     if (peer_pid_)
-      new_filters[i]->OnChannelConnected(peer_pid_);
+      filter->OnChannelConnected(peer_pid_);
   }
 }
 
@@ -205,15 +268,26 @@ void ChannelProxy::Context::OnRemoveFilter(MessageFilter* filter) {
   if (!channel_.get())
     return;  // The filters have already been deleted.
 
-  for (size_t i = 0; i < filters_.size(); ++i) {
-    if (filters_[i].get() == filter) {
+  bool filter_exists = false;
+  for (size_t i = 0; i < all_filters_.size(); ++i) {
+    if (all_filters_[i].get() == filter) {
       filter->OnFilterRemoved();
-      filters_.erase(filters_.begin() + i);
-      return;
+      all_filters_.erase(all_filters_.begin() + i);
+      filter_exists = true;
+      break;
     }
   }
 
-  NOTREACHED() << "filter to be removed not found";
+  if (!filter_exists) {
+    NOTREACHED() << "filter to be removed not found";
+    return;
+  }
+
+  if (EraseFilter(filter, message_global_filters_))
+    return;
+
+  for (size_t i = 0; i < LastIPCMsgStart; ++i)
+    EraseFilter(filter, message_class_filters_[i]);
 }
 
 // Called on the listener's thread