Merge pull request #276 from lucab/ups/listeners-always-unset

activation: remove unsafe support for fd re-use

Author: Luca Bruno

.travis.yml

@@ -10,7 +10,7 @@ env:
     - GOPATH=/opt
     - BUILD_DIR=/opt/src/github.com/coreos/go-systemd
   matrix:
-    - DOCKER_BASE=ubuntu:16.04
+    - DOCKER_BASE=ubuntu:18.04
     - DOCKER_BASE=debian:stretch
 
 before_install:

activation/files.go

@@ -22,11 +22,17 @@ import (
 	"syscall"
 )
 
-// based on: https://gist.github.com/alberts/4640792
 const (
+	// listenFdsStart corresponds to `SD_LISTEN_FDS_START`.
 	listenFdsStart = 3
 )
 
+// Files returns a slice containing a `os.File` object for each
+// file descriptor passed to this process via systemd fd-passing protocol.
+//
+// The order of the file descriptors is preserved in the returned slice.
+// `unsetEnv` is typically set to `true` in order to avoid clashes in
+// fd usage and to avoid leaking environment flags to child processes.
 func Files(unsetEnv bool) []*os.File {
 	if unsetEnv {
 		defer os.Unsetenv("LISTEN_PID")

activation/listeners.go

@@ -25,24 +25,22 @@ import (
 // The order of the file descriptors is preserved in the returned slice.
 // Nil values are used to fill any gaps. For example if systemd were to return file descriptors
 // corresponding with "udp, tcp, tcp", then the slice would contain {nil, net.Listener, net.Listener}
-func Listeners(unsetEnv bool) ([]net.Listener, error) {
-	files := Files(unsetEnv)
+func Listeners() ([]net.Listener, error) {
+	files := Files(true)
 	listeners := make([]net.Listener, len(files))
 
 	for i, f := range files {
 		if pc, err := net.FileListener(f); err == nil {
 			listeners[i] = pc
-			if unsetEnv {
-				f.Close()
-			}
+			f.Close()
 		}
 	}
 	return listeners, nil
 }
 
 // ListenersWithNames maps a listener name to a set of net.Listener instances.
-func ListenersWithNames(unsetEnv bool) (map[string][]net.Listener, error) {
-	files := Files(unsetEnv)
+func ListenersWithNames() (map[string][]net.Listener, error) {
+	files := Files(true)
 	listeners := map[string][]net.Listener{}
 
 	for _, f := range files {
@@ -53,9 +51,7 @@ func ListenersWithNames(unsetEnv bool) (map[string][]net.Listener, error) {
 			} else {
 				listeners[f.Name()] = append(current, pc)
 			}
-			if unsetEnv {
-				f.Close()
-			}
+			f.Close()
 		}
 	}
 	return listeners, nil
@@ -64,8 +60,8 @@ func ListenersWithNames(unsetEnv bool) (map[string][]net.Listener, error) {
 // TLSListeners returns a slice containing a net.listener for each matching TCP socket type
 // passed to this process.
 // It uses default Listeners func and forces TCP sockets handlers to use TLS based on tlsConfig.
-func TLSListeners(unsetEnv bool, tlsConfig *tls.Config) ([]net.Listener, error) {
-	listeners, err := Listeners(unsetEnv)
+func TLSListeners(tlsConfig *tls.Config) ([]net.Listener, error) {
+	listeners, err := Listeners()
 
 	if listeners == nil || err != nil {
 		return nil, err
@@ -85,8 +81,8 @@ func TLSListeners(unsetEnv bool, tlsConfig *tls.Config) ([]net.Listener, error)
 
 // TLSListenersWithNames maps a listener name to a net.Listener with
 // the associated TLS configuration.
-func TLSListenersWithNames(unsetEnv bool, tlsConfig *tls.Config) (map[string][]net.Listener, error) {
-	listeners, err := ListenersWithNames(unsetEnv)
+func TLSListenersWithNames(tlsConfig *tls.Config) (map[string][]net.Listener, error) {
+	listeners, err := ListenersWithNames()
 
 	if listeners == nil || err != nil {
 		return nil, err

activation/packetconns.go

@@ -24,16 +24,14 @@ import (
 // The order of the file descriptors is preserved in the returned slice.
 // Nil values are used to fill any gaps. For example if systemd were to return file descriptors
 // corresponding with "udp, tcp, udp", then the slice would contain {net.PacketConn, nil, net.PacketConn}
-func PacketConns(unsetEnv bool) ([]net.PacketConn, error) {
-	files := Files(unsetEnv)
+func PacketConns() ([]net.PacketConn, error) {
+	files := Files(true)
 	conns := make([]net.PacketConn, len(files))
 
 	for i, f := range files {
 		if pc, err := net.FilePacketConn(f); err == nil {
 			conns[i] = pc
-			if unsetEnv {
-				f.Close()
-			}
+			f.Close()
 		}
 	}
 	return conns, nil

examples/activation/listen.go

@@ -36,17 +36,7 @@ func fixListenPid() {
 func main() {
 	fixListenPid()
 
-	listeners, _ := activation.Listeners(false)
-
-	if len(listeners) == 0 {
-		panic("No listeners")
-	}
-
-	if os.Getenv("LISTEN_PID") == "" || os.Getenv("LISTEN_FDS") == "" || os.Getenv("LISTEN_FDNAMES") == "" {
-		panic("Should not unset envs")
-	}
-
-	listenersWithNames, err := activation.ListenersWithNames(true)
+	listenersWithNames, err := activation.ListenersWithNames()
 	if err != nil {
 		panic(err)
 	}

examples/activation/udpconn.go

@@ -37,17 +37,7 @@ func fixListenPid() {
 func main() {
 	fixListenPid()
 
-	pc, _ := activation.PacketConns(false)
-
-	if len(pc) == 0 {
-		panic("No packetConns")
-	}
-
-	if os.Getenv("LISTEN_PID") == "" || os.Getenv("LISTEN_FDS") == "" || os.Getenv("LISTEN_FDNAMES") == "" {
-		panic("Should not unset envs")
-	}
-
-	pc, err := activation.PacketConns(true)
+	pc, err := activation.PacketConns()
 	if err != nil {
 		panic(err)
 	}