dbus: revamp fd passing for i/o restrictions

dbus/message.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 "dbus/message.h"
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/platform_file.h"
 #include "base/stringprintf.h"
+#include "base/threading/thread_restrictions.h"
 #include "dbus/object_path.h"
 #include "third_party/protobuf/src/google/protobuf/message_lite.h"
 
 namespace {
 
 // Appends the header name and the value to |output|, if the value is
 // not empty.
 static void AppendStringHeader(const std::string& header_name,
                                const std::string& header_value,
                                std::string* output) {
   if (!header_value.empty()) {
     *output += header_name + ": " + header_value + "\n";
   }
 }
 
 // Appends the header name and the value to |output|, if the value is
 // nonzero.
 static void AppendUint32Header(const std::string& header_name,
                                uint32 header_value,
                                std::string* output) {
   if (header_value != 0) {
     *output += (header_name + ": " + base::StringPrintf("%u", header_value) +
                 "\n");
   }
 }
 
+// Checks if |fd| is suitable for sending/receiving.  We disallow
+// directories to avoid potential sandbox escapes.
+static bool CheckFileDescriptor(int fd) {
+  // NB: bypass i/o restrictions, current uses will not block
+  bool prev = base::ThreadRestrictions::SetIOAllowed(true);

[satorux1, 2012/05/04 22:31:22]

ThreadRestrictions::ScopedAllowIO would be simpler.

+  base::PlatformFileInfo info;
+  bool ok = base::GetPlatformFileInfo(fd, &info);
+  base::ThreadRestrictions::SetIOAllowed(prev);
+  return (ok && !info.is_directory);
+}
+
 }  // namespace
 
 namespace dbus {
 
 Message::Message()
     : raw_message_(NULL) {
 }
 
 Message::~Message() {
   if (raw_message_)
@@ skipping 634 matching lines @@
   const std::string signature = base::StringPrintf("%c", dbus_type);
   MessageWriter variant_writer(message_);
   OpenVariant(signature, &variant_writer);
   variant_writer.AppendBasic(dbus_type, value);
   CloseContainer(&variant_writer);
 }
 
 void MessageWriter::AppendFileDescriptor(const FileDescriptor& value) {
   CHECK(kDBusTypeUnixFdIsSupported);
 
-  base::PlatformFileInfo info;
   int fd = value.value();
-  bool ok = base::GetPlatformFileInfo(fd, &info);
-  if (!ok || info.is_directory) {
+  if (!CheckFileDescriptor(fd)) {

[satorux1, 2012/05/04 22:31:22]

Maybe this wasn't the right place to validate this. What about instead letting
the caller to to validate this, and we check it here:

if (!value.is_validated())
  LOG(FATAL) << "The caller should validate before passing a file descriptor";

     // NB: sending a directory potentially enables sandbox escape
     LOG(FATAL) << "Attempt to pass invalid file descriptor";
   }
   AppendBasic(DBUS_TYPE_UNIX_FD, &fd);
 }
 
 //
 // MessageReader implementation.
 //
 
@@ skipping 253 matching lines @@
 }
 
 bool MessageReader::PopFileDescriptor(FileDescriptor* value) {
   CHECK(kDBusTypeUnixFdIsSupported);
 
   int fd = -1;
   const bool success = PopBasic(DBUS_TYPE_UNIX_FD, &fd);
   if (!success)
     return false;
 
-  base::PlatformFileInfo info;
-  bool ok = base::GetPlatformFileInfo(fd, &info);
-  if (!ok || info.is_directory) {
+  if (!CheckFileDescriptor(fd)) {
     base::ClosePlatformFile(fd);
     // NB: receiving a directory potentially enables sandbox escape
     LOG(FATAL) << "Attempt to receive invalid file descriptor";
     return false;  // NB: not reached
   }
   value->PutValue(fd);
   return true;
 }
 
 }  // namespace dbus