From 8bd00cee8c4335f9598ef6e97a06736c925862a7 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Wed, 18 Feb 2015 14:08:39 +0000 Subject: [PATCH] fix blacklist checking logic, integrating tracking of sessions with blacklist module and fixing dangling session issue which caused misc. problems when blacklists were in use --- src/transport/gnunet-service-transport.c | 248 +++++------------- src/transport/gnunet-service-transport_ats.c | 2 +- .../gnunet-service-transport_blacklist.c | 182 ++++++++++--- .../gnunet-service-transport_blacklist.h | 27 +- .../gnunet-service-transport_neighbours.c | 145 +++++----- .../gnunet-service-transport_validation.c | 15 +- .../test_transport_api_blacklisting.c | 4 +- src/transport/test_transport_api_data.conf | 2 - .../test_transport_api_tcp_peer1.conf | 2 + .../test_transport_api_tcp_peer2.conf | 2 + 10 files changed, 322 insertions(+), 307 deletions(-) diff --git a/src/transport/gnunet-service-transport.c b/src/transport/gnunet-service-transport.c index 46503a5bf..7dfd994b4 100644 --- a/src/transport/gnunet-service-transport.c +++ b/src/transport/gnunet-service-transport.c @@ -69,53 +69,10 @@ struct SessionKiller /** * The kill task. */ - struct GNUNET_SCHEDULER_Task * task; + struct GNUNET_SCHEDULER_Task *task; }; -/** - * We track active blacklist checks in a DLL so we can cancel them if - * necessary. We typically check against the blacklist a few times - * during connection setup, as the check is asynchronous and the - * blacklist may change its mind before the connection goes fully up. - * Similarly, the session may die during the asynchronous check, so - * we use this list to then cancel ongoing checks. - */ -struct BlacklistCheckContext -{ - /** - * We keep these in a DLL. - */ - struct BlacklistCheckContext *prev; - - /** - * We keep these in a DLL. - */ - struct BlacklistCheckContext *next; - - /** - * Handle with the blacklist subsystem. - */ - struct GST_BlacklistCheck *blc; - - /** - * The address we are checking. - */ - struct GNUNET_HELLO_Address *address; - - /** - * Session associated with the address (or NULL). - */ - struct Session *session; - - /** - * Message to process in the continuation if the - * blacklist check is ok, can be NULL. - */ - struct GNUNET_MessageHeader *msg; - -}; - /* globals */ /** @@ -178,20 +135,6 @@ static struct SessionKiller *sk_tail; */ struct GNUNET_ATS_InterfaceScanner *GST_is; -/** - * Head of DLL of blacklist checks we have pending for - * incoming sessions and/or SYN requests. We may - * want to move this into the blacklist-logic at some - * point. - */ -struct BlacklistCheckContext *bc_head; - -/** - * Tail of DLL of blacklist checks we have pending for - * incoming sessions and/or SYN requests. - */ -struct BlacklistCheckContext *bc_tail; - /** * Transmit our HELLO message to the given (connected) neighbour. @@ -322,44 +265,6 @@ kill_session_task (void *cls, } -/** - * Cancel all blacklist checks that are pending for the given address and session. - * NOTE: Consider moving the "bc_*" logic into blacklist.h? - * - * @param address address to remove from check - * @param sesssion session that must match to remove for check - */ -static void -cancel_pending_blacklist_checks (const struct GNUNET_HELLO_Address *address, - struct Session *session) -{ - struct BlacklistCheckContext *blctx; - struct BlacklistCheckContext *next; - - next = bc_head; - for (blctx = next; NULL != blctx; blctx = next) - { - next = blctx->next; - if ( (NULL != blctx->address) && - (0 == GNUNET_HELLO_address_cmp(blctx->address, address)) && - (blctx->session == session)) - { - GNUNET_CONTAINER_DLL_remove (bc_head, - bc_tail, - blctx); - if (NULL != blctx->blc) - { - GST_blacklist_test_cancel (blctx->blc); - blctx->blc = NULL; - } - GNUNET_HELLO_address_free (blctx->address); - GNUNET_free_non_null (blctx->msg); - GNUNET_free (blctx); - } - } -} - - /** * Force plugin to terminate session due to communication * issue. @@ -398,54 +303,49 @@ kill_session (const char *plugin_name, * 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 cls the message * @param peer the peer + * @param address the address + * @param session the session * @param result the result */ static void connect_bl_check_cont (void *cls, const struct GNUNET_PeerIdentity *peer, + const struct GNUNET_HELLO_Address *address, + struct Session *session, int result) { - struct BlacklistCheckContext *blctx = cls; + struct GNUNET_MessageHeader *msg = cls; - GNUNET_CONTAINER_DLL_remove (bc_head, - bc_tail, - blctx); - blctx->blc = NULL; if (GNUNET_OK == result) { /* Blacklist allows to speak to this peer, forward SYN to neighbours */ GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Received SYN message from peer `%s' at `%s'\n", GNUNET_i2s (peer), - GST_plugins_a2s (blctx->address)); + GST_plugins_a2s (address)); if (GNUNET_OK != - GST_neighbours_handle_session_syn (blctx->msg, - &blctx->address->peer)) + GST_neighbours_handle_session_syn (msg, + peer)) { - cancel_pending_blacklist_checks (blctx->address, - blctx->session); - kill_session (blctx->address->transport_name, - blctx->session); + GST_blacklist_abort_matching (address, + session); + kill_session (address->transport_name, + session); } + GNUNET_free (msg); + return; } - else - { - /* Blacklist denies to speak to this peer */ - GNUNET_log (GNUNET_ERROR_TYPE_INFO, - "Discarding SYN message from `%s' due to denied blacklist check\n", - GNUNET_i2s (peer)); - cancel_pending_blacklist_checks (blctx->address, - blctx->session); - kill_session (blctx->address->transport_name, - blctx->session); - } - - if (NULL != blctx->address) - GNUNET_HELLO_address_free (blctx->address); - GNUNET_free (blctx->msg); - GNUNET_free (blctx); + GNUNET_free (msg); + if (GNUNET_SYSERR == result) + return; /* check was aborted, session destroyed */ + /* Blacklist denies to speak to this peer */ + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Discarding SYN message from `%s' due to denied blacklist check\n", + GNUNET_i2s (peer)); + kill_session (address->transport_name, + session); } @@ -470,8 +370,6 @@ 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; @@ -498,8 +396,8 @@ GST_receive_callback (void *cls, if (GNUNET_OK != GST_validation_handle_hello (message)) { GNUNET_break_op (0); - cancel_pending_blacklist_checks (address, - session); + GST_blacklist_abort_matching (address, + session); } return ret; case GNUNET_MESSAGE_TYPE_TRANSPORT_PING: @@ -512,8 +410,8 @@ GST_receive_callback (void *cls, address, session)) { - cancel_pending_blacklist_checks (address, - session); + GST_blacklist_abort_matching (address, + session); kill_session (plugin_name, session); } @@ -524,31 +422,20 @@ GST_receive_callback (void *cls, GST_plugins_a2s (address)); if (GNUNET_OK != GST_validation_handle_pong (&address->peer, message)) { - GNUNET_break_op(0); - cancel_pending_blacklist_checks (address, session); + GNUNET_break_op (0); + GST_blacklist_abort_matching (address, + session); kill_session (plugin_name, session); } break; case GNUNET_MESSAGE_TYPE_TRANSPORT_SESSION_SYN: /* 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))) - { - blctx->blc = blc; - } + (void) GST_blacklist_test_allowed (&address->peer, + NULL, + &connect_bl_check_cont, + GNUNET_copy_message (message), + address, + session); break; case GNUNET_MESSAGE_TYPE_TRANSPORT_SESSION_SYN_ACK: if (GNUNET_OK != @@ -556,7 +443,7 @@ GST_receive_callback (void *cls, address, session)) { - cancel_pending_blacklist_checks (address, session); + GST_blacklist_abort_matching (address, session); kill_session (plugin_name, session); } break; @@ -567,7 +454,7 @@ GST_receive_callback (void *cls, session)) { GNUNET_break_op(0); - cancel_pending_blacklist_checks (address, session); + GST_blacklist_abort_matching (address, session); kill_session (plugin_name, session); } break; @@ -684,7 +571,7 @@ plugin_env_session_end (void *cls, GST_neighbours_session_terminated (&address->peer, session); GST_ats_del_session (address, session); - cancel_pending_blacklist_checks (address, session); + GST_blacklist_abort_matching (address, session); for (sk = sk_head; NULL != sk; sk = sk->next) { @@ -704,39 +591,34 @@ plugin_env_session_end (void *cls, * plugin gave us a new session in #plugin_env_session_start(). If * connection to the peer is disallowed, kill the session. * - * @param cls blc_ctx bl context + * @param cls NULL * @param peer the peer + * @param address address associated with the request + * @param session session associated with the request * @param result the result */ static void plugin_env_session_start_bl_check_cont (void *cls, const struct GNUNET_PeerIdentity *peer, + const struct GNUNET_HELLO_Address *address, + struct Session *session, int result) { - struct BlacklistCheckContext *blctx = cls; - - GNUNET_CONTAINER_DLL_remove (bc_head, - bc_tail, - blctx); - blctx->blc = NULL; if (GNUNET_OK != result) { - cancel_pending_blacklist_checks (blctx->address, - blctx->session); - kill_session (blctx->address->transport_name, - blctx->session); + kill_session (address->transport_name, + session); + return; } - else if (GNUNET_YES != - GNUNET_HELLO_address_check_option (blctx->address, - GNUNET_HELLO_ADDRESS_INFO_INBOUND)) + if (GNUNET_YES != + GNUNET_HELLO_address_check_option (address, + GNUNET_HELLO_ADDRESS_INFO_INBOUND)) { GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Informing verifier about inbound session's address `%s'\n", - GST_plugins_a2s (blctx->address)); - GST_validation_handle_address (blctx->address); + GST_plugins_a2s (address)); + GST_validation_handle_address (address); } - GNUNET_HELLO_address_free (blctx->address); - GNUNET_free (blctx); } @@ -754,8 +636,6 @@ plugin_env_session_start (void *cls, struct Session *session, enum GNUNET_ATS_Network_Type scope) { - struct BlacklistCheckContext *blctx; - struct GST_BlacklistCheck *blc; struct GNUNET_ATS_Properties prop; if (NULL == address) @@ -788,20 +668,12 @@ plugin_env_session_start (void *cls, &prop); } /* 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; - 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; - } + (void) GST_blacklist_test_allowed (&address->peer, + address->transport_name, + &plugin_env_session_start_bl_check_cont, + NULL, + address, + session); } diff --git a/src/transport/gnunet-service-transport_ats.c b/src/transport/gnunet-service-transport_ats.c index 5301090ff..3a321e786 100644 --- a/src/transport/gnunet-service-transport_ats.c +++ b/src/transport/gnunet-service-transport_ats.c @@ -303,7 +303,7 @@ GST_ats_block_address (const struct GNUNET_HELLO_Address *address, ai = find_ai (address, session); if (NULL == ai) { - GNUNET_break (0); + GNUNET_assert (0); return; } if (NULL == ai->ar) diff --git a/src/transport/gnunet-service-transport_blacklist.c b/src/transport/gnunet-service-transport_blacklist.c index 195e3439f..22d62570a 100644 --- a/src/transport/gnunet-service-transport_blacklist.c +++ b/src/transport/gnunet-service-transport_blacklist.c @@ -145,6 +145,16 @@ struct GST_BlacklistCheck */ void *cont_cls; + /** + * Address for #GST_blacklist_abort_matching(), can be NULL. + */ + struct GNUNET_HELLO_Address *address; + + /** + * Session for #GST_blacklist_abort_matching(), can be NULL. + */ + struct Session *session; + /** * Current transmission request handle for this client, or NULL if no * request is pending. @@ -159,7 +169,7 @@ struct GST_BlacklistCheck /** * Current task performing the check. */ - struct GNUNET_SCHEDULER_Task * task; + struct GNUNET_SCHEDULER_Task *task; }; @@ -198,7 +208,8 @@ static struct GNUNET_CONTAINER_MultiPeerMap *blacklist; * @param tc unused */ static void -do_blacklist_check (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc); +do_blacklist_check (void *cls, + const struct GNUNET_SCHEDULER_TaskContext *tc); /** @@ -209,7 +220,8 @@ do_blacklist_check (void *cls, const struct GNUNET_SCHEDULER_TaskContext *tc); * @param client identification of the client */ static void -client_disconnect_notification (void *cls, struct GNUNET_SERVER_Client *client) +client_disconnect_notification (void *cls, + struct GNUNET_SERVER_Client *client) { struct Blacklisters *bl; struct GST_BlacklistCheck *bc; @@ -220,17 +232,17 @@ client_disconnect_notification (void *cls, struct GNUNET_SERVER_Client *client) { if (bl->client != client) continue; - for (bc = bc_head; bc != NULL; bc = bc->next) + for (bc = bc_head; NULL != bc; bc = bc->next) { if (bc->bl_pos != bl) continue; bc->bl_pos = bl->next; - if (bc->th != NULL) + if (NULL != bc->th) { GNUNET_SERVER_notify_transmit_ready_cancel (bc->th); bc->th = NULL; } - if (bc->task == NULL) + if (NULL == bc->task) bc->task = GNUNET_SCHEDULER_add_now (&do_blacklist_check, bc); } GNUNET_CONTAINER_DLL_remove (bl_head, bl_tail, bl); @@ -383,17 +395,20 @@ GST_blacklist_stop () * @return number of bytes copied to @a buf */ static size_t -transmit_blacklist_message (void *cls, size_t size, void *buf) +transmit_blacklist_message (void *cls, + size_t size, + void *buf) { struct GST_BlacklistCheck *bc = cls; struct Blacklisters *bl; struct BlacklistMessage bm; bc->th = NULL; - if (size == 0) + if (0 == size) { - GNUNET_assert (bc->task == NULL); - bc->task = GNUNET_SCHEDULER_add_now (&do_blacklist_check, bc); + GNUNET_assert (NULL == bc->task); + bc->task = GNUNET_SCHEDULER_add_now (&do_blacklist_check, + bc); GNUNET_log (GNUNET_ERROR_TYPE_WARNING, "Failed to send blacklist test for peer `%s' to client\n", GNUNET_i2s (&bc->peer)); @@ -401,16 +416,20 @@ transmit_blacklist_message (void *cls, size_t size, void *buf) } GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Sending blacklist test for peer `%s' to client %p\n", - GNUNET_i2s (&bc->peer), bc->bl_pos->client); + GNUNET_i2s (&bc->peer), + bc->bl_pos->client); bl = bc->bl_pos; bm.header.size = htons (sizeof (struct BlacklistMessage)); bm.header.type = htons (GNUNET_MESSAGE_TYPE_TRANSPORT_BLACKLIST_QUERY); bm.is_allowed = htonl (0); bm.peer = bc->peer; - memcpy (buf, &bm, sizeof (bm)); + memcpy (buf, + &bm, + sizeof (bm)); if (GNUNET_YES == bl->call_receive_done) { - GNUNET_SERVER_receive_done (bl->client, GNUNET_OK); + GNUNET_SERVER_receive_done (bl->client, + GNUNET_OK); bl->call_receive_done = GNUNET_NO; } @@ -434,25 +453,30 @@ do_blacklist_check (void *cls, bc->task = NULL; bl = bc->bl_pos; - if (bl == NULL) + if (NULL == bl) { GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "No other blacklist clients active, will allow neighbour `%s'\n", GNUNET_i2s (&bc->peer)); - bc->cont (bc->cont_cls, &bc->peer, GNUNET_OK); - GNUNET_CONTAINER_DLL_remove(bc_head, bc_tail, bc); - GNUNET_free (bc); + bc->cont (bc->cont_cls, + &bc->peer, + bc->address, + bc->session, + GNUNET_OK); + GST_blacklist_test_cancel (bc); return; } - if ((bl->bc != NULL) || (bl->waiting_for_reply != GNUNET_NO)) + if ( (NULL != bl->bc) || + (GNUNET_NO != bl->waiting_for_reply) ) return; /* someone else busy with this client */ bl->bc = bc; bc->th = GNUNET_SERVER_notify_transmit_ready (bl->client, sizeof (struct BlacklistMessage), GNUNET_TIME_UNIT_FOREVER_REL, - &transmit_blacklist_message, bc); + &transmit_blacklist_message, + bc); } @@ -462,25 +486,30 @@ do_blacklist_check (void *cls, * * @param cls unused * @param peer the neighbour that was investigated + * @param address address associated with the request + * @param session session associated with the request * @param allowed #GNUNET_OK if we can keep it, * #GNUNET_NO if we must shutdown the connection */ static void confirm_or_drop_neighbour (void *cls, const struct GNUNET_PeerIdentity *peer, + const struct GNUNET_HELLO_Address *address, + struct Session *session, int allowed) { if (GNUNET_OK == allowed) return; /* we're done */ GNUNET_STATISTICS_update (GST_stats, - gettext_noop ("# disconnects due to blacklist"), 1, + gettext_noop ("# disconnects due to blacklist"), + 1, GNUNET_NO); GST_neighbours_force_disconnect (peer); } /** - * Closure for 'test_connection_ok'. + * Closure for #test_connection_ok(). */ struct TestConnectionContext { @@ -525,6 +554,7 @@ test_connection_ok (void *cls, bc_tail, bc); bc->peer = *peer; + bc->address = GNUNET_HELLO_address_copy (address); bc->cont = &confirm_or_drop_neighbour; bc->cont_cls = NULL; bc->bl_pos = tcc->bl; @@ -548,7 +578,8 @@ test_connection_ok (void *cls, * @param message the blacklist-init message that was sent */ void -GST_blacklist_handle_init (void *cls, struct GNUNET_SERVER_Client *client, +GST_blacklist_handle_init (void *cls, + struct GNUNET_SERVER_Client *client, const struct GNUNET_MessageHeader *message) { struct Blacklisters *bl; @@ -590,7 +621,8 @@ GST_blacklist_handle_init (void *cls, struct GNUNET_SERVER_Client *client, * @param message the blacklist-init message that was sent */ void -GST_blacklist_handle_reply (void *cls, struct GNUNET_SERVER_Client *client, +GST_blacklist_handle_reply (void *cls, + struct GNUNET_SERVER_Client *client, const struct GNUNET_MessageHeader *message) { const struct BlacklistMessage *msg = @@ -605,14 +637,15 @@ GST_blacklist_handle_reply (void *cls, struct GNUNET_SERVER_Client *client, { GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Blacklist client disconnected\n"); - GNUNET_SERVER_receive_done (client, GNUNET_SYSERR); + GNUNET_SERVER_receive_done (client, + GNUNET_SYSERR); return; } GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Blacklist client %p sent reply for `%s'\n", client, - GNUNET_i2s(&msg->peer)); + GNUNET_i2s (&msg->peer)); bc = bl->bc; bl->bc = NULL; @@ -622,32 +655,48 @@ GST_blacklist_handle_reply (void *cls, struct GNUNET_SERVER_Client *client, { /* only run this if the blacklist check has not been * cancelled in the meantime... */ + GNUNET_assert (bc->bl_pos == bl); if (ntohl (msg->is_allowed) == GNUNET_SYSERR) { GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Blacklist check failed, peer not allowed\n"); - bc->cont (bc->cont_cls, &bc->peer, GNUNET_NO); - GNUNET_CONTAINER_DLL_remove (bc_head, bc_tail, bc); + /* For the duration of the continuation, make the ongoing + check invisible (to avoid double-cancellation); then + add it back again so we can re-use GST_blacklist_test_cancel() */ + GNUNET_CONTAINER_DLL_remove (bc_head, + bc_tail, + bc); + bc->cont (bc->cont_cls, + &bc->peer, + bc->address, + bc->session, + GNUNET_NO); + GNUNET_CONTAINER_DLL_insert (bc_head, + bc_tail, + bc); + GST_blacklist_test_cancel (bc); GNUNET_SERVER_receive_done (bl->client, GNUNET_OK); bl->call_receive_done = GNUNET_NO; - GNUNET_free (bc); return; } else { GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Blacklist check succeeded, continuing with checks\n"); - GNUNET_SERVER_receive_done (bl->client, GNUNET_OK); + GNUNET_SERVER_receive_done (bl->client, + GNUNET_OK); bl->call_receive_done = GNUNET_NO; - bc->bl_pos = bc->bl_pos->next; - bc->task = GNUNET_SCHEDULER_add_now (&do_blacklist_check, bc); + bc->bl_pos = bl->next; + bc->task = GNUNET_SCHEDULER_add_now (&do_blacklist_check, + bc); } } /* check if any other blacklist checks are waiting for this blacklister */ for (bc = bc_head; bc != NULL; bc = bc->next) if ((bc->bl_pos == bl) && (NULL == bc->task)) { - bc->task = GNUNET_SCHEDULER_add_now (&do_blacklist_check, bc); + bc->task = GNUNET_SCHEDULER_add_now (&do_blacklist_check, + bc); break; } } @@ -687,6 +736,38 @@ GST_blacklist_add_peer (const struct GNUNET_PeerIdentity *peer, } +/** + * Abort blacklist if @a address and @a session match. + * + * @param address address used to abort matching checks + * @param session session used to abort matching checks + */ +void +GST_blacklist_abort_matching (const struct GNUNET_HELLO_Address *address, + struct Session *session) +{ + struct GST_BlacklistCheck *bc; + struct GST_BlacklistCheck *n; + + n = bc_head; + while (NULL != (bc = n)) + { + n = bc->next; + if ( (bc->session == session) && + (0 == GNUNET_HELLO_address_cmp (bc->address, + address)) ) + { + bc->cont (bc->cont_cls, + &bc->peer, + bc->address, + bc->session, + GNUNET_SYSERR); + GST_blacklist_test_cancel (bc); + } + } +} + + /** * Test if the given blacklist entry matches. If so, * abort the iteration. @@ -725,8 +806,9 @@ test_blacklisted (void *cls, /* blacklist check for specific transport */ if ((NULL != transport_name) && (NULL != value)) { - if (0 == strcmp (transport_name, be)) - return GNUNET_NO; /* plugin is blacklisted! */ + if (0 == strcmp (transport_name, + be)) + return GNUNET_NO; /* plugin is blacklisted! */ } return GNUNET_OK; } @@ -739,6 +821,8 @@ test_blacklisted (void *cls, * @param transport_name name of the transport to test, never NULL * @param cont function to call with result * @param cont_cls closure for @a cont + * @param address address to pass back to @a cont, can be NULL + * @param session session to pass back to @a cont, can be NULL * @return handle to the blacklist check, NULL if the decision * was made instantly and @a cont was already called */ @@ -746,7 +830,9 @@ struct GST_BlacklistCheck * GST_blacklist_test_allowed (const struct GNUNET_PeerIdentity *peer, const char *transport_name, GST_BlacklistTestContinuation cont, - void *cont_cls) + void *cont_cls, + const struct GNUNET_HELLO_Address *address, + struct Session *session) { struct GST_BlacklistCheck *bc; @@ -767,21 +853,30 @@ GST_blacklist_test_allowed (const struct GNUNET_PeerIdentity *peer, /* Disallowed by config, disapprove instantly */ GNUNET_STATISTICS_update (GST_stats, gettext_noop ("# disconnects due to blacklist"), - 1, GNUNET_NO); + 1, + GNUNET_NO); GNUNET_log (GNUNET_ERROR_TYPE_INFO, _("Disallowing connection to peer `%s' on transport %s\n"), GNUNET_i2s (peer), (NULL != transport_name) ? transport_name : "unspecified"); - if (cont != NULL) - cont (cont_cls, peer, GNUNET_NO); + if (NULL != cont) + cont (cont_cls, + peer, + address, + session, + GNUNET_NO); return NULL; } if (NULL == bl_head) { /* no blacklist clients, approve instantly */ - if (cont != NULL) - cont (cont_cls, peer, GNUNET_OK); + if (NULL != cont) + cont (cont_cls, + peer, + address, + session, + GNUNET_OK); GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Allowing connection to peer `%s' %s\n", GNUNET_i2s (peer), @@ -791,8 +886,12 @@ GST_blacklist_test_allowed (const struct GNUNET_PeerIdentity *peer, /* need to query blacklist clients */ bc = GNUNET_new (struct GST_BlacklistCheck); - GNUNET_CONTAINER_DLL_insert (bc_head, bc_tail, bc); + GNUNET_CONTAINER_DLL_insert (bc_head, + bc_tail, + bc); bc->peer = *peer; + bc->address = GNUNET_HELLO_address_copy (address); + bc->session = session; bc->cont = cont; bc->cont_cls = cont_cls; bc->bl_pos = bl_head; @@ -830,6 +929,7 @@ GST_blacklist_test_cancel (struct GST_BlacklistCheck *bc) GNUNET_SERVER_notify_transmit_ready_cancel (bc->th); bc->th = NULL; } + GNUNET_free_non_null (bc->address); GNUNET_free (bc); } diff --git a/src/transport/gnunet-service-transport_blacklist.h b/src/transport/gnunet-service-transport_blacklist.h index 068556778..1a8bdebf4 100644 --- a/src/transport/gnunet-service-transport_blacklist.h +++ b/src/transport/gnunet-service-transport_blacklist.h @@ -27,6 +27,7 @@ #define GNUNET_SERVICE_TRANSPORT_BLACKLIST_H #include "gnunet_statistics_service.h" +#include "gnunet_ats_service.h" #include "gnunet_util_lib.h" /** @@ -99,12 +100,17 @@ struct GST_BlacklistCheck; * * @param cls closure * @param peer identity of peer that was tested + * @param address address associated with the request + * @param session session associated with the request * @param result #GNUNET_OK if the connection is allowed, - * #GNUNET_NO if not + * #GNUNET_NO if not, + * #GNUNET_SYSERR if operation was aborted */ typedef void (*GST_BlacklistTestContinuation) (void *cls, const struct GNUNET_PeerIdentity *peer, + const struct GNUNET_HELLO_Address *address, + struct Session *session, int result); @@ -115,13 +121,30 @@ typedef void * @param transport_name name of the transport to test, never NULL * @param cont function to call with result * @param cont_cls closure for @a cont + * @param address address to pass back to @a cont, can be NULL + * @param session session to pass back to @a cont, can be NULL * @return handle to the blacklist check, NULL if the decision * was made instantly and @a cont was already called */ struct GST_BlacklistCheck * GST_blacklist_test_allowed (const struct GNUNET_PeerIdentity *peer, const char *transport_name, - GST_BlacklistTestContinuation cont, void *cont_cls); + GST_BlacklistTestContinuation cont, + void *cont_cls, + const struct GNUNET_HELLO_Address *address, + struct Session *session); + + +/** + * Abort blacklist if @a address and @a session match. + * + * @param address address used to abort matching checks + * @param session session used to abort matching checks + */ +void +GST_blacklist_abort_matching (const struct GNUNET_HELLO_Address *address, + struct Session *session); + /** diff --git a/src/transport/gnunet-service-transport_neighbours.c b/src/transport/gnunet-service-transport_neighbours.c index 404dadb80..7c596fbae 100644 --- a/src/transport/gnunet-service-transport_neighbours.c +++ b/src/transport/gnunet-service-transport_neighbours.c @@ -2141,16 +2141,6 @@ struct BlacklistCheckSwitchContext */ struct GST_BlacklistCheck *blc; - /** - * Address we are asking the blacklist subsystem about. - */ - struct GNUNET_HELLO_Address *address; - - /** - * Session we should use in conjunction with @e address, can be NULL. - */ - struct Session *session; - /** * Inbound bandwidth that was assigned to @e address. */ @@ -2169,11 +2159,17 @@ struct BlacklistCheckSwitchContext * * @param cls blc_ctx bl context * @param peer the peer - * @param result the result + * @param address address associated with the request + * @param session session associated with the request + * @param result #GNUNET_OK if the connection is allowed, + * #GNUNET_NO if not, + * #GNUNET_SYSERR if operation was aborted */ static void try_connect_bl_check_cont (void *cls, const struct GNUNET_PeerIdentity *peer, + const struct GNUNET_HELLO_Address *address, + struct Session *session, int result) { struct BlacklistCheckSwitchContext *blc_ctx = cls; @@ -2281,7 +2277,9 @@ GST_neighbours_try_connect (const struct GNUNET_PeerIdentity *target) (blc = GST_blacklist_test_allowed (target, NULL, &try_connect_bl_check_cont, - blc_ctx))) + blc_ctx, + NULL, + NULL))) { blc_ctx->blc = blc; } @@ -2475,54 +2473,66 @@ try_run_fast_ats_update (const struct GNUNET_HELLO_Address *address, * @param cls the `struct BlacklistCheckSwitchContext` with * the information about the future address * @param peer the peer we may switch addresses on - * @param result #GNUNET_NO if we are not allowed to use the new - * address + * @param address address associated with the request + * @param session session associated with the request + * @param result #GNUNET_OK if the connection is allowed, + * #GNUNET_NO if not, + * #GNUNET_SYSERR if operation was aborted */ static void switch_address_bl_check_cont (void *cls, const struct GNUNET_PeerIdentity *peer, + const struct GNUNET_HELLO_Address *address, + struct Session *session, int result) { struct BlacklistCheckSwitchContext *blc_ctx = cls; struct GNUNET_TRANSPORT_PluginFunctions *papi; struct NeighbourMapEntry *n; - if (result == GNUNET_NO) + if (GNUNET_SYSERR == result) + goto cleanup; + + papi = GST_plugins_find (address->transport_name); + GNUNET_assert (NULL != papi); + + if (GNUNET_NO == result) { GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Blacklist denied to switch to suggested address `%s' session %p for peer `%s'\n", - GST_plugins_a2s (blc_ctx->address), - blc_ctx->session, - GNUNET_i2s (&blc_ctx->address->peer)); + GST_plugins_a2s (address), + session, + GNUNET_i2s (peer)); GNUNET_STATISTICS_update (GST_stats, "# ATS suggestions ignored (blacklist denied)", 1, GNUNET_NO); - /* FIXME: tell plugin to force killing session here and now - (note: _proper_ plugin API for this does not yet exist) */ - GST_ats_block_address (blc_ctx->address, - blc_ctx->session); + papi->disconnect_session (papi->cls, + session); + if (GNUNET_YES != + GNUNET_HELLO_address_check_option (address, + GNUNET_HELLO_ADDRESS_INFO_INBOUND)) + GST_ats_block_address (address, + NULL); goto cleanup; } - papi = GST_plugins_find (blc_ctx->address->transport_name); - GNUNET_assert (NULL != papi); - if (NULL == blc_ctx->session) + if (NULL == session) { /* need to create a session, ATS only gave us an address */ - blc_ctx->session = papi->get_session (papi->cls, - blc_ctx->address); + session = papi->get_session (papi->cls, + address); GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Obtained new session for peer `%s' and address '%s': %p\n", - GNUNET_i2s (&blc_ctx->address->peer), - GST_plugins_a2s (blc_ctx->address), - blc_ctx->session); - if (NULL != blc_ctx->session) - GST_ats_new_session (blc_ctx->address, - blc_ctx->session); + GNUNET_i2s (&address->peer), + GST_plugins_a2s (address), + session); + if (NULL != session) + GST_ats_new_session (address, + session); } - if (NULL == blc_ctx->session) + if (NULL == session) { /* session creation failed, bad!, fail! */ GNUNET_STATISTICS_update (GST_stats, @@ -2532,10 +2542,10 @@ switch_address_bl_check_cont (void *cls, /* No session could be obtained, remove blacklist check and clean up */ GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Failed to obtain new session for peer `%s' and address '%s'\n", - GNUNET_i2s (&blc_ctx->address->peer), - GST_plugins_a2s (blc_ctx->address)); - GST_ats_block_address (blc_ctx->address, - blc_ctx->session); + GNUNET_i2s (&address->peer), + GST_plugins_a2s (address)); + GST_ats_block_address (address, + session); goto cleanup; } @@ -2543,8 +2553,8 @@ switch_address_bl_check_cont (void *cls, it is theoretically possible that the situation changed in the meantime, hence we check again here */ if (GNUNET_OK == - try_run_fast_ats_update (blc_ctx->address, - blc_ctx->session, + try_run_fast_ats_update (address, + session, blc_ctx->bandwidth_in, blc_ctx->bandwidth_out)) goto cleanup; /* was just a minor update, we're done */ @@ -2558,23 +2568,23 @@ switch_address_bl_check_cont (void *cls, GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Peer `%s' switches to address `%s'\n", - GNUNET_i2s (&blc_ctx->address->peer), - GST_plugins_a2s (blc_ctx->address)); + GNUNET_i2s (&address->peer), + GST_plugins_a2s (address)); switch (n->state) { case GNUNET_TRANSPORT_PS_NOT_CONNECTED: GNUNET_break (0); - GST_ats_block_address (blc_ctx->address, - blc_ctx->session); + GST_ats_block_address (address, + session); free_neighbour (n); return; case GNUNET_TRANSPORT_PS_INIT_ATS: /* We requested an address and ATS suggests one: * set primary address and send SYN message*/ set_primary_address (n, - blc_ctx->address, - blc_ctx->session, + address, + session, blc_ctx->bandwidth_in, blc_ctx->bandwidth_out); if (ACK_SEND_SYN_ACK == n->ack_state) @@ -2594,8 +2604,8 @@ switch_address_bl_check_cont (void *cls, * Switch and send new SYN */ /* ATS suggests a different address, switch again */ set_primary_address (n, - blc_ctx->address, - blc_ctx->session, + address, + session, blc_ctx->bandwidth_in, blc_ctx->bandwidth_out); if (ACK_SEND_SYN_ACK == n->ack_state) @@ -2614,8 +2624,8 @@ switch_address_bl_check_cont (void *cls, /* We requested an address and ATS suggests one: * set primary address and send SYN_ACK message*/ set_primary_address (n, - blc_ctx->address, - blc_ctx->session, + address, + session, blc_ctx->bandwidth_in, blc_ctx->bandwidth_out); /* Send an ACK message as a response to the SYN msg */ @@ -2638,8 +2648,8 @@ switch_address_bl_check_cont (void *cls, n->connect_ack_timestamp); } set_primary_address (n, - blc_ctx->address, - blc_ctx->session, + address, + session, blc_ctx->bandwidth_in, blc_ctx->bandwidth_out); set_state_and_timeout (n, @@ -2649,12 +2659,12 @@ switch_address_bl_check_cont (void *cls, case GNUNET_TRANSPORT_PS_CONNECTED: GNUNET_assert (NULL != n->primary_address.address); GNUNET_assert (NULL != n->primary_address.session); - GNUNET_break (n->primary_address.session != blc_ctx->session); + GNUNET_break (n->primary_address.session != session); /* ATS asks us to switch a life connection; see if we can get a SYN_ACK on it before we actually do this! */ set_alternative_address (n, - blc_ctx->address, - blc_ctx->session, + address, + session, blc_ctx->bandwidth_in, blc_ctx->bandwidth_out); set_state_and_timeout (n, @@ -2668,8 +2678,8 @@ switch_address_bl_check_cont (void *cls, break; case GNUNET_TRANSPORT_PS_RECONNECT_ATS: set_primary_address (n, - blc_ctx->address, - blc_ctx->session, + address, + session, blc_ctx->bandwidth_in, blc_ctx->bandwidth_out); if (ACK_SEND_SYN_ACK == n->ack_state) @@ -2688,8 +2698,8 @@ switch_address_bl_check_cont (void *cls, /* ATS asks us to switch while we were trying to reconnect; switch to new address and send SYN again */ set_primary_address (n, - blc_ctx->address, - blc_ctx->session, + address, + session, blc_ctx->bandwidth_in, blc_ctx->bandwidth_out); set_state_and_timeout (n, @@ -2699,8 +2709,8 @@ switch_address_bl_check_cont (void *cls, break; case GNUNET_TRANSPORT_PS_SWITCH_SYN_SENT: if ( (0 == GNUNET_HELLO_address_cmp (n->primary_address.address, - blc_ctx->address)) && - (n->primary_address.session == blc_ctx->session) ) + address)) && + (n->primary_address.session == session) ) { /* ATS switches back to still-active session */ GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, @@ -2713,8 +2723,8 @@ switch_address_bl_check_cont (void *cls, } /* ATS asks us to switch a life connection, send */ set_alternative_address (n, - blc_ctx->address, - blc_ctx->session, + address, + session, blc_ctx->bandwidth_in, blc_ctx->bandwidth_out); set_state_and_timeout (n, @@ -2743,7 +2753,6 @@ switch_address_bl_check_cont (void *cls, GNUNET_CONTAINER_DLL_remove (pending_bc_head, pending_bc_tail, blc_ctx); - GNUNET_HELLO_address_free (blc_ctx->address); GNUNET_free (blc_ctx); } @@ -2811,8 +2820,6 @@ GST_neighbours_switch_to_address (const struct GNUNET_HELLO_Address *address, /* Perform blacklist check */ blc_ctx = GNUNET_new (struct BlacklistCheckSwitchContext); - blc_ctx->address = GNUNET_HELLO_address_copy (address); - blc_ctx->session = session; blc_ctx->bandwidth_in = bandwidth_in; blc_ctx->bandwidth_out = bandwidth_out; GNUNET_CONTAINER_DLL_insert (pending_bc_head, @@ -2821,7 +2828,9 @@ GST_neighbours_switch_to_address (const struct GNUNET_HELLO_Address *address, if (NULL != (blc = GST_blacklist_test_allowed (&address->peer, address->transport_name, &switch_address_bl_check_cont, - blc_ctx))) + blc_ctx, + address, + session))) { blc_ctx->blc = blc; } @@ -3835,8 +3844,6 @@ GST_neighbours_stop () GST_blacklist_test_cancel (cur->blc); cur->blc = NULL; } - if (NULL != cur->address) - GNUNET_HELLO_address_free (cur->address); GNUNET_free (cur); } } diff --git a/src/transport/gnunet-service-transport_validation.c b/src/transport/gnunet-service-transport_validation.c index 2022a8c47..de6edc90a 100644 --- a/src/transport/gnunet-service-transport_validation.c +++ b/src/transport/gnunet-service-transport_validation.c @@ -505,11 +505,17 @@ timeout_hello_validation (void *cls, * * @param cls our `struct ValidationEntry` * @param pid identity of the other peer - * @param result #GNUNET_OK if the connection is allowed, #GNUNET_NO if not + * @param address_null address associated with the request, always NULL + * @param session_null session associated with the request, always NULL + * @param result #GNUNET_OK if the connection is allowed, + * #GNUNET_NO if not, + * #GNUNET_SYSERR if operation was aborted */ static void transmit_ping_if_allowed (void *cls, const struct GNUNET_PeerIdentity *pid, + const struct GNUNET_HELLO_Address *address_null, + struct Session *session_null, int result) { struct ValidationEntry *ve = cls; @@ -524,7 +530,7 @@ transmit_ping_if_allowed (void *cls, struct Session *session; ve->bc = NULL; - if (GNUNET_NO == result) + if (GNUNET_OK != result) { GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Blacklist denies sending PING to `%s' `%s' `%s'\n", @@ -743,7 +749,10 @@ revalidate_address (void *cls, GNUNET_NO); bc = GST_blacklist_test_allowed (&ve->address->peer, ve->address->transport_name, - &transmit_ping_if_allowed, ve); + &transmit_ping_if_allowed, + ve, + NULL, + NULL); if (NULL != bc) ve->bc = bc; /* only set 'bc' if 'transmit_ping_if_allowed' was not already * called... */ diff --git a/src/transport/test_transport_api_blacklisting.c b/src/transport/test_transport_api_blacklisting.c index 7338dabe7..601ab27f1 100644 --- a/src/transport/test_transport_api_blacklisting.c +++ b/src/transport/test_transport_api_blacklisting.c @@ -367,7 +367,8 @@ blacklist_cb (void *cls, return res; } -void + +static void start_cb (struct PeerContext *p, void *cls) { static int started; @@ -392,6 +393,7 @@ start_cb (struct PeerContext *p, void *cls) } + static void run (void *cls, char *const *args, const char *cfgfile, const struct GNUNET_CONFIGURATION_Handle *cfg) diff --git a/src/transport/test_transport_api_data.conf b/src/transport/test_transport_api_data.conf index 164a38f69..58b8e17b0 100644 --- a/src/transport/test_transport_api_data.conf +++ b/src/transport/test_transport_api_data.conf @@ -7,5 +7,3 @@ PORT = 2094 [transport-udp] PORT = 2094 - - diff --git a/src/transport/test_transport_api_tcp_peer1.conf b/src/transport/test_transport_api_tcp_peer1.conf index 72b8006b1..4bfe9b6ca 100644 --- a/src/transport/test_transport_api_tcp_peer1.conf +++ b/src/transport/test_transport_api_tcp_peer1.conf @@ -5,3 +5,5 @@ GNUNET_TEST_HOME = /tmp/test-transport/api-tcp-p1/ [transport] PLUGINS = tcp +#[transport] +#PREFIX = valgrind diff --git a/src/transport/test_transport_api_tcp_peer2.conf b/src/transport/test_transport_api_tcp_peer2.conf index afb412cd0..e68cdbee4 100644 --- a/src/transport/test_transport_api_tcp_peer2.conf +++ b/src/transport/test_transport_api_tcp_peer2.conf @@ -5,3 +5,5 @@ GNUNET_TEST_HOME = /tmp/test-transport/api-tcp-p2/ [transport] PLUGINS = tcp +#[transport] +#PREFIX = valgrind -- 2.25.1