Browse code

Refactor setting close-on-exec for socket FDs

The existing code can leak socket FDs to the "--up" script, which is
not desired. Brought up by Alberto Gonzalez Iniesta, based on debian
bug 367716.

Since different sockets get create at different times, just moving the
set_cloexec() to link_socket_init_phase1() is not good enough - so move
the call into create_socket_<family>(), so we will catch ALL socket
creations, no matter when or under which conditions they will be
created (SOCKS proxy socket, listening socket, ...).

--inetd gets an extra fd_cloexec() call, as socket FD is inherited.

URL: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=367716

v2: remove set_cloexec() calls from manage.c

v3: add set_cloexec() calls to accept()ed TCP/unix child sockets

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1481027162-12165-1-git-send-email-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13405.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Gert Doering authored on 2016/12/06 21:26:02
Showing 2 changed files
... ...
@@ -1499,7 +1499,6 @@ man_new_connection_post (struct management *man, const char *description)
1499 1499
   struct gc_arena gc = gc_new ();
1500 1500
 
1501 1501
   set_nonblock (man->connection.sd_cli);
1502
-  set_cloexec (man->connection.sd_cli);
1503 1502
 
1504 1503
   man_connection_settings_reset (man);
1505 1504
 
... ...
@@ -1640,7 +1639,6 @@ man_listen (struct management *man)
1640 1640
        * Set misc socket properties
1641 1641
        */
1642 1642
       set_nonblock (man->connection.sd_top);
1643
-      set_cloexec (man->connection.sd_top);
1644 1643
 
1645 1644
 #if UNIX_SOCK_SUPPORT
1646 1645
       if (man->settings.flags & MF_UNIX_SOCK)
... ...
@@ -771,6 +771,10 @@ create_socket_tcp (struct addrinfo* addrinfo)
771 771
   }
772 772
 #endif
773 773
 
774
+  /* set socket file descriptor to not pass across execs, so that
775
+     scripts don't have access to it */
776
+  set_cloexec (sd);
777
+
774 778
   return sd;
775 779
 }
776 780
 
... ...
@@ -815,6 +819,11 @@ create_socket_udp (struct addrinfo* addrinfo, const unsigned int flags)
815 815
         }
816 816
     }
817 817
 #endif
818
+
819
+  /* set socket file descriptor to not pass across execs, so that
820
+     scripts don't have access to it */
821
+  set_cloexec (sd);
822
+
818 823
   return sd;
819 824
 }
820 825
 
... ...
@@ -968,6 +977,12 @@ socket_do_accept (socket_descriptor_t sd,
968 968
       openvpn_close_socket (new_sd);
969 969
       new_sd = SOCKET_UNDEFINED;
970 970
     }
971
+  else
972
+    {
973
+      /* set socket file descriptor to not pass across execs, so that
974
+	 scripts don't have access to it */
975
+      set_cloexec (sd);
976
+    }
971 977
   return new_sd;
972 978
 }
973 979
 
... ...
@@ -1617,6 +1632,7 @@ link_socket_init_phase1 (struct link_socket *sock,
1617 1617
       ASSERT (sock->info.proto != PROTO_TCP_CLIENT);
1618 1618
       ASSERT (socket_defined (inetd_socket_descriptor));
1619 1619
       sock->sd = inetd_socket_descriptor;
1620
+      set_cloexec (sock->sd);		/* not created by create_socket*() */
1620 1621
     }
1621 1622
   else if (mode != LS_MODE_TCP_ACCEPT_FROM)
1622 1623
     {
... ...
@@ -1677,13 +1693,6 @@ phase2_set_socket_flags (struct link_socket* sock)
1677 1677
   /* set socket to non-blocking mode */
1678 1678
   set_nonblock (sock->sd);
1679 1679
 
1680
-  /* set socket file descriptor to not pass across execs, so that
1681
-     scripts don't have access to it */
1682
-  set_cloexec (sock->sd);
1683
-
1684
-  if (socket_defined (sock->ctrl_sd))
1685
-    set_cloexec (sock->ctrl_sd);
1686
-
1687 1680
   /* set Path MTU discovery options on the socket */
1688 1681
   set_mtu_discover_type (sock->sd, sock->mtu_discover_type, sock->info.af);
1689 1682
 
... ...
@@ -3476,6 +3485,11 @@ create_socket_unix (void)
3476 3476
 
3477 3477
   if ((sd = socket (PF_UNIX, SOCK_STREAM, 0)) < 0)
3478 3478
     msg (M_ERR, "Cannot create unix domain socket");
3479
+
3480
+  /* set socket file descriptor to not pass across execs, so that
3481
+     scripts don't have access to it */
3482
+  set_cloexec (sd);
3483
+
3479 3484
   return sd;
3480 3485
 }
3481 3486
 
... ...
@@ -3516,6 +3530,12 @@ socket_accept_unix (socket_descriptor_t sd,
3516 3516
 
3517 3517
   CLEAR (*remote);
3518 3518
   ret = accept (sd, (struct sockaddr *) remote, &remote_len);
3519
+  if ( ret >= 0 )
3520
+    {
3521
+      /* set socket file descriptor to not pass across execs, so that
3522
+	 scripts don't have access to it */
3523
+      set_cloexec (ret);
3524
+    }
3519 3525
   return ret;
3520 3526
 }
3521 3527