Merge pull request #74 from painless-security/jennifer/set_realm_apcs
authormrw42 <margaret@painless-security.com>
Fri, 4 May 2018 19:04:10 +0000 (15:04 -0400)
committerGitHub <noreply@github.com>
Fri, 4 May 2018 19:04:10 +0000 (15:04 -0400)
Handle APC correctly when a realm is discovered from an APC community update

1  2 
common/tr_comm.c
include/tr_comm.h
tr/tr_trp.c
trp/trp_conn.c
trp/trps.c

diff --combined common/tr_comm.c
  #include <tr_rp.h>
  #include <tr_idp.h>
  #include <tr_name_internal.h>
 +#include <trp_internal.h>
  #include <tr_comm.h>
  #include <tr_debug.h>
 -
 +#include <tr_util.h>
  
  static int tr_comm_destructor(void *obj)
  {
@@@ -762,9 -761,53 +762,53 @@@ TR_COMM_MEMB *tr_comm_memb_iter_next(TR
  }
  
  
- /* iterate over all memberships in the table */
+ /* iterate over all memberships in the table
+  *
+  * The table is structured as a vertical list of memberships, each for a
+  * different community/realm. Each element in this list has a horizontal "origin list,"
+  * each for the same community/realm but with a different origin for the membership.
+  * Only the first element in the origin list has a vertical link ("next" pointer).
+  * Any element may have a horizontal link ("origin_next" pointer).
+  *
+  *  (A) - (B) - (C) - X
+  *   |
+  *  (D) - (E) - X
+  *   |
+  *  (F) - X
+  *   |
+  *  (G) - (H) - X
+  *   |
+  *   X
+  *
+  *  A, B, and C are all community/realm pair membership 1, with different origins
+  *  D, E are a second community/realm pair, with different origins
+  *  F is a third...
+  *  G, H are a fourth pair, with different origins
+  *
+  * This iterator will return every element in the grid.
+  *
+  * Algorithm:
+  *   The iterator struct stores the current member (cur_memb) and the origin head (cur_orig_head).
+  *   The latter is a pointer to the head of the current origin list (i.e., first element in a row).
+  *   The former can point to any element in the list. Both start at the root of the list (A in the
+  *   diagram above).
+  *
+  *   After each call to _first() or _next(), cur_memb points to the element just returned.
+  *
+  *   0. _first() just returns the first element. The rest of the steps are in _next()
+  *
+  *   1. If cur_memb has an origin_next element, walk the origin list. Move cur_memb to
+  *      the origin_list element (next one in this row) and return it.
+  *   2. If cur_memb does not have an origin_next element, we've finished the current origin
+  *      list. Move cur_memb to cur_orig_head's next element (the start of the next column),
+  *      move cur_orig_head to that same place, and return it.
+  *   3. If neither cur_memb has an origin_next element nor cur_orig_head has a next element,
+  *      then we have already reached the end of the list and there's nothing more to do.
+  *      Return NULL.
+  */
  TR_COMM_MEMB *tr_comm_memb_iter_all_first(TR_COMM_ITER *iter, TR_COMM_TABLE *ctab)
  {
+   /* step 0: return the root of the list */
    iter->cur_memb=ctab->memberships;
    iter->cur_orig_head=ctab->memberships;
    return iter->cur_memb;
  
  TR_COMM_MEMB *tr_comm_memb_iter_all_next(TR_COMM_ITER *iter)
  {
-   if (iter->cur_memb->next==NULL) {
-     if (iter->cur_orig_head->next==NULL) {
-       /* we're done */
-       return NULL;
-     } else {
-       iter->cur_memb=iter->cur_orig_head->next;
-       iter->cur_orig_head=iter->cur_orig_head->next;
-     }
+   if (iter->cur_memb->origin_next) {
+     /* step 1: return the next element in the current origin list */
+     iter->cur_memb = iter->cur_memb->origin_next;
+   } else if (iter->cur_orig_head->next) {
+     /* step 2: move to the start of the next row and return the first element */
+     iter->cur_orig_head = iter->cur_memb = iter->cur_orig_head->next;
    } else {
-     iter->cur_memb=iter->cur_memb->origin_next;
+     /* step 3: both cur_memb->origin_next and cur_orig_head->next are null */
+     iter->cur_orig_head = iter->cur_memb = NULL;
    }
    return iter->cur_memb;
  }
@@@ -1018,18 -1060,6 +1061,18 @@@ struct timespec *tr_comm_memb_get_expir
    return memb->expiry;
  }
  
 +/**
 + * Get the expiration according to the realtime clock
 + *
 + * @param memb
 + * @param result space to store the result
 + * @return pointer to the result, or null on error
 + */
 +struct timespec *tr_comm_memb_get_expiry_realtime(TR_COMM_MEMB *memb, struct timespec *result)
 +{
 +  return tr_clock_convert(TRP_CLOCK, memb->expiry, CLOCK_REALTIME, result);
 +}
 +
  int tr_comm_memb_is_expired(TR_COMM_MEMB *memb, struct timespec *curtime)
  {
    tr_debug("tr_comm_memb_is_expired: (cur->tv_sec>memb->expiry->tv_sec)=(%u > %u)=%s",
@@@ -1335,11 -1365,24 +1378,24 @@@ TR_COMM *tr_comm_table_find_comm(TR_COM
    return tr_comm_lookup(ctab->comms, comm_id);
  }
  
- void tr_comm_table_add_comm(TR_COMM_TABLE *ctab, TR_COMM *new)
+ /**
+  * Add a community to the table.
+  *
+  * Does not allow duplicate community ids.
+  *
+  * @param ctab
+  * @param new
+  * @return 0 on success, -1 on failure
+  */
+ int tr_comm_table_add_comm(TR_COMM_TABLE *ctab, TR_COMM *new)
  {
+   if (tr_comm_table_find_comm(ctab, tr_comm_get_id(new)) != NULL)
+     return -1;
    tr_comm_add(ctab->comms, new);
    if (ctab->comms!=NULL)
      talloc_steal(ctab, ctab->comms); /* make sure it's in the right context */
+   return 0;
  }
  
  void tr_comm_table_remove_comm(TR_COMM_TABLE *ctab, TR_COMM *comm)
diff --combined include/tr_comm.h
@@@ -113,7 -113,7 +113,7 @@@ void tr_comm_table_free(TR_COMM_TABLE *
  TR_COMM_TABLE *tr_comm_table_new(TALLOC_CTX *mem_ctx);
  void tr_comm_table_free(TR_COMM_TABLE *ctab);
  void tr_comm_table_sweep(TR_COMM_TABLE *ctab);
void tr_comm_table_add_comm(TR_COMM_TABLE *ctab, TR_COMM *new);
int tr_comm_table_add_comm(TR_COMM_TABLE *ctab, TR_COMM *new);
  void tr_comm_table_remove_comm(TR_COMM_TABLE *ctab, TR_COMM *comm);
  TR_RP_REALM *tr_comm_table_find_rp_realm(TR_COMM_TABLE *ctab, TR_NAME *realm_id);
  void tr_comm_table_add_rp_realm(TR_COMM_TABLE *ctab, TR_RP_REALM *new);
@@@ -155,7 -155,6 +155,7 @@@ void tr_comm_memb_set_interval(TR_COMM_
  unsigned int tr_comm_memb_get_interval(TR_COMM_MEMB *memb);
  void tr_comm_memb_set_expiry(TR_COMM_MEMB *memb, struct timespec *time);
  struct timespec *tr_comm_memb_get_expiry(TR_COMM_MEMB *memb);
 +struct timespec *tr_comm_memb_get_expiry_realtime(TR_COMM_MEMB *memb, struct timespec *result);
  int tr_comm_memb_is_expired(TR_COMM_MEMB *memb, struct timespec *curtime);
  void tr_comm_memb_set_triggered(TR_COMM_MEMB *memb, int trig);
  int tr_comm_memb_is_triggered(TR_COMM_MEMB *memb);
diff --combined tr/tr_trp.c
@@@ -114,7 -114,7 +114,7 @@@ static int tr_trps_gss_handler(gss_name
  
    tr_debug("tr_trps_gss_handler()");
  
 -  if ((!client_name) || (!gss_name) || (!trps) || (!cfg_mgr)) {
 +  if ((!client_name) || (!trps) || (!cfg_mgr)) {
      tr_debug("tr_trps_gss_handler: Bad parameters.");
      return -1;
    }
@@@ -184,23 -184,22 +184,22 @@@ static void tr_trps_event_cb(int listen
      tr_debug("tr_trps_event_cb: unexpected event on TRPS socket (event=0x%X)", event);
    } else {
      /* create a thread to handle this connection */
-     if (asprintf(&name, "trustrouter@%s", trps->hostname)==-1) {
+     name = talloc_asprintf(tmp_ctx, "trustrouter@%s", trps->hostname);
+     if (name == NULL)
        goto cleanup;
-     }
-     gssname=tr_new_name(name);
-     free(name); name=NULL;
+     gssname=tr_new_name(name); /* name cleaned up with tmp_ctx */
      conn=trp_connection_accept(tmp_ctx, listener, gssname);
      if (conn!=NULL) {
        /* need to monitor this fd and trigger events when read becomes possible */
        thread_data=talloc(conn, struct trps_thread_data);
        if (thread_data==NULL) {
          tr_err("tr_trps_event_cb: unable to allocate trps_thread_data");
-         talloc_free(tmp_ctx);
-         return;
+         goto cleanup;
        }
        thread_data->conn=conn;
        thread_data->trps=trps;
-       trps_add_connection(trps, conn); /* remember the connection */
+       trps_add_connection(trps, conn); /* remember the connection - this puts conn and the thread data in trps's talloc context */
        pthread_create(trp_connection_get_thread(conn), NULL, tr_trps_thread, thread_data);
      }
    }
@@@ -222,6 -221,17 +221,17 @@@ static void tr_trps_cleanup_conn(TRPS_I
  
  static void tr_trps_cleanup_trpc(TRPS_INSTANCE *trps, TRPC_INSTANCE *trpc)
  {
+   TR_MQ_MSG *msg;
+   /* tell the trpc thread to exit */
+   msg = tr_mq_msg_new(NULL, TR_MQMSG_TRPC_EXIT_OK, TR_MQ_PRIO_NORMAL);
+   if (msg) {
+     tr_debug("tr_trps_cleanup_trpc: Notifying thread that it may now exit");
+     trpc_mq_add(trpc, msg);
+   } else {
+     tr_crit("tr_trps_cleanup_trpc: Unable to acknowledge disconnection, thread will probably never terminate");
+   }
    pthread_join(*trp_connection_get_thread(trpc_get_conn(trpc)), NULL);
    trps_remove_trpc(trps, trpc);
    trpc_free(trpc);
@@@ -268,55 -278,65 +278,65 @@@ static void tr_trps_process_mq(int sock
    TRPS_INSTANCE *trps=talloc_get_type_abort(arg, TRPS_INSTANCE);
    TR_MQ_MSG *msg=NULL;
    const char *s=NULL;
+   TRP_PEER *peer = NULL;
+   char *tmp = NULL;
  
    msg=trps_mq_pop(trps);
    while (msg!=NULL) {
      s=tr_mq_msg_get_message(msg);
      if (0==strcmp(s, TR_MQMSG_TRPS_CONNECTED)) {
-       TR_NAME *gssname=(TR_NAME *)tr_mq_msg_get_payload(msg);
-       TRP_PEER *peer=trps_get_peer_by_gssname(trps, gssname);
+       TR_NAME *peer_gssname=(TR_NAME *)tr_mq_msg_get_payload(msg);
+       peer=trps_get_peer_by_gssname(trps, peer_gssname); /* get the peer record */
+       tmp = tr_name_strdup(peer_gssname); /* get the name as a null-terminated string */
        if (peer==NULL)
-         tr_err("tr_trps_process_mq: incoming connection from unknown peer (%s) reported.", gssname->buf);
+         tr_err("tr_trps_process_mq: incoming connection from unknown peer (%s) reported.", tmp);
        else {
          trp_peer_set_incoming_status(peer, PEER_CONNECTED);
-         tr_err("tr_trps_process_mq: incoming connection from %s established.", gssname->buf);
+         tr_err("tr_trps_process_mq: incoming connection from %s established.", tmp);
        }
+       free(tmp);
      }
      else if (0==strcmp(s, TR_MQMSG_TRPS_DISCONNECTED)) {
        TRP_CONNECTION *conn=talloc_get_type_abort(tr_mq_msg_get_payload(msg), TRP_CONNECTION);
-       TR_NAME *gssname=trp_connection_get_gssname(conn);
-       TRP_PEER *peer=trps_get_peer_by_gssname(trps, gssname);
+       TR_NAME *peer_gssname=trp_connection_get_peer(conn);
+       peer=trps_get_peer_by_gssname(trps, peer_gssname); /* get the peer record */
+       tmp = tr_name_strdup(peer_gssname); /* get the name as a null-terminated string */
        if (peer==NULL) {
-         tr_err("tr_trps_process_mq: incoming connection from unknown peer (%s) lost.",
-                trp_connection_get_gssname(conn)->buf);
+         tr_err("tr_trps_process_mq: incoming connection from unknown peer (%.*s) lost.", tmp);
        } else {
          trp_peer_set_incoming_status(peer, PEER_DISCONNECTED);
          tr_trps_cleanup_conn(trps, conn);
-         tr_err("tr_trps_process_mq: incoming connection from %s lost.", gssname->buf);
+         tr_err("tr_trps_process_mq: incoming connection from %s lost.", tmp);
        }
+       free(tmp);
      }
      else if (0==strcmp(s, TR_MQMSG_TRPC_CONNECTED)) {
        TR_NAME *svcname=(TR_NAME *)tr_mq_msg_get_payload(msg);
-       TRP_PEER *peer=trps_get_peer_by_servicename(trps, svcname);
+       peer=trps_get_peer_by_servicename(trps, svcname);
+       tmp = tr_name_strdup(svcname);
        if (peer==NULL)
-         tr_err("tr_trps_process_mq: outgoing connection to unknown peer (%s) reported.", svcname->buf);
+         tr_err("tr_trps_process_mq: outgoing connection to unknown peer (%s) reported.", tmp);
        else {
          trp_peer_set_outgoing_status(peer, PEER_CONNECTED);
-         tr_err("tr_trps_process_mq: outgoing connection to %s established.", svcname->buf);
+         tr_err("tr_trps_process_mq: outgoing connection to %s established.", tmp);
        }
+       free(tmp);
      }
      else if (0==strcmp(s, TR_MQMSG_TRPC_DISCONNECTED)) {
-       /* trpc connection died */
+       /* The trpc connection died - see note above tr_trpc_thread() regarding the shutdown protocol.
+        * The EXIT_OK message is sent in tr_trps_cleanup_trpc(). */
        TRPC_INSTANCE *trpc=talloc_get_type_abort(tr_mq_msg_get_payload(msg), TRPC_INSTANCE);
-       TR_NAME *gssname=trpc_get_gssname(trpc);
-       TRP_PEER *peer=trps_get_peer_by_servicename(trps, gssname);
+       TR_NAME *svcname=trpc_get_gssname(trpc);
+       peer=trps_get_peer_by_servicename(trps, svcname);
+       tmp = tr_name_strdup(svcname);
        if (peer==NULL)
-         tr_err("tr_trps_process_mq: outgoing connection to unknown peer (%s) lost.", gssname->buf);
+         tr_err("tr_trps_process_mq: outgoing connection to unknown peer (%s) lost.", tmp);
        else {
          trp_peer_set_outgoing_status(peer, PEER_DISCONNECTED);
-         tr_err("tr_trps_process_mq: outgoing connection to %s lost.", gssname->buf);
+         tr_err("tr_trps_process_mq: outgoing connection to %s lost.", tmp);
          tr_trps_cleanup_trpc(trps, trpc);
        }
+       free(tmp);
      }
  
      else if (0==strcmp(s, TR_MQMSG_MSG_RECEIVED)) {
@@@ -573,6 -593,49 +593,49 @@@ struct trpc_thread_data 
    TRPC_INSTANCE *trpc;
    TRPS_INSTANCE *trps;
  };
+ /**
+  * Returns when a message is ready
+  */
+ static void trpc_thread_wait_for_message(struct trpc_notify_cb_data *cb)
+ {
+   cb->msg_ready = 0;
+   do
+     pthread_cond_wait(&(cb->cond), &(cb->mutex));
+   while (!cb->msg_ready);
+ }
+ /**
+  * Thread for handling TRPC (outgoing) connections
+  *
+  * Locking protocol:
+  *
+  * This thread uses a mutex and condition variable to wait for incoming messages
+  * on its queue. The main thread interacts with these through the tr_trpc_mq_cb()
+  * callback. This callback is called by the TR_MQ whenever the queue goes from empty
+  * to non-empty. The callback locks the thread's mutex, then sets a msg_ready flag
+  * signals the condition variable, then unlocks the mutex.
+  *
+  * This thread waits for the condition variable to be signaled, then checks that the
+  * msg_ready flag has actually been set (pthreads does not guarantee against false
+  * wakes). If so, it holds the mutex lock while reading any messages in its queue and
+  * sending responses over the GSS connection. If it receives an ABORT message from
+  * the main thread (not currently used), or if sending the GSS message fails (which
+  * usually indicates that the connection has been lost), the thread begins the shutdown
+  * process. It still holds the mutex lock at the start of this process.
+  *
+  * This begins by setting trpc->shutting_down = 1, then sending a message to the
+  * TRPS (main) thread indicating that it has DISCONNECTED. It then releases the mutex
+  * and waits for more messages, again using the mutex and the condition variable. It
+  * ignores any except for an EXIT_OK message. When that is received, it exits, terminating
+  * the thread.
+  *
+  * When the main thread receives the DISCONNECTED message, it must stop sending messages
+  * to this thread's message queue. When it is certain that it will not queue any more messages,
+  * it sends the EXIT_OK message as its last interaction with this thread. Once that message
+  * has been queued, queueing additional messages may result in deadlock or segfaults.
+  * (It is ok if there are other messages in the queue, but it must not queue more.)
+  */
  static void *tr_trpc_thread(void *arg)
  {
    TALLOC_CTX *tmp_ctx=talloc_new(NULL);
    const char *msg_type=NULL;
    char *encoded_msg=NULL;
    TR_NAME *peer_gssname=NULL;
-   int n_sent=0;
+   int n_sent = 0;
+   int n_popped = 0;
    int exit_loop=0;
  
    struct trpc_notify_cb_data cb_data={0,
      msg=NULL;
  
      while(!exit_loop) {
-       cb_data.msg_ready=0;
-       pthread_cond_wait(&(cb_data.cond), &(cb_data.mutex));
-       /* verify the condition */
-       if (cb_data.msg_ready) {
-         for (msg=trpc_mq_pop(trpc),n_sent=0; msg!=NULL; msg=trpc_mq_pop(trpc),n_sent++) {
-           msg_type=tr_mq_msg_get_message(msg);
-           if (0==strcmp(msg_type, TR_MQMSG_ABORT)) {
-             exit_loop=1;
-             break;
-           }
-           else if (0==strcmp(msg_type, TR_MQMSG_TRPC_SEND)) {
-             encoded_msg=tr_mq_msg_get_payload(msg);
-             if (encoded_msg==NULL)
-               tr_notice("tr_trpc_thread: null outgoing TRP message.");
-             else {
-               rc = trpc_send_msg(trpc, encoded_msg);
-               if (rc!=TRP_SUCCESS) {
-                 tr_notice("tr_trpc_thread: trpc_send_msg failed.");
-                 exit_loop=1;
-                 break;
-               }
+       trpc_thread_wait_for_message(&cb_data); /* handles locking */
+       n_popped = 0; /* have not popped any messages from the queue */
+       n_sent = 0; /* have not sent any messages yet */
+       for (msg = trpc_mq_pop(trpc);
+            msg != NULL;
+            msg = trpc_mq_pop(trpc)) {
+         n_popped++;
+         msg_type = tr_mq_msg_get_message(msg);
+         if (0 == strcmp(msg_type, TR_MQMSG_ABORT)) {
+           exit_loop = 1;
+           break;
+         } else if (0 == strcmp(msg_type, TR_MQMSG_TRPC_SEND)) {
+           encoded_msg = tr_mq_msg_get_payload(msg);
+           if (encoded_msg == NULL)
+             tr_notice("tr_trpc_thread: null outgoing TRP message.");
+           else {
+             rc = trpc_send_msg(trpc, encoded_msg);
+             if (rc == TRP_SUCCESS) {
+               n_sent++;
+             } else {
+               tr_notice("tr_trpc_thread: trpc_send_msg failed.");
+               /* Assume this means we lost the connection. */
+               exit_loop = 1;
+               break;
              }
            }
-           else
-             tr_notice("tr_trpc_thread: unknown message '%s' received.", msg_type);
+         } else
+           tr_notice("tr_trpc_thread: unknown message '%s' received.", msg_type);
  
-           tr_mq_msg_free(msg);
-         }
-         if (n_sent==0)
-           tr_err("tr_trpc_thread: notified of msg, but queue empty");
-         else 
-           tr_debug("tr_trpc_thread: sent %d messages.", n_sent);
+         tr_mq_msg_free(msg);
        }
+       /* if n_popped == 0, then n_sent must be zero (it's only set after at
+        * least one msg is popped) */
+       if (n_popped==0)
+         tr_err("tr_trpc_thread: notified of message, but queue empty");
+       else
+         tr_debug("tr_trpc_thread: sent %d messages.", n_sent);
      }
    }
  
-   tr_debug("tr_trpc_thread: exiting.");
-   msg=tr_mq_msg_new(tmp_ctx, TR_MQMSG_TRPC_DISCONNECTED, TR_MQ_PRIO_HIGH);
+   tr_debug("tr_trpc_thread: Disconnected. Waiting to terminate thread.");
+   trpc->shutting_down = 1;
+   /* Send a DISCONNECTED message to the main thread */
+   msg=tr_mq_msg_new(tmp_ctx, TR_MQMSG_TRPC_DISCONNECTED, TR_MQ_PRIO_NORMAL);
    tr_mq_msg_set_payload(msg, (void *)trpc, NULL); /* do not pass a free routine */
-   if (msg==NULL)
+   if (msg==NULL) {
+     /* can't notify main thread of exit - just do it and hope for the best */
      tr_err("tr_trpc_thread: error allocating TR_MQ_MSG");
-   else
+   } else {
      trps_mq_add(trps, msg);
  
-   trpc_mq_clear(trpc); /* clear any queued messages */
+     /* DISCONNECTED sent, now wait for an acknowledgement before exiting */
+     exit_loop = 0;
+     while (!exit_loop) {
+       trpc_thread_wait_for_message(&cb_data);
+       while (NULL != (msg = trpc_mq_pop(trpc))) {
+         msg_type = tr_mq_msg_get_message(msg);
+         /* ignore anything except an EXIT_OK */
+         if (0 == strcmp(msg_type, TR_MQMSG_TRPC_EXIT_OK)) {
+           exit_loop = 1;
+           break; /* skip any further messages */
+         }
+       }
+     }
+   }
  
+   tr_debug("tr_trpc_thread: thread terminating.");
    talloc_free(tmp_ctx);
    return NULL;
  }
@@@ -881,11 -970,7 +970,11 @@@ void tr_config_changed(TR_CFG *new_cfg
      tr_debug("tr_config_changed: freeing tr->mons->authorized_gss_names");
      tr_gss_names_free(tr->mons->authorized_gss_names);
    }
 -  tr->mons->authorized_gss_names = tr_gss_names_dup(tr->mons, new_cfg->internal->monitoring_credentials);
 +  if (new_cfg->internal->monitoring_credentials != NULL) {
 +    tr->mons->authorized_gss_names = tr_gss_names_dup(tr->mons, new_cfg->internal->monitoring_credentials);
 +  } else {
 +    tr->mons->authorized_gss_names = tr_gss_names_new(tr->mons);
 +  }
    if (tr->mons->authorized_gss_names == NULL) {
      tr_err("tr_config_changed: Error configuring monitoring credentials");
    }
diff --combined trp/trp_conn.c
@@@ -40,7 -40,6 +40,7 @@@
  
  #include <tr_debug.h>
  #include <trp_internal.h>
 +#include <tr_socket.h>
  
  /* Threading note: mutex lock is only used for protecting get_status() and set_status().
   * If needed, locking for other operations (notably adding/removing connections) must be managed
@@@ -339,21 -338,26 +339,26 @@@ int trp_connection_auth(TRP_CONNECTION 
    return !auth;
  }
  
- /* Accept connection */
- TRP_CONNECTION *trp_connection_accept(TALLOC_CTX *mem_ctx, int listen, TR_NAME *gssname)
+ /**
+  * Accept connection
+  *
+  * @param mem_ctx talloc context for return value
+  * @param listen socket fd for incoming connection
+  * @param gss_servicename our GSS service name to use for passive auth */
+ TRP_CONNECTION *trp_connection_accept(TALLOC_CTX *mem_ctx, int listen, TR_NAME *gss_servicename)
  {
    int conn_fd=-1;
    TRP_CONNECTION *conn=NULL;
  
 -  conn_fd = accept(listen, NULL, NULL);
 +  conn_fd = tr_sock_accept(listen);
  
    if (0 > conn_fd) {
 -    tr_notice("trp_connection_accept: accept() returned error.");
 +    tr_notice("trp_connection_accept: Error accepting connection.");
      return NULL;
    }
    conn=trp_connection_new(mem_ctx);
    trp_connection_set_fd(conn, conn_fd);
-   trp_connection_set_gssname(conn, gssname);
+   trp_connection_set_gssname(conn, gss_servicename);
    trp_connection_set_status(conn, TRP_CONNECTION_AUTHORIZING);
    return conn;
  }
diff --combined trp/trps.c
@@@ -269,6 -269,9 +269,9 @@@ TRP_RC trps_send_msg(TRPS_INSTANCE *trp
     * connect fails */
    if (trpc==NULL) {
      tr_warning("trps_send_msg: skipping message queued for missing TRP client entry.");
+   } else if (trpc->shutting_down) {
+     tr_debug("trps_send_msg: skipping message because TRP client is shutting down.");
+     rc = TRP_SUCCESS; /* it's ok that this didn't get sent, the connection will be gone in a moment */
    } else {
      mq_msg=tr_mq_msg_new(tmp_ctx, TR_MQMSG_TRPC_SEND, TR_MQ_PRIO_NORMAL);
      msg_dup=talloc_strdup(mq_msg, msg); /* get local copy in mq_msg context */
@@@ -653,36 -656,51 +656,51 @@@ static TRP_RC trps_accept_update(TRPS_I
  static TRP_RC trps_handle_inforec_route(TRPS_INSTANCE *trps, TRP_UPD *upd, TRP_INFOREC *rec)
  {
    TRP_ROUTE *route=NULL;
+   TR_COMM *comm = NULL;
    unsigned int feas=0;
  
    /* determine feasibility */
    feas=trps_check_feasibility(trps, trp_upd_get_realm(upd), trp_upd_get_comm(upd), rec);
    tr_debug("trps_handle_update: record feasibility=%d", feas);
  
-   /* do we have an existing route? */
-   route=trps_get_route(trps,
-                        trp_upd_get_comm(upd),
-                        trp_upd_get_realm(upd),
-                        trp_upd_get_peer(upd));
-   if (route!=NULL) {
-     /* there was a route table entry already */
-     tr_debug("trps_handle_updates: route entry already exists.");
-     if (feas) {
-       /* Update is feasible. Accept it. */
-       trps_accept_update(trps, upd, rec);
-     } else {
-       /* Update is infeasible. Ignore it unless the trust router has changed. */
-       if (0!=tr_name_cmp(trp_route_get_trust_router(route),
-                          trp_inforec_get_trust_router(rec))) {
-         /* the trust router associated with the route has changed, treat update as a retraction */
-         trps_retract_route(trps, route);
+   /* verify that the community is an APC */
+   comm = tr_comm_table_find_comm(trps->ctable, trp_upd_get_comm(upd));
+   if (comm == NULL) {
+     /* We don't know this community. Reject the route. */
+     tr_debug("trps_handle_updates: community %.*s unknown, ignoring route for %.*s",
+              trp_upd_get_comm(upd)->len, trp_upd_get_comm(upd)->buf,
+              trp_upd_get_realm(upd)->len, trp_upd_get_realm(upd)->buf);
+   } else if (tr_comm_get_type(comm) != TR_COMM_APC) {
+     /* The community in a route request *must* be an APC. This was not - ignore it. */
+     tr_debug("trps_handle_updates: community %.*s is not an APC, ignoring route for %.*s",
+              trp_upd_get_comm(upd)->len, trp_upd_get_comm(upd)->buf,
+              trp_upd_get_realm(upd)->len, trp_upd_get_realm(upd)->buf);
+   } else {
+     /* do we have an existing route? */
+     route=trps_get_route(trps,
+                          trp_upd_get_comm(upd),
+                          trp_upd_get_realm(upd),
+                          trp_upd_get_peer(upd));
+     if (route!=NULL) {
+       /* there was a route table entry already */
+       tr_debug("trps_handle_updates: route entry already exists.");
+       if (feas) {
+         /* Update is feasible. Accept it. */
+         trps_accept_update(trps, upd, rec);
+       } else {
+         /* Update is infeasible. Ignore it unless the trust router has changed. */
+         if (0!=tr_name_cmp(trp_route_get_trust_router(route),
+                            trp_inforec_get_trust_router(rec))) {
+           /* the trust router associated with the route has changed, treat update as a retraction */
+           trps_retract_route(trps, route);
+         }
        }
+     } else {
+       /* No existing route table entry. Ignore it unless it is feasible and not a retraction. */
+       tr_debug("trps_handle_update: no route entry exists yet.");
+       if (feas && trp_metric_is_finite(trp_inforec_get_metric(rec)))
+         trps_accept_update(trps, upd, rec);
      }
-   } else {
-     /* No existing route table entry. Ignore it unless it is feasible and not a retraction. */
-     tr_debug("trps_handle_update: no route entry exists yet.");
-     if (feas && trp_metric_is_finite(trp_inforec_get_metric(rec)))
-       trps_accept_update(trps, upd, rec);
    }
  
    return TRP_SUCCESS;
@@@ -768,7 -786,7 +786,7 @@@ cleanup
    return comm;
  }
  
- static TR_RP_REALM *trps_create_new_rp_realm(TALLOC_CTX *mem_ctx, TR_NAME *realm_id, TRP_INFOREC *rec)
+ static TR_RP_REALM *trps_create_new_rp_realm(TALLOC_CTX *mem_ctx, TR_NAME *comm, TR_NAME *realm_id, TRP_INFOREC *rec)
  {
    TALLOC_CTX *tmp_ctx=talloc_new(NULL);
    TR_RP_REALM *rp=tr_rp_realm_new(tmp_ctx);
@@@ -791,11 -809,15 +809,15 @@@ cleanup
    return rp;
  }
  
- static TR_IDP_REALM *trps_create_new_idp_realm(TALLOC_CTX *mem_ctx, TR_NAME *realm_id, TRP_INFOREC *rec)
+ static TR_IDP_REALM *trps_create_new_idp_realm(TALLOC_CTX *mem_ctx,
+                                                TR_NAME *comm_id,
+                                                TR_NAME *realm_id,
+                                                TRP_INFOREC *rec)
  {
    TALLOC_CTX *tmp_ctx=talloc_new(NULL);
    TR_IDP_REALM *idp=tr_idp_realm_new(tmp_ctx);
-   
+   TR_APC *realm_apcs = NULL;
    if (idp==NULL) {
      tr_debug("trps_create_new_idp_realm: unable to allocate new realm.");
      goto cleanup;
      idp=NULL;
      goto cleanup;
    }
-   if (trp_inforec_get_apcs(rec)!=NULL) {
-     tr_idp_realm_set_apcs(idp, tr_apc_dup(tmp_ctx, trp_inforec_get_apcs(rec)));
-     if (tr_idp_realm_get_apcs(idp)==NULL) {
-       tr_debug("trps_create_new_idp_realm: unable to allocate APC list.");
-       idp=NULL;
+   /* Set the APCs. If the community is a CoI, copy its APCs. If it is an APC, then
+    * that community itself is the APC for the realm. */
+   if (trp_inforec_get_comm_type(rec) == TR_COMM_APC) {
+     /* the community is an APC for this realm */
+     realm_apcs = tr_apc_new(tmp_ctx);
+     if (realm_apcs == NULL) {
+       tr_debug("trps_create_new_idp_realm: unable to allocate new APC list.");
+       idp = NULL;
+       goto cleanup;
+     }
+     tr_apc_set_id(realm_apcs, tr_dup_name(comm_id));
+     if (tr_apc_get_id(realm_apcs) == NULL) {
+       tr_debug("trps_create_new_idp_realm: unable to allocate new APC name.");
+       idp = NULL;
+       goto cleanup;
+     }
+   } else {
+     /* the community is not an APC for this realm */
+     realm_apcs = trp_inforec_get_apcs(rec);
+     if (realm_apcs == NULL) {
+       tr_debug("trps_create_new_idp_realm: no APCs for realm %.*s/%.*s, cannot add.",
+                realm_id->len, realm_id->buf,
+                comm_id->len, comm_id->buf);
+       idp = NULL;
+       goto cleanup;
+     }
+     /* we have APCs, make our own copy */
+     realm_apcs = tr_apc_dup(tmp_ctx, realm_apcs);
+     if (realm_apcs == NULL) {
+       tr_debug("trps_create_new_idp_realm: unable to duplicate APC list.");
+       idp = NULL;
        goto cleanup;
      }
    }
+   /* Whether the community is an APC or CoI, the APCs for the realm are in realm_apcs */
+   tr_idp_realm_set_apcs(idp, realm_apcs); /* takes realm_apcs out of tmp_ctx on success */
+   if (tr_idp_realm_get_apcs(idp) == NULL) {
+     tr_debug("trps_create_new_idp_realm: unable to set APC list for new realm.");
+     idp=NULL;
+     goto cleanup;
+   }
    idp->origin=TR_REALM_DISCOVERED;
    
    talloc_steal(mem_ctx, idp);
@@@ -864,7 -924,11 +924,11 @@@ static TRP_RC trps_handle_inforec_comm(
          tr_debug("trps_handle_inforec_comm: unable to create new community.");
          goto cleanup;
        }
-       tr_comm_table_add_comm(trps->ctable, comm);
+       if (tr_comm_table_add_comm(trps->ctable, comm) != 0)
+       {
+         tr_debug("trps_handle_inforec_comm: unable to add community to community table.");
+         goto cleanup;
+       }
      }
      /* TODO: see if other comm data match the new inforec and update or complain */
  
        if (rp_realm==NULL) {
          tr_debug("trps_handle_inforec_comm: unknown RP realm %.*s in inforec, creating it.",
                   realm_id->len, realm_id->buf);
-         rp_realm=trps_create_new_rp_realm(tmp_ctx, realm_id, rec);
+         rp_realm= trps_create_new_rp_realm(tmp_ctx, tr_comm_get_id(comm), realm_id, rec);
          if (rp_realm==NULL) {
            tr_debug("trps_handle_inforec_comm: unable to create new RP realm.");
            /* we may leave an unused community in the table, but it will only last until
        if (idp_realm==NULL) {
          tr_debug("trps_handle_inforec_comm: unknown IDP realm %.*s in inforec, creating it.",
                   realm_id->len, realm_id->buf);
-         idp_realm=trps_create_new_idp_realm(tmp_ctx, realm_id, rec);
+         idp_realm= trps_create_new_idp_realm(tmp_ctx, tr_comm_get_id(comm), realm_id, rec);
          if (idp_realm==NULL) {
            tr_debug("trps_handle_inforec_comm: unable to create new IDP realm.");
            /* we may leave an unused community in the table, but it will only last until
@@@ -1179,7 -1243,6 +1243,7 @@@ TRP_RC trps_sweep_ctable(TRPS_INSTANCE 
  {
    TALLOC_CTX *tmp_ctx=talloc_new(NULL);
    struct timespec sweep_time={0,0};
 +  struct timespec tmp = {0};
    TR_COMM_MEMB *memb=NULL;
    TR_COMM_ITER *iter=NULL;
    TRP_RC rc=TRP_ERROR;
                   tr_comm_memb_get_realm_id(memb)->len, tr_comm_memb_get_realm_id(memb)->buf,
                   tr_comm_get_id(tr_comm_memb_get_comm(memb))->len, tr_comm_get_id(tr_comm_memb_get_comm(memb))->buf,
                   tr_comm_memb_get_origin(memb)->len, tr_comm_memb_get_origin(memb)->buf,
 -                 timespec_to_str(tr_comm_memb_get_expiry(memb)));
 +                 timespec_to_str(tr_comm_memb_get_expiry_realtime(memb, &tmp)));
          tr_comm_table_remove_memb(trps->ctable, memb);
          tr_comm_memb_free(memb);
        } else {
          tr_comm_memb_expire(memb);
          trps_compute_expiry(trps, tr_comm_memb_get_interval(memb), tr_comm_memb_get_expiry(memb));
          tr_debug("trps_sweep_ctable: community membership expired at %s, resetting expiry to %s (%.*s in %.*s, origin %.*s).",
 -                 timespec_to_str(&sweep_time),
 -                 timespec_to_str(tr_comm_memb_get_expiry(memb)),
 +                 timespec_to_str(tr_clock_convert(TRP_CLOCK, &sweep_time, CLOCK_REALTIME, &tmp)),
 +                 timespec_to_str(tr_comm_memb_get_expiry_realtime(memb, &tmp)),
                   tr_comm_memb_get_realm_id(memb)->len, tr_comm_memb_get_realm_id(memb)->buf,
                   tr_comm_get_id(tr_comm_memb_get_comm(memb))->len, tr_comm_get_id(tr_comm_memb_get_comm(memb))->buf,
                   tr_comm_memb_get_origin(memb)->len, tr_comm_memb_get_origin(memb)->buf);