[PATCH] dnsproxy: Close the UDP socket on error condition.

Forest Bond forest at alittletooquiet.net
Tue Nov 13 08:57:00 PST 2012


From: Forest Bond <forest.bond at rapidrollout.com>

The socket is created again on the next request.

Previously, an error condition only caused the event watch to be removed
and then added again on the next request to avoid processing spurious
error events.  But in some network environments, the error condition is
caused by an overly aggressive firewall that rejects our UDP packets and
sends an ICMP error packet indicating an unreachable host and/or port.
In some of these cases we can recover by using a different UDP source
port for outgoing requests (i.e. by closing and re-opening the socket).
---
 src/dnsproxy.c |  212 ++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 128 insertions(+), 84 deletions(-)

diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 17d9a20..473a47d 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -1750,6 +1750,28 @@ static gboolean try_remove_cache(gpointer user_data)
 	return FALSE;
 }
 
+static void server_destroy_socket(struct server_data *data)
+{
+	DBG("interface %s server %s proto %d", data->interface,
+					data->server, data->protocol);
+
+	if (data->watch > 0) {
+		g_source_remove(data->watch);
+		data->watch = 0;
+	}
+
+	if (data->timeout > 0) {
+		g_source_remove(data->timeout);
+		data->timeout = 0;
+	}
+
+	g_io_channel_unref(data->channel);
+	data->channel = NULL;
+
+	g_free(data->incoming_reply);
+	data->incoming_reply = NULL;
+}
+
 static void destroy_server(struct server_data *server)
 {
 	GList *list;
@@ -1758,19 +1780,11 @@ static void destroy_server(struct server_data *server)
 		g_io_channel_unix_get_fd(server->channel));
 
 	server_list = g_slist_remove(server_list, server);
-
-	if (server->watch > 0)
-		g_source_remove(server->watch);
-
-	if (server->timeout > 0)
-		g_source_remove(server->timeout);
-
-	g_io_channel_unref(server->channel);
+	server_destroy_socket(server);
 
 	if (server->protocol == IPPROTO_UDP)
 		DBG("Removing DNS server %s", server->server);
 
-	g_free(server->incoming_reply);
 	g_free(server->server);
 	for (list = server->domains; list; list = list->next) {
 		char *domain = list->data;
@@ -1804,7 +1818,7 @@ static gboolean udp_server_event(GIOChannel *channel, GIOCondition condition,
 
 	if (condition & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
 		connman_error("Error with UDP server %s", data->server);
-		data->watch = 0;
+		server_destroy_socket(data);
 		return FALSE;
 	}
 
@@ -2028,19 +2042,17 @@ static gboolean tcp_idle_timeout(gpointer user_data)
 	return FALSE;
 }
 
-static struct server_data *create_server(const char *interface,
-					const char *domain, const char *server,
-					int protocol)
+static int server_create_socket(struct server_data *data)
 {
-	struct addrinfo hints, *rp;
-	struct server_data *data;
-	int sk, ret;
+	struct addrinfo hints, *rp = NULL;
+	int sk = 0, ret, err;
 
-	DBG("interface %s server %s proto %d", interface, server, protocol);
+	DBG("interface %s server %s proto %d", data->interface,
+					data->server, data->protocol);
 
 	memset(&hints, 0, sizeof(hints));
 
-	switch (protocol) {
+	switch (data->protocol) {
 	case IPPROTO_UDP:
 		hints.ai_socktype = SOCK_DGRAM;
 		break;
@@ -2050,16 +2062,18 @@ static struct server_data *create_server(const char *interface,
 		break;
 
 	default:
-		return NULL;
+		err = EINVAL;
+		goto error_out;
 	}
 	hints.ai_family = AF_UNSPEC;
 	hints.ai_flags = AI_NUMERICSERV | AI_NUMERICHOST;
 
-	ret = getaddrinfo(server, "53", &hints, &rp);
+	ret = getaddrinfo(data->server, "53", &hints, &rp);
 	if (ret) {
 		connman_error("Failed to parse server %s address: %s\n",
-			      server, gai_strerror(ret));
-		return NULL;
+			      data->server, gai_strerror(ret));
+		err = EINVAL;
+		goto error_out;
 	}
 	/* Do not blindly copy this code elsewhere; it doesn't loop over the
 	   results using ->ai_next as it should. That's OK in *this* case
@@ -2067,45 +2081,37 @@ static struct server_data *create_server(const char *interface,
 
 	sk = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol);
 	if (sk < 0) {
-		connman_error("Failed to create server %s socket", server);
-		freeaddrinfo(rp);
-		return NULL;
+		err = errno;
+		connman_error("Failed to create server %s socket",
+							data->server);
+		goto error_out;
 	}
 
 	DBG("sk %d", sk);
 
-	if (interface != NULL) {
-		if (setsockopt(sk, SOL_SOCKET, SO_BINDTODEVICE,
-				interface, strlen(interface) + 1) < 0) {
+	if (data->interface != NULL) {
+		ret = setsockopt(sk, SOL_SOCKET, SO_BINDTODEVICE,
+				data->interface, strlen(data->interface) + 1);
+		if (ret < 0) {
+			err = errno;
 			connman_error("Failed to bind server %s "
 						"to interface %s",
-							server, interface);
-			freeaddrinfo(rp);
-			close(sk);
-			return NULL;
+						data->server, data->interface);
+			goto error_out;
 		}
 	}
 
-	data = g_try_new0(struct server_data, 1);
-	if (data == NULL) {
-		connman_error("Failed to allocate server %s data", server);
-		freeaddrinfo(rp);
-		close(sk);
-		return NULL;
-	}
-
 	data->channel = g_io_channel_unix_new(sk);
 	if (data->channel == NULL) {
-		connman_error("Failed to create server %s channel", server);
-		freeaddrinfo(rp);
-		close(sk);
-		g_free(data);
-		return NULL;
+		err = ENOMEM;
+		connman_error("Failed to create server %s channel",
+							data->server);
+		goto error_out;
 	}
 
 	g_io_channel_set_close_on_unref(data->channel, TRUE);
 
-	if (protocol == IPPROTO_TCP) {
+	if (data->protocol == IPPROTO_TCP) {
 		g_io_channel_set_flags(data->channel, G_IO_FLAG_NONBLOCK, NULL);
 		data->watch = g_io_add_watch(data->channel,
 			G_IO_OUT | G_IO_IN | G_IO_HUP | G_IO_NVAL | G_IO_ERR,
@@ -2117,61 +2123,39 @@ static struct server_data *create_server(const char *interface,
 			G_IO_IN | G_IO_NVAL | G_IO_ERR | G_IO_HUP,
 						udp_server_event, data);
 
-	data->interface = g_strdup(interface);
-	if (domain)
-		data->domains = g_list_append(data->domains, g_strdup(domain));
-	data->server = g_strdup(server);
-	data->protocol = protocol;
 	data->server_addr_len = rp->ai_addrlen;
 
 	switch (rp->ai_family) {
 	case AF_INET:
 		data->server_addr = (struct sockaddr *)
 					g_try_new0(struct sockaddr_in, 1);
+		err = ENOMEM;
 		break;
 	case AF_INET6:
 		data->server_addr = (struct sockaddr *)
 					g_try_new0(struct sockaddr_in6, 1);
+		err = ENOMEM;
 		break;
 	default:
 		connman_error("Wrong address family %d", rp->ai_family);
+		err = EINVAL;
 		break;
 	}
 	if (data->server_addr == NULL) {
-		freeaddrinfo(rp);
-		close(sk);
-		g_free(data);
-		return NULL;
+		goto error_out;
 	}
 	memcpy(data->server_addr, rp->ai_addr, rp->ai_addrlen);
 
 	ret = connect(sk, rp->ai_addr, rp->ai_addrlen);
-	freeaddrinfo(rp);
 	if (ret < 0) {
-		if ((protocol == IPPROTO_TCP && errno != EINPROGRESS) ||
-				protocol == IPPROTO_UDP) {
-			GList *list;
-
-			connman_error("Failed to connect to server %s", server);
-			if (data->watch > 0)
-				g_source_remove(data->watch);
-			if (data->timeout > 0)
-				g_source_remove(data->timeout);
-
-			g_io_channel_unref(data->channel);
-			close(sk);
-
-			g_free(data->server);
-			g_free(data->interface);
-			for (list = data->domains; list; list = list->next) {
-				char *tmp_domain = list->data;
-
-				data->domains = g_list_remove(data->domains,
-								tmp_domain);
-				g_free(tmp_domain);
-			}
-			g_free(data);
-			return NULL;
+		err = errno;
+
+		if ((data->protocol == IPPROTO_TCP && errno != EINPROGRESS) ||
+				data->protocol == IPPROTO_UDP) {
+
+			connman_error("Failed to connect to server %s",
+								data->server);
+			goto error_out;
 		}
 	}
 
@@ -2181,6 +2165,68 @@ static struct server_data *create_server(const char *interface,
 					NULL,
 					cache_element_destroy);
 
+	freeaddrinfo(rp);
+
+	return 0;
+
+error_out:
+	if (data->watch > 0) {
+		g_source_remove(data->watch);
+		data->watch = 0;
+	}
+	if (data->timeout > 0) {
+		g_source_remove(data->timeout);
+		data->timeout = 0;
+	}
+
+	g_io_channel_unref(data->channel);
+
+	if (rp != NULL)
+		freeaddrinfo(rp);
+	if (sk > 0)
+		close(sk);
+
+	return -err;
+}
+
+static struct server_data *create_server(const char *interface,
+					const char *domain, const char *server,
+					int protocol)
+{
+	struct server_data *data;
+
+	DBG("interface %s server %s", interface, server);
+
+	data = g_try_new0(struct server_data, 1);
+	if (data == NULL) {
+		connman_error("Failed to allocate server %s data", server);
+		return NULL;
+	}
+
+	data->interface = g_strdup(interface);
+	if (domain)
+		data->domains = g_list_append(data->domains, g_strdup(domain));
+	data->server = g_strdup(server);
+	data->protocol = protocol;
+
+	if (server_create_socket(data) != 0) {
+		GList *list;
+
+		g_free(data->interface);
+
+		for (list = data->domains; list; list = list->next) {
+			char *tmp_domain = list->data;
+
+			data->domains = g_list_remove(data->domains,
+							tmp_domain);
+			g_free(tmp_domain);
+		}
+
+		g_free(data->server);
+		g_free(data);
+		return NULL;
+	}
+
 	if (protocol == IPPROTO_UDP) {
 		/* Enable new servers by default */
 		data->enabled = TRUE;
@@ -2205,10 +2251,8 @@ static gboolean resolv(struct request_data *req,
 		if (data->enabled == FALSE)
 			continue;
 
-		if (data->watch == 0 && data->protocol == IPPROTO_UDP)
-			data->watch = g_io_add_watch(data->channel,
-				G_IO_IN | G_IO_NVAL | G_IO_ERR | G_IO_HUP,
-						udp_server_event, data);
+		if (data->channel == NULL && data->protocol == IPPROTO_UDP)
+			server_create_socket(data);
 
 		if (ns_resolv(data, req, request, name) > 0)
 			return TRUE;
-- 
1.7.0.4



More information about the connman mailing list