Do blacklist checks on CONNECT before giving CONNECT to neighbours.

If peer is blacklisted we do not need to to anything, this simplifies the state machine:
If peer is blacklisted: CONNECT is not given to neighbours
If address is blacklisted: address is not given to ATS and will therefore not be suggested 
So neighbour can use this information without additional blacklist checks

2 files changed, 265 insertions, 42 deletions

diff --git a/src/transport/gnunet-service-transport.c b/src/transport/gnunet-service-transport.c
index 0be5b4391..f4878a54f 100644
--- a/src/transport/gnunet-service-transport.c
+++ b/src/transport/gnunet-service-transport.c
@@ -71,6 +71,21 @@ struct SessionKiller
   GNUNET_SCHEDULER_TaskIdentifier task;
 };
 
+struct BlacklistCheckContext
+{
+  struct BlacklistCheckContext *prev;
+  struct BlacklistCheckContext *next;
+
+
+  struct GST_BlacklistCheck *blc;
+
+  struct GNUNET_HELLO_Address *address;
+  struct Session *session;
+  struct GNUNET_MessageHeader *msg;
+  struct GNUNET_ATS_Information *ats;
+  uint32_t ats_count;
+};
+
 /* globals */
 
 /**
@@ -128,6 +143,10 @@ static struct SessionKiller *sk_head;
  */
 static struct SessionKiller *sk_tail;
 
+struct BlacklistCheckContext *bc_head;
+struct BlacklistCheckContext *bc_tail;
+
+
 /**
  * Transmit our HELLO message to the given (connected) neighbour.
  *
@@ -264,6 +283,86 @@ kill_session (const char *plugin_name, struct Session *session)
 }
 
 /**
+ * Black list check result for try_connect call
+ * If connection to the peer is allowed request adddress and
+ *
+ * @param cls blc_ctx bl context
+ * @param peer the peer
+ * @param result the result
+ */
+static void
+connect_address_bl_check_cont (void *cls,
+    const struct GNUNET_PeerIdentity *peer, int result)
+{
+  struct BlacklistCheckContext *blctx = cls;
+
+  if (GNUNET_OK == result)
+  {
+    GST_ats_add_address (blctx->address, blctx->session, NULL, 0);
+  }
+  else
+  {
+    kill_session (blctx->address->transport_name, blctx->session);
+  }
+
+  GNUNET_CONTAINER_DLL_remove (bc_head, bc_tail, blctx);
+  GNUNET_HELLO_address_free (blctx->address);
+  GNUNET_free (blctx);
+}
+
+/**
+ * Black list check result for try_connect call
+ * If connection to the peer is allowed request adddress and
+ *
+ * @param cls blc_ctx bl context
+ * @param peer the peer
+ * @param result the result
+ */
+static void
+connect_bl_check_cont (void *cls,
+    const struct GNUNET_PeerIdentity *peer, int result)
+{
+  struct BlacklistCheckContext *blctx = cls;
+  struct BlacklistCheckContext *blctx_address;
+  struct GST_BlacklistCheck *blc;
+  if (GNUNET_OK == result)
+  {
+    /* Check if incoming address can be used to communicate */
+    blctx_address = GNUNET_new (struct BlacklistCheckContext);
+    blctx_address->address = GNUNET_HELLO_address_copy (blctx->address);
+    blctx_address->session = blctx->session;
+
+    GNUNET_CONTAINER_DLL_insert (bc_head, bc_tail, blctx_address);
+    if (NULL != (blc = GST_blacklist_test_allowed (&blctx_address->address->peer,
+          blctx_address->address->transport_name,
+          &connect_address_bl_check_cont, blctx_address)))
+    {
+      blctx_address->blc = blc;
+    }
+
+    /* Blacklist allows to speak to this peer, forward CONNECT to neighbours  */
+    if (GNUNET_OK != GST_neighbours_handle_connect (blctx->msg,
+          &blctx->address->peer, blctx->address, blctx->session))
+    {
+      kill_session (blctx->address->transport_name, blctx->session);
+    }
+  }
+  else
+  {
+    /* Blacklist denies to speak to this peer */
+    GNUNET_break (0);
+    kill_session (blctx->address->transport_name, blctx->session);
+    GNUNET_break (0);
+  }
+
+  GNUNET_CONTAINER_DLL_remove (bc_head, bc_tail, blctx);
+  if (NULL != blctx->address)
+    GNUNET_HELLO_address_free (blctx->address);
+  GNUNET_free (blctx->msg);
+  GNUNET_free (blctx);
+}
+
+/**
  * Function called by the transport for each received message.
  *
  * @param cls closure, const char* with the name of the plugin we received the message from
@@ -284,6 +383,8 @@ GST_receive_callback (void *cls,
 {
   const char *plugin_name = cls;
   struct GNUNET_TIME_Relative ret;
+  struct BlacklistCheckContext *blctx;
+  struct GST_BlacklistCheck *blc;
   uint16_t type;
 
   ret = GNUNET_TIME_UNIT_ZERO;
@@ -328,16 +429,22 @@ GST_receive_callback (void *cls,
     }
     break;
   case GNUNET_MESSAGE_TYPE_TRANSPORT_SESSION_CONNECT:
-    if (GNUNET_OK
-        != GST_neighbours_handle_connect (message, &address->peer, address, session))
+    /* Do blacklist check if communication with this peer is allowed */
+    blctx = GNUNET_new (struct BlacklistCheckContext);
+    blctx->address = GNUNET_HELLO_address_copy (address);
+    blctx->session = session;
+    blctx->msg = GNUNET_malloc (ntohs(message->size));
+    memcpy (blctx->msg, message, ntohs(message->size));
+    GNUNET_CONTAINER_DLL_insert (bc_head, bc_tail, blctx);
+    if (NULL != (blc = GST_blacklist_test_allowed (&address->peer, NULL,
+          &connect_bl_check_cont, blctx)))
     {
-      GNUNET_break_op(0);
-      kill_session (plugin_name, session);
+      blctx->blc = blc;
     }
     break;
   case GNUNET_MESSAGE_TYPE_TRANSPORT_SESSION_CONNECT_ACK:
-    if (GNUNET_OK
-        != GST_neighbours_handle_connect_ack (message, &address->peer, address, session))
+    if (GNUNET_OK != GST_neighbours_handle_connect_ack (message,
+        &address->peer, address, session))
     {
       kill_session (plugin_name, session);
     }
@@ -611,6 +718,37 @@ plugin_env_update_metrics (void *cls,
 }
 
 /**
+ * Black list check result for try_connect call
+ * If connection to the peer is allowed request adddress and
+ *
+ * @param cls blc_ctx bl context
+ * @param peer the peer
+ * @param result the result
+ */
+static void
+plugin_env_session_start_bl_check_cont (void *cls,
+    const struct GNUNET_PeerIdentity *peer, int result)
+{
+  struct BlacklistCheckContext *blctx = cls;
+
+  if (GNUNET_OK == result)
+  {
+    GST_ats_add_address (blctx->address, blctx->session,
+        blctx->ats, blctx->ats_count);
+  }
+  else
+  {
+    kill_session (blctx->address->transport_name, blctx->session);
+  }
+
+  GNUNET_CONTAINER_DLL_remove (bc_head, bc_tail, blctx);
+  GNUNET_HELLO_address_free (blctx->address);
+  GNUNET_free_non_null (blctx->ats);
+  GNUNET_free (blctx);
+}
+
+
+/**
  * Plugin tells transport service about a new inbound session
  *
  * @param cls unused
@@ -624,6 +762,10 @@ plugin_env_session_start (void *cls, struct GNUNET_HELLO_Address *address,
     struct Session *session, const struct GNUNET_ATS_Information *ats,
     uint32_t ats_count)
 {
+  struct BlacklistCheckContext *blctx;
+  struct GST_BlacklistCheck *blc;
+  int c;
+
   if (NULL == address)
   {
     GNUNET_break(0);
@@ -640,7 +782,27 @@ plugin_env_session_start (void *cls, struct GNUNET_HELLO_Address *address,
       GNUNET_HELLO_address_check_option (address,
           GNUNET_HELLO_ADDRESS_INFO_INBOUND) ? "inbound" : "outbound",
       session, GNUNET_i2s (&address->peer), GST_plugins_a2s (address));
-  GST_ats_add_address (address, session, ats, ats_count);
+
+  /* Do blacklist check if communication with this peer is allowed */
+  blctx = GNUNET_new (struct BlacklistCheckContext);
+  blctx->address = GNUNET_HELLO_address_copy (address);
+  blctx->session = session;
+  if (ats_count > 0)
+  {
+    blctx->ats = GNUNET_malloc (ats_count * sizeof (struct GNUNET_ATS_Information));
+    for (c = 0; c < ats_count; c++)
+    {
+      blctx->ats[c].type = ats[c].type;
+      blctx->ats[c].value = ats[c].value;
+    }
+  }
+
+  GNUNET_CONTAINER_DLL_insert (bc_head, bc_tail, blctx);
+  if (NULL != (blc = GST_blacklist_test_allowed (&address->peer, address->transport_name,
+        &plugin_env_session_start_bl_check_cont, blctx)))
+  {
+    blctx->blc = blc;
+  }
 }
 
 /**
diff --git a/src/transport/gnunet-service-transport_neighbours.c b/src/transport/gnunet-service-transport_neighbours.c
index 5ce09223c..258967261 100644
--- a/src/transport/gnunet-service-transport_neighbours.c
+++ b/src/transport/gnunet-service-transport_neighbours.c
@@ -1673,7 +1673,6 @@ send_session_connect_cont (void *cls,
   set_state_and_timeout (n, GNUNET_TRANSPORT_PS_INIT_ATS,
       GNUNET_TIME_relative_to_absolute (ATS_RESPONSE_TIMEOUT));
   return;
-
 }
 
 /**
@@ -1750,6 +1749,55 @@ send_session_connect (struct NeighbourAddress *na)
 }
 
 
+static void
+send_session_connect_ack_cont (void *cls,
+                      const struct GNUNET_PeerIdentity *target,
+                      int result,
+                      size_t size_payload,
+                      size_t size_on_wire)
+{
+  struct NeighbourMapEntry *n;
+
+  n = lookup_neighbour (target);
+  if (NULL == n)
+  {
+    /* CONNECT_ACK continuation was called after neighbor was freed,
+     * for example due to a time out for the state or the session
+     * used was already terminated: nothing to do here... */
+    return;
+  }
+
+  if (GNUNET_TRANSPORT_PS_CONNECT_RECV_ACK != n->state)
+  {
+    /* CONNECT_ACK continuation was called after neighbor changed state,
+     * for example due to a time out for the state or the session
+     * used was already terminated: nothing to do here... */
+    return;
+  }
+  if (GNUNET_OK == result)
+    return;
+
+  GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+            _("Failed to send CONNECT_ACK message to peer `%s' using address `%s' session %p\n"),
+            GNUNET_i2s (target),
+            GST_plugins_a2s (n->primary_address.address),
+            n->primary_address.session);
+
+  /* Failed to send CONNECT_ACK message with this address */
+  GNUNET_ATS_address_destroyed (GST_ats, n->primary_address.address,
+      n->primary_address.session);
+  GNUNET_ATS_address_destroyed (GST_ats, n->primary_address.address,
+      NULL);
+
+  /* Remove address and request and additional one */
+  unset_primary_address (n);
+
+  set_state_and_timeout (n, GNUNET_TRANSPORT_PS_CONNECT_RECV_ATS,
+      GNUNET_TIME_relative_to_absolute (ATS_RESPONSE_TIMEOUT));
+  return;
+}
+
+
 /**
  * Send a CONNECT_ACK message via the given address.
  *
@@ -1795,7 +1843,7 @@ send_connect_ack_message (const struct GNUNET_HELLO_Address *address,
                      (const char *) &connect_msg, sizeof (struct SessionConnectMessage),
                      UINT_MAX,
                      GNUNET_TIME_UNIT_FOREVER_REL,
-                     NULL, NULL))
+                     send_session_connect_ack_cont, NULL))
   {
     GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
                 _("Failed to transmit CONNECT_ACK message via plugin to %s\n"),
@@ -1816,13 +1864,11 @@ send_connect_ack_message (const struct GNUNET_HELLO_Address *address,
       GNUNET_ATS_address_destroyed (GST_ats, address, NULL);
     }
     else
-    {
       GNUNET_ATS_address_destroyed (GST_ats, address, session);
-    }
 
     /* Remove address and request and additional one */
     unset_primary_address (n);
-    set_state_and_timeout (n, GNUNET_TRANSPORT_PS_INIT_ATS,
+    set_state_and_timeout (n, GNUNET_TRANSPORT_PS_CONNECT_RECV_ATS,
         GNUNET_TIME_relative_to_absolute (ATS_RESPONSE_TIMEOUT));
     return;
   }
@@ -2182,6 +2228,7 @@ GST_neighbours_try_connect (const struct GNUNET_PeerIdentity *target)
     case GNUNET_TRANSPORT_PS_DISCONNECT_FINISHED:
       /* should not be possible */
       GNUNET_assert (0);
+      return;
     default:
       GNUNET_log (GNUNET_ERROR_TYPE_ERROR,
                   "Unhandled state `%s'\n",
@@ -2227,9 +2274,25 @@ handle_connect_blacklist_check_cont (void *cls,
               "Connection to new address of peer `%s' based on blacklist is `%s'\n",
               GNUNET_i2s (peer),
               (GNUNET_OK == result) ? "allowed" : "FORBIDDEN");
+
+  if (NULL == (n = lookup_neighbour (peer)))
+  {
+    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+                "No neighbor entry for peer `%s', ignoring blacklist result\n",
+                GNUNET_i2s (peer));
+    goto cleanup; /* nobody left to care about new address */
+  }
+
+  GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+              "Blacklist check after CONNECT for peer `%s' in state %s/%s: %s\n",
+              GNUNET_i2s (peer),
+              GNUNET_TRANSPORT_ps2s (n->state),
+              print_ack_state (n->ack_state),
+              (GNUNET_OK == result) ? "OK" : "FAIL");
+
   if (GNUNET_OK == result)
   {
-    /* Blacklist agreed on connecting to a peer with this address */
+    /* Blacklist agreed on connecting to a peer with this address, notify ATS */
     GNUNET_log (GNUNET_ERROR_TYPE_INFO,
         "Notifying ATS peer's `%s' %s address `%s' session %p\n",
         GNUNET_i2s (peer),
@@ -2239,28 +2302,15 @@ handle_connect_blacklist_check_cont (void *cls,
     GST_ats_add_address (bcc->na.address, bcc->na.session, NULL, 0);
   }
 
-  if (NULL == (n = lookup_neighbour (peer)))
-  {
-    GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-                "No neighbor entry for peer `%s', ignoring blacklist result\n",
-                GNUNET_i2s (peer));
-    goto cleanup; /* nobody left to care about new address */
-  }
-  GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "Received connect blacklist check result for peer `%s' in state %s/%s\n",
-              GNUNET_i2s (peer),
-              GNUNET_TRANSPORT_ps2s (n->state),
-              print_ack_state (n->ack_state));
   switch (n->state)
   {
   case GNUNET_TRANSPORT_PS_NOT_CONNECTED:
-    /* this should not be possible */
+    /* This should not be possible */
     GNUNET_break (0);
     free_neighbour (n, GNUNET_NO);
     break;
   case GNUNET_TRANSPORT_PS_INIT_ATS:
-    /* waiting on ATS suggestion; still, pass address to ATS as a
-       possibility */
+    /* Waiting on ATS suggestion */
     break;
   case GNUNET_TRANSPORT_PS_CONNECT_SENT:
 #if 0
@@ -2284,19 +2334,25 @@ handle_connect_blacklist_check_cont (void *cls,
        * with this peer, request an address from ATS*/
       set_state_and_timeout (n, GNUNET_TRANSPORT_PS_CONNECT_RECV_ATS,
           GNUNET_TIME_relative_to_absolute (ATS_RESPONSE_TIMEOUT));
-      GNUNET_ATS_reset_backoff (GST_ats, peer);
+
       GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
                   "Requesting address for peer %s to ATS\n",
                   GNUNET_i2s (peer));
       if (NULL == n->suggest_handle)
         n->suggest_handle = GNUNET_ATS_suggest_address (GST_ats, peer,
             &address_suggest_cont, n);
-      else
-        GNUNET_ATS_reset_backoff (GST_ats, peer);
+      GNUNET_ATS_reset_backoff (GST_ats, peer);
     }
     else
     {
-      /* FIXME: state handling required! */
+      /* We received a CONNECT message from a peer, but blacklist denies to
+       * communicate with this peer and this address
+       * - Previous state: NOT_CONNECTED:
+       * We can free the neighbour, since the CONNECT created it
+       * - Previous state INIT_ATS:
+       *
+       * */
+      free_neighbour (n, GNUNET_NO);
     }
     break;
   case GNUNET_TRANSPORT_PS_CONNECT_RECV_ATS:
@@ -2343,6 +2399,9 @@ handle_connect_blacklist_check_cont (void *cls,
     if ( (GNUNET_OK == result) &&
          (ACK_SEND_CONNECT_ACK == n->ack_state) )
     {
+      /* TODO: Why should this happen? */
+      /* *Debug message: */ GNUNET_break (0);
+
       n->ack_state = ACK_SEND_SESSION_ACK;
       send_connect_ack_message (n->primary_address.address,
                                         n->primary_address.session,
@@ -2536,10 +2595,11 @@ GST_neighbours_handle_connect (const struct GNUNET_MessageHeader *message,
   n->connect_ack_timestamp = ts;
 
   GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
-              "Received SESSION_CONNECT for peer `%s' in state %s/%s\n",
+              "Received CONNECT for peer `%s' in state %s/%s\n",
               GNUNET_i2s (peer),
               GNUNET_TRANSPORT_ps2s (n->state),
               print_ack_state (n->ack_state));
+
   switch (n->state)
   {
   case GNUNET_TRANSPORT_PS_NOT_CONNECTED:
@@ -2552,7 +2612,8 @@ GST_neighbours_handle_connect (const struct GNUNET_MessageHeader *message,
     /* CONNECT message takes priority over us asking ATS for address */
     set_state_and_timeout (n, GNUNET_TRANSPORT_PS_CONNECT_RECV_BLACKLIST_INBOUND,
         GNUNET_TIME_relative_to_absolute (BLACKLIST_RESPONSE_TIMEOUT));
-    /* fallthrough */
+    connect_check_blacklist (peer, ts, address, session);
+    break;
   case GNUNET_TRANSPORT_PS_CONNECT_SENT:
   case GNUNET_TRANSPORT_PS_CONNECT_RECV_BLACKLIST_INBOUND:
   case GNUNET_TRANSPORT_PS_CONNECT_RECV_ATS:
@@ -2732,24 +2793,24 @@ switch_address_bl_check_cont (void *cls,
         GNUNET_TIME_relative_to_absolute (SETUP_CONNECTION_TIMEOUT));
     send_session_connect (&n->primary_address);
     break;
+  case GNUNET_TRANSPORT_PS_CONNECT_RECV_BLACKLIST_INBOUND:
+    /* We received an suggestion while waiting for a CONNECT blacklist check,
+     * this suggestion was permitted by a blacklist check, so send ACK*/
   case GNUNET_TRANSPORT_PS_CONNECT_RECV_ATS:
+    /* We requested an address and ATS suggests one:
+     * set primary address and send CONNECT_ACK message*/
     set_primary_address (n, blc_ctx->address, blc_ctx->session,
         blc_ctx->bandwidth_in, blc_ctx->bandwidth_out, GNUNET_NO);
     /* Send an ACK message as a response to the CONNECT msg */
     set_state_and_timeout (n, GNUNET_TRANSPORT_PS_CONNECT_RECV_ACK,
         GNUNET_TIME_relative_to_absolute (SETUP_CONNECTION_TIMEOUT));
     send_connect_ack_message (n->primary_address.address,
-                                      n->primary_address.session,
-                                      n->connect_ack_timestamp);
+                              n->primary_address.session,
+                              n->connect_ack_timestamp);
     if (ACK_SEND_CONNECT_ACK == n->ack_state)
       n->ack_state = ACK_SEND_SESSION_ACK;
 
     break;
-  case GNUNET_TRANSPORT_PS_CONNECT_RECV_BLACKLIST_INBOUND:
-    set_timeout (n, GNUNET_TIME_relative_to_absolute (BLACKLIST_RESPONSE_TIMEOUT));
-    /* REMOVE */ connect_check_blacklist (&n->id, n->connect_ack_timestamp,
-        blc_ctx->address, blc_ctx->session);
-    break;
   case GNUNET_TRANSPORT_PS_CONNECT_RECV_BLACKLIST:
   case GNUNET_TRANSPORT_PS_CONNECT_RECV_ACK:
     /* ATS asks us to switch while we were trying to connect; switch to new
@@ -3161,7 +3222,7 @@ master_task (void *cls,
   case GNUNET_TRANSPORT_PS_CONNECT_RECV_BLACKLIST_INBOUND:
     if (0 == delay.rel_value_us)
     {
-      GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
+      GNUNET_log (GNUNET_ERROR_TYPE_INFO,
                   "Connection to `%s' timed out waiting BLACKLIST to approve address to use for received CONNECT\n",
                   GNUNET_i2s (&n->id));
       free_neighbour (n, GNUNET_NO);