Pepper: Stop using SRPC for irt_open_resource().

This registers an IRT interface in Chromium instead of using the one provided
by NaCl. This reuses the ManifestServiceChannel used for providing
irt_open_resource() in non-SFI mode.

In this change, the Chromium implementation of NACL_IRT_RESOURCE_OPEN_v0_1
takes precedence over the one supplied by NaCl (which is SRPC-based).

The SRPC-based codepath in service_runtime.cc needs to be kept because the
PNaCl translator doesn't have the IRT available yet. I've added a check to
enforce that's the only user of that codepath.

BUG=394130
TEST=Manually tested with a file token that didn't resolve with a local patch
that forced GetFilePath to fail in nacl_process_host.cc and confirmed
that URLLoader* still passed.
CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:linux_rel_precise32
R=dmichael@chromium.org, mseaborn@chromium.org, tsepez@chromium.org

Review URL: https://codereview.chromium.org/418423002

Cr-Commit-Position: refs/heads/master@{#294208}

components/nacl/browser/nacl_process_host.cc

@@ -664,6 +664,9 @@ bool NaClProcessHost::OnMessageReceived(const IPC::Message& msg) {
                           OnSetKnownToValidate)
       IPC_MESSAGE_HANDLER_DELAY_REPLY(NaClProcessMsg_ResolveFileToken,
                                       OnResolveFileToken)
+      IPC_MESSAGE_HANDLER(NaClProcessMsg_ResolveFileTokenAsync,
+                          OnResolveFileTokenAsync)
+
 #if defined(OS_WIN)
       IPC_MESSAGE_HANDLER_DELAY_REPLY(
           NaClProcessMsg_AttachDebugExceptionHandler,
@@ -1027,27 +1030,6 @@ void NaClProcessHost::OnSetKnownToValidate(const std::string& signature) {
       signature, off_the_record_);
 }
 
-void NaClProcessHost::FileResolved(
-    const base::FilePath& file_path,
-    IPC::Message* reply_msg,
-    base::File file) {
-  if (file.IsValid()) {
-    IPC::PlatformFileForTransit handle = IPC::TakeFileHandleForProcess(
-        file.Pass(),
-        process_->GetData().handle);
-    NaClProcessMsg_ResolveFileToken::WriteReplyParams(
-        reply_msg,
-        handle,
-        file_path);
-  } else {
-    NaClProcessMsg_ResolveFileToken::WriteReplyParams(
-        reply_msg,
-        IPC::InvalidPlatformFileForTransit(),
-        base::FilePath());
-  }
-  Send(reply_msg);
-}
-
 void NaClProcessHost::OnResolveFileToken(uint64 file_token_lo,
                                          uint64 file_token_hi,
                                          IPC::Message* reply_msg) {
@@ -1103,6 +1085,83 @@ void NaClProcessHost::OnResolveFileToken(uint64 file_token_lo,
   }
 }
 
+void NaClProcessHost::OnResolveFileTokenAsync(uint64 file_token_lo,
+                                              uint64 file_token_hi) {
+  // See the comment at OnResolveFileToken() for details of the file path cache
+  // behavior.
+  CHECK(!uses_nonsfi_mode_);
+  base::FilePath file_path;
+  if (!NaClBrowser::GetInstance()->GetFilePath(
+        file_token_lo, file_token_hi, &file_path)) {
+    Send(new NaClProcessMsg_ResolveFileTokenAsyncReply(
+             file_token_lo,
+             file_token_hi,
+             IPC::PlatformFileForTransit(),
+             base::FilePath()));
+    return;
+  }
+
+  // Open the file.
+  if (!base::PostTaskAndReplyWithResult(
+          content::BrowserThread::GetBlockingPool(),
+          FROM_HERE,
+          base::Bind(OpenNaClReadExecImpl, file_path, true /* is_executable */),
+          base::Bind(&NaClProcessHost::FileResolvedAsync,
+                     weak_factory_.GetWeakPtr(),
+                     file_token_lo,
+                     file_token_hi,
+                     file_path))) {
+    Send(new NaClProcessMsg_ResolveFileTokenAsyncReply(
+            file_token_lo,
+            file_token_hi,
+            IPC::PlatformFileForTransit(),
+            base::FilePath()));
+  }
+}
+
+void NaClProcessHost::FileResolved(
+    const base::FilePath& file_path,
+    IPC::Message* reply_msg,
+    base::File file) {
+  if (file.IsValid()) {
+    IPC::PlatformFileForTransit handle = IPC::TakeFileHandleForProcess(
+        file.Pass(),
+        process_->GetData().handle);
+    NaClProcessMsg_ResolveFileToken::WriteReplyParams(
+        reply_msg,
+        handle,
+        file_path);
+  } else {
+    NaClProcessMsg_ResolveFileToken::WriteReplyParams(
+        reply_msg,
+        IPC::InvalidPlatformFileForTransit(),
+        base::FilePath());
+  }
+  Send(reply_msg);
+}
+
+void NaClProcessHost::FileResolvedAsync(
+    uint64_t file_token_lo,
+    uint64_t file_token_hi,
+    const base::FilePath& file_path,
+    base::File file) {
+  base::FilePath out_file_path;
+  IPC::PlatformFileForTransit out_handle;
+  if (file.IsValid()) {
+    out_file_path = file_path;
+    out_handle = IPC::TakeFileHandleForProcess(
+        file.Pass(),
+        process_->GetData().handle);
+  } else {
+    out_handle = IPC::InvalidPlatformFileForTransit();
+  }
+  Send(new NaClProcessMsg_ResolveFileTokenAsyncReply(
+           file_token_lo,
+           file_token_hi,
+           out_handle,
+           out_file_path));
+}
+
 #if defined(OS_WIN)
 void NaClProcessHost::OnAttachDebugExceptionHandler(const std::string& info,
                                                     IPC::Message* reply_msg) {

components/nacl/browser/nacl_process_host.h

@@ -172,10 +172,14 @@ class NaClProcessHost : public content::BrowserChildProcessHostDelegate {
   void OnSetKnownToValidate(const std::string& signature);
   void OnResolveFileToken(uint64 file_token_lo, uint64 file_token_hi,
                           IPC::Message* reply_msg);
+  void OnResolveFileTokenAsync(uint64 file_token_lo, uint64 file_token_hi);
   void FileResolved(const base::FilePath& file_path,
                     IPC::Message* reply_msg,
                     base::File file);
-
+  void FileResolvedAsync(uint64_t file_token_lo,
+                         uint64_t file_token_hi,
+                         const base::FilePath& file_path,
+                         base::File file);
 #if defined(OS_WIN)
   // Message handler for Windows hardware exception handling.
   void OnAttachDebugExceptionHandler(const std::string& info,

components/nacl/common/nacl_messages.h

@@ -91,12 +91,23 @@ IPC_MESSAGE_CONTROL1(NaClProcessMsg_SetKnownToValidate,
 // Used by the NaCl process to acquire trusted information about a file directly
 // from the browser, including the file's path as well as a fresh version of the
 // file handle.
+// TODO(teravest): Remove the synchronous version of this message once initial
+// nexe validation caching stops using this.
 IPC_SYNC_MESSAGE_CONTROL2_2(NaClProcessMsg_ResolveFileToken,
                             uint64, /* file_token_lo */
                             uint64, /* file_token_hi */
                             IPC::PlatformFileForTransit, /* fd */
                             base::FilePath /* Path opened to get fd */)
 
+IPC_MESSAGE_CONTROL2(NaClProcessMsg_ResolveFileTokenAsync,
+                     uint64, /* file_token_lo */
+                     uint64 /* file_token_hi */)
+IPC_MESSAGE_CONTROL4(NaClProcessMsg_ResolveFileTokenAsyncReply,
+                     uint64, /* file_token_lo */
+                     uint64, /* file_token_hi */
+                     IPC::PlatformFileForTransit, /* fd */
+                     base::FilePath /* Path opened to get fd */)
+
 // Notify the browser process that the server side of the PPAPI channel was
 // created successfully.
 IPC_MESSAGE_CONTROL4(NaClProcessHostMsg_PpapiChannelsCreated,

components/nacl/loader/DEPS

@@ -16,6 +16,7 @@ include_rules = [
   "+native_client/src/trusted/desc",
   "+native_client/src/trusted/service_runtime/include",
   "+native_client/src/trusted/service_runtime/nacl_error_code.h",
+  "+native_client/src/trusted/validator/rich_file_info.h",
   "+native_client/src/trusted/validator/validation_cache.h",
   "+native_client/src/untrusted/irt/irt.h",
 

components/nacl/loader/nacl_ipc_adapter.cc

@@ -12,6 +12,7 @@
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/shared_memory.h"
+#include "base/task_runner_util.h"
 #include "build/build_config.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_platform_file.h"
@@ -24,6 +25,7 @@
 #include "native_client/src/trusted/desc/nacl_desc_sync_socket.h"
 #include "native_client/src/trusted/desc/nacl_desc_wrapper.h"
 #include "native_client/src/trusted/service_runtime/include/sys/fcntl.h"
+#include "native_client/src/trusted/validator/rich_file_info.h"
 #include "ppapi/c/ppb_file_io.h"
 #include "ppapi/proxy/ppapi_messages.h"
 #include "ppapi/proxy/serialized_handle.h"
@@ -465,6 +467,7 @@ int NaClIPCAdapter::TakeClientFileDescriptor() {
 
 bool NaClIPCAdapter::OnMessageReceived(const IPC::Message& msg) {
   uint32_t type = msg.type();
+
   if (type == IPC_REPLY_ID) {
     int id = IPC::SyncMessage::GetMessageId(msg);
     IOThreadData::PendingSyncMsgMap::iterator it =
@@ -475,7 +478,49 @@ bool NaClIPCAdapter::OnMessageReceived(const IPC::Message& msg) {
       io_thread_data_.pending_sync_msgs_.erase(it);
     }
   }
+  // Handle PpapiHostMsg_OpenResource outside the lock as it requires sending
+  // IPC to handle properly.
+  if (type == PpapiHostMsg_OpenResource::ID) {
+    PickleIterator iter = IPC::SyncMessage::GetDataIterator(&msg);
+    ppapi::proxy::SerializedHandle sh;
+    uint64_t token_lo;
+    uint64_t token_hi;
+    if (!IPC::ReadParam(&msg, &iter, &sh) ||
+        !IPC::ReadParam(&msg, &iter, &token_lo) ||
+        !IPC::ReadParam(&msg, &iter, &token_hi)) {
+      return false;
+    }
 
+    if (sh.IsHandleValid() && (token_lo != 0 || token_hi != 0)) {
+      // We've received a valid file token. Instead of using the file
+      // descriptor received, we send the file token to the browser in
+      // exchange for a new file descriptor and file path information.
+      // That file descriptor can be used to construct a NaClDesc with
+      // identity-based validation caching.
+      //
+      // We do not use file descriptors from the renderer with validation
+      // caching; a compromised renderer should not be able to run
+      // arbitrary code in a plugin process.
+      DCHECK(!resolve_file_token_cb_.is_null());
+
+      // resolve_file_token_cb_ must be invoked from the main thread.
+      resolve_file_token_cb_.Run(
+          token_lo,
+          token_hi,
+          base::Bind(&NaClIPCAdapter::OnFileTokenResolved,
+                     this,
+                     msg));
+
+      // In this case, we don't release the message to NaCl untrusted code
+      // immediately. We defer it until we get an async message back from the
+      // browser process.
+      return true;
+    }
+  }
+  return RewriteMessage(msg, type);
+}
+
+bool NaClIPCAdapter::RewriteMessage(const IPC::Message& msg, uint32_t type) {
   {
     base::AutoLock lock(lock_);
     scoped_refptr<RewrittenMessage> rewritten_msg(new RewrittenMessage);
@@ -538,8 +583,7 @@ bool NaClIPCAdapter::OnMessageReceived(const IPC::Message& msg) {
         }
 
         case ppapi::proxy::SerializedHandle::INVALID: {
-          // Nothing to do. TODO(dmichael): Should we log this? Or is it
-          // sometimes okay to pass an INVALID handle?
+          // Nothing to do.
           break;
         }
         // No default, so the compiler will warn us if new types get added.
@@ -556,6 +600,105 @@ bool NaClIPCAdapter::OnMessageReceived(const IPC::Message& msg) {
   return true;
 }
 
+scoped_ptr<IPC::Message> CreateOpenResourceReply(
+    const IPC::Message& orig_msg,
+    ppapi::proxy::SerializedHandle sh) {
+  // The creation of new_msg must be kept in sync with
+  // SyncMessage::WriteSyncHeader.
+  scoped_ptr<IPC::Message> new_msg(new IPC::Message(
+      orig_msg.routing_id(),
+      orig_msg.type(),
+      IPC::Message::PRIORITY_NORMAL));
+  new_msg->set_reply();
+  new_msg->WriteInt(IPC::SyncMessage::GetMessageId(orig_msg));
+
+  ppapi::proxy::SerializedHandle::WriteHeader(sh.header(),
+                                              new_msg.get());
+  new_msg->WriteBool(true);  // valid == true
+  // The file descriptor is at index 0. There's only ever one file
+  // descriptor provided for this message type, so this will be correct.
+  new_msg->WriteInt(0);
+
+  // Write empty file tokens.
+  new_msg->WriteUInt64(0);  // token_lo
+  new_msg->WriteUInt64(0);  // token_hi
+  return new_msg.Pass();
+}
+
+void NaClIPCAdapter::OnFileTokenResolved(const IPC::Message& orig_msg,
+                                         IPC::PlatformFileForTransit ipc_fd,
+                                         base::FilePath file_path) {
+  // The path where an invalid ipc_fd is returned isn't currently
+  // covered by any tests.
+  if (ipc_fd == IPC::InvalidPlatformFileForTransit()) {
+    // The file token didn't resolve successfully, so we give the
+    // original FD to the client without making a validated NaClDesc.
+    // However, we must rewrite the message to clear the file tokens.
+    PickleIterator iter = IPC::SyncMessage::GetDataIterator(&orig_msg);
+    ppapi::proxy::SerializedHandle sh;
+
+    // We know that this can be read safely; see the original read in
+    // OnMessageReceived().
+    CHECK(IPC::ReadParam(&orig_msg, &iter, &sh));
+    scoped_ptr<IPC::Message> new_msg = CreateOpenResourceReply(orig_msg, sh);
+
+    scoped_ptr<NaClDescWrapper> desc_wrapper(new NaClDescWrapper(
+        NaClDescIoDescFromHandleAllocCtor(
+#if defined(OS_WIN)
+            sh.descriptor(),
+#else
+            sh.descriptor().fd,
+#endif
+            NACL_ABI_O_RDONLY)));
+
+    scoped_refptr<RewrittenMessage> rewritten_msg(new RewrittenMessage);
+    rewritten_msg->AddDescriptor(desc_wrapper.release());
+    {
+      base::AutoLock lock(lock_);
+      SaveMessage(*new_msg, rewritten_msg.get());
+      cond_var_.Signal();
+    }
+    return;
+  }
+
+  // The file token was sucessfully resolved.
+  std::string file_path_str = file_path.AsUTF8Unsafe();
+  base::PlatformFile handle =
+      IPC::PlatformFileForTransitToPlatformFile(ipc_fd);
+  // The file token was resolved successfully, so we populate the new
+  // NaClDesc with that information.
+  char* alloc_file_path = static_cast<char*>(
+      malloc(file_path_str.length() + 1));
+  strcpy(alloc_file_path, file_path_str.c_str());
+  scoped_ptr<NaClDescWrapper> desc_wrapper(new NaClDescWrapper(
+      NaClDescIoDescFromHandleAllocCtor(handle, NACL_ABI_O_RDONLY)));
+
+  // Mark the desc as OK for mapping as executable memory.
+  NaClDescMarkSafeForMmap(desc_wrapper->desc());
+
+  // Provide metadata for validation.
+  struct NaClRichFileInfo info;
+  NaClRichFileInfoCtor(&info);
+  info.known_file = 1;
+  info.file_path = alloc_file_path;  // Takes ownership.
+  info.file_path_length =
+      static_cast<uint32_t>(file_path_str.length());
+  NaClSetFileOriginInfo(desc_wrapper->desc(), &info);
+  NaClRichFileInfoDtor(&info);
+
+  ppapi::proxy::SerializedHandle sh;
+  sh.set_file_handle(ipc_fd, PP_FILEOPENFLAG_READ, 0);
+  scoped_ptr<IPC::Message> new_msg = CreateOpenResourceReply(orig_msg, sh);
+  scoped_refptr<RewrittenMessage> rewritten_msg(new RewrittenMessage);
+
+  rewritten_msg->AddDescriptor(desc_wrapper.release());
+  {
+    base::AutoLock lock(lock_);
+    SaveMessage(*new_msg, rewritten_msg.get());
+    cond_var_.Signal();
+  }
+}
+
 void NaClIPCAdapter::OnChannelConnected(int32 peer_pid) {
 }
 

components/nacl/loader/nacl_ipc_adapter.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
@@ -20,6 +21,7 @@
 #include "base/synchronization/lock.h"
 #include "base/task_runner.h"
 #include "ipc/ipc_listener.h"
+#include "ipc/ipc_platform_file.h"
 #include "ppapi/c/pp_stdint.h"
 #include "ppapi/proxy/nacl_message_scanner.h"
 
@@ -112,6 +114,22 @@ class NaClIPCAdapter : public base::RefCountedThreadSafe<NaClIPCAdapter>,
   virtual void OnChannelConnected(int32 peer_pid) OVERRIDE;
   virtual void OnChannelError() OVERRIDE;
 
+  typedef base::Callback<void(IPC::PlatformFileForTransit, base::FilePath)>
+      ResolveFileTokenReplyCallback;
+
+  typedef base::Callback<void(uint64_t,  // file_token_lo
+                              uint64_t,  // file_token_hi
+                              ResolveFileTokenReplyCallback)>
+      ResolveFileTokenCallback;
+
+  // Sets a callback to be invoked for resolving file tokens received from the
+  // renderer. When the file token is resolved, the
+  // ResolveFileTokenReplyCallback passed inside the ResolveFileTokenCallback
+  // will be invoked.
+  void set_resolve_file_token_callback(ResolveFileTokenCallback cb) {
+    resolve_file_token_cb_ = cb;
+  }
+
  private:
   friend class base::RefCountedThreadSafe<NaClIPCAdapter>;
 
@@ -158,6 +176,12 @@ class NaClIPCAdapter : public base::RefCountedThreadSafe<NaClIPCAdapter>,
 
   virtual ~NaClIPCAdapter();
 
+  void OnFileTokenResolved(const IPC::Message& orig_msg,
+                           IPC::PlatformFileForTransit ipc_fd,
+                           base::FilePath file_path);
+
+  bool RewriteMessage(const IPC::Message& msg, uint32_t type);
+
   // Returns 0 if nothing is waiting.
   int LockedReceive(NaClImcTypedMsgHdr* msg);
 
@@ -183,6 +207,8 @@ class NaClIPCAdapter : public base::RefCountedThreadSafe<NaClIPCAdapter>,
 
   scoped_refptr<base::TaskRunner> task_runner_;
 
+  ResolveFileTokenCallback resolve_file_token_cb_;
+
   // To be accessed inside of lock_ only.
   LockedData locked_data_;
 

components/nacl/loader/nacl_listener.cc

@@ -136,10 +136,11 @@ void DebugStubPortSelectedHandler(uint16_t port) {
 // the given message_loop_proxy runs.
 // Also, creates and sets the corresponding NaClDesc to the given nap with
 // the FD #.
-void SetUpIPCAdapter(IPC::ChannelHandle* handle,
-                     scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
-                     struct NaClApp* nap,
-                     int nacl_fd) {
+scoped_refptr<NaClIPCAdapter> SetUpIPCAdapter(
+    IPC::ChannelHandle* handle,
+    scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
+    struct NaClApp* nap,
+    int nacl_fd) {
   scoped_refptr<NaClIPCAdapter> ipc_adapter(
       new NaClIPCAdapter(*handle, message_loop_proxy.get()));
   ipc_adapter->ConnectChannel();
@@ -151,6 +152,7 @@ void SetUpIPCAdapter(IPC::ChannelHandle* handle,
   // Pass a NaClDesc to the untrusted side. This will hold a ref to the
   // NaClIPCAdapter.
   NaClAppSetDesc(nap, nacl_fd, ipc_adapter->MakeNaClDesc());
+  return ipc_adapter;
 }
 
 }  // namespace
@@ -181,6 +183,9 @@ class BrowserValidationDBProxy : public NaClValidationDB {
     }
   }
 
+  // This is the "old" code path for resolving file tokens. It's only
+  // used for resolving the main nexe.
+  // TODO(teravest): Remove this.
   virtual bool ResolveFileToken(struct NaClFileToken* file_token,
                                 int32* fd, std::string* path) OVERRIDE {
     *fd = -1;
@@ -251,6 +256,34 @@ bool NaClListener::Send(IPC::Message* msg) {
   }
 }
 
+// The NaClProcessMsg_ResolveFileTokenAsyncReply message must be
+// processed in a MessageFilter so it can be handled on the IO thread.
+// The main thread used by NaClListener is busy in
+// NaClChromeMainAppStart(), so it can't be used for servicing messages.
+class FileTokenMessageFilter : public IPC::MessageFilter {
+ public:
+  virtual bool OnMessageReceived(const IPC::Message& msg) OVERRIDE {
+    bool handled = true;
+    IPC_BEGIN_MESSAGE_MAP(FileTokenMessageFilter, msg)
+      IPC_MESSAGE_HANDLER(NaClProcessMsg_ResolveFileTokenAsyncReply,
+                          OnResolveFileTokenAsyncReply)
+      IPC_MESSAGE_UNHANDLED(handled = false)
+    IPC_END_MESSAGE_MAP()
+    return handled;
+  }
+
+  void OnResolveFileTokenAsyncReply(
+      uint64_t token_lo,
+      uint64_t token_hi,
+      IPC::PlatformFileForTransit ipc_fd,
+      base::FilePath file_path) {
+    CHECK(g_listener);
+    g_listener->OnFileTokenResolved(token_lo, token_hi, ipc_fd, file_path);
+  }
+ private:
+  virtual ~FileTokenMessageFilter() { }
+};
+
 void NaClListener::Listen() {
   std::string channel_name =
       CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
@@ -259,6 +292,7 @@ void NaClListener::Listen() {
       this, io_thread_.message_loop_proxy().get(), &shutdown_event_);
   filter_ = new IPC::SyncMessageFilter(&shutdown_event_);
   channel_->AddFilter(filter_.get());
+  channel_->AddFilter(new FileTokenMessageFilter());
   channel_->Init(channel_name, IPC::Channel::MODE_CLIENT, true);
   main_loop_ = base::MessageLoop::current();
   main_loop_->Run();
@@ -298,10 +332,12 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) {
 
   IPC::ChannelHandle browser_handle;
   IPC::ChannelHandle ppapi_renderer_handle;
+  IPC::ChannelHandle manifest_service_handle;
 
   if (params.enable_ipc_proxy) {
     browser_handle = IPC::Channel::GenerateVerifiedChannelID("nacl");
     ppapi_renderer_handle = IPC::Channel::GenerateVerifiedChannelID("nacl");
+    manifest_service_handle = IPC::Channel::GenerateVerifiedChannelID("nacl");
 
     // Create the PPAPI IPC channels between the NaCl IRT and the host
     // (browser/renderer) processes. The IRT uses these channels to
@@ -310,6 +346,14 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) {
                     nap, NACL_CHROME_DESC_BASE);
     SetUpIPCAdapter(&ppapi_renderer_handle, io_thread_.message_loop_proxy(),
                     nap, NACL_CHROME_DESC_BASE + 1);
+
+    scoped_refptr<NaClIPCAdapter> manifest_ipc_adapter =
+        SetUpIPCAdapter(&manifest_service_handle,
+                        io_thread_.message_loop_proxy(),
+                        nap,
+                        NACL_CHROME_DESC_BASE + 2);
+    manifest_ipc_adapter->set_resolve_file_token_callback(
+        base::Bind(&NaClListener::ResolveFileToken, base::Unretained(this)));
   }
 
   trusted_listener_ = new NaClTrustedListener(
@@ -320,7 +364,7 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) {
           browser_handle,
           ppapi_renderer_handle,
           trusted_listener_->TakeClientChannelHandle(),
-          IPC::ChannelHandle())))
+          manifest_service_handle)))
     LOG(ERROR) << "Failed to send IPC channel handle to NaClProcessHost.";
 
   std::vector<nacl::FileDescriptor> handles = params.handles;
@@ -427,3 +471,23 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) {
   trusted_listener_->Send(new NaClRendererMsg_ReportExitStatus(exit_status));
   NaClExit(exit_status);
 }
+
+void NaClListener::ResolveFileToken(
+    uint64_t token_lo,
+    uint64_t token_hi,
+    base::Callback<void(IPC::PlatformFileForTransit, base::FilePath)> cb) {
+  if (!Send(new NaClProcessMsg_ResolveFileTokenAsync(token_lo, token_hi))) {
+    cb.Run(IPC::PlatformFileForTransit(), base::FilePath());
+    return;
+  }
+  resolved_cb_ = cb;
+}
+
+void NaClListener::OnFileTokenResolved(
+    uint64_t token_lo,
+    uint64_t token_hi,
+    IPC::PlatformFileForTransit ipc_fd,
+    base::FilePath file_path) {
+  resolved_cb_.Run(ipc_fd, file_path);
+  resolved_cb_.Reset();
+}

components/nacl/loader/nacl_listener.h

@@ -48,6 +48,16 @@ class NaClListener : public IPC::Listener {
 
   void* crash_info_shmem_memory() const { return crash_info_shmem_->memory(); }
 
+  typedef base::Callback<void(IPC::PlatformFileForTransit, base::FilePath)>
+      ResolveFileTokenCallback;
+  void ResolveFileToken(uint64_t token_lo,
+                        uint64_t token_hi,
+                        ResolveFileTokenCallback cb);
+  void OnFileTokenResolved(uint64_t token_lo,
+                           uint64_t token_hi,
+                           IPC::PlatformFileForTransit ipc_fd,
+                           base::FilePath file_path);
+
  private:
   virtual bool OnMessageReceived(const IPC::Message& msg) OVERRIDE;
 
@@ -78,6 +88,8 @@ class NaClListener : public IPC::Listener {
 
   scoped_refptr<NaClTrustedListener> trusted_listener_;
 
+  ResolveFileTokenCallback resolved_cb_;
+
   // Used to identify what thread we're on.
   base::MessageLoop* main_loop_;
 

components/nacl/renderer/manifest_service_channel.cc

@@ -7,10 +7,12 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/callback_helpers.h"
+#include "content/public/common/sandbox_init.h"
 #include "content/public/renderer/render_thread.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_sync_channel.h"
 #include "ppapi/c/pp_errors.h"
+#include "ppapi/c/ppb_file_io.h"
 #include "ppapi/proxy/ppapi_messages.h"
 
 namespace nacl {
@@ -29,6 +31,7 @@ ManifestServiceChannel::ManifestServiceChannel(
           content::RenderThread::Get()->GetIOMessageLoopProxy(),
           true,
           waitable_event)),
+      peer_pid_(base::kNullProcessId),
       weak_ptr_factory_(this) {
 }
 
@@ -54,6 +57,7 @@ bool ManifestServiceChannel::OnMessageReceived(const IPC::Message& message) {
 }
 
 void ManifestServiceChannel::OnChannelConnected(int32 peer_pid) {
+  peer_pid_ = peer_pid;
   if (!connected_callback_.is_null())
     base::ResetAndReturn(&connected_callback_).Run(PP_OK);
 }
@@ -69,31 +73,38 @@ void ManifestServiceChannel::OnStartupInitializationComplete() {
 
 void ManifestServiceChannel::OnOpenResource(
     const std::string& key, IPC::Message* reply) {
-  // Currently this is used only for non-SFI mode, which is not supported on
-  // windows.
-#if !defined(OS_WIN)
   delegate_->OpenResource(
       key,
       base::Bind(&ManifestServiceChannel::DidOpenResource,
                  weak_ptr_factory_.GetWeakPtr(), reply));
-#else
-  PpapiHostMsg_OpenResource::WriteReplyParams(
-      reply, ppapi::proxy::SerializedHandle());
-  Send(reply);
-#endif
 }
 
-#if !defined(OS_WIN)
-void ManifestServiceChannel::DidOpenResource(
-    IPC::Message* reply, base::File file) {
-  // Here, PlatformFileForTransit is alias of base::FileDescriptor.
-  PpapiHostMsg_OpenResource::WriteReplyParams(
-      reply,
-      ppapi::proxy::SerializedHandle(
-          ppapi::proxy::SerializedHandle::FILE,
-          base::FileDescriptor(file.Pass())));
+void ManifestServiceChannel::DidOpenResource(IPC::Message* reply,
+                                             base::File file,
+                                             uint64_t token_lo,
+                                             uint64_t token_hi) {
+  ppapi::proxy::SerializedHandle handle;
+  if (file.IsValid()) {
+    IPC::PlatformFileForTransit file_for_transit;
+#if defined(OS_WIN)
+    bool ok = content::BrokerDuplicateHandle(
+        file.TakePlatformFile(),
+        peer_pid_,
+        &file_for_transit,
+        0,  // desired_access is 0 since we're using DUPLICATE_SAME_ACCESS.
+        DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE);
+    if (ok)
+      handle.set_file_handle(file_for_transit, PP_FILEOPENFLAG_READ, 0);
+#else
+    file_for_transit = base::FileDescriptor(file.Pass());
+    handle.set_file_handle(file_for_transit, PP_FILEOPENFLAG_READ, 0);
+#endif
+  }
+  PpapiHostMsg_OpenResource::WriteReplyParams(reply,
+                                              handle,
+                                              token_lo,
+                                              token_hi);
   Send(reply);
 }
-#endif
 
 }  // namespace nacl

components/nacl/renderer/manifest_service_channel.h

@@ -28,7 +28,8 @@ namespace nacl {
 
 class ManifestServiceChannel : public IPC::Listener {
  public:
-  typedef base::Callback<void(base::File)> OpenResourceCallback;
+  typedef base::Callback<void(base::File, uint64_t, uint64_t)>
+      OpenResourceCallback;
 
   class Delegate {
    public:
@@ -61,14 +62,16 @@ class ManifestServiceChannel : public IPC::Listener {
  private:
   void OnStartupInitializationComplete();
   void OnOpenResource(const std::string& key, IPC::Message* reply);
-#if !defined(OS_WIN)
-  void DidOpenResource(IPC::Message* reply, base::File file);
-#endif
-
+  void DidOpenResource(IPC::Message* reply,
+                       base::File file,
+                       uint64_t token_lo,
+                       uint64_t token_hi);
   base::Callback<void(int32_t)> connected_callback_;
   scoped_ptr<Delegate> delegate_;
   scoped_ptr<IPC::SyncChannel> channel_;
 
+  base::ProcessId peer_pid_;
+
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate the weak pointers before any other members are destroyed.
   base::WeakPtrFactory<ManifestServiceChannel> weak_ptr_factory_;

components/nacl/renderer/ppb_nacl_private_impl.cc

@@ -203,7 +203,7 @@ class ManifestServiceProxy : public ManifestServiceChannel::Delegate {
     if (!ManifestResolveKey(pp_instance_, false, key, &url, &pnacl_options)) {
       base::MessageLoop::current()->PostTask(
           FROM_HERE,
-          base::Bind(callback, base::Passed(base::File())));
+          base::Bind(callback, base::Passed(base::File()), 0, 0));
       return;
     }
 
@@ -224,10 +224,12 @@ class ManifestServiceProxy : public ManifestServiceChannel::Delegate {
       int32_t pp_error,
       const PP_NaClFileInfo& file_info) {
     if (pp_error != PP_OK) {
-      callback.Run(base::File());
+      callback.Run(base::File(), 0, 0);
       return;
     }
-    callback.Run(base::File(file_info.handle));
+    callback.Run(base::File(file_info.handle),
+                 file_info.token_lo,
+                 file_info.token_hi);
   }
 
   PP_Instance pp_instance_;
@@ -421,13 +423,11 @@ void LaunchSelLdr(PP_Instance instance,
 
   // Create the manifest service handle as well.
   // For security hardening, disable the IPCs for open_resource() when they
-  // aren't needed.  PNaCl doesn't expose open_resource(), and the new
-  // open_resource() IPCs are currently only used for Non-SFI NaCl so far,
-  // not SFI NaCl. Note that enable_dyncode_syscalls is true if and only if
-  // the plugin is a non-PNaCl plugin.
+  // aren't needed.  PNaCl doesn't expose open_resource().  Note that
+  // enable_dyncode_syscalls is true if and only if the plugin is a non-PNaCl
+  // plugin.
   if (load_manager &&
       enable_dyncode_syscalls &&
-      uses_nonsfi_mode &&
       IsValidChannelHandle(
           launch_result.manifest_service_ipc_channel_handle)) {
     scoped_ptr<ManifestServiceChannel> manifest_service_channel(

ppapi/nacl_irt/irt_ppapi.cc

@@ -6,6 +6,7 @@
 #include "native_client/src/trusted/service_runtime/include/sys/unistd.h"
 #include "native_client/src/untrusted/irt/irt.h"
 #include "native_client/src/untrusted/irt/irt_private.h"
+#include "ppapi/nacl_irt/irt_manifest.h"
 #include "ppapi/nacl_irt/irt_ppapi.h"
 #include "ppapi/nacl_irt/plugin_main.h"
 #include "ppapi/nacl_irt/public/irt_ppapi.h"
@@ -46,19 +47,35 @@ static int ppapihook_pnacl_private_filter(void) {
   return pnacl_mode;
 }
 
+static const nacl_irt_resource_open kIrtResourceOpen = {
+  ppapi::IrtOpenResource,
+};
+
+static int not_pnacl_filter(void) {
+  int pnacl_mode = sysconf(NACL_ABI__SC_NACL_PNACL_MODE);
+  if (pnacl_mode == -1)
+    return 0;
+  return !pnacl_mode;
+}
+
 static const struct nacl_irt_interface irt_interfaces[] = {
   { NACL_IRT_PPAPIHOOK_v0_1, &nacl_irt_ppapihook, sizeof(nacl_irt_ppapihook),
     NULL },
   { NACL_IRT_PPAPIHOOK_PNACL_PRIVATE_v0_1,
     &nacl_irt_ppapihook_pnacl_private, sizeof(nacl_irt_ppapihook_pnacl_private),
     ppapihook_pnacl_private_filter },
+  { NACL_IRT_RESOURCE_OPEN_v0_1, &kIrtResourceOpen,
+    sizeof(kIrtResourceOpen), not_pnacl_filter },
 };
 
 size_t chrome_irt_query(const char* interface_ident,
                         void* table, size_t tablesize) {
-  size_t result = nacl_irt_query_core(interface_ident, table, tablesize);
+  size_t result = nacl_irt_query_list(interface_ident,
+                                      table,
+                                      tablesize,
+                                      irt_interfaces,
+                                      sizeof(irt_interfaces));
   if (result != 0)
     return result;
-  return nacl_irt_query_list(interface_ident, table, tablesize,
-                             irt_interfaces, sizeof(irt_interfaces));
+  return nacl_irt_query_core(interface_ident, table, tablesize);
 }

ppapi/nacl_irt/irt_start.cc

@@ -19,7 +19,7 @@ void nacl_irt_start(uint32_t* info) {
   ppapi::SetIPCFileDescriptors(
       NACL_CHROME_DESC_BASE,
       NACL_CHROME_DESC_BASE + 1,
-      -1);  // Currently manifest service is disabled on NaCl in SFI mode.
+      NACL_CHROME_DESC_BASE + 2);
   ppapi::StartUpPlugin();
 
   nacl_irt_enter_user_code(info, chrome_irt_query);

ppapi/nacl_irt/manifest_service.cc

@@ -80,18 +80,42 @@ void ManifestService::StartupInitializationComplete() {
 }
 
 bool ManifestService::OpenResource(const char* file, int* fd) {
+  // We currently restrict to only allow one concurrent open_resource() call
+  // per plugin. This could be fixed by doing a token lookup with
+  // NaClProcessMsg_ResolveFileTokenAsyncReply instead of using a
+  // global inside components/nacl/loader/nacl_listener.cc
+  base::AutoLock lock(open_resource_lock_);
+
   // OpenResource will return INVALID SerializedHandle, if it is not supported.
   // Specifically, PNaCl doesn't support open resource.
   ppapi::proxy::SerializedHandle ipc_fd;
+
+  // File tokens are ignored here, but needed when the message is processed
+  // inside NaClIPCAdapter.
+  uint64_t file_token_lo;
+  uint64_t file_token_hi;
   if (!filter_->Send(new PpapiHostMsg_OpenResource(
-          std::string(kFilePrefix) + file, &ipc_fd)) ||
-      !ipc_fd.is_file()) {
+          std::string(kFilePrefix) + file,
+          &ipc_fd,
+          &file_token_lo,
+          &file_token_hi))) {
     LOG(ERROR) << "ManifestService::OpenResource failed:" << file;
     *fd = -1;
     return false;
   }
 
-  *fd = ipc_fd.descriptor().fd;
+  // File tokens are used internally by NaClIPCAdapter and should have
+  // been cleared from the message when it is received here.
+  CHECK(file_token_lo == 0);
+  CHECK(file_token_hi == 0);
+
+  // Copy the file if we received a valid file descriptor. Otherwise, if we got
+  // a reply, the file doesn't exist, so provide an fd of -1.
+  // See IrtOpenResource() for how this function's result is interpreted.
+  if (ipc_fd.is_file())
+    *fd = ipc_fd.descriptor().fd;
+  else
+    *fd = -1;
   return true;
 }
 
@@ -105,7 +129,6 @@ int IrtOpenResource(const char* file, int* fd) {
       !manifest_service->OpenResource(file, fd)) {
     return NACL_ABI_EIO;
   }
-
   return (*fd == -1) ? NACL_ABI_ENOENT : 0;
 }
 

ppapi/nacl_irt/manifest_service.h

@@ -8,6 +8,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/synchronization/lock.h"
 
 namespace base {
 class MessageLoopProxy;
@@ -36,6 +37,8 @@ class ManifestService {
   scoped_ptr<IPC::ChannelProxy> channel_;
   scoped_refptr<IPC::SyncMessageFilter> filter_;
 
+  base::Lock open_resource_lock_;
+
   DISALLOW_COPY_AND_ASSIGN(ManifestService);
 };
 

ppapi/native_client/src/trusted/plugin/service_runtime.cc

@@ -105,6 +105,16 @@ void PluginReverseInterface::StartupInitializationComplete() {
 // GetPOSIXFileDesc.
 bool PluginReverseInterface::OpenManifestEntry(nacl::string url_key,
                                                struct NaClFileInfo* info) {
+  // This method should only ever be called from the PNaCl translator, as the
+  // IRT is not available there.
+  // TODO(teravest): Remove support for OpenManifestEntry here once
+  // crbug.com/302078 is resolved.
+  if (service_runtime_->main_service_runtime()) {
+    NaClLog(LOG_ERROR,
+            "OpenManifestEntry should only be used by PNaCl translator.\n");
+    return false;
+  }
+
   bool op_complete = false;  // NB: mu_ and cv_ also controls access to this!
   // The to_open object is owned by the weak ref callback. Because this function
   // waits for the callback to finish, the to_open object will be deallocated on

ppapi/proxy/nacl_message_scanner.cc

@@ -53,9 +53,12 @@ void WriteHandle(int handle_index,
                  IPC::Message* msg) {
   SerializedHandle::WriteHeader(handle.header(), msg);
 
-  // Now write the handle itself in POSIX style.
-  msg->WriteBool(true);  // valid == true
-  msg->WriteInt(handle_index);
+  if (handle.type() != SerializedHandle::INVALID) {
+    // Now write the handle itself in POSIX style.
+    // See ParamTraits<FileDescriptor>::Read for where these values are read.
+    msg->WriteBool(true);  // valid == true
+    msg->WriteInt(handle_index);
+  }
 }
 
 // Define overloads for each kind of message parameter that requires special
@@ -318,6 +321,7 @@ bool NaClMessageScanner::ScanMessage(
     CASE_FOR_MESSAGE(PpapiMsg_PPBAudio_NotifyAudioStreamCreated)
     CASE_FOR_MESSAGE(PpapiMsg_PPPMessaging_HandleMessage)
     CASE_FOR_MESSAGE(PpapiPluginMsg_ResourceReply)
+    CASE_FOR_REPLY(PpapiHostMsg_OpenResource)
     CASE_FOR_REPLY(PpapiHostMsg_PPBGraphics3D_Create)
     CASE_FOR_REPLY(PpapiHostMsg_PPBGraphics3D_CreateTransferBuffer)
     CASE_FOR_REPLY(PpapiHostMsg_PPBImageData_CreateSimple)

ppapi/proxy/ppapi_messages.h

@@ -840,9 +840,11 @@ IPC_MESSAGE_CONTROL1(PpapiHostMsg_ChannelCreated,
 IPC_MESSAGE_CONTROL0(PpapiHostMsg_StartupInitializationComplete)
 
 // Calls renderer to open a resource file for nacl_irt_open_resource().
-IPC_SYNC_MESSAGE_CONTROL1_1(PpapiHostMsg_OpenResource,
+IPC_SYNC_MESSAGE_CONTROL1_3(PpapiHostMsg_OpenResource,
                             std::string /* key */,
-                            ppapi::proxy::SerializedHandle /* fd */)
+                            ppapi::proxy::SerializedHandle /* fd */,
+                            uint64_t /* file_token_lo */,
+                            uint64_t /* file_token_hi */)
 
 // Logs the given message to the console of all instances.
 IPC_MESSAGE_CONTROL4(PpapiHostMsg_LogWithSource,