From cbfc7a98c389545028ae8714cc9fa20a03907785 Mon Sep 17 00:00:00 2001 From: Alex Barter Date: Sun, 21 Apr 2024 20:45:20 +0000 Subject: [PATCH 1/5] Allow more control over file descriptors --- activation/filesmethod.go | 34 +++++++++++++++++++ activation/filesmethod_unix.go | 54 +++++++++++++++++++++++++++++++ activation/filesmethod_windows.go | 21 ++++++++++++ activation/listeners.go | 30 +++++++++++------ activation/options.go | 30 +++++++++++++++++ activation/packetconns.go | 9 ++++-- 6 files changed, 166 insertions(+), 12 deletions(-) create mode 100644 activation/filesmethod.go create mode 100644 activation/filesmethod_unix.go create mode 100644 activation/filesmethod_windows.go create mode 100644 activation/options.go diff --git a/activation/filesmethod.go b/activation/filesmethod.go new file mode 100644 index 00000000..98f237aa --- /dev/null +++ b/activation/filesmethod.go @@ -0,0 +1,34 @@ +// Copyright 2015 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package activation + +// Method decides what happens to the file descriptors that are passed in by systemd. +type Method int + +const ( + // ConsumeFiles is the default, and removes the original file descriptors passed in by + // systemd. This means that new file descriptors created by the program may use the + // file descriptors indices. + ConsumeFiles Method = iota + + // ReserveFiles stores placeholder file descriptors, which point to /dev/null. This + // stops new file descriptors consuming the indices. + ReserveFiles + + // CloneFiles duplicates and leaves the original file descriptors in tack. This + // stops new file descriptors consuming the indices like [ReserveFiles] but + // consider the possible secuirty risk of leaving the sockets exposed. + ConserveFiles +) diff --git a/activation/filesmethod_unix.go b/activation/filesmethod_unix.go new file mode 100644 index 00000000..3e251ff5 --- /dev/null +++ b/activation/filesmethod_unix.go @@ -0,0 +1,54 @@ +// Copyright 2015 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !windows +// +build !windows + +package activation + +import ( + "fmt" + "os" + "syscall" +) + +func (m Method) Apply(f *os.File) error { + saveFd := int(f.Fd()) // get the idx before being closed. + + switch m { + case ConsumeFiles: + f.Close() + case ReserveFiles: + devNull, err := os.OpenFile(os.DevNull, os.O_RDWR, 0755) + if err != nil { + return fmt.Errorf("accessing /dev/null: %w", err) + } + + nullFd := int(devNull.Fd()) + + // "If oldfd equals newfd, then dup3() fails with the error EINVAL." + if saveFd == nullFd { + syscall.CloseOnExec(nullFd) + } else { + // "makes newfd be the copy of oldfd, closing newfd first if necessary" + if err := syscall.Dup3(nullFd, saveFd, syscall.O_CLOEXEC); err != nil { + return fmt.Errorf("setting %d fd to /dev/null: %w", saveFd, err) + } + } + case ConserveFiles: + // no action + } + + return nil +} diff --git a/activation/filesmethod_windows.go b/activation/filesmethod_windows.go new file mode 100644 index 00000000..58119eae --- /dev/null +++ b/activation/filesmethod_windows.go @@ -0,0 +1,21 @@ +// Copyright 2015 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package activation + +import "os" + +func (m Method) Apply(f *os.File) error { + return nil +} diff --git a/activation/listeners.go b/activation/listeners.go index 3dbe2b08..179755fe 100644 --- a/activation/listeners.go +++ b/activation/listeners.go @@ -25,22 +25,29 @@ 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() ([]net.Listener, error) { - files := Files(true) +func Listeners(opts ...option) ([]net.Listener, error) { + o := Options(opts...) + + files := Files(o.unsetEnv) listeners := make([]net.Listener, len(files)) for i, f := range files { if pc, err := net.FileListener(f); err == nil { listeners[i] = pc - f.Close() + + if err := o.method.Apply(f); err != nil { + + } } } return listeners, nil } // ListenersWithNames maps a listener name to a set of net.Listener instances. -func ListenersWithNames() (map[string][]net.Listener, error) { - files := Files(true) +func ListenersWithNames(opts ...option) (map[string][]net.Listener, error) { + o := Options(opts...) + + files := Files(o.unsetEnv) listeners := map[string][]net.Listener{} for _, f := range files { @@ -51,7 +58,10 @@ func ListenersWithNames() (map[string][]net.Listener, error) { } else { listeners[f.Name()] = append(current, pc) } - f.Close() + + if err := o.method.Apply(f); err != nil { + + } } } return listeners, nil @@ -60,8 +70,8 @@ func ListenersWithNames() (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(tlsConfig *tls.Config) ([]net.Listener, error) { - listeners, err := Listeners() +func TLSListeners(tlsConfig *tls.Config, opts ...option) ([]net.Listener, error) { + listeners, err := Listeners(opts...) if listeners == nil || err != nil { return nil, err @@ -81,8 +91,8 @@ func TLSListeners(tlsConfig *tls.Config) ([]net.Listener, error) { // TLSListenersWithNames maps a listener name to a net.Listener with // the associated TLS configuration. -func TLSListenersWithNames(tlsConfig *tls.Config) (map[string][]net.Listener, error) { - listeners, err := ListenersWithNames() +func TLSListenersWithNames(tlsConfig *tls.Config, opts ...option) (map[string][]net.Listener, error) { + listeners, err := ListenersWithNames(opts...) if listeners == nil || err != nil { return nil, err diff --git a/activation/options.go b/activation/options.go new file mode 100644 index 00000000..5e6782c6 --- /dev/null +++ b/activation/options.go @@ -0,0 +1,30 @@ +package activation + +type options struct { + unsetEnv bool + method Method +} + +type option func(*options) + +func UnsetEnv(f bool) option { + return func(o *options) { + o.unsetEnv = f + } +} + +func UseMethod(m Method) option { + return func(o *options) { + o.method = m + } +} + +func Options(opts ...option) *options { + o := &options{} + + for _, opt := range opts { + opt(o) + } + + return o +} diff --git a/activation/packetconns.go b/activation/packetconns.go index a9720678..3c942091 100644 --- a/activation/packetconns.go +++ b/activation/packetconns.go @@ -24,14 +24,19 @@ 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() ([]net.PacketConn, error) { +func PacketConns(opts ...option) ([]net.PacketConn, error) { + o := Options(opts...) + 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 - f.Close() + + if err := o.method.Apply(f); err != nil { + + } } } return conns, nil From 220c27de949dd8434f1a181ae856192667df7f52 Mon Sep 17 00:00:00 2001 From: Alex Barter Date: Sat, 4 May 2024 11:14:52 +0000 Subject: [PATCH 2/5] Set unsetEnv default to true to match previous behaviour --- activation/options.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/activation/options.go b/activation/options.go index 5e6782c6..f2750c1a 100644 --- a/activation/options.go +++ b/activation/options.go @@ -20,7 +20,9 @@ func UseMethod(m Method) option { } func Options(opts ...option) *options { - o := &options{} + o := &options{ + unsetEnv: true, + } for _, opt := range opts { opt(o) From d619fd459fded6766a70b41a1d16b215900079e5 Mon Sep 17 00:00:00 2001 From: Alex Barter Date: Sun, 5 May 2024 07:37:17 +0000 Subject: [PATCH 3/5] Use custom error struct instead of fmt wrapped error The latter was not around in 1.12 (min targetted version) --- activation/filesmethod_unix.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/activation/filesmethod_unix.go b/activation/filesmethod_unix.go index 3e251ff5..192ac586 100644 --- a/activation/filesmethod_unix.go +++ b/activation/filesmethod_unix.go @@ -18,11 +18,23 @@ package activation import ( - "fmt" "os" "syscall" ) +type ErrorDevNullSetup struct { + fd int + err error +} + +func (e ErrorDevNullSetup) Error() string { + return "setting up %d fd to /dev/null: " + e.err.Error() +} + +func (e ErrorDevNullSetup) Unwrap() error { + return e.err +} + func (m Method) Apply(f *os.File) error { saveFd := int(f.Fd()) // get the idx before being closed. @@ -32,7 +44,7 @@ func (m Method) Apply(f *os.File) error { case ReserveFiles: devNull, err := os.OpenFile(os.DevNull, os.O_RDWR, 0755) if err != nil { - return fmt.Errorf("accessing /dev/null: %w", err) + return ErrorDevNullSetup{err: err, fd: saveFd} } nullFd := int(devNull.Fd()) @@ -43,7 +55,7 @@ func (m Method) Apply(f *os.File) error { } else { // "makes newfd be the copy of oldfd, closing newfd first if necessary" if err := syscall.Dup3(nullFd, saveFd, syscall.O_CLOEXEC); err != nil { - return fmt.Errorf("setting %d fd to /dev/null: %w", saveFd, err) + return ErrorDevNullSetup{err: err, fd: saveFd} } } case ConserveFiles: From 20105fa2ff213bb396fc54c314d64c42e9548736 Mon Sep 17 00:00:00 2001 From: Alex Barter Date: Sun, 5 May 2024 07:41:19 +0000 Subject: [PATCH 4/5] Return fd close error & close dev null file on error --- activation/filesmethod_unix.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/activation/filesmethod_unix.go b/activation/filesmethod_unix.go index 192ac586..e99494eb 100644 --- a/activation/filesmethod_unix.go +++ b/activation/filesmethod_unix.go @@ -40,7 +40,7 @@ func (m Method) Apply(f *os.File) error { switch m { case ConsumeFiles: - f.Close() + return f.Close() case ReserveFiles: devNull, err := os.OpenFile(os.DevNull, os.O_RDWR, 0755) if err != nil { @@ -55,6 +55,8 @@ func (m Method) Apply(f *os.File) error { } else { // "makes newfd be the copy of oldfd, closing newfd first if necessary" if err := syscall.Dup3(nullFd, saveFd, syscall.O_CLOEXEC); err != nil { + devNull.Close() // on an error tidy up. + return ErrorDevNullSetup{err: err, fd: saveFd} } } From 0ad2d1ea56acc8a7a6467f99bbde617f619fffb4 Mon Sep 17 00:00:00 2001 From: Alex Barter Date: Sun, 5 May 2024 07:45:13 +0000 Subject: [PATCH 5/5] Add docs to options.go file --- activation/options.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/activation/options.go b/activation/options.go index f2750c1a..fc69612c 100644 --- a/activation/options.go +++ b/activation/options.go @@ -7,18 +7,26 @@ type options struct { type option func(*options) +// UnsetEnv controls if the LISTEN_PID, LISTEN_FDS & LISTEN_FDNAMES environment +// variables are unset. +// +// This is useful to avoid clashes in fd usage and to avoid leaking environment +// flags to child processes. func UnsetEnv(f bool) option { return func(o *options) { o.unsetEnv = f } } +// UseMethod chooses the [Method] applied to the file descriptor passed in by +// systemd socket activation. func UseMethod(m Method) option { return func(o *options) { o.method = m } } +// Options takes some [option]s and produces a stuct containing the flags and settings. func Options(opts ...option) *options { o := &options{ unsetEnv: true,