Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to enable auto-upgrade on Linux/Unix despite CAP_NET_BIND_SERVICE capability #1944

Open
CampinCarl opened this issue Jul 25, 2020 · 5 comments

Comments

@CampinCarl
Copy link

Problem Description

The if statement linked below rightly describes the issue with setting CAP_NET_BIND_SERVICE on binary files in Linux but doesn't account for setting this option using systemd's AmbientCapabilities directive instead, which doesn't require setting the capability on the binary itself.

if runtime.GOOS != "windows" &&
((tlsConf.Enabled && (tlsConf.PortHTTPS < 1024 || tlsConf.PortDNSOverTLS < 1024)) ||
config.BindPort < 1024 ||
config.DNS.Port < 1024) {
// On UNIX, if we're running under a regular user,
// but with CAP_NET_BIND_SERVICE set on a binary file,
// and we're listening on ports <1024,
// we won't be able to restart after we replace the binary file,
// because we'll lose CAP_NET_BIND_SERVICE capability.
canUpdate, _ = util.HaveAdminRights()
}

Proposed Solution

Modify the if statement logic to allow users to override the behavior, perhaps with a command line flag like --allow-auto-update? When combined with the AmbientCapabilities systemd directive, this would allow users to auto upgrade the binary even when running AdGuardHome without root permissions.

Systemd Service File Example

AmbientCapabilities=CAP_NET_BIND_SERVICE

Alternatives Considered

Script the upgrade myself or fork the code, but a native solution would be much easier and a benefit for other Linux users. Thanks for the great application!

Additional Information

@ameshkov
Copy link
Member

Modify the if statement logic to allow users to override the behavior, perhaps with a command line flag like --allow-auto-update?

I am not a fan of adding new flags because most people simply wouldn't know about it.

Is there any way to detect this automatically?

@CampinCarl
Copy link
Author

That's fair, this is admittedly a bit of an edge case though I'd argue it's the best way to set it up on Linux without root (at least with systemd). Would you consider a configuration file directive instead or is that just as bad/worse?

As for how to detect it, I suspect you'd need to interact with systemd directly to check. I only know a little Go so here's how one could check from the shell.

  1. Check if AdGuard Home is running under systemd and check the service unit's name
    • systemctl status <AdGuardHome PID>; or
    • ps -o unit <AdGuardHome PID>
  2. Check the AmbientCapabilities property using the service unit's name found in step 1
    • systemctl show -p AmbientCapabilities <AdGuardHome Service File>

If the output from step 2 includes cap_net_bind_service then it should be safe to upgrade AdGuard Home.

@Aikatsui
Copy link
Contributor

#813 - automatic update.

@ameshkov
Copy link
Member

Well, we can run shell commands from Go, it's a perfectly viable solution.

We will need to implement a method that will call your commands one by one, something like util.HaveAmbientCapabilities() and use it.

Something like this:

if runtime.GOOS != "linux" {
    return false;
}

cmd := "systemctl status " + os.Getpid()
if cmd, e := exec.Run(cmd, nil, nil, exec.DevNull, exec.Pipe, exec.MergeWithStdout); e == nil {
    b, _ := ioutil.ReadAll(cmd.Stdout)
    output := string(b)

    // parse, find config file
    // read the config file, check the capabilities
}

Marked as "help wanted" for now that means that we're looking for anyone who can contribute this change to AGH.
If it gets more upvotes and no one volunteers to implement it, we'll do it.

@CampinCarl
Copy link
Author

That makes sense; thanks for the insight. I'm a bit of a novice with Go but I'll play with it and see if I can't get something working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants