Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1915?usp=email

to look at the new patch set (#2).


Change subject: socket: let an optional UDP socket give up on a failed bind
......................................................................

socket: let an optional UDP socket give up on a failed bind

create_socket_udp_configured()'s "optional" argument covers only the
socket() call: socket_bind() is fatal, so a caller that can do without
the socket still dies if the address is unavailable.

That is a problem for --server-probe, which binds a probe socket for
every address family in use, while a connection binds only the family of
the remote it is about to try. A --lport whose IPv6 port happens to be
taken by another process therefore kills a client that would have
connected over IPv4.

Give socket_bind() a msglevel and a return value, and let "optional"
apply to the bind as well: the socket is closed and SOCKET_UNDEFINED is
returned, so the caller can skip that address family. Every existing
caller passes M_FATAL and keeps its behaviour.

Change-Id: I24e5925a72aaede53ef9e863fe96fcd7363df402
Signed-off-by: Lev Stipakov <[email protected]>
---
M src/openvpn/manage.c
M src/openvpn/socket.c
M src/openvpn/socket.h
3 files changed, 32 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/15/1915/2

diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index e121b38..46b0abe 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -2044,7 +2044,7 @@
         {
             man->connection.sd_top = create_socket_tcp(man->settings.local);
             socket_bind(man->connection.sd_top, man->settings.local, 
man->settings.local->ai_family,
-                        "MANAGEMENT", false);
+                        "MANAGEMENT", false, M_FATAL);
         }

         /*
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 41c8bac..4c0aab5 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -627,12 +627,13 @@
     {
         if (sock->socks_proxy && sock->info.proto == PROTO_UDP)
         {
-            socket_bind(sock->ctrl_sd, sock->info.lsa->bind_local, 
sock->info.af, "SOCKS", false);
+            socket_bind(sock->ctrl_sd, sock->info.lsa->bind_local, 
sock->info.af, "SOCKS", false,
+                        M_FATAL);
         }
         else
         {
             socket_bind(sock->sd, sock->info.lsa->bind_local, sock->info.af, 
"TCP/UDP",
-                        sock->info.bind_ipv6_only);
+                        sock->info.bind_ipv6_only, M_FATAL);
         }
     }
 }
@@ -678,9 +679,11 @@
     }

     socket_apply_options(sd, sbs, mark, bind_dev);
-    if (bind_addr)
+    if (bind_addr
+        && !socket_bind(sd, bind_addr, af, "TCP/UDP", bind_ipv6_only, optional 
? D_LOW : M_FATAL))
     {
-        socket_bind(sd, bind_addr, af, "TCP/UDP", bind_ipv6_only);
+        openvpn_close_socket(sd);
+        return SOCKET_UNDEFINED;
     }
     return sd;
 }
@@ -914,9 +917,9 @@
     return new_sd;
 }

-void
+bool
 socket_bind(socket_descriptor_t sd, struct addrinfo *local, int ai_family, 
const char *prefix,
-            bool ipv6only)
+            bool ipv6only, msglvl_t msglevel)
 {
     struct gc_arena gc = gc_new();

@@ -940,8 +943,10 @@
     }
     if (!cur)
     {
-        msg(M_FATAL, "%s: Socket bind failed: Addr to bind has no %s record", 
prefix,
+        msg(msglevel, "%s: Socket bind failed: Addr to bind has no %s record", 
prefix,
             addr_family_name(ai_family));
+        gc_free(&gc);
+        return false;
     }

     if (ai_family == AF_INET6)
@@ -956,10 +961,13 @@
     }
     if (openvpn_bind(sd, cur->ai_addr, cur->ai_addrlen))
     {
-        msg(M_FATAL | M_ERRNO, "%s: Socket bind failed on local address %s", 
prefix,
+        msg(msglevel | M_ERRNO, "%s: Socket bind failed on local address %s", 
prefix,
             print_sockaddr_ex(local->ai_addr, ":", PS_SHOW_PORT, &gc));
+        gc_free(&gc);
+        return false;
     }
     gc_free(&gc);
+    return true;
 }

 int
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index 46fa29c..4db3f3b 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -348,8 +348,15 @@

 struct link_socket *link_socket_new(void);

-void socket_bind(socket_descriptor_t sd, struct addrinfo *local, int 
af_family, const char *prefix,
-                 bool ipv6only);
+/**
+ * Bind \p sd to the first \p af_family address of \p local. A failure is
+ * reported at \p msglevel, so a caller that can do without the bind passes a
+ * non-fatal level and checks the result.
+ *
+ * @return whether the socket was bound
+ */
+bool socket_bind(socket_descriptor_t sd, struct addrinfo *local, int 
af_family, const char *prefix,
+                 bool ipv6only, msglvl_t msglevel);

 int openvpn_connect(socket_descriptor_t sd, const struct sockaddr *remote, int 
connect_timeout,
                     volatile int *signal_received);
@@ -402,12 +409,12 @@
 socket_descriptor_t create_socket_tcp(struct addrinfo *);

 /**
- * Create a UDP socket for @p af set up the way a link socket is: 
--sndbuf/--rcvbuf,
- * --mark, --bind-dev and, when @p bind_addr is given, the local bind 
(IPV6_V6ONLY
- * per @p bind_ipv6_only). Shared by create_socket() and the --server-probe 
sockets,
- * so a probe socket is the connection socket it may become. A failed bind is
- * fatal; with @p optional a socket the host cannot create is not, so
- * --server-probe can skip that address family (SOCKET_UNDEFINED is returned).
+ * Create a UDP socket for \p af set up the way a link socket is: 
--sndbuf/--rcvbuf,
+ * --mark, --bind-dev and, when \p bind_addr is given, the local bind 
(IPV6_V6ONLY
+ * per \p bind_ipv6_only). Shared by create_socket() and the --server-probe 
sockets,
+ * so a probe socket is the connection socket it may become. With \p optional a
+ * socket the host cannot create or bind is not fatal, so --server-probe can 
skip
+ * that address family (SOCKET_UNDEFINED is returned).
  */
 socket_descriptor_t create_socket_udp_configured(sa_family_t af, unsigned int 
sockflags,
                                                  const struct 
socket_buffer_size *sbs, int mark,

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1915?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I24e5925a72aaede53ef9e863fe96fcd7363df402
Gerrit-Change-Number: 1915
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to