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

CanAutoUpdate should not decide by util.HaveAdminRights #2231

Closed
ihipop opened this issue Oct 29, 2020 · 7 comments
Closed

CanAutoUpdate should not decide by util.HaveAdminRights #2231

ihipop opened this issue Oct 29, 2020 · 7 comments
Assignees
Milestone

Comments

@ihipop
Copy link

ihipop commented Oct 29, 2020

if runtime.GOOS != "windows" &&
((tlsConf.Enabled && (tlsConf.PortHTTPS < 1024 ||
tlsConf.PortDNSOverTLS < 1024 ||
tlsConf.PortDNSOverQUIC < 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()
}
ret["can_autoupdate"] = canUpdate

if we run AdGuardHome by systemd like this

[Unit]
Description=Adguard Home
Requires=network.target
After=network.target

[Service]
Type=simple
####ExecStartPre=-/sbin/setcap cap_net_bind_service=+ep /config/ihipop/AdGuardHome/AdGuardHome
AmbientCapabilities=CAP_NET_BIND_SERVICE
ExecStart=/config/ihipop/AdGuardHome/AdGuardHome
Restart=always
RestartSec=2s
User=dnsmasq
PermissionsStartOnly=true
LimitNOFILE=infinity

[Install]
WantedBy=multi-user.target

The CAP_NET_BIND_SERVICE capability is grant by systemd and not exists in filesystem
AmbientCapabilities doc https://www.freedesktop.org/software/systemd/man/systemd.exec.html#AmbientCapabilities=
It's safe to upgrade then

@ihipop
Copy link
Author

ihipop commented Oct 29, 2020

#1193 7ff743a
#1944

@ihipop
Copy link
Author

ihipop commented Oct 29, 2020

IMO,the simple way is to have a auto update policy option, let the administrator to decide whether it’s safe or not to auto upgrade
The administrator is responsible for the failure of auto update
To interactive with systemd is not a good idea because it’s tooo heavy behavior and bundle to much to platform code
Let‘s keep it simple 。

@ihipop
Copy link
Author

ihipop commented Oct 30, 2020

Suggestions:

  • deprecated no-check-update option
  • add new update-policy option

New policy:

  • auto:
    auto update if possible, with a new version and execute file is located and writable on file system ,ignore the Capabilities stuff
  • manual: (default)
    same as auto but will not update self until administrator click it on admin panel
  • disable:
    same as the old no-check-update, will not perform any update check so will no self-update

@ainar-g ainar-g assigned EugeneOne1 and unassigned ainar-g Nov 19, 2020
adguard pushed a commit that referenced this issue Nov 30, 2020
Merge in DNS/adguard-home from 2231-autoupdate to master

Updates #2231.

Squashed commit of the following:

commit 4ee9148
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Mon Nov 30 19:08:14 2020 +0300

    sysutil: provide os-independent interface

commit 778097c
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Mon Nov 30 16:40:33 2020 +0300

    all: add sysutil package
@EugeneOne1
Copy link
Member

EugeneOne1 commented Nov 30, 2020

This should be fixed as of snapshot 641db73. Could you please check if our solution fixes the issue for you?

@ihipop
Copy link
Author

ihipop commented Dec 1, 2020

This should be fixed as of snapshot 641db73. Could you please check if our solution fixes the issue for you?

I take a glance at the code ,Nice and clean fix than what I've suggested !!!
I will take a try and close my issue if it works fine

adguard pushed a commit that referenced this issue Dec 4, 2020
Merge in DNS/adguard-home from 2391-updating-bug to master

Updates #2391.
Updates #2231.

Squashed commit of the following:

commit b321884
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Fri Dec 4 16:54:20 2020 +0300

    all: log changes

commit 5aa0202
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Fri Dec 4 14:42:10 2020 +0300

    sysutil: fix binding capability defining
@ainar-g
Copy link
Contributor

ainar-g commented Dec 7, 2020

@ihipop, hi, any news? Can we close the issue for now?

adguard pushed a commit that referenced this issue Dec 10, 2020
Merge in DNS/adguard-home from 2231-autoupdate to master

Updates #2231.

Squashed commit of the following:

commit 4ee9148
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Mon Nov 30 19:08:14 2020 +0300

    sysutil: provide os-independent interface

commit 778097c
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Mon Nov 30 16:40:33 2020 +0300

    all: add sysutil package
adguard pushed a commit that referenced this issue Dec 10, 2020
Merge in DNS/adguard-home from 2391-updating-bug to master

Updates #2391.
Updates #2231.

Squashed commit of the following:

commit b321884
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Fri Dec 4 16:54:20 2020 +0300

    all: log changes

commit 5aa0202
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Fri Dec 4 14:42:10 2020 +0300

    sysutil: fix binding capability defining
@ainar-g
Copy link
Contributor

ainar-g commented Dec 10, 2020

I'll close this issue for now. Please feel free to reopen with more details if you think it should be reopened.

@ainar-g ainar-g closed this as completed Dec 10, 2020
adguard pushed a commit that referenced this issue Jun 10, 2024
…Node 18 support

Merge in DNS/adguard-home from ADG-8368-typescript-node-18 to master

Squashed commit of the following:

commit daa288a
Merge: 4c89cf7 1085d59
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Mon Jun 10 17:22:20 2024 +0200

    merge

commit 4c89cf7
Author: Ildar Kamalov <ik@adguard.com>
Date:   Thu Jun 6 13:27:18 2024 +0300

    remove install from initial state

commit b943f20
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 23:10:55 2024 +0200

    frontend production build fix

commit cd1be2d
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 20:23:14 2024 +0200

    production build quickfix

commit 7b8ac01
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Wed Jun 5 19:57:31 2024 +0300

    all: upd node docker

commit 02afed6
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 18:23:12 2024 +0200

    changelog fixes

commit 9c0f736
Merge: 62c4fbf e04775c
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 18:18:29 2024 +0200

    merge

commit 62c4fbf
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 16:22:22 2024 +0200

    empty line in changelog

commit 76b1e44
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 16:20:37 2024 +0200

    changelog

commit f783e90
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 16:19:13 2024 +0200

    filters.js -> filters.ts

commit 3d4ce65
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 16:18:03 2024 +0200

    generated file removed

commit e35ba58
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 15:45:21 2024 +0200

    rollback unwanted changes

commit 1f30d42
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 15:27:36 2024 +0200

    review fix

commit 6cd4e44
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 11:55:39 2024 +0200

    missing generated file restoresd

commit 2ab738b
Author: Igor Lobanov <bniwredyc@gmail.com>
Date:   Wed Jun 5 11:40:32 2024 +0200

    Frontend rewritten in TypeScript, added Node 18 support
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

4 participants