From 48c44c29aba472839b25fe1132c6ee8b91a27bba Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Fri, 4 Jun 2021 16:35:34 +0300 Subject: [PATCH] Pull request: all: support setgid, setuid on unix Updates #2763. Squashed commit of the following: commit bd2077c6569b53ae341a58aa73de6063d7037e8e Author: Ainar Garipov Date: Fri Jun 4 16:25:17 2021 +0300 all: move rlimit_nofile, imp docs commit ba95d4ab7c722bf83300d626a598aface37539ad Author: Ainar Garipov Date: Fri Jun 4 15:12:23 2021 +0300 all: support setgid, setuid on unix --- CHANGELOG.md | 4 ++ HACKING.md | 3 ++ internal/aghos/os.go | 29 +++++++++---- internal/aghos/os_windows.go | 5 +-- internal/aghos/user.go | 11 +++++ internal/aghos/user_unix.go | 50 +++++++++++++++++++++ internal/aghos/user_windows.go | 16 +++++++ internal/dhcpd/checkother.go | 3 +- internal/dhcpd/checkother_windows.go | 10 +++-- internal/dhcpd/os_windows.go | 10 +++-- internal/home/config.go | 23 ++++++++-- internal/home/home.go | 65 ++++++++++++++++++++++++---- internal/home/upgrade.go | 38 +++++++++++++++- internal/home/upgrade_test.go | 47 ++++++++++++++++++++ 14 files changed, 283 insertions(+), 31 deletions(-) create mode 100644 internal/aghos/user.go create mode 100644 internal/aghos/user_unix.go create mode 100644 internal/aghos/user_windows.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b534e63f..7097ce6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to ### Added +- The ability to change group and user ID on startup on Unix ([#2763]). - Experimental OpenBSD support for AMD64 and 64-bit ARM CPUs ([#2439]). - Support for custom port in DNS-over-HTTPS profiles for Apple's devices ([#3172]). @@ -31,6 +32,8 @@ and this project adheres to ### Changed +- The setting `rlimit_nofile` is now in the `os` block of the configuration + file, together with the new `group` and `user` settings ([#2763]). - Permissions on filter files are now `0o644` instead of `0o600` ([#3198]). ### Deprecated @@ -56,6 +59,7 @@ released by then. [#2439]: https://github.com/AdguardTeam/AdGuardHome/issues/2439 [#2441]: https://github.com/AdguardTeam/AdGuardHome/issues/2441 [#2443]: https://github.com/AdguardTeam/AdGuardHome/issues/2443 +[#2763]: https://github.com/AdguardTeam/AdGuardHome/issues/2763 [#3136]: https://github.com/AdguardTeam/AdGuardHome/issues/3136 [#3172]: https://github.com/AdguardTeam/AdGuardHome/issues/3172 [#3184]: https://github.com/AdguardTeam/AdGuardHome/issues/3184 diff --git a/HACKING.md b/HACKING.md index 3eae4ba7..3090a182 100644 --- a/HACKING.md +++ b/HACKING.md @@ -126,6 +126,9 @@ on GitHub and most other Markdown renderers. --> ) ``` + * Don't rely only on file names for build tags to work. Always add build tags + as well. + * Don't use `fmt.Sprintf` where a more structured approach to string conversion could be used. For example, `net.JoinHostPort` or `url.(*URL).String`. diff --git a/internal/aghos/os.go b/internal/aghos/os.go index 0c3db64a..6807ad69 100644 --- a/internal/aghos/os.go +++ b/internal/aghos/os.go @@ -4,17 +4,30 @@ package aghos import ( "fmt" "os/exec" + "runtime" "syscall" - - "github.com/AdguardTeam/golibs/errors" ) -// ErrUnsupported is returned when the functionality is unsupported on the -// current operating system. -// -// TODO(a.garipov): Make a structured error and use it everywhere instead of -// a bunch of fmt.Errorf and all that. -const ErrUnsupported errors.Error = "unsupported" +// UnsupportedError is returned by functions and methods when a particular +// operation Op cannot be performed on the current OS. +type UnsupportedError struct { + Op string + OS string +} + +// Error implements the error interface for *UnsupportedError. +func (err *UnsupportedError) Error() (msg string) { + return fmt.Sprintf("%s is unsupported on %s", err.Op, err.OS) +} + +// Unsupported is a helper that returns an *UnsupportedError with the Op field +// set to op and the OS field set to the current OS. +func Unsupported(op string) (err error) { + return &UnsupportedError{ + Op: op, + OS: runtime.GOOS, + } +} // CanBindPrivilegedPorts checks if current process can bind to privileged // ports. diff --git a/internal/aghos/os_windows.go b/internal/aghos/os_windows.go index baf7d821..026665e3 100644 --- a/internal/aghos/os_windows.go +++ b/internal/aghos/os_windows.go @@ -5,7 +5,6 @@ package aghos import ( - "fmt" "syscall" "golang.org/x/sys/windows" @@ -16,7 +15,7 @@ func canBindPrivilegedPorts() (can bool, err error) { } func setRlimit(val uint64) (err error) { - return ErrUnsupported + return Unsupported("setrlimit") } func haveAdminRights() (bool, error) { @@ -41,7 +40,7 @@ func haveAdminRights() (bool, error) { } func sendProcessSignal(pid int, sig syscall.Signal) error { - return fmt.Errorf("not supported on Windows") + return Unsupported("kill") } func isOpenWrt() (ok bool) { diff --git a/internal/aghos/user.go b/internal/aghos/user.go new file mode 100644 index 00000000..9ab6a637 --- /dev/null +++ b/internal/aghos/user.go @@ -0,0 +1,11 @@ +package aghos + +// SetGroup sets the effective group ID of the calling process. +func SetGroup(groupName string) (err error) { + return setGroup(groupName) +} + +// SetUser sets the effective user ID of the calling process. +func SetUser(userName string) (err error) { + return setUser(userName) +} diff --git a/internal/aghos/user_unix.go b/internal/aghos/user_unix.go new file mode 100644 index 00000000..f8d228f7 --- /dev/null +++ b/internal/aghos/user_unix.go @@ -0,0 +1,50 @@ +// +build darwin freebsd linux netbsd openbsd + +//go:build darwin || freebsd || linux || netbsd || openbsd + +package aghos + +import ( + "fmt" + "os/user" + "strconv" + "syscall" +) + +func setGroup(groupName string) (err error) { + g, err := user.LookupGroup(groupName) + if err != nil { + return fmt.Errorf("looking up group: %w", err) + } + + gid, err := strconv.Atoi(g.Gid) + if err != nil { + return fmt.Errorf("parsing gid: %w", err) + } + + err = syscall.Setgid(gid) + if err != nil { + return fmt.Errorf("setting gid: %w", err) + } + + return nil +} + +func setUser(userName string) (err error) { + u, err := user.Lookup(userName) + if err != nil { + return fmt.Errorf("looking up user: %w", err) + } + + uid, err := strconv.Atoi(u.Uid) + if err != nil { + return fmt.Errorf("parsing uid: %w", err) + } + + err = syscall.Setuid(uid) + if err != nil { + return fmt.Errorf("setting uid: %w", err) + } + + return nil +} diff --git a/internal/aghos/user_windows.go b/internal/aghos/user_windows.go new file mode 100644 index 00000000..bb39caab --- /dev/null +++ b/internal/aghos/user_windows.go @@ -0,0 +1,16 @@ +// +build windows + +//go:build windows + +package aghos + +// TODO(a.garipov): Think of a way to implement these. Perhaps by using +// syscall.CreateProcessAsUser or something from the golang.org/x/sys module. + +func setGroup(_ string) (err error) { + return Unsupported("setgid") +} + +func setUser(_ string) (err error) { + return Unsupported("setuid") +} diff --git a/internal/dhcpd/checkother.go b/internal/dhcpd/checkother.go index c6345cd7..8406e836 100644 --- a/internal/dhcpd/checkother.go +++ b/internal/dhcpd/checkother.go @@ -13,6 +13,7 @@ import ( "time" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/insomniacslk/dhcp/dhcpv4" @@ -41,7 +42,7 @@ func CheckIfOtherDHCPServersPresentV4(ifaceName string) (ok bool, err error) { // TODO(a.garipov): Find out what this is about. Perhaps this // information is outdated or at least incomplete. if runtime.GOOS == "darwin" { - return false, fmt.Errorf("can't find DHCP server: not supported on macOS") + return false, aghos.Unsupported("CheckIfOtherDHCPServersPresentV4") } srcIP := ifaceIPNet[0] diff --git a/internal/dhcpd/checkother_windows.go b/internal/dhcpd/checkother_windows.go index 49d90f25..c57a28c6 100644 --- a/internal/dhcpd/checkother_windows.go +++ b/internal/dhcpd/checkother_windows.go @@ -1,11 +1,15 @@ +// +build windows + +//go:build windows + package dhcpd -import "fmt" +import "github.com/AdguardTeam/AdGuardHome/internal/aghos" func CheckIfOtherDHCPServersPresentV4(ifaceName string) (bool, error) { - return false, fmt.Errorf("not supported") + return false, aghos.Unsupported("CheckIfOtherDHCPServersPresentV4") } func CheckIfOtherDHCPServersPresentV6(ifaceName string) (bool, error) { - return false, fmt.Errorf("not supported") + return false, aghos.Unsupported("CheckIfOtherDHCPServersPresentV6") } diff --git a/internal/dhcpd/os_windows.go b/internal/dhcpd/os_windows.go index 6c0417a0..33606537 100644 --- a/internal/dhcpd/os_windows.go +++ b/internal/dhcpd/os_windows.go @@ -1,13 +1,17 @@ +// +build windows + +//go:build windows + package dhcpd import ( "net" - "github.com/AdguardTeam/golibs/errors" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" "golang.org/x/net/ipv4" ) // Create a socket for receiving broadcast packets -func newBroadcastPacketConn(bindAddr net.IP, port int, ifname string) (*ipv4.PacketConn, error) { - return nil, errors.Error("newBroadcastPacketConn(): not supported on Windows") +func newBroadcastPacketConn(_ net.IP, _ int, _ string) (*ipv4.PacketConn, error) { + return nil, aghos.Unsupported("newBroadcastPacketConn") } diff --git a/internal/home/config.go b/internal/home/config.go index 4a3d7705..82f89178 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -35,6 +35,19 @@ type logSettings struct { Verbose bool `yaml:"verbose"` // If true, verbose logging is enabled } +// osConfig contains OS-related configuration. +type osConfig struct { + // Group is the name of the group which AdGuard Home must switch to on + // startup. Empty string means no switching. + Group string `yaml:"group"` + // User is the name of the user which AdGuard Home must switch to on + // startup. Empty string means no switching. + User string `yaml:"user"` + // RlimitNoFile is the maximum number of opened fd's per process. Zero + // means use the default value. + RlimitNoFile uint64 `yaml:"rlimit_nofile"` +} + // configuration is loaded from YAML // field ordering is important -- yaml fields will mirror ordering from here type configuration struct { @@ -52,10 +65,9 @@ type configuration struct { // AuthBlockMin is the duration, in minutes, of the block of new login // attempts after AuthAttempts unsuccessful login attempts. AuthBlockMin uint `yaml:"block_auth_min"` - ProxyURL string `yaml:"http_proxy"` // Proxy address for our HTTP client - Language string `yaml:"language"` // two-letter ISO 639-1 language code - RlimitNoFile uint64 `yaml:"rlimit_nofile"` // Maximum number of opened fd's per process (0: default) - DebugPProf bool `yaml:"debug_pprof"` // Enable pprof HTTP server on port 6060 + ProxyURL string `yaml:"http_proxy"` // Proxy address for our HTTP client + Language string `yaml:"language"` // two-letter ISO 639-1 language code + DebugPProf bool `yaml:"debug_pprof"` // Enable pprof HTTP server on port 6060 // TTL for a web session (in hours) // An active session is automatically refreshed once a day. @@ -75,6 +87,8 @@ type configuration struct { logSettings `yaml:",inline"` + OSConfig *osConfig `yaml:"os"` + sync.RWMutex `yaml:"-"` SchemaVersion int `yaml:"schema_version"` // keeping last so that users will be less tempted to change it -- used when upgrading between versions @@ -184,6 +198,7 @@ var config = configuration{ LogMaxSize: 100, LogMaxAge: 3, }, + OSConfig: &osConfig{}, SchemaVersion: currentSchemaVersion, } diff --git a/internal/home/home.go b/internal/home/home.go index 82d449e5..61c613c9 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -119,7 +119,7 @@ func Main(clientBuildFS fs.FS) { // support OpenBSD currently. Either patch it to do so or make // our own implementation of the service.System interface. if runtime.GOOS == "openbsd" { - log.Fatal("service actions are not supported on openbsd") + log.Fatal("service actions are not supported on openbsd, see issue 3226") } handleServiceControlAction(args, clientBuildFS) @@ -183,6 +183,59 @@ func setupContext(args options) { Context.mux = http.NewServeMux() } +// logIfUnsupported logs a formatted warning if the error is one of the +// unsupported errors and returns nil. If err is nil, logIfUnsupported returns +// nil. Otherise, it returns err. +func logIfUnsupported(msg string, err error) (outErr error) { + if unsupErr := (&aghos.UnsupportedError{}); errors.As(err, &unsupErr) { + log.Debug(msg, err) + } else if err != nil { + return err + } + + return nil +} + +// configureOS sets the OS-related configuration. +func configureOS(conf *configuration) (err error) { + osConf := conf.OSConfig + if osConf == nil { + return nil + } + + if osConf.Group != "" { + err = aghos.SetGroup(osConf.Group) + err = logIfUnsupported("warning: setting group", err) + if err != nil { + return fmt.Errorf("setting group: %w", err) + } + + log.Info("group set to %s", osConf.Group) + } + + if osConf.User != "" { + err = aghos.SetUser(osConf.User) + err = logIfUnsupported("warning: setting user", err) + if err != nil { + return fmt.Errorf("setting user: %w", err) + } + + log.Info("user set to %s", osConf.User) + } + + if osConf.RlimitNoFile != 0 { + err = aghos.SetRlimit(osConf.RlimitNoFile) + err = logIfUnsupported("warning: setting rlimit", err) + if err != nil { + return fmt.Errorf("setting rlimit: %w", err) + } + + log.Info("rlimit_nofile set to %d", osConf.RlimitNoFile) + } + + return nil +} + func setupConfig(args options) (err error) { config.DHCP.WorkDir = Context.workDir config.DHCP.HTTPRegister = httpRegister @@ -216,13 +269,6 @@ func setupConfig(args options) (err error) { Context.clients.Init(config.Clients, Context.dhcpServer, Context.etcHosts) config.Clients = nil - if config.RlimitNoFile != 0 { - err = aghos.SetRlimit(config.RlimitNoFile) - if err != nil && !errors.Is(err, aghos.ErrUnsupported) { - return fmt.Errorf("setting rlimit: %w", err) - } - } - // override bind host/port from the console if args.bindHost != nil { config.BindHost = args.bindHost @@ -309,6 +355,9 @@ func run(args options, clientBuildFS fs.FS) { setupContext(args) + err = configureOS(&config) + fatalOnError(err) + // clients package uses filtering package's static data (filtering.BlockedSvcKnown()), // so we have to initialize filtering's static data first, // but also avoid relying on automatic Go init() function diff --git a/internal/home/upgrade.go b/internal/home/upgrade.go index c8e54319..1e471ce6 100644 --- a/internal/home/upgrade.go +++ b/internal/home/upgrade.go @@ -19,7 +19,7 @@ import ( ) // currentSchemaVersion is the current schema version. -const currentSchemaVersion = 10 +const currentSchemaVersion = 11 // These aliases are provided for convenience. type ( @@ -81,6 +81,7 @@ func upgradeConfigSchema(oldVersion int, diskConf yobj) (err error) { upgradeSchema7to8, upgradeSchema8to9, upgradeSchema9to10, + upgradeSchema10to11, } n := 0 @@ -611,6 +612,41 @@ func upgradeSchema9to10(diskConf yobj) (err error) { return nil } +// upgradeSchema10to11 performs the following changes: +// +// # BEFORE: +// 'rlimit_nofile': 42 +// +// # AFTER: +// 'os': +// 'group': '' +// 'rlimit_nofile': 42 +// 'user': '' +// +func upgradeSchema10to11(diskConf yobj) (err error) { + log.Printf("Upgrade yaml: 10 to 11") + + diskConf["schema_version"] = 11 + + rlimit := 0 + rlimitVal, ok := diskConf["rlimit_nofile"] + if ok { + rlimit, ok = rlimitVal.(int) + if !ok { + return fmt.Errorf("unexpected type of rlimit_nofile: %T", rlimitVal) + } + } + + delete(diskConf, "rlimit_nofile") + diskConf["os"] = yobj{ + "group": "", + "rlimit_nofile": rlimit, + "user": "", + } + + return nil +} + // TODO(a.garipov): Replace with log.Output when we port it to our logging // package. func funcName() string { diff --git a/internal/home/upgrade_test.go b/internal/home/upgrade_test.go index 78d66aa9..fe6733e8 100644 --- a/internal/home/upgrade_test.go +++ b/internal/home/upgrade_test.go @@ -368,3 +368,50 @@ func TestUpgradeSchema9to10(t *testing.T) { assert.Equal(t, "unexpected type of dns: int", err.Error()) }) } + +func TestUpgradeSchema10to11(t *testing.T) { + check := func(t *testing.T, conf yobj) { + rlimit, _ := conf["rlimit_nofile"].(int) + + err := upgradeSchema10to11(conf) + require.NoError(t, err) + + require.Equal(t, conf["schema_version"], 11) + + _, ok := conf["rlimit_nofile"] + assert.False(t, ok) + + osVal, ok := conf["os"] + require.True(t, ok) + + newOSConf, ok := osVal.(yobj) + require.True(t, ok) + + _, ok = newOSConf["group"] + assert.True(t, ok) + + _, ok = newOSConf["user"] + assert.True(t, ok) + + rlimitVal, ok := newOSConf["rlimit_nofile"].(int) + require.True(t, ok) + + assert.Equal(t, rlimit, rlimitVal) + } + + const rlimit = 42 + t.Run("with_rlimit", func(t *testing.T) { + conf := yobj{ + "rlimit_nofile": rlimit, + "schema_version": 10, + } + check(t, conf) + }) + + t.Run("without_rlimit", func(t *testing.T) { + conf := yobj{ + "schema_version": 10, + } + check(t, conf) + }) +}