Skip to content

[Security] do not run as root. #816

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

[Security] do not run as root. #816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aa46ee5
[Security] do not run as root.
zander Jul 24, 2021
0c7abaa
Match sock file default to systemd setup.
zander Jul 24, 2021
2fbfa4d
Update to make systemd create the user
zander Jan 9, 2022
b3eb5f6
Merge branch 'develop' into RunAsUser
neilalexander Jan 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- in case of vulnerabilities.
-->

## [future]

### Changed
- The `AdminListen` option and `yggdrasilctl` now default to `unix:///var/run/yggdrasil/yggdrasil.sock` on Linux


## [0.4.2] - 2021-11-03
### Fixed
- Reverted a dependency update which resulted in problems building with Go 1.16 and running on Windows
Expand Down
13 changes: 11 additions & 2 deletions contrib/systemd/yggdrasil.service
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,25 @@ After=network-online.target
After=yggdrasil-default-config.service

[Service]
Group=yggdrasil
ProtectHome=true
ProtectSystem=true
SyslogIdentifier=yggdrasil
CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_BIND_SERVICE
ExecStartPre=+-/sbin/modprobe tun
ExecStart=/usr/bin/yggdrasil -useconffile /etc/yggdrasil.conf
ExecReload=/bin/kill -HUP $MAINPID
Restart=always
TimeoutStopSec=5
Group=yggdrasil
User=yggdrasil-dyn
DynamicUser=true
ProtectSystem=strict
NoNewPrivileges=true
ReadWritePaths=/var/run/yggdrasil /run/yggdrasil

# make sure /var/run/yggdrasil/ is created writable for the user.
RuntimeDirectory=yggdrasil
# the small list of admin capabilities we need to do our job
AmbientCapabilities=CAP_NET_ADMIN CAP_NET_BIND_SERVICE

[Install]
WantedBy=multi-user.target
2 changes: 1 addition & 1 deletion src/defaults/defaults_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ package defaults
func GetDefaults() platformDefaultParameters {
return platformDefaultParameters{
// Admin
DefaultAdminListen: "unix:///var/run/yggdrasil.sock",
DefaultAdminListen: "unix:///var/run/yggdrasil/yggdrasil.sock",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@zander zander Jul 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

For instance wayland creates its sockets there.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).


// Configuration (used for yggdrasilctl)
DefaultConfigFile: "/etc/yggdrasil.conf",
Expand Down