From 6446293d028db5fe66c53f70ab8194f6c47f3fa1 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Mon, 25 Jul 2016 21:34:47 +0000 Subject: [PATCH] -ensure clean DNS shutdown, fix DNS-STUN termination issues --- src/nat/nat.c | 92 +++++++++++++++++++++++------------------ src/nat/nat_stun.c | 44 +++++++++++--------- src/util/resolver_api.c | 83 +++++++++++++++++++++++++++++-------- 3 files changed, 142 insertions(+), 77 deletions(-) diff --git a/src/nat/nat.c b/src/nat/nat.c index 6e9fcfd0e..9a9ae18ac 100644 --- a/src/nat/nat.c +++ b/src/nat/nat.c @@ -429,7 +429,7 @@ struct GNUNET_NAT_Handle /** * STUN request task */ - struct GNUNET_SCHEDULER_Task * stun_task; + struct GNUNET_SCHEDULER_Task *stun_task; /** * Head of List of STUN servers @@ -459,15 +459,6 @@ static void start_gnunet_nat_server (struct GNUNET_NAT_Handle *h); -/** - * Call task to process STUN - * - * @param cls handle to NAT - */ -static void -process_stun (void *cls); - - /** * Remove all addresses from the list of 'local' addresses * that originated from the given source. @@ -688,7 +679,8 @@ process_external_ip (void *cls, /* Current iteration is over, remove 'old' IPs now */ LOG (GNUNET_ERROR_TYPE_DEBUG, "Purging old IPs for external address\n"); - remove_from_address_list_by_source (h, LAL_EXTERNAL_IP_OLD); + remove_from_address_list_by_source (h, + LAL_EXTERNAL_IP_OLD); if (1 == inet_pton (AF_INET, h->external_address, &dummy)) @@ -704,9 +696,13 @@ process_external_ip (void *cls, } LOG (GNUNET_ERROR_TYPE_DEBUG, "Got IP `%s' for external address `%s'\n", - GNUNET_a2s (addr, addrlen), + GNUNET_a2s (addr, + addrlen), h->external_address); - add_to_address_list (h, LAL_EXTERNAL_IP, addr, addrlen); + add_to_address_list (h, + LAL_EXTERNAL_IP, + addr, + addrlen); } @@ -740,10 +736,14 @@ process_hostname_ip (void *cls, h->hostname_dns = NULL; h->hostname_task = GNUNET_SCHEDULER_add_delayed (h->hostname_dns_frequency, - &resolve_hostname, h); + &resolve_hostname, + h); return; } - add_to_address_list (h, LAL_HOSTNAME_DNS, addr, addrlen); + add_to_address_list (h, + LAL_HOSTNAME_DNS, + addr, + addrlen); } @@ -1204,7 +1204,7 @@ static void process_stun (void *cls) { struct GNUNET_NAT_Handle *h = cls; - struct StunServerList* elem = h->actual_stun_server; + struct StunServerList *elem = h->actual_stun_server; h->stun_task = NULL; /* Make the request */ @@ -1260,9 +1260,12 @@ resolve_hostname (void *cls) h->hostname_task = NULL; remove_from_address_list_by_source (h, LAL_HOSTNAME_DNS); + GNUNET_assert (NULL == h->hostname_dns); h->hostname_dns = - GNUNET_RESOLVER_hostname_resolve (AF_UNSPEC, HOSTNAME_RESOLVE_TIMEOUT, - &process_hostname_ip, h); + GNUNET_RESOLVER_hostname_resolve (AF_UNSPEC, + HOSTNAME_RESOLVE_TIMEOUT, + &process_hostname_ip, + h); } @@ -1285,10 +1288,13 @@ resolve_dns (void *cls) LOG (GNUNET_ERROR_TYPE_DEBUG, "Resolving external address `%s'\n", h->external_address); + GNUNET_assert (NULL == h->ext_dns); h->ext_dns = - GNUNET_RESOLVER_ip_get (h->external_address, AF_INET, + GNUNET_RESOLVER_ip_get (h->external_address, + AF_INET, GNUNET_TIME_UNIT_MINUTES, - &process_external_ip, h); + &process_external_ip, + h); } @@ -1636,7 +1642,6 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg, size_t pos_port; h->socket = sock; - h->actual_stun_server = NULL; stun_servers = NULL; /* Lets process the servers*/ (void) GNUNET_CONFIGURATION_get_value_string (cfg, @@ -1644,9 +1649,6 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg, "STUN_SERVERS", &stun_servers); urls = 0; - h->stun_servers_head = NULL; - h->stun_servers_tail = NULL; - h->actual_stun_server = NULL; if ( (NULL != stun_servers) && (strlen (stun_servers) > 0) ) { @@ -1665,8 +1667,8 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg, { struct StunServerList *ml; - /*Check if we do have a port*/ - if((0 == pos_port) || (pos_port <= pos)) + /* Check if we do have a port */ + if ((0 == pos_port) || (pos_port <= pos)) { LOG (GNUNET_ERROR_TYPE_WARNING, "STUN server format mistake\n"); @@ -1677,7 +1679,7 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg, ml->port = atoi (&stun_servers[pos_port]); /* Remove trailing space */ - if(stun_servers[pos] == ' ') + if (stun_servers[pos] == ' ') ml->address = GNUNET_strdup (&stun_servers[pos + 1]); else ml->address = GNUNET_strdup (&stun_servers[pos]); @@ -1688,6 +1690,7 @@ GNUNET_NAT_register (const struct GNUNET_CONFIGURATION_Handle *cfg, GNUNET_CONTAINER_DLL_insert (h->stun_servers_head, h->stun_servers_tail, ml); + stun_servers[pos] = '\0'; } } } @@ -1767,9 +1770,30 @@ GNUNET_NAT_unregister (struct GNUNET_NAT_Handle *h) unsigned int i; struct LocalAddressList *lal; struct MiniList *ml; - + struct StunServerList *ssl; + LOG (GNUNET_ERROR_TYPE_DEBUG, "NAT unregister called\n"); + while (NULL != (ssl = h->stun_servers_head)) + { + GNUNET_CONTAINER_DLL_remove (h->stun_servers_head, + h->stun_servers_tail, + ssl); + GNUNET_free (ssl->address); + GNUNET_free (ssl); + } + while (NULL != (lal = h->lal_head)) + { + GNUNET_CONTAINER_DLL_remove (h->lal_head, + h->lal_tail, + lal); + if (NULL != h->address_callback) + h->address_callback (h->callback_cls, + GNUNET_NO, + (const struct sockaddr *) &lal[1], + lal->addrlen); + GNUNET_free (lal); + } while (NULL != (ml = h->mini_head)) { GNUNET_CONTAINER_DLL_remove (h->mini_head, @@ -1839,18 +1863,6 @@ GNUNET_NAT_unregister (struct GNUNET_NAT_Handle *h) h->server_stdout = NULL; h->server_stdout_handle = NULL; } - while (NULL != (lal = h->lal_head)) - { - GNUNET_CONTAINER_DLL_remove (h->lal_head, - h->lal_tail, - lal); - if (NULL != h->address_callback) - h->address_callback (h->callback_cls, - GNUNET_NO, - (const struct sockaddr *) &lal[1], - lal->addrlen); - GNUNET_free (lal); - } for (i = 0; i < h->num_local_addrs; i++) GNUNET_free (h->local_addrs[i]); GNUNET_free_non_null (h->local_addrs); diff --git a/src/nat/nat_stun.c b/src/nat/nat_stun.c index 4aa9094ea..b914abb2e 100644 --- a/src/nat/nat_stun.c +++ b/src/nat/nat_stun.c @@ -461,9 +461,9 @@ stun_dns_callback (void *cls, int reqlen; struct sockaddr_in server; - rh->dns_active = NULL; if (NULL == addr) { + rh->dns_active = NULL; if (GNUNET_NO == rh->dns_success) { LOG (GNUNET_ERROR_TYPE_INFO, @@ -471,8 +471,18 @@ stun_dns_callback (void *cls, rh->stun_server); rh->cb (rh->cb_cls, GNUNET_NAT_ERROR_NOT_ONLINE); - GNUNET_NAT_stun_make_request_cancel (rh); } + else if (GNUNET_SYSERR == rh->dns_success) + { + rh->cb (rh->cb_cls, + GNUNET_NAT_ERROR_INTERNAL_NETWORK_ERROR); + } + else + { + rh->cb (rh->cb_cls, + GNUNET_NAT_ERROR_SUCCESS); + } + GNUNET_NAT_stun_make_request_cancel (rh); return; } @@ -487,31 +497,27 @@ stun_dns_callback (void *cls, /*Craft the simplest possible STUN packet. A request binding*/ req = (struct stun_header *)reqdata; - generate_request_id(req); + generate_request_id (req); reqlen = 0; req->msgtype = 0; req->msglen = 0; - req->msglen = htons(reqlen); - req->msgtype = htons(encode_message(STUN_REQUEST, STUN_BINDING)); + req->msglen = htons (reqlen); + req->msgtype = htons (encode_message (STUN_REQUEST, + STUN_BINDING)); /* Send the packet */ - if (-1 == GNUNET_NETWORK_socket_sendto (rh->sock, - req, - ntohs(req->msglen) + sizeof(*req), - (const struct sockaddr *) &server, - sizeof (server))) + if (-1 == + GNUNET_NETWORK_socket_sendto (rh->sock, + req, + ntohs(req->msglen) + sizeof(*req), + (const struct sockaddr *) &server, + sizeof (server))) { GNUNET_log_strerror (GNUNET_ERROR_TYPE_ERROR, - "Fail to sendto"); - rh->cb (rh->cb_cls, - GNUNET_NAT_ERROR_INTERNAL_NETWORK_ERROR); - GNUNET_NAT_stun_make_request_cancel (rh); + "sendto"); + rh->dns_success = GNUNET_SYSERR; return; } - /* sending STUN request done, let's wait for replies... */ - rh->cb (rh->cb_cls, - GNUNET_NAT_ERROR_SUCCESS); - GNUNET_NAT_stun_make_request_cancel (rh); } @@ -550,8 +556,6 @@ GNUNET_NAT_stun_make_request (const char *server, &stun_dns_callback, rh); if (NULL == rh->dns_active) { - GNUNET_log_strerror (GNUNET_ERROR_TYPE_ERROR, - "Failed DNS"); GNUNET_NAT_stun_make_request_cancel (rh); return NULL; } diff --git a/src/util/resolver_api.c b/src/util/resolver_api.c index 299fdfef9..c4ce1ccc2 100644 --- a/src/util/resolver_api.c +++ b/src/util/resolver_api.c @@ -249,8 +249,16 @@ GNUNET_RESOLVER_connect (const struct GNUNET_CONFIGURATION_Handle *cfg) void GNUNET_RESOLVER_disconnect () { - GNUNET_assert (NULL == req_head); - GNUNET_assert (NULL == req_tail); + struct GNUNET_RESOLVER_RequestHandle *rh; + + while (NULL != (rh = req_head)) + { + GNUNET_assert (GNUNET_SYSERR == rh->was_transmitted); + GNUNET_CONTAINER_DLL_remove (req_head, + req_tail, + rh); + GNUNET_free (rh); + } if (NULL != mq) { LOG (GNUNET_ERROR_TYPE_DEBUG, @@ -271,6 +279,42 @@ GNUNET_RESOLVER_disconnect () } +/** + * Task executed on system shutdown. + */ +static void +shutdown_task (void *cls) +{ + s_task = NULL; + GNUNET_RESOLVER_disconnect (); + backoff = GNUNET_TIME_UNIT_MILLISECONDS; +} + + +/** + * Consider disconnecting if we have no further requests pending. + */ +static void +check_disconnect () +{ + struct GNUNET_RESOLVER_RequestHandle *rh; + + for (rh = req_head; NULL != rh; rh = rh->next) + if (GNUNET_SYSERR != rh->was_transmitted) + return; + if (NULL != r_task) + { + GNUNET_SCHEDULER_cancel (r_task); + r_task = NULL; + } + if (NULL != s_task) + return; + s_task = GNUNET_SCHEDULER_add_delayed (GNUNET_TIME_UNIT_MILLISECONDS, + &shutdown_task, + NULL); +} + + /** * Convert IP address to string without DNS resolution. * @@ -339,22 +383,12 @@ mq_error_handler (void *cls, { GNUNET_MQ_destroy (mq); mq = NULL; + LOG (GNUNET_ERROR_TYPE_DEBUG, + "MQ error, reconnecting\n"); reconnect (); } -/** - * Task executed on system shutdown. - */ -static void -shutdown_task (void *cls) -{ - s_task = NULL; - GNUNET_RESOLVER_disconnect (); - backoff = GNUNET_TIME_UNIT_MILLISECONDS; -} - - /** * Process pending requests to the resolver. */ @@ -599,7 +633,9 @@ numeric_resolution (void *cls) } if ( ( (rh->af == AF_UNSPEC) || (rh->af == AF_INET6) ) && - (1 == inet_pton (AF_INET6, hostname, &v6.sin6_addr) ) ) + (1 == inet_pton (AF_INET6, + hostname, + &v6.sin6_addr) ) ) { rh->addr_callback (rh->cls, (const struct sockaddr *) &v6, @@ -671,6 +707,9 @@ loopback_resolution (void *cls) rh->addr_callback (rh->cls, NULL, 0); + LOG (GNUNET_ERROR_TYPE_DEBUG, + "Finished resolving hostname `%s'.\n", + (const char *) &rh[1]); GNUNET_free (rh); } @@ -740,6 +779,7 @@ reconnect () req_tail, rh); GNUNET_free (rh); + check_disconnect (); break; default: GNUNET_assert (0); @@ -841,13 +881,16 @@ GNUNET_RESOLVER_ip_get (const char *hostname, GNUNET_break (0); return NULL; } + LOG (GNUNET_ERROR_TYPE_DEBUG, + "Trying to resolve hostname `%s'.\n", + hostname); rh = GNUNET_malloc (sizeof (struct GNUNET_RESOLVER_RequestHandle) + slen); rh->af = af; rh->addr_callback = callback; rh->cls = callback_cls; GNUNET_memcpy (&rh[1], - hostname, - slen); + hostname, + slen); rh->data_len = slen; rh->timeout = GNUNET_TIME_relative_to_absolute (timeout); rh->direction = GNUNET_NO; @@ -1133,6 +1176,10 @@ GNUNET_RESOLVER_hostname_resolve (int af, void GNUNET_RESOLVER_request_cancel (struct GNUNET_RESOLVER_RequestHandle *rh) { + if (GNUNET_NO == rh->direction) + LOG (GNUNET_ERROR_TYPE_DEBUG, + "Asked to cancel request to resolve hostname `%s'.\n", + (const char *) &rh[1]); if (NULL != rh->task) { GNUNET_SCHEDULER_cancel (rh->task); @@ -1145,10 +1192,12 @@ GNUNET_RESOLVER_request_cancel (struct GNUNET_RESOLVER_RequestHandle *rh) req_tail, rh); GNUNET_free (rh); + check_disconnect (); return; } GNUNET_assert (GNUNET_YES == rh->was_transmitted); rh->was_transmitted = GNUNET_SYSERR; /* mark as cancelled */ + check_disconnect (); } -- 2.25.1