Make file descriptors from stdlib non-inheritable by default (#13201)

* io: make file descriptors non-inheritable by default

This prevents file descriptors/handles leakage to child processes
that might cause issues like running out of file descriptors, or potential
security issues like leaking a file descriptor to a restricted file.

While this breaks backward compatibility, I'm rather certain that not
many programs (if any) actually make use of this implementation detail.
A new API `setInheritable` is provided for the few that actually want to
use this functionality.

* io: disable inheritance at file creation time for supported platforms

Some platforms provide extension to fopen-family of functions to allow
for disabling descriptor inheritance atomically during File creation.
This guards against possible leaks when a child process is spawned
before we managed to disable the file descriptor inheritance
(ie. in a multi-threaded program).

* net, nativesockets: make sockets non inheritable by default

With this commit, sockets will no longer leak to child processes when
you don't want it to. Should solves a lot of "address in use" that might
occur when your server has just restarted.

All APIs that create sockets in these modules now expose a `inheritable`
flag that allow users to toggle inheritance for the resulting sockets.
An implementation of `setInheritance()` is also provided for SocketHandle.

While atomically disabling inheritance at creation time is supported on
Windows, it's only implemented by native winsock2, which is too much for
now. This support can be implemented in a future patch.

* posix: add F_DUPFD_CLOEXEC

This command duplicates file descriptor with close-on-exec flag set.

Defined in POSIX.1-2008.

* ioselectors_kqueue: don't leak file descriptors

File descriptors internally used by ioselectors on BSD/OSX are now
shielded from leakage.

* posix: add O_CLOEXEC

This flag allows file descriptors to be open() with close-on-exec flag
set atomically.

This flag is specified in POSIX.1-2008

* tfdleak: test for selectors leakage

Also simplified the test by using handle-type agnostic APIs to test for
validity.

* ioselectors_epoll: mark all fd created close-on-exec

File descriptors from ioselectors should no longer leaks on Linux.

* tfdleak: don't check for selector leakage on Windows

The getFd proc for ioselectors_select returns a hardcoded -1

* io: add NoInheritFlag at compile time

* io: add support for ioctl-based close-on-exec

This allows for the flag to be set/unset in one syscall. While the
performance gains might be negliable, we have one less failure point
to deal with.

* tfdleak: add a test for setInheritable

* stdlib: add nimInheritHandles to restore old behaviors

* memfiles: make file handle not inheritable by default for posix

* io: setInheritable now operates on OS file handle

On Windows, the native handle is the only thing that's inheritable, thus
we can assume that users of this function will already have the handle
available to them. This also allows users to pass down file descriptors
from memfiles on Windows with ease, should that be desired.

With this, nativesockets.setInheritable can be made much simpler.

* changelog: clarify

* nativesockets: document setInheritable return value

* posix_utils: atomically disable fd inheritance for mkstemp

5 files changed, 95 insertions, 38 deletions

diff --git a/lib/pure/ioselects/ioselectors_epoll.nim b/lib/pure/ioselects/ioselectors_epoll.nim
index bf13cc83e..83f6001ea 100644
--- a/lib/pure/ioselects/ioselectors_epoll.nim
+++ b/lib/pure/ioselects/ioselectors_epoll.nim
@@ -81,7 +81,7 @@ proc newSelector*[T](): Selector[T] =
   # Start with a reasonable size, checkFd() will grow this on demand
   const numFD = 1024
 
-  var epollFD = epoll_create(MAX_EPOLL_EVENTS)
+  var epollFD = epoll_create1(O_CLOEXEC)
   if epollFD < 0:
     raiseOSError(osLastError())
 
@@ -110,7 +110,7 @@ proc close*[T](s: Selector[T]) =
     raiseIOSelectorsError(osLastError())
 
 proc newSelectEvent*(): SelectEvent =
-  let fdci = eventfd(0, 0)
+  let fdci = eventfd(0, O_CLOEXEC)
   if fdci == -1:
     raiseIOSelectorsError(osLastError())
   setNonBlocking(fdci)
@@ -269,7 +269,7 @@ proc registerTimer*[T](s: Selector[T], timeout: int, oneshot: bool,
   var
     newTs: Itimerspec
     oldTs: Itimerspec
-  let fdi = timerfd_create(CLOCK_MONOTONIC, 0).int
+  let fdi = timerfd_create(CLOCK_MONOTONIC, O_CLOEXEC).int
   if fdi == -1:
     raiseIOSelectorsError(osLastError())
   setNonBlocking(fdi.cint)
@@ -314,7 +314,7 @@ when not defined(android):
     discard sigaddset(nmask, cint(signal))
     blockSignals(nmask, omask)
 
-    let fdi = signalfd(-1, nmask, 0).int
+    let fdi = signalfd(-1, nmask, O_CLOEXEC).int
     if fdi == -1:
       raiseIOSelectorsError(osLastError())
     setNonBlocking(fdi.cint)
@@ -341,7 +341,7 @@ when not defined(android):
     discard sigaddset(nmask, posix.SIGCHLD)
     blockSignals(nmask, omask)
 
-    let fdi = signalfd(-1, nmask, 0).int
+    let fdi = signalfd(-1, nmask, O_CLOEXEC).int
     if fdi == -1:
       raiseIOSelectorsError(osLastError())
     setNonBlocking(fdi.cint)
diff --git a/lib/pure/ioselects/ioselectors_kqueue.nim b/lib/pure/ioselects/ioselectors_kqueue.nim
index 83e15d479..7635a04d5 100644
--- a/lib/pure/ioselects/ioselectors_kqueue.nim
+++ b/lib/pure/ioselects/ioselectors_kqueue.nim
@@ -9,7 +9,7 @@
 
 #  This module implements BSD kqueue().
 
-import posix, times, kqueue
+import posix, times, kqueue, nativesockets
 
 const
   # Maximum number of events that can be returned.
@@ -76,7 +76,7 @@ type
 
 proc getUnique[T](s: Selector[T]): int {.inline.} =
   # we create duplicated handles to get unique indexes for our `fds` array.
-  result = posix.fcntl(s.sock, F_DUPFD, s.sock)
+  result = posix.fcntl(s.sock, F_DUPFD_CLOEXEC, s.sock)
   if result == -1:
     raiseIOSelectorsError(osLastError())
 
@@ -96,8 +96,8 @@ proc newSelector*[T](): owned(Selector[T]) =
   # we allocating empty socket to duplicate it handle in future, to get unique
   # indexes for `fds` array. This is needed to properly identify
   # {Event.Timer, Event.Signal, Event.Process} events.
-  let usock = posix.socket(posix.AF_INET, posix.SOCK_STREAM,
-                             posix.IPPROTO_TCP).cint
+  let usock = createNativeSocket(posix.AF_INET, posix.SOCK_STREAM,
+                                 posix.IPPROTO_TCP).cint
   if usock == -1:
     let err = osLastError()
     discard posix.close(kqFD)
diff --git a/lib/pure/memfiles.nim b/lib/pure/memfiles.nim
index 152b03ec6..dab5483e4 100644
--- a/lib/pure/memfiles.nim
+++ b/lib/pure/memfiles.nim
@@ -228,7 +228,7 @@ proc open*(filename: string, mode: FileMode = fmRead,
       if result.handle != -1: discard close(result.handle)
       raiseOSError(errCode)
 
-    var flags = if readonly: O_RDONLY else: O_RDWR
+    var flags = (if readonly: O_RDONLY else: O_RDWR) or O_CLOEXEC
 
     if newFileSize != -1:
       flags = flags or O_CREAT or O_TRUNC
diff --git a/lib/pure/nativesockets.nim b/lib/pure/nativesockets.nim
index d9256a921..8d0d6426d 100644
--- a/lib/pure/nativesockets.nim
+++ b/lib/pure/nativesockets.nim
@@ -185,19 +185,53 @@ proc toSockType*(protocol: Protocol): SockType =
   of IPPROTO_IP, IPPROTO_IPV6, IPPROTO_RAW, IPPROTO_ICMP, IPPROTO_ICMPV6:
     SOCK_RAW
 
-proc createNativeSocket*(domain: Domain = AF_INET,
-                      sockType: SockType = SOCK_STREAM,
-                      protocol: Protocol = IPPROTO_TCP): SocketHandle =
-  ## Creates a new socket; returns `osInvalidSocket` if an error occurs.
-  socket(toInt(domain), toInt(sockType), toInt(protocol))
+proc close*(socket: SocketHandle) =
+  ## closes a socket.
+  when useWinVersion:
+    discard winlean.closesocket(socket)
+  else:
+    discard posix.close(socket)
+  # TODO: These values should not be discarded. An OSError should be raised.
+  # http://stackoverflow.com/questions/12463473/what-happens-if-you-call-close-on-a-bsd-socket-multiple-times
 
-proc createNativeSocket*(domain: cint, sockType: cint,
-                      protocol: cint): SocketHandle =
+when declared(setInheritable) or defined(nimdoc):
+  proc setInheritable*(s: SocketHandle, inheritable: bool): bool {.inline.} =
+    ## Set whether a socket is inheritable by child processes. Returns `true`
+    ## on success.
+    ##
+    ## This function is not implemented on all platform, test for availability
+    ## with `declared() <system.html#declared,untyped>`.
+    setInheritable(FileHandle s, inheritable)
+
+proc createNativeSocket*(domain: cint, sockType: cint, protocol: cint,
+                         inheritable: bool = defined(nimInheritHandles)): SocketHandle =
   ## Creates a new socket; returns `osInvalidSocket` if an error occurs.
   ##
+  ## `inheritable` decides if the resulting SocketHandle can be inherited
+  ## by child processes.
+  ##
   ## Use this overload if one of the enums specified above does
   ## not contain what you need.
-  socket(domain, sockType, protocol)
+  let sockType =
+    when (defined(linux) or defined(bsd)) and not defined(nimdoc):
+      if inheritable: sockType and not SOCK_CLOEXEC else: sockType or SOCK_CLOEXEC
+    else:
+      sockType
+  result = socket(domain, sockType, protocol)
+  when declared(setInheritable) and not (defined(linux) or defined(bsd)):
+    if not setInheritable(result, inheritable):
+      close result
+      return osInvalidSocket
+
+proc createNativeSocket*(domain: Domain = AF_INET,
+                         sockType: SockType = SOCK_STREAM,
+                         protocol: Protocol = IPPROTO_TCP,
+                         inheritable: bool = defined(nimInheritHandles)): SocketHandle =
+  ## Creates a new socket; returns `osInvalidSocket` if an error occurs.
+  ##
+  ## `inheritable` decides if the resulting SocketHandle can be inherited
+  ## by child processes.
+  createNativeSocket(toInt(domain), toInt(sockType), toInt(protocol))
 
 proc newNativeSocket*(domain: Domain = AF_INET,
                       sockType: SockType = SOCK_STREAM,
@@ -215,15 +249,6 @@ proc newNativeSocket*(domain: cint, sockType: cint,
   ## not contain what you need.
   createNativeSocket(domain, sockType, protocol)
 
-proc close*(socket: SocketHandle) =
-  ## closes a socket.
-  when useWinVersion:
-    discard winlean.closesocket(socket)
-  else:
-    discard posix.close(socket)
-  # TODO: These values should not be discarded. An OSError should be raised.
-  # http://stackoverflow.com/questions/12463473/what-happens-if-you-call-close-on-a-bsd-socket-multiple-times
-
 proc bindAddr*(socket: SocketHandle, name: ptr SockAddr,
     namelen: SockLen): cint =
   result = bindSocket(socket, name, namelen)
@@ -652,14 +677,25 @@ proc selectWrite*(writefds: var seq[SocketHandle],
 
   pruneSocketSet(writefds, (wr))
 
-proc accept*(fd: SocketHandle): (SocketHandle, string) =
+proc accept*(fd: SocketHandle, inheritable = defined(nimInheritHandles)): (SocketHandle, string) =
   ## Accepts a new client connection.
   ##
+  ## `inheritable` decides if the resulting SocketHandle can be inherited by
+  ## child processes.
+  ##
   ## Returns (osInvalidSocket, "") if an error occurred.
   var sockAddress: Sockaddr_in
   var addrLen = sizeof(sockAddress).SockLen
-  var sock = accept(fd, cast[ptr SockAddr](addr(sockAddress)),
-                    addr(addrLen))
+  var sock =
+    when (defined(linux) or defined(bsd)) and not defined(nimdoc):
+      accept4(fd, cast[ptr SockAddr](addr(sockAddress)), addr(addrLen),
+              if inheritable: 0 else: SOCK_CLOEXEC)
+    else:
+      accept(fd, cast[ptr SockAddr](addr(sockAddress)), addr(addrLen))
+  when declared(setInheritable) and not (defined(linux) or defined(bsd)):
+    if not setInheritable(sock, inheritable):
+      close sock
+      sock = osInvalidSocket
   if sock == osInvalidSocket:
     return (osInvalidSocket, "")
   else:
diff --git a/lib/pure/net.nim b/lib/pure/net.nim
index 593e5c76e..30355226c 100644
--- a/lib/pure/net.nim
+++ b/lib/pure/net.nim
@@ -212,22 +212,32 @@ proc newSocket*(fd: SocketHandle, domain: Domain = AF_INET,
   when defined(macosx) and not defined(nimdoc):
     setSockOptInt(fd, SOL_SOCKET, SO_NOSIGPIPE, 1)
 
-proc newSocket*(domain, sockType, protocol: cint, buffered = true): owned(Socket) =
+proc newSocket*(domain, sockType, protocol: cint, buffered = true,
+                inheritable = defined(nimInheritHandles)): owned(Socket) =
   ## Creates a new socket.
   ##
+  ## The SocketHandle associated with the resulting Socket will not be
+  ## inheritable by child processes by default. This can be changed via
+  ## the `inheritable` parameter.
+  ##
   ## If an error occurs OSError will be raised.
-  let fd = createNativeSocket(domain, sockType, protocol)
+  let fd = createNativeSocket(domain, sockType, protocol, inheritable)
   if fd == osInvalidSocket:
     raiseOSError(osLastError())
   result = newSocket(fd, domain.Domain, sockType.SockType, protocol.Protocol,
                      buffered)
 
 proc newSocket*(domain: Domain = AF_INET, sockType: SockType = SOCK_STREAM,
-                protocol: Protocol = IPPROTO_TCP, buffered = true): owned(Socket) =
+                protocol: Protocol = IPPROTO_TCP, buffered = true,
+                inheritable = defined(nimInheritHandles)): owned(Socket) =
   ## Creates a new socket.
   ##
+  ## The SocketHandle associated with the resulting Socket will not be
+  ## inheritable by child processes by default. This can be changed via
+  ## the `inheritable` parameter.
+  ##
   ## If an error occurs OSError will be raised.
-  let fd = createNativeSocket(domain, sockType, protocol)
+  let fd = createNativeSocket(domain, sockType, protocol, inheritable)
   if fd == osInvalidSocket:
     raiseOSError(osLastError())
   result = newSocket(fd, domain, sockType, protocol, buffered)
@@ -872,7 +882,8 @@ proc bindAddr*(socket: Socket, port = Port(0), address = "") {.
   freeaddrinfo(aiList)
 
 proc acceptAddr*(server: Socket, client: var owned(Socket), address: var string,
-                 flags = {SocketFlag.SafeDisconn}) {.
+                 flags = {SocketFlag.SafeDisconn},
+                 inheritable = defined(nimInheritHandles)) {.
                  tags: [ReadIOEffect], gcsafe, locks: 0.} =
   ## Blocks until a connection is being made from a client. When a connection
   ## is made sets ``client`` to the client socket and ``address`` to the address
@@ -882,19 +893,23 @@ proc acceptAddr*(server: Socket, client: var owned(Socket), address: var string,
   ## The resulting client will inherit any properties of the server socket. For
   ## example: whether the socket is buffered or not.
   ##
+  ## The SocketHandle associated with the resulting client will not be
+  ## inheritable by child processes by default. This can be changed via
+  ## the `inheritable` parameter.
+  ##
   ## The ``accept`` call may result in an error if the connecting socket
   ## disconnects during the duration of the ``accept``. If the ``SafeDisconn``
   ## flag is specified then this error will not be raised and instead
   ## accept will be called again.
   if client.isNil:
     new(client)
-  let ret = accept(server.fd)
+  let ret = accept(server.fd, inheritable)
   let sock = ret[0]
 
   if sock == osInvalidSocket:
     let err = osLastError()
     if flags.isDisconnectionError(err):
-      acceptAddr(server, client, address, flags)
+      acceptAddr(server, client, address, flags, inheritable)
     raiseOSError(err)
   else:
     address = ret[1]
@@ -964,10 +979,16 @@ when false: #defineSsl:
         doHandshake()
 
 proc accept*(server: Socket, client: var owned(Socket),
-             flags = {SocketFlag.SafeDisconn}) {.tags: [ReadIOEffect].} =
+             flags = {SocketFlag.SafeDisconn},
+             inheritable = defined(nimInheritHandles))
+            {.tags: [ReadIOEffect].} =
   ## Equivalent to ``acceptAddr`` but doesn't return the address, only the
   ## socket.
   ##
+  ## The SocketHandle associated with the resulting client will not be
+  ## inheritable by child processes by default. This can be changed via
+  ## the `inheritable` parameter.
+  ##
   ## The ``accept`` call may result in an error if the connecting socket
   ## disconnects during the duration of the ``accept``. If the ``SafeDisconn``
   ## flag is specified then this error will not be raised and instead