Created attachment 494550 [details] redis.initd-5 The init script for redis gives ownership of its PID file directory to the daemon's runtime user: start_pre() { checkpath -d -m 0775 -o ${REDIS_USER}:${REDIS_GROUP} \ $(dirname ${REDIS_PID}) } This can be exploited by $REDIS_USER to kill root processes, since when the service is stopped, root will send a SIGTERM to the contents of the PID file (which are under the control of $REDIS_USER). I've rewritten the init script to work around this by having OpenRC manage the PID file itself, and store it directly in /run. I also utilized some other OpenRC variables to clean up the script and to eliminate everything except the depend() function. Finally, I removed "use net" and added an rc_need line to the conf.d file. Since the actual interface we need is determined by redis.conf, the correct dependency needs to be set by the user. (The default 127.0.0.1 isn't even covered by "use net").
Created attachment 494552 [details] redis.confd-r1
https://github.com/gentoo/gentoo/pull/5875
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=f689ea635ecbb7290b1c914000bf83e9289cd5e9 commit f689ea635ecbb7290b1c914000bf83e9289cd5e9 Author: Tomáš Mózes <hydrapolic@gmail.com> AuthorDate: 2017-10-06 20:40:02 +0000 Commit: Robin H. Johnson <robbat2@gentoo.org> CommitDate: 2017-10-08 23:23:40 +0000 dev-db/redis: bump to 4.0.2 (cherry picked from commit 93eac00c9ae82a63fceacdcc5b7ded98e44a3a9c) Bug: https://bugs.gentoo.org/631002 Closes: https://github.com/gentoo/gentoo/pull/5875#issuecomment-334886169 Signed-off-by: Robin H. Johnson <robbat2@gentoo.org> dev-db/redis/Manifest | 1 + dev-db/redis/files/redis-3.2.3-config.patch | 2 +- dev-db/redis/files/redis.confd-r1 | 20 +++++ dev-db/redis/files/redis.initd-5 | 23 ++++++ dev-db/redis/redis-4.0.2.ebuild | 122 ++++++++++++++++++++++++++++ 5 files changed, 167 insertions(+), 1 deletion(-)}
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=7733941faa2a1687dbc16d6cb88c214c0902a376 commit 7733941faa2a1687dbc16d6cb88c214c0902a376 Author: Robin H. Johnson <robbat2@gentoo.org> AuthorDate: 2017-10-09 00:03:11 +0000 Commit: Robin H. Johnson <robbat2@gentoo.org> CommitDate: 2017-10-09 00:13:07 +0000 dev-db/redis: backport security & Lua-combo fixes from 4.x to 3.2.8 release. Bug: https://bugs.gentoo.org/show_bug.cgi?id=631002 Package-Manager: Portage-2.3.8, Repoman-2.3.3 dev-db/redis/redis-3.2.8-r4.ebuild | 131 +++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+)}
arches, please test & stabilize redis-3.2.8-r4 AND redis-4.0.2. target keywords: amd64, arm, hppa, ppc, ppc64, x86, arm64 test instructions: Package with FEATURES=test
arches, see comment #5 for stabilizing please.
Hi, Since the last update, I end with 2 pid files : -rw-r--r--. 1 root root system_u:object_r:initrc_var_run_t 6 12 oct. 12:19 /run/redis.pid /run/redis: total 4 -rw-r--r--. 1 redis redis system_u:object_r:redis_var_run_t 6 12 oct. 12:19 redis.pid srwxrwx---. 1 redis redis system_u:object_r:redis_var_run_t 0 12 oct. 12:19 redis.sock The one monitored by OpenRC is /run/redis.pid, but if we look at the content of the file, the pid does not exist. The correct one is stored inside /run/redis/redis.pid. Thus it leaves the redis service in a crashed state, and you have to manually kill the correct pid to actually stop redis. Is this normal ?
(In reply to jack_mort from comment #7) > Hi, > Since the last update, I end with 2 pid files : > > -rw-r--r--. 1 root root system_u:object_r:initrc_var_run_t 6 12 oct. > 12:19 /run/redis.pid > > /run/redis: > total 4 > -rw-r--r--. 1 redis redis system_u:object_r:redis_var_run_t 6 12 oct. 12:19 > redis.pid > srwxrwx---. 1 redis redis system_u:object_r:redis_var_run_t 0 12 oct. 12:19 > redis.sock > > The one monitored by OpenRC is /run/redis.pid, but if we look at the content > of the file, the pid does not exist. The correct one is stored inside > /run/redis/redis.pid. > > Thus it leaves the redis service in a crashed state, and you have to > manually kill the correct pid to actually stop redis. > > Is this normal ? I forgot to change the pidfile in https://github.com/gentoo/gentoo/blob/master/dev-db/redis/files/redis-3.2.3-config.patch. This happens when you have supervised yes in your /etc/redis.conf, is that correct?
On a second thought, this won't be fixed by adjusting the pidfile in /etc/redis.conf. Even if we set it to /run/redis.pid, redis will not be able to write there with user redis, that was the whole point of the bug. So I'm afraid the setup with "daemonize yes" will not be supported. Sorry I wrongly asked about "supervised yes", it really is "daemonize yes". Robbat2, how about to print a warning that redis setups with "daemonize yes" are not supported any more for security reasons?
"daemonize yes" never quite worked... if you try changing either $REDIS_PID (in the conf.d file) or "pidfile" (in redis.conf) in an older version of redis, then you'll still wind up with an extra junk PID file. By default, $REDIS_PID and "pidfile" agreed, so the junk file was fortuitously overwritten with the real one, and nobody noticed. But ultimately, OpenRC is trying to background the process itself (in order to run as a non-root user), so we've always wanted "daemonize no". Fortunately, it looks like we can force "daemonize no" on the command-line, regardless of what the config file says. See "Passing arguments via the command line" in, https://redis.io/topics/config I'm way too tired to test it right now, but it looks like "--daemonize no" in $command_args should do it.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=b545fdf6233c35ef3173109dfcd3d0c2b05da205 commit b545fdf6233c35ef3173109dfcd3d0c2b05da205 Author: Robin H. Johnson <robbat2@gentoo.org> AuthorDate: 2017-10-12 20:30:01 +0000 Commit: Robin H. Johnson <robbat2@gentoo.org> CommitDate: 2017-10-12 20:36:58 +0000 dev-db/redis: use s-s-d to background, disallow daemonize. Bug: https://bugs.gentoo.org/631002#c10 Package-Manager: Portage-2.3.8, Repoman-2.3.3 dev-db/redis/files/redis.initd-5 | 4 +++- dev-db/redis/{redis-3.2.8-r4.ebuild => redis-3.2.8-r5.ebuild} | 0 dev-db/redis/{redis-4.0.2.ebuild => redis-4.0.2-r1.ebuild} | 0 3 files changed, 3 insertions(+), 1 deletion(-)}
Implemented & tested.
An automated check of this bug failed - the following atoms are unknown: dev-db/redis-3.2.8-r5 dev-db/redis-4.0.2-r1 Please verify the atom list.
Thanks for pointing out the solution and fixing this. However what to do with the /etc/redis.conf pidfile. I know it's not used, but to avoid confusion, we could possibly comment it. And also to print a warning that we enforce "daemonize no".
Hi, I was surprisingly using daemonize=yes, good catch. Now it's all OK with -r1 : only one pidfile and rc-status shows a started process :-) Thanks.
x86 stable
ppc64 stable
amd64 stable
removed unstable ARM64 arch from list.
arm stable
(In reply to Ultrabug from comment #19) > removed unstable ARM64 arch from list. Please do not do that when we have earlier versions stable.
arm64 stable despite test failure (bug 649556), as not a regression
hppa stable
ppc stable
@maintainer(s), please drop vulnerable.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=248dbab6baed691d61011be1a4115b25e2f05e4f commit 248dbab6baed691d61011be1a4115b25e2f05e4f Author: Robin H. Johnson <robbat2@gentoo.org> AuthorDate: 2018-08-15 03:09:02 +0000 Commit: Robin H. Johnson <robbat2@gentoo.org> CommitDate: 2018-08-15 03:09:45 +0000 dev-db/redis: cleanup old ebuilds Bug: https://bugs.gentoo.org/631002 Signed-off-by: Robin H. Johnson <robbat2@gentoo.org> Package-Manager: Portage-2.3.40, Repoman-2.3.9 dev-db/redis/Manifest | 3 - dev-db/redis/files/configure.ac-2.2 | 58 ----------- dev-db/redis/files/redis-2.8.17-config.patch | 46 --------- dev-db/redis/files/redis-2.8.3-shared.patch | 36 ------- dev-db/redis/files/redis-3.0.0-sharedlua.patch | 44 --------- dev-db/redis/files/redis.confd | 20 ---- dev-db/redis/files/redis.initd-4 | 31 ------ dev-db/redis/redis-3.0.7-r1.ebuild | 115 ---------------------- dev-db/redis/redis-3.0.7.ebuild | 115 ---------------------- dev-db/redis/redis-3.2.5.ebuild | 123 ------------------------ dev-db/redis/redis-3.2.8-r2.ebuild | 123 ------------------------ dev-db/redis/redis-4.0.1-r1.ebuild | 128 ------------------------- dev-db/redis/redis-4.0.1.ebuild | 122 ----------------------- 13 files changed, 964 deletions(-)
