correctly handle assignment of cid during channel open, send channel open ack, and detect duplicate open

3 files changed, 148 insertions, 52 deletions

diff --git a/src/cadet/gnunet-service-cadet-new_channel.c b/src/cadet/gnunet-service-cadet-new_channel.c
index 425422cb6..08704152b 100644
--- a/src/cadet/gnunet-service-cadet-new_channel.c
+++ b/src/cadet/gnunet-service-cadet-new_channel.c
@@ -212,7 +212,12 @@ struct CadetChannel
   /**
    * Task to resend/poll in case no ACK is received.
    */
-  struct GNUNET_SCHEDULER_Task *retry_task;
+  struct GNUNET_SCHEDULER_Task *retry_control_task;
+
+    /**
+   * Task to resend/poll in case no ACK is received.
+   */
+  struct GNUNET_SCHEDULER_Task *retry_data_task;
 
   /**
    * Last time the channel was used
@@ -385,10 +390,15 @@ channel_destroy (struct CadetChannel *ch)
     GCT_send_cancel (ch->last_control_qe);
     ch->last_control_qe = NULL;
   }
-  if (NULL != ch->retry_task)
+  if (NULL != ch->retry_data_task)
+  {
+    GNUNET_SCHEDULER_cancel (ch->retry_data_task);
+    ch->retry_data_task = NULL;
+  }
+  if (NULL != ch->retry_control_task)
   {
-    GNUNET_SCHEDULER_cancel (ch->retry_task);
-    ch->retry_task = NULL;
+    GNUNET_SCHEDULER_cancel (ch->retry_control_task);
+    ch->retry_control_task = NULL;
   }
   GCT_remove_channel (ch->t,
                       ch,
@@ -420,9 +430,10 @@ channel_open_sent_cb (void *cls)
   GNUNET_assert (NULL != ch->last_control_qe);
   ch->last_control_qe = NULL;
   ch->retry_time = GNUNET_TIME_STD_BACKOFF (ch->retry_time);
-  ch->retry_task = GNUNET_SCHEDULER_add_delayed (ch->retry_time,
-                                                 &send_channel_open,
-                                                 ch);
+  ch->retry_control_task
+    = GNUNET_SCHEDULER_add_delayed (ch->retry_time,
+                                    &send_channel_open,
+                                    ch);
 }
 
 
@@ -438,7 +449,7 @@ send_channel_open (void *cls)
   struct GNUNET_CADET_ChannelOpenMessage msgcc;
   uint32_t options;
 
-  ch->retry_task = NULL;
+  ch->retry_control_task = NULL;
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "Sending CHANNEL_OPEN message for %s\n",
        GCCH_2s (ch));
@@ -475,9 +486,10 @@ send_channel_open (void *cls)
 void
 GCCH_tunnel_up (struct CadetChannel *ch)
 {
-  GNUNET_assert (NULL == ch->retry_task);
-  ch->retry_task = GNUNET_SCHEDULER_add_now (&send_channel_open,
-                                             ch);
+  GNUNET_assert (NULL == ch->retry_control_task);
+  ch->retry_control_task
+    = GNUNET_SCHEDULER_add_now (&send_channel_open,
+                                ch);
 }
 
 
@@ -518,7 +530,7 @@ GCCH_channel_local_new (struct CadetClient *owner,
                             1,
                             GNUNET_NO);
   LOG (GNUNET_ERROR_TYPE_DEBUG,
-       "Created channel to port %s at peer %s for client %s using tunnel %s\n",
+       "Created channel to port %s at peer %s for %s using %s\n",
        GNUNET_h2s (port),
        GCP_2s (destination),
        GSC_2s (owner),
@@ -538,7 +550,7 @@ timeout_closed_cb (void *cls)
 {
   struct CadetChannel *ch = cls;
 
-  ch->retry_task = NULL;
+  ch->retry_control_task = NULL;
   LOG (GNUNET_ERROR_TYPE_DEBUG,
        "Closing incoming channel to port %s from peer %s due to timeout\n",
        GNUNET_h2s (&ch->port),
@@ -588,9 +600,10 @@ GCCH_channel_incoming_new (struct CadetTunnel *t,
                                               port,
                                               ch,
                                               GNUNET_CONTAINER_MULTIHASHMAPOPTION_MULTIPLE);
-    ch->retry_task = GNUNET_SCHEDULER_add_delayed (TIMEOUT_CLOSED_PORT,
-                                                   &timeout_closed_cb,
-                                                   ch);
+    ch->retry_control_task
+      = GNUNET_SCHEDULER_add_delayed (TIMEOUT_CLOSED_PORT,
+                                      &timeout_closed_cb,
+                                      ch);
     LOG (GNUNET_ERROR_TYPE_DEBUG,
          "Created loose incoming channel to port %s from peer %s\n",
          GNUNET_h2s (&ch->port),
@@ -657,12 +670,55 @@ send_channel_data_ack (struct CadetChannel *ch)
  * @param cls the `struct CadetChannel`
  */
 static void
-send_connect_ack (void *cls)
+send_open_ack (void *cls)
 {
   struct CadetChannel *ch = cls;
+  struct GNUNET_CADET_ChannelManageMessage msg;
+
+  LOG (GNUNET_ERROR_TYPE_DEBUG,
+       "Sending CHANNEL_OPEN_ACK on channel %s\n",
+       GCCH_2s (ch));
+  ch->retry_control_task = NULL;
+  msg.header.type = htons (GNUNET_MESSAGE_TYPE_CADET_CHANNEL_OPEN_ACK);
+  msg.header.size = htons (sizeof (msg));
+  msg.reserved = htonl (0);
+  msg.ctn = ch->ctn;
+  if (NULL != ch->last_control_qe)
+    GCT_send_cancel (ch->last_control_qe);
+  ch->last_control_qe = GCT_send (ch->t,
+                                  &msg.header,
+                                  &send_ack_cb,
+                                  ch);
+}
 
-  ch->retry_task = NULL;
-  send_channel_data_ack (ch);
+
+/**
+ * We got a #GNUNET_MESSAGE_TYPE_CADET_CHANNEL_OPEN message again for
+ * this channel.  If the binding was successful, (re)transmit the
+ * #GNUNET_MESSAGE_TYPE_CADET_CHANNEL_OPEN_ACK.
+ *
+ * @param ch channel that got the duplicate open
+ */
+void
+GCCH_handle_duplicate_open (struct CadetChannel *ch)
+{
+  if (NULL == ch->dest)
+  {
+    LOG (GNUNET_ERROR_TYPE_DEBUG,
+         "Ignoring duplicate channel OPEN on %s: port is closed\n",
+         GCCH_2s (ch));
+    return;
+  }
+  if (NULL != ch->retry_control_task)
+  {
+    LOG (GNUNET_ERROR_TYPE_DEBUG,
+         "Ignoring duplicate channel OPEN on %s: control message is pending\n",
+         GCCH_2s (ch));
+    return;
+  }
+  ch->retry_control_task
+    = GNUNET_SCHEDULER_add_now (&send_open_ack,
+                                ch);
 }
 
 
@@ -704,16 +760,16 @@ GCCH_bind (struct CadetChannel *ch,
   uint32_t options;
 
   LOG (GNUNET_ERROR_TYPE_DEBUG,
-       "Binding %s from tunnel %s to port %s of client %s\n",
+       "Binding %s from %s to port %s of %s\n",
        GCCH_2s (ch),
        GCT_2s (ch->t),
        GNUNET_h2s (&ch->port),
        GSC_2s (c));
-  if (NULL != ch->retry_task)
+  if (NULL != ch->retry_control_task)
   {
     /* there might be a timeout task here */
-    GNUNET_SCHEDULER_cancel (ch->retry_task);
-    ch->retry_task = NULL;
+    GNUNET_SCHEDULER_cancel (ch->retry_control_task);
+    ch->retry_control_task = NULL;
   }
   options = 0;
   if (ch->nobuffer)
@@ -731,8 +787,9 @@ GCCH_bind (struct CadetChannel *ch,
   ch->mid_recv.mid = htonl (1); /* The CONNECT counts as message 0! */
 
   /* notify other peer that we accepted the connection */
-  ch->retry_task = GNUNET_SCHEDULER_add_now (&send_connect_ack,
-                                             ch);
+  ch->retry_control_task
+    = GNUNET_SCHEDULER_add_now (&send_open_ack,
+                                ch);
   /* give client it's initial supply of ACKs */
   env = GNUNET_MQ_msg (tcm,
                        GNUNET_MESSAGE_TYPE_CADET_LOCAL_CHANNEL_CREATE);
@@ -833,8 +890,8 @@ GCCH_handle_channel_open_ack (struct CadetChannel *ch)
     LOG (GNUNET_ERROR_TYPE_DEBUG,
          "Received channel OPEN_ACK for waiting %s, entering READY state\n",
          GCCH_2s (ch));
-    GNUNET_SCHEDULER_cancel (ch->retry_task);
-    ch->retry_task = NULL;
+    GNUNET_SCHEDULER_cancel (ch->retry_control_task);
+    ch->retry_control_task = NULL;
     ch->state = CADET_CHANNEL_READY;
     /* On first connect, send client as many ACKs as we allow messages
        to be buffered! */
@@ -1062,7 +1119,7 @@ retry_transmission (void *cls)
   struct CadetChannel *ch = cls;
   struct CadetReliableMessage *crm = ch->head_sent;
 
-  ch->retry_task = NULL;
+  ch->retry_data_task = NULL;
   GNUNET_assert (NULL == crm->qe);
   crm->qe = GCT_send (ch->t,
                       &crm->data_message.header,
@@ -1161,11 +1218,12 @@ data_sent_cb (void *cls)
     GNUNET_CONTAINER_DLL_insert (ch->head_sent,
                                  ch->tail_sent,
                                  crm);
-    if (NULL != ch->retry_task)
-      GNUNET_SCHEDULER_cancel (ch->retry_task);
-    ch->retry_task = GNUNET_SCHEDULER_add_delayed (crm->retry_delay,
-                                                   &retry_transmission,
-                                                   ch);
+    if (NULL != ch->retry_data_task)
+      GNUNET_SCHEDULER_cancel (ch->retry_data_task);
+    ch->retry_data_task
+      = GNUNET_SCHEDULER_add_delayed (crm->retry_delay,
+                                      &retry_transmission,
+                                      ch);
     return;
   }
   for (off = ch->head_sent; NULL != off; off = off->next)
diff --git a/src/cadet/gnunet-service-cadet-new_channel.h b/src/cadet/gnunet-service-cadet-new_channel.h
index ae4c5da5a..d4a592518 100644
--- a/src/cadet/gnunet-service-cadet-new_channel.h
+++ b/src/cadet/gnunet-service-cadet-new_channel.h
@@ -160,6 +160,17 @@ GCCH_channel_incoming_destroy (struct CadetChannel *ch);
 
 
 /**
+ * We got a #GNUNET_MESSAGE_TYPE_CADET_CHANNEL_OPEN message again for
+ * this channel.  If the binding was successful, (re)transmit the
+ * #GNUNET_MESSAGE_TYPE_CADET_CHANNEL_OPEN_ACK.
+ *
+ * @param ch channel that got the duplicate open
+ */
+void
+GCCH_handle_duplicate_open (struct CadetChannel *ch);
+
+
+/**
  * We got payload data for a channel.  Pass it on to the client.
  *
  * @param ch channel that got data
diff --git a/src/cadet/gnunet-service-cadet-new_tunnels.c b/src/cadet/gnunet-service-cadet-new_tunnels.c
index 9754040cf..20ab8f4e9 100644
--- a/src/cadet/gnunet-service-cadet-new_tunnels.c
+++ b/src/cadet/gnunet-service-cadet-new_tunnels.c
@@ -1472,19 +1472,28 @@ GCT_handle_kx (struct CadetTConnection *ct,
 static struct GNUNET_CADET_ChannelTunnelNumber
 get_next_free_ctn (struct CadetTunnel *t)
 {
+#define HIGH_BIT 0x8000000
   struct GNUNET_CADET_ChannelTunnelNumber ret;
   uint32_t ctn;
-
-  /* FIXME: this logic does NOT prevent both ends of the
-     channel from picking the same CTN!
-     Need to reserve one bit of the CTN for the
-     direction, i.e. which side established the connection! */
+  int cmp;
+  uint32_t highbit;
+
+  cmp = GNUNET_CRYPTO_cmp_peer_identity (&my_full_id,
+                                         GCP_get_id (GCT_get_destination (t)));
+  if (0 < cmp)
+    highbit = HIGH_BIT;
+  else if (0 > cmp)
+    highbit = 0;
+  else
+    GNUNET_assert (0); // loopback must never go here!
   ctn = ntohl (t->next_ctn.cn);
   while (NULL !=
          GNUNET_CONTAINER_multihashmap32_get (t->channels,
                                               ctn))
-    ctn++;
-  t->next_ctn.cn = htonl (ctn + 1);
+  {
+    ctn = ((ctn + 1) & (~ HIGH_BIT)) | highbit;
+  }
+  t->next_ctn.cn = htonl (((ctn + 1) & (~ HIGH_BIT)) | highbit);
   ret.cn = ntohl (ctn);
   return ret;
 }
@@ -2029,28 +2038,38 @@ handle_plaintext_data_ack (void *cls,
  * another peer.  Creates the incoming channel.
  *
  * @param cls the `struct CadetTunnel` for which we decrypted the message
- * @param cc the message we received on the tunnel
+ * @param copen the message we received on the tunnel
  */
 static void
 handle_plaintext_channel_open (void *cls,
-                               const struct GNUNET_CADET_ChannelOpenMessage *cc)
+                               const struct GNUNET_CADET_ChannelOpenMessage *copen)
 {
   struct CadetTunnel *t = cls;
   struct CadetChannel *ch;
-  struct GNUNET_CADET_ChannelTunnelNumber ctn;
 
+  ch = GNUNET_CONTAINER_multihashmap32_get (t->channels,
+                                            ntohl (copen->ctn.cn));
+  if (NULL != ch)
+  {
+    LOG (GNUNET_ERROR_TYPE_DEBUG,
+         "Receicved duplicate channel OPEN on port %s from %s (%s), resending ACK\n",
+         GNUNET_h2s (&copen->port),
+         GCT_2s (t),
+         GCCH_2s (ch));
+    GCCH_handle_duplicate_open (ch);
+    return;
+  }
   LOG (GNUNET_ERROR_TYPE_DEBUG,
-       "Receicved channel OPEN on port %s from peer %s\n",
-       GNUNET_h2s (&cc->port),
-       GCP_2s (GCT_get_destination (t)));
-  ctn = get_next_free_ctn (t);
+       "Receicved channel OPEN on port %s from %s\n",
+       GNUNET_h2s (&copen->port),
+       GCT_2s (t));
   ch = GCCH_channel_incoming_new (t,
-                                  ctn,
-                                  &cc->port,
-                                  ntohl (cc->opt));
+                                  copen->ctn,
+                                  &copen->port,
+                                  ntohl (copen->opt));
   GNUNET_assert (GNUNET_OK ==
                  GNUNET_CONTAINER_multihashmap32_put (t->channels,
-                                                      ntohl (ctn.cn),
+                                                      ntohl (copen->ctn.cn),
                                                       ch,
                                                       GNUNET_CONTAINER_MULTIHASHMAPOPTION_UNIQUE_ONLY));
 }
@@ -2110,6 +2129,10 @@ handle_plaintext_channel_open_ack (void *cls,
                               cm->ctn);
     return;
   }
+  LOG (GNUNET_ERROR_TYPE_DEBUG,
+       "Receicved channel OPEN_ACK on channel %s from %s\n",
+       GCCH_2s (ch),
+       GCT_2s (t));
   GCCH_handle_channel_open_ack (ch);
 }
 
@@ -2139,6 +2162,10 @@ handle_plaintext_channel_destroy (void *cls,
          GCCH_2s (ch));
     return;
   }
+  LOG (GNUNET_ERROR_TYPE_DEBUG,
+       "Receicved channel DESTROY on %s from %s\n",
+       GCCH_2s (ch),
+       GCT_2s (t));
   GCCH_handle_remote_destroy (ch);
 }
 
@@ -2385,7 +2412,7 @@ GCT_send (struct CadetTunnel *t,
 
   payload_size = ntohs (message->size);
   LOG (GNUNET_ERROR_TYPE_DEBUG,
-       "Encrypting %u bytes of for tunnel %s\n",
+       "Encrypting %u bytes for tunnel %s\n",
        (unsigned int) payload_size,
        GCT_2s (t));
   env = GNUNET_MQ_msg_extra (ax_msg,