conncheck: don't ignore local socket errors

With this patch we ensure that local socket errors during connection
establishment are properly transmitted to the connection check layer, so
the related pair can be put in state failed when needed. The local
socket errors we are interested in, are those occuring when the local
source address cannot be bound anymore, because the underlying interface
vanished for example.

In particular, we don't ignore errors coming from g_socket_bind() with
tcp active sockets, that perform a bind to *whatever* local address is
available, in case it cannot bind to the specified address.

This behaviour was demonstrated with the test-tcp example, that tried
to bind to the IPv4 loopback address on a socket initially created for
the IPv6 loopback.

5 files changed, 61 insertions, 20 deletions

diff --git a/agent/conncheck.c b/agent/conncheck.c
index 53faba3..cd8f9cf 100644
--- a/agent/conncheck.c
+++ b/agent/conncheck.c
@@ -2975,12 +2975,18 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair)
             component2);
 
         nice_component_attach_socket (component2, new_socket);
+      } else {
+        priv_remove_stun_transaction (pair, stun, component);
+        return -1;
       }
     }
   }
   /* send the conncheck */
-  agent_socket_send (pair->sockptr, &pair->remote->addr,
-      buffer_len, (gchar *)stun->buffer);
+  if (agent_socket_send (pair->sockptr, &pair->remote->addr,
+      buffer_len, (gchar *)stun->buffer) < 0) {
+    priv_remove_stun_transaction (pair, stun, component);
+    return -1;
+  }
 
   if (agent->compatibility == NICE_COMPATIBILITY_OC2007R2)
     ms_ice2_legacy_conncheck_send (&stun->message, pair->sockptr,
diff --git a/socket/tcp-active.c b/socket/tcp-active.c
index eb7cd7f..558b303 100644
--- a/socket/tcp-active.c
+++ b/socket/tcp-active.c
@@ -41,6 +41,7 @@
 
 #include "socket.h"
 #include "tcp-active.h"
+#include "agent-priv.h"
 
 #include <string.h>
 #include <errno.h>
@@ -192,7 +193,14 @@ nice_tcp_active_socket_connect (NiceSocket *sock, NiceAddress *addr)
   gboolean gret = FALSE;
   GSocketAddress *gaddr;
   NiceAddress local_addr;
+  NiceAddress remote_addr;
   NiceSocket *new_socket = NULL;
+  union {
+    struct sockaddr_storage ss;
+    struct sockaddr sa;
+  } sa;
+  char remote_addr_str[INET6_ADDRSTRLEN];
+  char local_addr_str[INET6_ADDRSTRLEN];
 
   if (addr == NULL) {
     /* We can't connect a tcp socket with no destination address */
@@ -234,29 +242,27 @@ nice_tcp_active_socket_connect (NiceSocket *sock, NiceAddress *addr)
   /* setting TCP_NODELAY to TRUE in order to avoid packet batching */
   g_socket_set_option (gsock, IPPROTO_TCP, TCP_NODELAY, TRUE, NULL);
 
-  /* Allow g_socket_bind to fail */
-  g_socket_bind (gsock, priv->local_addr, FALSE, NULL);
+  gret = g_socket_bind (gsock, priv->local_addr, FALSE, &gerr);
+
+  if (gret == FALSE) {
+    if (g_error_matches (gerr, G_IO_ERROR, G_IO_ERROR_PENDING) == FALSE)
+      goto error;
+    g_clear_error (&gerr);
+  }
 
   gret = g_socket_connect (gsock, gaddr, NULL, &gerr);
   g_object_unref (gaddr);
 
   if (gret == FALSE) {
-    if (g_error_matches (gerr, G_IO_ERROR, G_IO_ERROR_PENDING) == FALSE) {
-      g_error_free (gerr);
-      g_socket_close (gsock, NULL);
-      g_object_unref (gsock);
-      return NULL;
-    }
+    if (g_error_matches (gerr, G_IO_ERROR, G_IO_ERROR_PENDING) == FALSE)
+      goto error;
     g_error_free (gerr);
   }
 
   gaddr = g_socket_get_local_address (gsock, NULL);
   if (gaddr == NULL ||
-      !g_socket_address_to_native (gaddr, &name.addr, sizeof (name), NULL)) {
-    g_socket_close (gsock, NULL);
-    g_object_unref (gsock);
-    return NULL;
-  }
+      !g_socket_address_to_native (gaddr, &name.addr, sizeof (name), NULL))
+    goto error2;
   g_object_unref (gaddr);
 
   nice_address_set_from_sockaddr (&local_addr, &name.addr);
@@ -266,4 +272,26 @@ nice_tcp_active_socket_connect (NiceSocket *sock, NiceAddress *addr)
   g_object_unref (gsock);
 
   return new_socket;
+
+error:
+  g_socket_address_to_native (gaddr, &sa, sizeof (sa), NULL);
+  nice_address_set_from_sockaddr (&remote_addr, &sa.sa);
+  nice_address_to_string (&remote_addr, remote_addr_str);
+
+  g_socket_address_to_native (priv->local_addr, &sa, sizeof (sa), NULL);
+  nice_address_set_from_sockaddr (&local_addr, &sa.sa);
+  nice_address_to_string (&local_addr, local_addr_str);
+
+  nice_debug ("%s: tcp-active socket %p %s:%u -> %s:%u: error: %s",
+      G_STRFUNC, sock,
+      local_addr_str, nice_address_get_port (&local_addr),
+      remote_addr_str, nice_address_get_port (&remote_addr),
+      gerr->message);
+
+  g_error_free (gerr);
+
+error2:
+  g_socket_close (gsock, NULL);
+  g_object_unref (gsock);
+  return NULL;
 }
diff --git a/socket/udp-bsd.c b/socket/udp-bsd.c
index d1c8513..7064408 100644
--- a/socket/udp-bsd.c
+++ b/socket/udp-bsd.c
@@ -335,7 +335,7 @@ socket_send_messages (NiceSocket *sock, const NiceAddress *to,
       nice_address_to_string (&local_addr, local_addr_str);
       g_object_unref (gsocket);
 
-      nice_debug_verbose ("%s: udp-bsd socket %p %s:%u -> %s:%u: error: %s",
+      nice_debug ("%s: udp-bsd socket %p %s:%u -> %s:%u: error: %s",
           G_STRFUNC, sock,
           local_addr_str, nice_address_get_port (&local_addr),
           remote_addr_str, nice_address_get_port (&remote_addr),
diff --git a/socket/udp-turn.c b/socket/udp-turn.c
index 1e86a0a..c1dac62 100644
--- a/socket/udp-turn.c
+++ b/socket/udp-turn.c
@@ -948,7 +948,8 @@ socket_send_message (NiceSocket *sock, const NiceAddress *to,
     if (priv->compatibility == NICE_TURN_SOCKET_COMPATIBILITY_RFC5766 &&
         !priv_has_permission_for_peer (priv, to)) {
       if (!priv_has_sent_permission_for_peer (priv, to)) {
-        priv_send_create_permission (priv, to);
+        if (!priv_send_create_permission (priv, to))
+          goto error;
       }
 
       /* enque data */
@@ -2012,7 +2013,7 @@ priv_send_create_permission(UdpTurnPriv *priv,
     const NiceAddress *peer)
 {
   guint msg_buf_len;
-  gboolean res = FALSE;
+  gssize res = 0;
   TURNMessage *msg = g_new0 (TURNMessage, 1);
   union {
     struct sockaddr_storage storage;
@@ -2051,6 +2052,11 @@ priv_send_create_permission(UdpTurnPriv *priv,
             msg_buf_len, (gchar *) msg->buffer, FALSE);
     }
 
+    if (res < 0) {
+      g_free(msg);
+      goto done;
+    }
+
     if (nice_socket_is_reliable (priv->base_socket)) {
       stun_timer_start_reliable (&msg->timer,
         STUN_TIMER_DEFAULT_RELIABLE_TIMEOUT);
@@ -2065,7 +2071,8 @@ priv_send_create_permission(UdpTurnPriv *priv,
     g_free(msg);
   }
 
-  return res;
+done:
+  return (res > 0);
 }
 
 static gboolean
diff --git a/tests/test-tcp.c b/tests/test-tcp.c
index f94ac88..fb38943 100644
--- a/tests/test-tcp.c
+++ b/tests/test-tcp.c
@@ -93,7 +93,7 @@ main (void)
   mainloop = g_main_loop_new (NULL, FALSE);
 
   nice_address_init (&active_bind_addr);
-  g_assert_true (nice_address_set_from_string (&active_bind_addr, "::1"));
+  g_assert_true (nice_address_set_from_string (&active_bind_addr, "127.0.0.1"));
 
   nice_address_init (&passive_bind_addr);
   g_assert_true (nice_address_set_from_string (&passive_bind_addr, "127.0.0.1"));