[Security] do not run as root. #816

zander · 2021-07-24T11:02:28Z

An Internet accessible service should aim to have as little as
possible attack surface, which is much easier to do when running
with the absolute minimum number of priviledges at all times.

This makes the systemd setup run the service as a user 'yggdrasil'
and uses the systemd feature RuntimeDirectory to auto-create
the /var/run/yggdrasil dir so our non-elevated client can still
create the socket. (see #802)

The sysusers file will cause the user be created on first install
using the sysusers subsystem, this can be used by packagers as well.

cofob

Looks fine

Arceliar · 2021-07-24T13:07:36Z

src/defaults/defaults_linux.go

@@ -7,7 +7,7 @@ package defaults
 func GetDefaults() platformDefaultParameters {
 	return platformDefaultParameters{
 		// Admin
-		DefaultAdminListen: "unix:///var/run/yggdrasil.sock",
+		DefaultAdminListen: "unix:///var/run/yggdrasil/yggdrasil.sock",


This breaks running if the /var/run/yggdrasil folder doesn't exist, so the default depends on the systemd package (or otherwise creating the folder). The binary produced by ./build needs to be able to run with default settings (i.e. -autoconf) with no further changes to the OS/environment.

I didn't test this and indeed if yggdrasil does not try to create the directory in which it wants to place the socket (which IMOHO is a bug) then this will fail.

An actual go progammer is required to make ygg create the directory before it attempts to create a file in it. I am not a Go programmer, would appreciate a patch. I guess its not too complex.

No problem, default socket path can be different if run with -autoconf. Probably, it should be created in user's home directory if run this way.

good point. Likely best to use /run/user/{UID}/ which is where Linux by default makes those.

For instance wayland creates its sockets there.

No, it's bad to hardcode this path. The better way would be to use contents XDG_RUNTIME_DIR environment variable if it's present, otherwise something like .yggdrasil.sock in user's home directory. I presume, this will be a good default for those who still insist on not using systemd.

Actually, defaults should be dependent on whether config is generated by regular user or root (not on -autoconf option).

Regular user should have everything in the home directory (and socket $XDG_RUNTIME_DIR if this variable is present).
But for root user everything should be in LSB-conforming directories (config in /etc/yggdrasil, socket in /run/yggdrasil).

contrib/systemd/yggdrasil.sysusers

src/defaults/defaults_linux.go

zander · 2021-07-24T18:42:56Z

To followup after the discussion on matrix.

the executable as compiled by build should, when using -autoconf use the XDG_RUNTIME_DIR (typically /run/user/UID) as a basdir instead. The rationale here is that services (daemons in Linux) do not have this variable set and thus finding this env-var means its started by a user. The yggdrasilctl executable can follow the same logic and thus find the socket.
the package-managers will be very happy to reuse the files this PR updates, they lack the XDG env-var and thus will make the file go to /run/yggdrasil/. Packagers for distro's not using systemd will need a packager-note to make clear that need to create this dir.
maybe users are used to running this as root, as such I suggest that yggdrasil auto-creates the directory the user requested the socket to be inserted in. This code will only hit when the two above cases are not used as they ensure the dir to exist.

I would be very sad if yggdrasil would turn off the admin interface before there is a replacement as that makes deployment and sysadmin-jobs much harder.

Arceliar · 2021-07-24T19:17:50Z

One more thing to add:

Running as an unprivileged user means the old AdminListen value (/var/run/yggdrasil.sock) will need to be recognized and replaced when updating. Otherwise, updating the package (via apt-get upgrade or similar) will leave the system in a broken state. The new systemd file would try to run ygg as an unprivileged user, but using a config file with the old (root-only) path, which leads to a runtime crash. This is true regardless of what the new default value is.

EDIT: One more thing to add. We can make the AdminListen something that's configurable at compile time. The default value would be "none" for all platforms, but the package maintainers could override. Then we'd update our scripts (mainly contrib/deb/generate.sh) accordingly, so packages that use the example systemd service file will also use that path for the admin socket by default. This is independent of any code changes we may want to introduce, to e.g. create the socket's directory at run time (if it does not already exist), or to migrate old configs (with the old default value of /var/run/yggdrasil.sock) to the new defaults.

zander · 2021-07-24T20:06:49Z

Running as an unprivileged user means the old AdminListen value (/var/run/yggdrasil.sock) will need to be recognized and replaced when updating.

Yes, this is one of those problems: what is the ideal time to do this? Before 0.4.0, the second best time to do this? Now.

The solution there is for packagers to add an 'upgrade' line. Ygg does not have to do anything as the problem does not exist for people using its compiled binary.
perl -spi.orig -e 's/(AdminListen: unix:\/\/\/var\/run\/)(yggdrasil.sock)/$1\yggdrasil\/$2/;' /etc/yggdrasil.conf

We can make the AdminListen something that's configurable at compile time. The default value would be "none" for all platforms, but the package maintainers could override.

That is possible. Sounds complicated compared to simply doing a makedir.
You might consider instead having a new argument that is used in combination wih --genconf to create a useful line for AdminListen.

This has the advantage that your second systemd config file is the one that actually calls genconf and thus this change has lower impact.

kotovalexarian · 2021-08-01T07:37:45Z

Why not just drop root privileges after socket file setup?

jgoerzen · 2022-01-05T23:14:15Z

Here are some suggestions to make this work better:

Don't create a yggdrasil sysusers file, and instead use the DynamicUser option:

User=yggdrasil-dyn
Group=yggdrasil
DynamicUser=true

(Note: User must not match yggdrasil here, since it isn't defined statically)

Also, set:

ProtectSystem=strict
NoNewPrivileges=true
ReadWritePaths=/var/run/yggdrasil /run/yggdrasil

ProtectSystem=strict prevents it from modifying basically anything on the filesystem, so therefore we have to specify the possible locations for the runtime directory under ReadWritePaths.

zander · 2022-01-06T09:53:56Z

@jgoerzen sounds like an interesting idea. Seems to make the systemd deployment one file, which is nice.

Would you be able to comment on the minimum version of systemd needed for all the features your idea uses?

jgoerzen · 2022-01-06T15:27:52Z

Sure. Per the systemd NEWS file:

DynamicUser was added in version 232, 2016-11-03
NoNewPrivileges was introduced before NEWS started; it was already mentioned as existing in v227, 2015-10-07
ReadWritePaths was also introduced before NEWS started; it was already mentioned as existing in v231, 2016-07-25
ProtectSystem was already in use, and was introduced in v214, 2014-06-11

As far as I can tell, this is supported by all major distributions that are under active support.

Versions of systemd in Debian that are still under standard or Long-Term Support (LTS):

stretch (June 2017): 232
buster (July 2019): 241
bullseye (August 2021): 247
Source: https://packages.debian.org/search?keywords=systemd

Versions of systemd in Ubuntu that are still under general support:

bionic (April 2018): 237
focal (April 2020): 245
hirsute (April 2021): 247
impish (October 2021: 248
Source: https://packages.ubuntu.com/search?keywords=systemd

Versions of systemd in Fedora that are still under general support:

Fedora 34: 248
Fedora 35: 249
Source: https://packages.fedoraproject.org/pkgs/systemd/systemd/

RHEL/CentOS forks RockyLinux and AlmaLinux are both shipping version 239, as did CentOS 8.

Various version comparisons can be found at https://pkgs.org/download/systemd

An Internet accessible service should aim to have as little as possible attack surface, which is much easier to do when running with the absolute minimum number of priviledges. This makes the systemd setup run the service as a user 'yggdrasil' and uses the systemd feature `RuntimeDirectory` to auto-create the /var/run/yggdrasil dir so our non-elevated client can still create the socket. The sysusers file will cause the user be created on first install using the sysusers subsystem.

The `AdminListen` option and `yggdrasilctl` now default to `unix:///var/run/yggdrasil/yggdrasil.sock` on Linux This allows yggdrasil to be run as its own user. Closes yggdrasil-network#802

zander · 2022-01-09T14:35:31Z

I tested this new idea on my archlinux (i.e. latest) and also on my server which is debian old-stable (v 9).
It works great on both;

systemctl --version gives;

systemd 249 (249.7-2-arch) and systemd 232 respectively

I'll push an update shortly.

@jgoerzen

Also use some stricter security features systemd provides. This change from github user John Goerzen @jgoerzen as provided in his comment: yggdrasil-network#816 (comment) ProtectSystem=strict prevents it from modifying basically anything on the filesystem, so therefore we have to specify the possible locations for the runtime directory under ReadWritePaths.

jgoerzen · 2022-01-20T18:54:01Z

An update:

Per discussion in Matrix, CAP_NET_RAW is no longer necessary.

You may be interested in the service file I'm shipping with the package that will go into Debian; it's linked at #874 (comment)

neilalexander · 2022-01-20T20:07:00Z

I removed CAP_NET_RAW already in a4bdf3d.

neilalexander · 2023-10-21T18:07:57Z

Closing in favour of #927.

cofob approved these changes Jul 24, 2021

View reviewed changes

Arceliar requested changes Jul 24, 2021

View reviewed changes

zhoreeq reviewed Jul 24, 2021

View reviewed changes

contrib/systemd/yggdrasil.sysusers Outdated Show resolved Hide resolved

src/defaults/defaults_linux.go Show resolved Hide resolved

zander commented Jul 24, 2021

View reviewed changes

src/defaults/defaults_linux.go Show resolved Hide resolved

Arceliar mentioned this pull request Jul 24, 2021

Make it possible to override the default AdminListen option at compile time #818

Closed

zander added 2 commits January 9, 2022 14:50

Match sock file default to systemd setup.

0c7abaa

The `AdminListen` option and `yggdrasilctl` now default to `unix:///var/run/yggdrasil/yggdrasil.sock` on Linux This allows yggdrasil to be run as its own user. Closes yggdrasil-network#802

zander force-pushed the RunAsUser branch from 5370597 to 2fbfa4d Compare January 9, 2022 14:41

zander requested a review from Arceliar January 9, 2022 14:46

jgoerzen mentioned this pull request Jan 20, 2022

Multiple quality issues with the generated deb #874

Open

Merge branch 'develop' into RunAsUser

b3eb5f6

neilalexander force-pushed the develop branch from 5a0470e to 8ce7c86 Compare October 15, 2022 16:46

neilalexander closed this Oct 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security] do not run as root. #816

[Security] do not run as root. #816

zander commented Jul 24, 2021

cofob left a comment

Arceliar Jul 24, 2021

zander Jul 24, 2021 •

edited

Loading

OlegGirko Jul 24, 2021

zander Jul 24, 2021

OlegGirko Jul 24, 2021

OlegGirko Jul 24, 2021

zander commented Jul 24, 2021

Arceliar commented Jul 24, 2021 •

edited

Loading

zander commented Jul 24, 2021 •

edited

Loading

kotovalexarian commented Aug 1, 2021

jgoerzen commented Jan 5, 2022

zander commented Jan 6, 2022

jgoerzen commented Jan 6, 2022

zander commented Jan 9, 2022

jgoerzen commented Jan 20, 2022

neilalexander commented Jan 20, 2022

neilalexander commented Oct 21, 2023

[Security] do not run as root. #816

[Security] do not run as root. #816

Conversation

zander commented Jul 24, 2021

cofob left a comment

Choose a reason for hiding this comment

Arceliar Jul 24, 2021

Choose a reason for hiding this comment

zander Jul 24, 2021 • edited Loading

Choose a reason for hiding this comment

OlegGirko Jul 24, 2021

Choose a reason for hiding this comment

zander Jul 24, 2021

Choose a reason for hiding this comment

OlegGirko Jul 24, 2021

Choose a reason for hiding this comment

OlegGirko Jul 24, 2021

Choose a reason for hiding this comment

zander commented Jul 24, 2021

Arceliar commented Jul 24, 2021 • edited Loading

zander commented Jul 24, 2021 • edited Loading

kotovalexarian commented Aug 1, 2021

jgoerzen commented Jan 5, 2022

zander commented Jan 6, 2022

jgoerzen commented Jan 6, 2022

zander commented Jan 9, 2022

jgoerzen commented Jan 20, 2022

neilalexander commented Jan 20, 2022

neilalexander commented Oct 21, 2023

zander Jul 24, 2021 •

edited

Loading

Arceliar commented Jul 24, 2021 •

edited

Loading

zander commented Jul 24, 2021 •

edited

Loading