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

changelog.md

@@ -10,6 +10,25 @@
 - The default hash for `Ordinal` has changed to something more bit-scrambling.
   `import hashes; proc hash(x: myInt): Hash = hashIdentity(x)` recovers the old
   one in an instantiation context while `-d:nimIntHash1` recovers it globally.
+- File handles created from high-level abstractions in the stdlib will no longer
+  be inherited by child processes. In particular, these modules are affected:
+  `system`, `nativesockets`, `net` and `selectors`.
+
+  For `net` and `nativesockets`, an `inheritable` flag has been added to all
+  `proc`s that create sockets, allowing the user to control whether the
+  resulting socket is inheritable. This flag is provided to ease the writing of
+  multi-process servers, where sockets inheritance is desired.
+
+  For a transistion period, define `nimInheritHandles` to enable file handle
+  inheritance by default. This flag does **not** affect the `selectors` module
+  due to the differing semantics between operating systems.
+
+  `system.setInheritable` and `nativesockets.setInheritable` is also introduced
+  for setting file handle or socket inheritance. Not all platform have these
+  `proc`s defined.
+
+- The file descriptors created for internal bookkeeping by `ioselector_kqueue`
+  and `ioselector_epoll` will no longer be leaked to child processes.
 
 ## Language changes
 - In newruntime it is now allowed to assign discriminator field without restrictions as long as case object doesn't have custom destructor. Discriminator value doesn't have to be a constant either. If you have custom destructor for case object and you do want to freely assign discriminator fields, it is recommended to refactor object into 2 objects like this: 

lib/posix/posix.nim

@@ -896,6 +896,10 @@ proc `==`*(x, y: SocketHandle): bool {.borrow.}
 proc accept*(a1: SocketHandle, a2: ptr SockAddr, a3: ptr SockLen): SocketHandle {.
   importc, header: "<sys/socket.h>", sideEffect.}
 
+when defined(linux) or defined(bsd):
+  proc accept4*(a1: SocketHandle, a2: ptr SockAddr, a3: ptr SockLen,
+                flags: cint): SocketHandle {.importc, header: "<sys/socket.h>".}
+
 proc bindSocket*(a1: SocketHandle, a2: ptr SockAddr, a3: SockLen): cint {.
   importc: "bind", header: "<sys/socket.h>".}
   ## is Posix's ``bind``, because ``bind`` is a reserved word
@@ -1036,6 +1040,9 @@ proc mkstemp*(tmpl: cstring): cint {.importc, header: "<stdlib.h>", sideEffect.}
 
 proc mkdtemp*(tmpl: cstring): pointer {.importc, header: "<stdlib.h>", sideEffect.}
 
+when defined(linux) or defined(bsd):
+  proc mkostemp*(tmpl: cstring, oflags: cint): cint {.importc, header: "<stdlib.h>", sideEffect.}
+
 proc utimes*(path: cstring, times: ptr array[2, Timeval]): int {.
   importc: "utimes", header: "<sys/time.h>", sideEffect.}
   ## Sets file access and modification times.

lib/posix/posix_linux_amd64_consts.nim

@@ -100,6 +100,7 @@ const EXDEV* = cint(18)
 
 # <fcntl.h>
 const F_DUPFD* = cint(0)
+const F_DUPFD_CLOEXEC* = cint(1030)
 const F_GETFD* = cint(1)
 const F_SETFD* = cint(2)
 const F_GETFL* = cint(3)
@@ -126,6 +127,7 @@ const O_ACCMODE* = cint(3)
 const O_RDONLY* = cint(0)
 const O_RDWR* = cint(2)
 const O_WRONLY* = cint(1)
+const O_CLOEXEC* = cint(524288)
 const POSIX_FADV_NORMAL* = cint(0)
 const POSIX_FADV_SEQUENTIAL* = cint(2)
 const POSIX_FADV_RANDOM* = cint(1)
@@ -469,6 +471,7 @@ const SOCK_DGRAM* = cint(2)
 const SOCK_RAW* = cint(3)
 const SOCK_SEQPACKET* = cint(5)
 const SOCK_STREAM* = cint(1)
+const SOCK_CLOEXEC* = cint(524288)
 const SOL_SOCKET* = cint(1)
 const SOMAXCONN* = cint(128)
 const SO_REUSEPORT* = cint(15)

lib/posix/posix_macos_amd64.nim

@@ -557,6 +557,9 @@ when defined(linux) or defined(nimdoc):
 else:
   var SO_REUSEPORT* {.importc, header: "<sys/socket.h>".}: cint
 
+when defined(linux) or defined(bsd):
+  var SOCK_CLOEXEC* {.importc, header: "<sys/socket.h>".}: cint
+
 when defined(macosx):
   # We can't use the NOSIGNAL flag in the ``send`` function, it has no effect
   # Instead we should use SO_NOSIGPIPE in setsockopt

lib/posix/posix_openbsd_amd64.nim

@@ -532,6 +532,8 @@ when defined(nimdoc):
 else:
   var SO_REUSEPORT* {.importc, header: "<sys/socket.h>".}: cint
 
+var SOCK_CLOEXEC* {.importc, header: "<sys/socket.h>".}: cint
+
 var MSG_NOSIGNAL* {.importc, header: "<sys/socket.h>".}: cint
 
 when hasSpawnH:

lib/posix/posix_other.nim

@@ -562,6 +562,9 @@ when defined(linux) or defined(nimdoc):
 else:
   var SO_REUSEPORT* {.importc, header: "<sys/socket.h>".}: cint
 
+when defined(linux) or defined(bsd):
+  var SOCK_CLOEXEC* {.importc, header: "<sys/socket.h>".}: cint
+
 when defined(macosx):
   # We can't use the NOSIGNAL flag in the ``send`` function, it has no effect
   # Instead we should use SO_NOSIGPIPE in setsockopt

lib/posix/posix_other_consts.nim

@@ -99,6 +99,7 @@ var EXDEV* {.importc: "EXDEV", header: "<errno.h>".}: cint
 
 # <fcntl.h>
 var F_DUPFD* {.importc: "F_DUPFD", header: "<fcntl.h>".}: cint
+var F_DUPFD_CLOEXEC* {.importc: "F_DUPFD", header: "<fcntl.h>".}: cint
 var F_GETFD* {.importc: "F_GETFD", header: "<fcntl.h>".}: cint
 var F_SETFD* {.importc: "F_SETFD", header: "<fcntl.h>".}: cint
 var F_GETFL* {.importc: "F_GETFL", header: "<fcntl.h>".}: cint
@@ -125,6 +126,7 @@ var O_ACCMODE* {.importc: "O_ACCMODE", header: "<fcntl.h>".}: cint
 var O_RDONLY* {.importc: "O_RDONLY", header: "<fcntl.h>".}: cint
 var O_RDWR* {.importc: "O_RDWR", header: "<fcntl.h>".}: cint
 var O_WRONLY* {.importc: "O_WRONLY", header: "<fcntl.h>".}: cint
+var O_CLOEXEC* {.importc: "O_CLOEXEC", header: "<fcntl.h>".}: cint
 var POSIX_FADV_NORMAL* {.importc: "POSIX_FADV_NORMAL", header: "<fcntl.h>".}: cint
 var POSIX_FADV_SEQUENTIAL* {.importc: "POSIX_FADV_SEQUENTIAL", header: "<fcntl.h>".}: cint
 var POSIX_FADV_RANDOM* {.importc: "POSIX_FADV_RANDOM", header: "<fcntl.h>".}: cint

lib/posix/posix_utils.nim

@@ -83,7 +83,11 @@ proc mkstemp*(prefix: string): (string, File) =
   ## The file is created with perms 0600.
   ## Returns the filename and a file opened in r/w mode.
   var tmpl = cstring(prefix & "XXXXXX")
-  let fd = mkstemp(tmpl)
+  let fd =
+    when declared(mkostemp):
+      mkostemp(tmpl, O_CLOEXEC)
+    else:
+      mkstemp(tmpl)
   var f: File
   if open(f, fd, fmReadWrite):
     return ($tmpl, f)

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)

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)

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

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: