From d3b0b69e7d360b4b82c5a5580a7ea21317dfee24 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Fri, 27 Jan 2017 14:07:49 +0100 Subject: [PATCH] implement tie-breaking in case both peers establish a connection to each other via the same path at the same time, so that only one connection survives --- .../gnunet-service-cadet-new_connection.c | 119 +++++++++++++++--- .../gnunet-service-cadet-new_connection.h | 21 +++- src/cadet/gnunet-service-cadet-new_core.c | 32 ++++- src/cadet/gnunet-service-cadet-new_paths.c | 5 - src/cadet/gnunet-service-cadet-new_tunnels.c | 42 ++++++- src/cadet/gnunet-service-cadet-new_tunnels.h | 14 ++- 6 files changed, 197 insertions(+), 36 deletions(-) diff --git a/src/cadet/gnunet-service-cadet-new_connection.c b/src/cadet/gnunet-service-cadet-new_connection.c index 5f1ff61e1..a098674f5 100644 --- a/src/cadet/gnunet-service-cadet-new_connection.c +++ b/src/cadet/gnunet-service-cadet-new_connection.c @@ -158,31 +158,21 @@ struct CadetConnection /** - * Destroy a connection. + * Destroy a connection, part of the internal implementation. Called + * only from #GCC_destroy_from_core() or #GCC_destroy_from_tunnel(). * * @param cc connection to destroy */ -void +static void GCC_destroy (struct CadetConnection *cc) { - struct GNUNET_MQ_Envelope *env = NULL; - LOG (GNUNET_ERROR_TYPE_DEBUG, "Destroying %s\n", GCC_2s (cc)); - if (CADET_CONNECTION_SENDING_CREATE != cc->state) - { - struct GNUNET_CADET_ConnectionDestroyMessage *destroy_msg; - - /* Need to notify next hop that we are down. */ - env = GNUNET_MQ_msg (destroy_msg, - GNUNET_MESSAGE_TYPE_CADET_CONNECTION_DESTROY); - destroy_msg->cid = cc->cid; - } if (NULL != cc->mq_man) { GCP_request_mq_cancel (cc->mq_man, - env); + NULL); cc->mq_man = NULL; } if (NULL != cc->task) @@ -206,6 +196,56 @@ GCC_destroy (struct CadetConnection *cc) } + +/** + * Destroy a connection, called when the CORE layer is already done + * (i.e. has received a BROKEN message), but if we still have to + * communicate the destruction of the connection to the tunnel (if one + * exists). + * + * @param cc connection to destroy + */ +void +GCC_destroy_without_core (struct CadetConnection *cc) +{ + if (NULL != cc->ct) + { + GCT_connection_lost (cc->ct); + cc->ct = NULL; + } + GCC_destroy (cc); +} + + +/** + * Destroy a connection, called if the tunnel association with the + * connection was already broken, but we still need to notify the CORE + * layer about the breakage. + * + * @param cc connection to destroy + */ +void +GCC_destroy_without_tunnel (struct CadetConnection *cc) +{ + cc->ct = NULL; + if ( (CADET_CONNECTION_SENDING_CREATE != cc->state) && + (NULL != cc->mq_man) ) + { + struct GNUNET_MQ_Envelope *env; + struct GNUNET_CADET_ConnectionDestroyMessage *destroy_msg; + + /* Need to notify next hop that we are down. */ + env = GNUNET_MQ_msg (destroy_msg, + GNUNET_MESSAGE_TYPE_CADET_CONNECTION_DESTROY); + destroy_msg->cid = cc->cid; + GCP_request_mq_cancel (cc->mq_man, + env); + cc->mq_man = NULL; + } + GCC_destroy (cc); +} + + /** * Return the tunnel associated with this connection. * @@ -630,7 +670,8 @@ connection_create (struct CadetPeer *destination, * @param ct which tunnel uses this connection * @param ready_cb function to call when ready to transmit * @param ready_cb_cls closure for @a cb - * @return handle to the connection + * @return handle to the connection, NULL if we already have + * a connection that takes precedence on @a path */ struct CadetConnection * GCC_create_inbound (struct CadetPeer *destination, @@ -640,6 +681,54 @@ GCC_create_inbound (struct CadetPeer *destination, GCC_ReadyCallback ready_cb, void *ready_cb_cls) { + struct CadetConnection *cc; + unsigned int off; + + off = GCPP_find_peer (path, + destination); + GNUNET_assert (UINT_MAX != off); + cc = GCPP_get_connection (path, + destination, + off); + if (NULL != cc) + { + int cmp; + + cmp = memcmp (cid, + &cc->cid, + sizeof (*cid)); + if (0 == cmp) + { + /* Two peers picked the SAME random connection identifier at the + same time for the same path? Must be malicious. Drop + connection (existing and inbound), even if it is the only + one. */ + GNUNET_break_op (0); + GCT_connection_lost (cc->ct); + GCC_destroy_without_tunnel (cc); + return NULL; + } + if (0 < cmp) + { + /* drop existing */ + LOG (GNUNET_ERROR_TYPE_DEBUG, + "Got two connections on %s, dropping my existing %s\n", + GCPP_2s (path), + GCC_2s (cc)); + GCT_connection_lost (cc->ct); + GCC_destroy_without_tunnel (cc); + } + else + { + /* keep existing */ + LOG (GNUNET_ERROR_TYPE_DEBUG, + "Got two connections on %s, keeping my existing %s\n", + GCPP_2s (path), + GCC_2s (cc)); + return NULL; + } + } + return connection_create (destination, path, ct, diff --git a/src/cadet/gnunet-service-cadet-new_connection.h b/src/cadet/gnunet-service-cadet-new_connection.h index efdf9929a..692d03e4c 100644 --- a/src/cadet/gnunet-service-cadet-new_connection.h +++ b/src/cadet/gnunet-service-cadet-new_connection.h @@ -47,12 +47,26 @@ typedef void /** - * Destroy a connection. + * Destroy a connection, called when the CORE layer is already done + * (i.e. has received a BROKEN message), but if we still have to + * communicate the destruction of the connection to the tunnel (if one + * exists). * * @param cc connection to destroy */ void -GCC_destroy (struct CadetConnection *cc); +GCC_destroy_without_core (struct CadetConnection *cc); + + +/** + * Destroy a connection, called if the tunnel association with the + * connection was already broken, but we still need to notify the CORE + * layer about the breakage. + * + * @param cc connection to destroy + */ +void +GCC_destroy_without_tunnel (struct CadetConnection *cc); /** @@ -84,7 +98,8 @@ GCC_create (struct CadetPeer *destination, * @param ct which tunnel uses this connection * @param ready_cb function to call when ready to transmit * @param ready_cb_cls closure for @a cb - * @return handle to the connection + * @return handle to the connection, NULL if we already have + * a connection that takes precedence on @a path */ struct CadetConnection * GCC_create_inbound (struct CadetPeer *destination, diff --git a/src/cadet/gnunet-service-cadet-new_core.c b/src/cadet/gnunet-service-cadet-new_core.c index 4a4ead05b..086337c9a 100644 --- a/src/cadet/gnunet-service-cadet-new_core.c +++ b/src/cadet/gnunet-service-cadet-new_core.c @@ -479,10 +479,30 @@ handle_connection_create (void *cls, GNUNET_sh2s (&msg->cid.connection_of_tunnel)); path = GCPP_get_path_from_route (path_length - 1, pids); - GCT_add_inbound_connection (GCP_get_tunnel (origin, - GNUNET_YES), - &msg->cid, - path); + if (GNUNET_OK != + GCT_add_inbound_connection (GCP_get_tunnel (origin, + GNUNET_YES), + &msg->cid, + path)) + { + /* Send back BROKEN: duplicate connection on the same path, + we will use the other one. */ + struct GNUNET_MQ_Envelope *env; + struct GNUNET_CADET_ConnectionBrokenMessage *bm; + + LOG (GNUNET_ERROR_TYPE_DEBUG, + "Received CADET_CONNECTION_CREATE from %s for %s, but %s already has a connection. Sending BROKEN\n", + GCP_2s (sender), + GNUNET_sh2s (&msg->cid.connection_of_tunnel), + GCPP_2s (path)); + env = GNUNET_MQ_msg (bm, + GNUNET_MESSAGE_TYPE_CADET_CONNECTION_BROKEN); + bm->cid = msg->cid; + bm->peer1 = my_full_id; + GCP_send_ooo (sender, + env); + return; + } return; } /* We are merely a hop on the way, check if we can support the route */ @@ -611,7 +631,7 @@ handle_connection_broken (void *cls, LOG (GNUNET_ERROR_TYPE_DEBUG, "Received CONNECTION_BROKEN for connection %s. Destroying it.\n", GNUNET_sh2s (&msg->cid.connection_of_tunnel)); - GCC_destroy (cc); + GCC_destroy_without_core (cc); /* FIXME: also destroy the path up to the specified link! */ return; @@ -661,7 +681,7 @@ handle_connection_destroy (void *cls, "Received CONNECTION_DESTROY for connection %s. Destroying connection.\n", GNUNET_sh2s (&msg->cid.connection_of_tunnel)); - GCC_destroy (cc); + GCC_destroy_without_core (cc); return; } diff --git a/src/cadet/gnunet-service-cadet-new_paths.c b/src/cadet/gnunet-service-cadet-new_paths.c index a5d201e0b..685656ec3 100644 --- a/src/cadet/gnunet-service-cadet-new_paths.c +++ b/src/cadet/gnunet-service-cadet-new_paths.c @@ -24,11 +24,6 @@ * @author Christian Grothoff * * TODO: - * - BUG: currently only allowing one unique connection per path, - * but need to allow 2 in case WE are establishing one from A to B - * while at the same time B establishes one to A. - * Also, must not ASSERT if B establishes a 2nd one to us. - * Need to have some reasonable tie-breaking to only keep ONE. * - path desirability score calculations are not done */ #include "platform.h" diff --git a/src/cadet/gnunet-service-cadet-new_tunnels.c b/src/cadet/gnunet-service-cadet-new_tunnels.c index e677a7436..bd46dc151 100644 --- a/src/cadet/gnunet-service-cadet-new_tunnels.c +++ b/src/cadet/gnunet-service-cadet-new_tunnels.c @@ -1522,6 +1522,24 @@ GCT_add_channel (struct CadetTunnel *t, } +/** + * We lost a connection, remove it from our list and clean up + * the connection object itself. + * + * @param ct binding of connection to tunnel of the connection that was lost. + */ +void +GCT_connection_lost (struct CadetTConnection *ct) +{ + struct CadetTunnel *t = ct->t; + + GNUNET_CONTAINER_DLL_remove (t->connection_head, + t->connection_tail, + ct); + GNUNET_free (ct); +} + + /** * This tunnel is no longer used, destroy it. * @@ -1541,12 +1559,12 @@ destroy_tunnel (void *cls) GNUNET_assert (0 == GNUNET_CONTAINER_multihashmap32_size (t->channels)); while (NULL != (ct = t->connection_head)) { + struct CadetConnection *cc; + GNUNET_assert (ct->t == t); - GNUNET_CONTAINER_DLL_remove (t->connection_head, - t->connection_tail, - ct); - GCC_destroy (ct->cc); - GNUNET_free (ct); + cc = ct->cc; + GCT_connection_lost (ct); + GCC_destroy_without_tunnel (cc); } while (NULL != (tq = t->tq_head)) { @@ -2291,8 +2309,10 @@ GCT_create_tunnel (struct CadetPeer *destination) * @param t a tunnel * @param cid connection identifer to use for the connection * @param path path to use for the connection + * @return #GNUNET_OK on success, + * #GNUNET_SYSERR on failure (duplicate connection) */ -void +int GCT_add_inbound_connection (struct CadetTunnel *t, const struct GNUNET_CADET_ConnectionTunnelIdentifier *cid, struct CadetPeerPath *path) @@ -2308,6 +2328,15 @@ GCT_add_inbound_connection (struct CadetTunnel *t, cid, &connection_ready_cb, ct); + if (NULL == ct->cc) + { + LOG (GNUNET_ERROR_TYPE_DEBUG, + "Tunnel %s refused inbound connection %s (duplicate)\n", + GCT_2s (t), + GCC_2s (ct->cc)); + GNUNET_free (ct); + return GNUNET_SYSERR; + } /* FIXME: schedule job to kill connection (and path?) if it takes too long to get ready! (And track performance data on how long other connections took with the tunnel!) @@ -2320,6 +2349,7 @@ GCT_add_inbound_connection (struct CadetTunnel *t, "Tunnel %s has new connection %s\n", GCT_2s (t), GCC_2s (ct->cc)); + return GNUNET_OK; } diff --git a/src/cadet/gnunet-service-cadet-new_tunnels.h b/src/cadet/gnunet-service-cadet-new_tunnels.h index c867a9e82..463780686 100644 --- a/src/cadet/gnunet-service-cadet-new_tunnels.h +++ b/src/cadet/gnunet-service-cadet-new_tunnels.h @@ -113,13 +113,25 @@ GCT_destroy_tunnel_now (struct CadetTunnel *t); * @param t a tunnel * @param cid connection identifer to use for the connection * @param path path to use for the connection + * @return #GNUNET_OK on success, + * #GNUNET_SYSERR on failure (duplicate connection) */ -void +int GCT_add_inbound_connection (struct CadetTunnel *t, const struct GNUNET_CADET_ConnectionTunnelIdentifier *cid, struct CadetPeerPath *path); +/** + * We lost a connection, remove it from our list and clean up + * the connection object itself. + * + * @param ct binding of connection to tunnel of the connection that was lost. + */ +void +GCT_connection_lost (struct CadetTConnection *ct); + + /** * Return the peer to which this tunnel goes. * -- 2.25.1