Split secure command channel and untrusted application channel

src/trusted/service_runtime/sel_ldr.c

 /*
  * Copyright (c) 2012 The Native Client 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 <string.h>
 
 /*
  * NaCl Simple/secure ELF loader (NaCl SEL).
@@ skipping 138 matching lines @@
   nap->num_dynamic_regions = 0;
   nap->dynamic_regions_allocated = 0;
   nap->dynamic_delete_generation = 0;
 
   nap->dynamic_mapcache_offset = 0;
   nap->dynamic_mapcache_size = 0;
   nap->dynamic_mapcache_ret = 0;
 
   nap->service_port = NULL;
   nap->service_address = NULL;
+  nap->secure_service_port = NULL;
+  nap->secure_service_address = NULL;
   nap->bootstrap_channel = NULL;
   nap->secure_service = NULL;
+
   nap->manifest_proxy = NULL;
   nap->kernel_service = NULL;
   nap->resource_phase = NACL_RESOURCE_PHASE_START;
   if (!NaClResourceNaClAppInit(&nap->resources, nap)) {
     goto cleanup_dynamic_load_mutex;
   }
   nap->reverse_client = NULL;
   nap->reverse_channel_initialization_state =
       NACL_REVERSE_CHANNEL_UNINITIALIZED;
 
@@ skipping 617 matching lines @@
                 ("NaClAppDescriptorHookupCheck: I/O redirection for %s => %s"
                  " failed\n"),
                 g_nacl_redir_control[ix].env_name, env);
       }
       NaClDescUnref(ndp);
     }
   }
 }
 
 void NaClCreateServiceSocket(struct NaClApp *nap) {
+  struct NaClDesc *secure_pair[2];
   struct NaClDesc *pair[2];
 
   NaClLog(3, "Entered NaClCreateServiceSocket\n");
+
+  if (NACL_FI_ERROR_COND("NaClCreateServiceSocket__secure_boundsock",
+                         0 != NaClCommonDescMakeBoundSock(secure_pair))) {
+    NaClLog(LOG_FATAL, "Cound not create secure service socket\n");
+  }
+  NaClLog(4,
+          "got bound socket at 0x%08"NACL_PRIxPTR", "
+          "addr at 0x%08"NACL_PRIxPTR"\n",
+          (uintptr_t) secure_pair[0],
+          (uintptr_t) secure_pair[1]);
+
+  NaClDescSafeUnref(nap->secure_service_port);
+  nap->secure_service_port = secure_pair[0];
+
+  NaClDescSafeUnref(nap->secure_service_address);
+  nap->secure_service_address = secure_pair[1];
+
   if (NACL_FI_ERROR_COND("NaClCreateServiceSocket__boundsock",
                          0 != NaClCommonDescMakeBoundSock(pair))) {
     NaClLog(LOG_FATAL, "Cound not create service socket\n");
   }
   NaClLog(4,
           "got bound socket at 0x%08"NACL_PRIxPTR", "
           "addr at 0x%08"NACL_PRIxPTR"\n",
           (uintptr_t) pair[0],
           (uintptr_t) pair[1]);
   NaClSetDesc(nap, NACL_SERVICE_PORT_DESCRIPTOR, pair[0]);
   NaClSetDesc(nap, NACL_SERVICE_ADDRESS_DESCRIPTOR, pair[1]);
 
   NaClDescSafeUnref(nap->service_port);
 
   nap->service_port = pair[0];
   NaClDescRef(nap->service_port);
 
   NaClDescSafeUnref(nap->service_address);
 
   nap->service_address = pair[1];
   NaClDescRef(nap->service_address);
+
   NaClLog(4, "Leaving NaClCreateServiceSocket\n");
 }
 
 /*
  * Import the |inherited_desc| descriptor as an IMC handle, save a
  * reference to it at nap->bootstrap_channel, then send the
  * service_address over that channel.
  */
 void NaClSetUpBootstrapChannel(struct NaClApp  *nap,
                                NaClHandle      inherited_desc) {
   struct NaClDescImcDesc      *channel;
   struct NaClImcTypedMsgHdr   hdr;
+  struct NaClDesc             *descs[2];
   ssize_t                     rv;
 
   NaClLog(4,
           "NaClSetUpBootstrapChannel(0x%08"NACL_PRIxPTR", %"NACL_PRIdPTR")\n",
           (uintptr_t) nap,
           (uintptr_t) inherited_desc);
 
   channel = (struct NaClDescImcDesc *) malloc(sizeof *channel);
   if (NULL == channel) {
     NaClLog(LOG_FATAL, "NaClSetUpBootstrapChannel: no memory\n");
   }
   if (!NaClDescImcDescCtor(channel, inherited_desc)) {
     NaClLog(LOG_FATAL,
             ("NaClSetUpBootstrapChannel: cannot construct IMC descriptor"
              " object for inherited descriptor %"NACL_PRIdPTR"\n"),
             (uintptr_t) inherited_desc);

[bsy, 2012/09/26 18:01:44]

just for consistency, add return after LOG_FATAL, or delete returns below that
occurs after similar LOG_FATAL statements.

[Petr Hosek, 2012/09/26 18:49:34]

Added.

   }
+  if (NULL == nap->secure_service_address) {
+    NaClLog(LOG_FATAL,
+            "NaClSetUpBootstrapChannel: secure service address not set\n");
+    return;
+  }
   if (NULL == nap->service_address) {
     NaClLog(LOG_FATAL,
             "NaClSetUpBootstrapChannel: service address not set\n");
     return;
   }
   /*
    * service_address and service_port are set together.
    */
+  descs[0] = nap->secure_service_address;
+  descs[1] = nap->service_address;
 
   hdr.iov = (struct NaClImcMsgIoVec *) NULL;
   hdr.iov_length = 0;
-  hdr.ndescv = &nap->service_address;
-  hdr.ndesc_length = 1;
+  hdr.ndescv = descs;
+  hdr.ndesc_length = NACL_ARRAY_SIZE(descs);
 
   rv = (*NACL_VTBL(NaClDesc, channel)->SendMsg)((struct NaClDesc *) channel,
                                                 &hdr, 0);
   NaClXMutexLock(&nap->mu);
   if (NULL != nap->bootstrap_channel) {
     NaClLog(LOG_FATAL,
             "NaClSetUpBootstrapChannel: cannot have two bootstrap channels\n");
   }
   nap->bootstrap_channel = (struct NaClDesc *) channel;
   channel = NULL;
@@ skipping 400 matching lines @@
   if (NULL != nap->secure_service) {
     for (;;) {
       struct nacl_abi_timespec req;
       req.tv_sec = 1000;
       req.tv_nsec = 0;
       NaClNanosleep(&req, (struct nacl_abi_timespec *) NULL);
     }
   }
 }
 
-
-/*
- * Secure command channels.
- */
-
-struct NaClSecureService {
-  struct NaClSimpleService      base;
-  struct NaClApp                *nap;
-};
-
-struct NaClSimpleServiceVtbl const kNaClSecureServiceVtbl;
-
-
-struct NaClConnectionHandler {
-  struct NaClConnectionHandler  *next;
-
-  /* used by NaClSimpleRevServiceClient's ClientCallback fn */
-  void                          (*handler)(
-      void                        *state,
-      struct NaClThreadInterface  *tif,
-      struct NaClDesc             *conn);
-  void                          *state;
-};
-
-struct NaClSecureReverseClient {
-  struct NaClSimpleRevClient        base;
-  struct NaClApp                    *nap;
-
-  struct NaClMutex                  mu;
-
-  struct NaClConnectionHandler      *queue_head;
-  struct NaClConnectionHandler      **queue_insert;
-};
-
-struct NaClSimpleRevClientVtbl const kNaClSecureReverseClientVtbl;
-
-
-int NaClSecureServiceCtor(struct NaClSecureService          *self,
-                          struct NaClSrpcHandlerDesc const  *srpc_handlers,
-                          struct NaClApp                    *nap) {
-  NaClLog(4,
-          "Entered NaClSecureServiceCtor: self 0x%"NACL_PRIxPTR"\n",
-          (uintptr_t) self);
-  if (NACL_FI_ERROR_COND(
-          "NaClSecureServiceCtor__NaClSimpleServiceWithSocketCtor",
-          !NaClSimpleServiceWithSocketCtor(
-              &self->base,
-              srpc_handlers,
-              NaClThreadInterfaceThreadFactory,
-              (void *) NULL,
-              nap->service_port,
-              nap->service_address))) {
-    goto failure_simple_ctor;
-  }
-  self->nap = nap;
-
-  NACL_VTBL(NaClRefCount, self) =
-      (struct NaClRefCountVtbl *) &kNaClSecureServiceVtbl;
-  return 1;
- failure_simple_ctor:
-  return 0;
-}
-
-void NaClSecureServiceDtor(struct NaClRefCount *vself) {
-  struct NaClSecureService *self = (struct NaClSecureService *) vself;
-
-  NACL_VTBL(NaClRefCount, self) = (struct NaClRefCountVtbl const *)
-      &kNaClSimpleServiceVtbl;
-  (*NACL_VTBL(NaClRefCount, self)->Dtor)(vself);
-}
-
 /*
  * The first connection is performed by this callback handler.  This
  * spawns a client thread that will bootstrap the other connections by
  * stashing the connection represented by |conn| to make reverse RPCs
  * to ask the peer to connect to us.  No thread is spawned; we just
  * wrap access to the connection with a lock.
  *
  * Subsequent connection callbacks will pass the connection to the
  * actual thread that made the connection request using |conn|
  * received in the first connection.
@@ skipping 39 matching lines @@
     NaClLog(LOG_FATAL, "Reverse quota interface Ctor failed\n");
   }
   nap->reverse_channel_initialization_state = NACL_REVERSE_CHANNEL_INITIALIZED;
 
   NaClXCondVarBroadcast(&nap->cv);
   NaClXMutexUnlock(&nap->mu);
 
   NaClLog(4, "Leaving NaClSecureReverseClientCallback\n");
 }
 
-/* fwd */
-int NaClSecureReverseClientCtor(
-    struct NaClSecureReverseClient *self,
-    void                            (*client_callback)(
-        void *, struct NaClThreadInterface *, struct NaClDesc *),
-    void                            *state,
-    struct NaClApp                  *nap);
-
 static void NaClSecureReverseClientSetup(struct NaClSrpcRpc     *rpc,
                                          struct NaClSrpcArg     **in_args,
                                          struct NaClSrpcArg     **out_args,
                                          struct NaClSrpcClosure *done) {
   struct NaClApp                  *nap =
       (struct NaClApp *) rpc->channel->server_instance_data;
   struct NaClSecureReverseClient  *rev;
 
   UNREFERENCED_PARAMETER(in_args);
   NaClLog(4, "Entered NaClSecureReverseClientSetup\n");
@@ skipping 48 matching lines @@
   if (!NaClSimpleRevClientStartServiceThread(&rev->base)) {
     NaClLog(LOG_FATAL, "Could not start reverse service thread\n");
   }
 
 done:
   NaClXMutexUnlock(&nap->mu);
   (*done->Run)(done);
   NaClLog(4, "Leaving NaClSecureReverseClientSetup\n");
 }
 
-/*
- * Only called at startup and thereafter by the reverse secure
- * channel, with |self| locked.
- */
-static
-int NaClSecureReverseClientInsertHandler_mu(
-    struct NaClSecureReverseClient  *self,
-    void                            (*h)(void *,
-                                         struct NaClThreadInterface *,
-                                         struct NaClDesc *),
-    void                            *d) {
-  struct NaClConnectionHandler *entry;
-
-  NaClLog(4,
-          ("NaClSecureReverseClientInsertHandler_mu: h 0x%"NACL_PRIxPTR","
-           " d 0x%"NACL_PRIxPTR"\n"),
-          (uintptr_t) h,
-          (uintptr_t) d);
-  entry = (struct NaClConnectionHandler *) malloc(sizeof *entry);
-  if (NULL == entry) {
-    return 0;
-  }
-  entry->handler = h;
-  entry->state = d;
-  entry->next = (struct NaClConnectionHandler *) NULL;
-  *self->queue_insert = entry;
-  self->queue_insert = &entry->next;
-
-  return 1;
-}
-
-/*
- * Caller must set up handler before issuing connection request RPC on
- * nap->reverse_channel, since otherwise the connection handler queue
- * may be empty and the connect code would abort.  Because the connect
- * doesn't wait for a handler, we don't need a condvar.
- *
- * We do not need to serialize on the handlers, since the
- * RPC-server/IMC-client implementation should not distinguish one
- * connection from another: it is okay for two handlers to be
- * inserted, and two connection request RPCs to be preformed
- * (sequentially, since they are over a single channel), and have the
- * server side spawn threads that asynchronously connect twice, in the
- * "incorrect" order, etc.
- */
-int NaClSecureReverseClientInsertHandler(
-    struct NaClSecureReverseClient *self,
-    void                            (*handler)(
-        void                        *handlr_state,
-        struct NaClThreadInterface  *thread_if,
-        struct NaClDesc             *new_conn),
-    void                            *handler_state) {
-  int retval;
-
-  NaClXMutexLock(&self->mu);
-  retval = NaClSecureReverseClientInsertHandler_mu(self,
-                                                   handler, handler_state);
-  NaClXMutexUnlock(&self->mu);
-  return retval;
-}
-
-static
-struct NaClConnectionHandler *NaClSecureReverseClientPopHandler(
-    struct NaClSecureReverseClient *self) {
-  struct NaClConnectionHandler *head;
-
-  NaClLog(4, "Entered NaClSecureReverseClientPopHandler, acquiring lock\n");
-  NaClXMutexLock(&self->mu);
-  NaClLog(4, "NaClSecureReverseClientPopHandler, got lock\n");
-  head = self->queue_head;
-  if (NULL == head) {
-    NaClLog(LOG_FATAL,
-            "NaClSecureReverseClientPopHandler:  empty handler queue\n");
-  }
-  if (NULL == (self->queue_head = head->next)) {
-    NaClLog(4, "NaClSecureReverseClientPopHandler, last elt patch up\n");
-    self->queue_insert = &self->queue_head;
-  }
-  NaClLog(4, "NaClSecureReverseClientPopHandler, unlocking\n");
-  NaClXMutexUnlock(&self->mu);
-
-  head->next = NULL;
-  NaClLog(4,
-          ("Leaving NaClSecureReverseClientPopHandler:"
-           " returning %"NACL_PRIxPTR"\n"),
-          (uintptr_t) head);
-  return head;
-}
-
-static
-void NaClSecureReverseClientInternalCallback(
-    void                        *state,
-    struct NaClThreadInterface  *tif,
-    struct NaClDesc             *conn) {
-  struct NaClSecureReverseClient *self =
-      (struct NaClSecureReverseClient *) state;
-  struct NaClConnectionHandler *hand_ptr;
-
-  UNREFERENCED_PARAMETER(tif);
-  NaClLog(4, "Entered NaClSecureReverseClientInternalCallback\n");
-  hand_ptr = NaClSecureReverseClientPopHandler(self);
-  NaClLog(4, " got callback object %"NACL_PRIxPTR"\n", (uintptr_t) hand_ptr);
-  NaClLog(4,
-          " callback:0x%"NACL_PRIxPTR"(0x%"NACL_PRIxPTR",0x%"NACL_PRIxPTR")\n",
-          (uintptr_t) hand_ptr->handler,
-          (uintptr_t) hand_ptr->state,
-          (uintptr_t) conn);
-  (*hand_ptr->handler)(hand_ptr->state, tif, conn);
-  NaClLog(4, "NaClSecureReverseClientInternalCallback: freeing memory\n");
-  free(hand_ptr);
-  NaClLog(4, "Leaving NaClSecureReverseClientInternalCallback\n");
-}
-
-/*
- * Require an initial connection handler in the Ctor, so that it's
- * obvious that a reverse client needs to accept an IMC connection
- * from the server to get things bootstrapped.
- */
-int NaClSecureReverseClientCtor(
-    struct NaClSecureReverseClient  *self,
-    void                            (*client_callback)(
-        void *, struct NaClThreadInterface*, struct NaClDesc *),
-    void                            *state,
-    struct NaClApp                  *nap) {
-  NaClLog(4,
-          ("Entered NaClSecureReverseClientCtor, self 0x%"NACL_PRIxPTR","
-           " nap 0x%"NACL_PRIxPTR"\n"),
-          (uintptr_t) self,
-          (uintptr_t) nap);
-  if (!NaClSimpleRevClientCtor(&self->base,
-                               NaClSecureReverseClientInternalCallback,
-                               (void *) self,
-                               NaClThreadInterfaceThreadFactory,
-                               (void *) NULL)) {
-    goto failure_simple_ctor;
-  }
-  NaClLog(4, "NaClSecureReverseClientCtor: Mutex\n");
-  if (!NaClMutexCtor(&self->mu)) {
-    goto failure_mutex_ctor;
-  }
-  self->nap = nap;
-  self->queue_head = (struct NaClConnectionHandler *) NULL;
-  self->queue_insert = &self->queue_head;
-
-  NACL_VTBL(NaClRefCount, self) =
-      (struct NaClRefCountVtbl *) &kNaClSecureReverseClientVtbl;
-
-  NaClLog(4, "NaClSecureReverseClientCtor: InsertHandler\n");
-  if (!NaClSecureReverseClientInsertHandler(self,
-                                            client_callback,
-                                            state)) {
-    goto failure_handler_insert;
-  }
-
-  NaClLog(4, "Leaving NaClSecureReverseClientCtor\n");
-  return 1;
-
- failure_handler_insert:
-  NaClLog(4, "NaClSecureReverseClientCtor: InsertHandler failed\n");
-  NACL_VTBL(NaClRefCount, self) =
-      (struct NaClRefCountVtbl *) &kNaClSimpleRevClientVtbl;
-
-  self->nap = NULL;
-  self->queue_insert = (struct NaClConnectionHandler **) NULL;
-  NaClMutexDtor(&self->mu);
-
- failure_mutex_ctor:
-  NaClLog(4, "NaClSecureReverseClientCtor: Mutex failed\n");
-  (*NACL_VTBL(NaClRefCount, self)->Dtor)((struct NaClRefCount *) self);
- failure_simple_ctor:
-  NaClLog(4, "Leaving NaClSecureReverseClientCtor\n");
-  return 0;
-}
-
-void NaClSecureReverseClientDtor(struct NaClRefCount *vself) {
-  struct NaClSecureReverseClient *self =
-      (struct NaClSecureReverseClient *) vself;
-
-  struct NaClConnectionHandler  *entry;
-  struct NaClConnectionHandler  *next;
-
-  for (entry = self->queue_head; NULL != entry; entry = next) {
-    next = entry->next;
-    free(entry);
-  }
-  NaClMutexDtor(&self->mu);
-
-  NACL_VTBL(NaClRefCount, self) = (struct NaClRefCountVtbl const *)
-      &kNaClSimpleRevClientVtbl;
-  (*NACL_VTBL(NaClRefCount, self)->Dtor)(vself);
-}
-
-int NaClSecureServiceConnectionFactory(
-    struct NaClSimpleService            *vself,
-    struct NaClDesc                     *conn,
-    struct NaClSimpleServiceConnection  **out) {
-  struct NaClSecureService *self =
-      (struct NaClSecureService *) vself;
-
-  /* our instance_data is not connection specific */
-  return NaClSimpleServiceConnectionFactoryWithInstanceData(
-      vself, conn, (void *) self->nap, out);
-}
-
-int NaClSecureServiceAcceptAndSpawnHandler(struct NaClSimpleService *vself) {
-  int rv;
-
-  NaClLog(4,
-          "NaClSecureServiceAcceptAndSpawnHandler: invoking base class vfn\n");
-  rv = (*kNaClSimpleServiceVtbl.AcceptAndSpawnHandler)(vself);
-  if (0 != rv) {
-    NaClLog(LOG_FATAL,
-            "Secure channel AcceptAndSpawnHandler returned %d\n",
-            rv);
-  }
-  NaClThreadExit(0);
-  /*
-   * NOTREACHED The port is now to be used by untrusted code: all
-   * subsequent connections are handled there.
-   */
-  return rv;
-}
-
-void NaClSecureServiceRpcHandler(struct NaClSimpleService           *vself,
-                                 struct NaClSimpleServiceConnection *vconn) {
-
-  NaClLog(4, "NaClSecureChannelThread started\n");
-  (*kNaClSimpleServiceVtbl.RpcHandler)(vself, vconn);
-  NaClLog(4, "NaClSecureChannelThread: channel closed, exiting.\n");
-  NaClExit(0);
-}
-
-struct NaClSimpleServiceVtbl const kNaClSecureServiceVtbl = {
-  {
-    NaClSecureServiceDtor,
-  },
-  NaClSecureServiceConnectionFactory,
-  NaClSimpleServiceAcceptConnection,
-  NaClSecureServiceAcceptAndSpawnHandler,
-  NaClSecureServiceRpcHandler,
-};
-
-struct NaClSimpleRevClientVtbl const kNaClSecureReverseClientVtbl = {
-  {
-    NaClSecureReverseClientDtor,
-  },
-};
+struct NaClSrpcHandlerDesc const secure_handlers[]; /* fwd */
 
 void NaClSecureCommandChannel(struct NaClApp *nap) {
   struct NaClSecureService *secure_command_server;
 
-  static struct NaClSrpcHandlerDesc const secure_handlers[] = {
-    { "hard_shutdown::", NaClSecureChannelShutdownRpc, },
-    { "start_module::i", NaClSecureChannelStartModuleRpc, },
-    { "log:is:", NaClSecureChannelLog, },
-    { "load_module:hs:", NaClLoadModuleRpc, },
-    { "load_irt:h:", NaClLoadIrtRpc, },
-    { "reverse_setup::h", NaClSecureReverseClientSetup, },
-    /* add additional calls here.  upcall set up?  start module signal? */
-    { (char const *) NULL, (NaClSrpcMethod) NULL, },
-  };
-
   NaClLog(4, "Entered NaClSecureCommandChannel\n");
 
   secure_command_server = (struct NaClSecureService *) malloc(
       sizeof *secure_command_server);
   if (NACL_FI_ERROR_COND("NaClSecureCommandChannel__malloc",
                          NULL == secure_command_server)) {
     NaClLog(LOG_FATAL, "Out of memory for secure command channel\n");
   }
   if (NACL_FI_ERROR_COND("NaClSecureCommandChannel__NaClSecureServiceCtor",
                          !NaClSecureServiceCtor(secure_command_server,
-                                                secure_handlers, nap))) {
+                                                secure_handlers,
+                                                nap,
+                                                nap->secure_service_port,
+                                                nap->secure_service_address))) {
     NaClLog(LOG_FATAL, "NaClSecureServiceCtor failed\n");
   }
   nap->secure_service = secure_command_server;
 
   NaClLog(4, "NaClSecureCommandChannel: starting service thread\n");
   if (NACL_FI_ERROR_COND(
           "NaClSecureCommandChannel__NaClSimpleServiceStartServiceThread",
           !NaClSimpleServiceStartServiceThread((struct NaClSimpleService *)
                                                secure_command_server))) {
     NaClLog(LOG_FATAL,
             "Could not start secure command channel service thread\n");
   }
 
   NaClLog(4, "Leaving NaClSecureCommandChannel\n");
 }
 
+struct NaClSrpcHandlerDesc const secure_handlers[] = {
+  { "hard_shutdown::", NaClSecureChannelShutdownRpc, },
+  { "start_module::i", NaClSecureChannelStartModuleRpc, },
+  { "log:is:", NaClSecureChannelLog, },
+  { "load_module:hs:", NaClLoadModuleRpc, },
+  { "load_irt:h:", NaClLoadIrtRpc, },
+  { "reverse_setup::h", NaClSecureReverseClientSetup, },
+  /* add additional calls here.  upcall set up?  start module signal? */
+  { (char const *) NULL, (NaClSrpcMethod) NULL, },
+};
 
 /*
  * It is fine to have multiple I/O operations read from memory in Write
  * or SendMsg like operations.
  */
 void NaClVmIoWillStart(struct NaClApp *nap,
                        uint32_t addr_first_usr,
                        uint32_t addr_last_usr) {
   NaClXMutexLock(&nap->mu);
   (*nap->mem_io_regions->vtbl->AddInterval)(nap->mem_io_regions,
@@ skipping 60 matching lines @@
   nacl_global_xlate_base = mem_start;
 
   NaClSandboxMemoryStartForValgrind(mem_start);
 
   _ovly_debug_event();
 }
 
 void NaClGdbHook(struct NaClApp const *nap) {
   StopForDebuggerInit(nap->mem_start);
 }