The init script for filebeat gives ownership of the PID file directory to the same user that the daemon runs as: checkpath -d -o "${FILEBEAT_USER}":"${FILEBEAT_GROUP}" -m750 "$(dirname "${pidfile}")" As a result, the ${FILEBEAT_USER} user can write whatever he wants into the PID file. Later, that may be exploitable: when the service is stopped, root will call "kill" on the contents of that file. But there's good news: there's absolutely no reason for the call to "checkpath" above. With command_background="true", OpenRC creates the PID file as root:root, and the problematic "checkpath" can be deleted if you store the PID file directly in /run. Some other minor improvements to consider while you're touching the init script: 1. Utilize command_user for the user/group instead of start_stop_daemon_args 2. Can the OpenRC "retry" variable be used to eliminate stop() ? This isn't a very big deal since ${FILEBEAT_USER} and ${FILEBEAT_GROUP} default to root. However, if there's a benefit to running as a restricted user, maybe now's a good time to rethink the way it's done? Rather than let the user/group be variable, I would suggest creating a dedicated user/group in the ebuild, and then hard-coding those in the init/conf scripts. The reason is, if you let the end-user change FILEBEAT_USER, you will quickly hit an issue: your files might be owned by the old user, and inaccessible to the new one. To work around that, you'll come up with some recursive chown thing, and introduce a new problem. (This is all tangential; just something to keep in mind.) If the user/group were hard-coded, the other two checkpath calls would also be redundant.
Thanks for sharing your finding Michael. The user/group is configurable because filebeat is made mostly for reading logs, some are owner by root, some may be owner by unprivileged users. Therefore I think it's nice to be able to set the user. The problem is that we cannot play nice when changing users without posing a security risk. To be on the safe side, the ebuild should note, that changing users is to be processed by the user itself, however they may be times we switch the default user and it may break user installations. I'm working on an improved version of the init script, will link a PR here (hopefully soon). So far it looks like this: # diff -u filebeat.initd /etc/init.d/filebeat --- filebeat.initd 2017-02-12 17:03:04.000000000 +0100 +++ /etc/init.d/filebeat 2017-08-22 14:46:05.205668639 +0200 @@ -12,11 +12,12 @@ command="/usr/bin/filebeat" command_args="-c ${FILEBEAT_CONFIG} ${FILEBEAT_OPTS} -path.config $(dirname $FILEBEAT_CONFIG) \ -path.data ${FILEBEAT_DATADIR} -path.home ${FILEBEAT_DATADIR} -path.logs ${FILEBEAT_LOGDIR}" -extra_commands="checkconfig" command_background="true" -start_stop_daemon_args="--user ${FILEBEAT_USER}:${FILEBEAT_GROUP} \ - --chdir ${FILEBEAT_DATADIR}" -pidfile="/run/filebeat/filebeat.pid" +command_user="${FILEBEAT_USER}" +extra_commands="checkconfig" +pidfile="/run/filebeat.pid" +retry="TERM/5/KILL/5" +start_stop_daemon_args="--chdir ${FILEBEAT_DATADIR}" depend() { use net @@ -37,14 +38,6 @@ start_pre() { checkconfig || return 1 - checkpath -d -o "${FILEBEAT_USER}":"${FILEBEAT_GROUP}" -m750 "$(dirname "${pidfile}")" checkpath -d -o "${FILEBEAT_USER}":"${FILEBEAT_GROUP}" -m750 "${FILEBEAT_DATADIR}" checkpath -d -o "${FILEBEAT_USER}":"${FILEBEAT_GROUP}" -m750 "${FILEBEAT_LOGDIR}" } - -stop() { - ebegin "Stopping filebeat" - start-stop-daemon --stop \ - --pidfile=${pidfile} \ - --retry=TERM/5/KILL/5 -}
https://github.com/gentoo/gentoo/pull/5498 We can then stabilize version 5.5.2.
Please stabilize app-admin/filebeat-5.5.2 on amd64 and x86. Thanks.
(In reply to Tomáš Mózes from comment #3) > Please stabilize app-admin/filebeat-5.5.2 on amd64 and x86. Thanks. Thank you, since there was no previous stable version on x86, that should go on a new report, we can stabilize arches that already were stable before the report. I'll call amd64 for stabilization. @Arch, please test and mark stable. Gentoo Security Padawan ChrisADR
amd64 stable GLSA Vote: No
