-
-
Notifications
You must be signed in to change notification settings - Fork 578
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
Use pledge(2) on OpenBSD to restrict system calls #1801
Conversation
As per sio_open(3) "Use with pledge(2)" and "cpath wpath dpath" - metadata pipe, coverart cache "inet dns" - metadata socket, RTSP, NQPTP, MQTT "unix" - Avahi, D-Bus, MPRIS "proc exec" - external hooks/scripts Filesystem access is limited to a handful of paths, but accounting for them takes careful work, thus full "[rwcd]path" is not dropped. More importantly, prevent fork(2) and execve(2) if no hooks have been defined (default configuration). (As `config.cmd_*` are split into individual argv[] strings, they cannot be unveil(2)ed to limit "x" permissions to those commands.)
First step in dropping privileges regardless of the user (unprivileged Consider this as Draft to ease and encourage testing as I try to further reduce the set of promises at runtime and/or limit filesystem access to only those paths actually in use (think: no https://man.openbsd.org/pledge.2 |
shairport.c
Outdated
(config.cmd_active_stop == NULL) && | ||
(config.cmd_start == NULL) && | ||
(config.cmd_stop == NULL) && | ||
(config.cmd_set_volume == NULL)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config.unfixable
aka. run_this_if_an_unfixable_error_is_detected
is ALSA only, which OpenBSD does not support, thus ignored here.
Did I miss any other command?
Easy to glance over -h
and only see -B/--cmd-start
and -E/--cmd-stop
as the others are only documented in the configuration file.
if (pledge("stdio rpath wpath cpath dpath inet unix dns audio", NULL) == -1) | ||
die("pledge"); | ||
} | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To limit the dangerous "proc exec"
set to only those commands, one had to hoist poptParseArgvString()
out of common.c`s command handlers, so the following becomes possible:
} else {
unveil(parsed_active_start_argv0, "x");
....
unveil("/", "rwc");
unveil(NULL, NULL); // or pledge("..." /* no "unveil" */, NULL);
}
Account for double-fork(2) when built `--with-libdaemon`.
Worst case is pleding the same unchanged promises a second time, but this draws a better frame around the code that runs with "proc exec" merely because it is between main() start and daemon_fork().
In case nothing is to be executed at runtime, lock down filesystem access down to - write/create metadata pipe/cache - read/rwite D-Bus socket - read glib2 locales If D-Bus setup were to happen earlier, we'd be able to ignore that entirely (socket already opened) at this point and, iff metadata is off, drop "wpath" as well, i.e. run with nothing but read-only locales.
'mkdir -p /tmp/shairport-sync/.cache/' before helps, but ideally, this would be done earlier during startup, or the whole unveil block gets deferred to whenever...
OpenBSD now uses an initial, simple patch. |
Many thanks for this, and apologies for the delay — work IRL intervened. |
No worries, all good. I'll rebase onto the port's patch soon. Either way, this PR remains a draft (mostly to ease testing for me). |
This PR has been inactive for 28 days so will be closed 7 days from now. To prevent this, please remove the "stale" label or post a comment. |
As per sio_open(3) "Use with pledge(2)" and
Filesystem access is limited to a handful of paths, but accounting for
them takes careful work, thus full "[rwcd]path" is not dropped.
More importantly, prevent fork(2) and execve(2) if no hooks have been
defined (default configuration).
(As
config.cmd_*
are split into individual argv[] strings, they cannotbe unveil(2)ed to limit "x" permissions to those commands.)