Discussion:
Bug#934500: dh-runit: permissions of supervise directory
(too old to reply)
Dmitry Bogatov
2019-08-11 17:50:02 UTC
Permalink
Package: dh-runit
Version: 2.8.13.2
Severity: normal

In #924903 permissions of /var/lib/runit/supervise were changed from 755
to 700. As pointed by latest Lintian version, Policy 10.9 recommends
755.

The simpliest fix is revert of #924903. More attractive way is to move
supervise directories into /run.

Opinions? Preferences? Any considerations I miss?
--
Note, that I send and fetch email in batch, once in a few days.
Please, mention in body of your reply when you add or remove recepients.
Lorenz
2019-08-12 08:20:02 UTC
Permalink
Post by Dmitry Bogatov
The simpliest fix is revert of #924903. More attractive way is to move
supervise directories into /run.
The permission issue is not essential to me, but i prefer to move supervise
directories into /run.
I don't see a strong reason to have persistent supervise directories: the
files inside are mostly (if not all) not recyclable, bug like #919296
proves that pre-create files with the porpose of speeding up the start of a
runsv process doesn't work.
Moreover, if i recall correcly, Debian's runit already need to rely on /run
if it wants to become /etc readonly proof.
Let me know in advance if you are going this way, I will prepare a patch
for update-service too.

Lorenzo
Dmitry Bogatov
2019-08-14 19:30:01 UTC
Permalink
Post by Lorenz
[...]
I don't see a strong reason to have persistent supervise directories: the
files inside are mostly (if not all) not recyclable, bug like #919296
proves that pre-create files with the porpose of speeding up the start of a
runsv process doesn't work.
Moreover, if i recall correcly, Debian's runit already need to rely on /run
if it wants to become /etc readonly proof.
Let me know in advance if you are going this way, I will prepare a patch
for update-service too.
Yes, I go this way.

PS. I am positive I did wrote reply to this mail several days ago. How
did I managed to blackhole it? Hm...
--
Note, that I send and fetch email in batch, once in a few days.
Please, mention in body of your reply when you add or remove recepients.
Lorenz
2019-08-16 21:30:02 UTC
Permalink
Post by Dmitry Bogatov
Yes, I go this way.
Ok.
in commit d07519ae you already create /run/runit/supervise directory,
but the directory for the appendant log of 'foo' service will be
/run/runit/log/supervise/foo
or
/run/runit/supervise/foo.log ?

Lorenzo
Dmitry Bogatov
2019-08-19 11:40:01 UTC
Permalink
part 2 text/plain 323
Post by Dmitry Bogatov
Yes, I go this way.
Ok.
in commit d07519ae you already create /run/runit/supervise directory,
but the directory for the appendant log of 'foo' service will be
/run/runit/log/supervise/foo
or
/run/runit/supervise/foo.log ?
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This. Looks fine to me.
--
Note, that I send and fetch email in batch, once in a few days.
Please, mention in body of your reply when you add or remove recepients.
Lorenzo Puliti
2019-08-19 13:30:02 UTC
Permalink
Package: dh-runit
Version: 2.8.13.2
Followup-For: Bug #934500

The patch for 'update-service' is attached


-- System Information:
Debian Release: bullseye/sid
APT prefers unstable
APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 4.20.3-van (SMP w/4 CPU cores; PREEMPT)
Kernel taint flags: TAINT_OOT_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US:en (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: runit (via /run/runit.stopit)

Versions of packages dh-runit depends on:
ii debhelper 12.4
ii libfile-copy-recursive-perl 0.44-1
ii libfile-slurp-perl 9999.27-1
ii libtext-hogan-perl 2.01-1

dh-runit recommends no packages.

dh-runit suggests no packages.

-- no debconf information
Dmitry Bogatov
2019-08-20 16:10:01 UTC
Permalink
control: tags -1 +patch
Post by Dmitry Bogatov
Package: dh-runit
Version: 2.8.13.2
Followup-For: Bug #934500
The patch for 'update-service' is attached
From 200a8e68089a30177798e1e5f5e6c6def45fef64 Mon Sep 17 00:00:00 2001
Date: Mon, 19 Aug 2019 14:58:38 +0200
Subject: [PATCH] update-service: move supervise directories in tmpfs
Move log and service's supervise directories under /run/
tmpfs (they were in /var/ previously)
---
debian/contrib/update-service | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/debian/contrib/update-service b/debian/contrib/update-service
index 7e72501..83f0ff8 100644
--- a/debian/contrib/update-service
+++ b/debian/contrib/update-service
@@ -63,11 +63,11 @@ case "$opt" in
if test "${svdir#/etc/}" != "$svdir"; then
if test ! -h "$svdir"/supervise; then
rm -rf "$svdir"/supervise
- ln -s /var/lib/runit/supervise/"$sv" "$svdir"/supervise
+ ln -s /run/runit/supervise/"$sv" "$svdir"/supervise
Will it handle both /var/lib and /run/runit location?

I expect this change of supervise location to propagate as packages are
rebuilt with new dh-runit, so for some time there will be packages that
use old location and packages that use new location.
Post by Dmitry Bogatov
[...]
And here is my patch for dh_runit. Since libghc-shake-dev is still in
transition, I think of temporary disabling testsuite and uploading next
weekend.

From 372f39c6d8bb05551a0910a7bf62f3d6cd6bc050 Mon Sep 17 00:00:00 2001
From: Dmitry Bogatov <***@debian.org>
Date: Tue, 20 Aug 2019 03:19:01 +0000
Subject: [PATCH] Move supervise directories of generated packages to tmpfs

* dh_runit: make /etc/sv/<foo>/supervise link to /run/runit/supervise/<foo>.
Link is dangling, runsv(8) will create its target at runtime.

* t/checks/924903/check: remove test about permissions of generated supervise directory.

Closes: #934500
---
dh_runit | 10 ++--------
t/checks/924903/check | 8 +-------
2 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/dh_runit b/dh_runit
index 9825a4d..03efc79 100755
--- a/dh_runit
+++ b/dh_runit
@@ -122,10 +122,7 @@ PKG: foreach my $pkg (@{$dh{DOPACKAGES}}) {
# Unfortunately, dh_fixperms does not handle executable bit here.
ensure_executable("$sv_dir/$name");
}
- make_symlink("/etc/sv/$name/supervise",
- "/var/lib/runit/supervise/$name", $tmp);
- install_dir("$tmp/var/lib/runit/supervise/$name");
- chmod 0700, "$tmp/var/lib/runit/supervise/$name";
+ make_symlink("/etc/sv/$name/supervise", "/run/runit/supervise/$name", $tmp);
install_dir("$tmp/etc/runit/runsvdir/default");

my $substitutions = {
@@ -146,10 +143,7 @@ PKG: foreach my $pkg (@{$dh{DOPACKAGES}}) {
template_from_data_directory('logscript', "$sv_dir/$name/log/run",
{ logdir => $logdir }, 0755);

- make_symlink("/etc/sv/$name/log/supervise",
- "/var/lib/runit/log/supervise/$name", $tmp);
- install_dir("$tmp/var/lib/runit/log/supervise/$name");
- chmod 0700, "$tmp/var/lib/runit/log/supervise/$name";
+ make_symlink("/etc/sv/$name/log/supervise", "/run/runit/supervise/$name.log", $tmp);
}
}
# runit=2.1.2-20 introduced 'runit-log' user
diff --git a/t/checks/924903/check b/t/checks/924903/check
index e2e9fa4..ec2aeff 100644
--- a/t/checks/924903/check
+++ b/t/checks/924903/check
@@ -1,14 +1,8 @@
#!/usr/bin/perl
use strict;
use warnings;
-use Test::More tests => 3;
+use Test::More tests => 1;
use File::stat;

-my $path = 'debian/dh-runit-test/var/lib/runit/supervise/test';
-ok(-d $path, 'supervise directory correctly created');
-my $info = stat($path);
-my $mode = sprintf("%o", $info->mode & 0777);
-is($mode, '700', 'supervise directory have conservative permissions');
-
my $noreplace = 'debian/dh-runit-test/var/lib/runit/noreplace/test';
ok(!-f $noreplace, 'noreplace file is correctly absent');
--
Note, that I send and fetch email in batch, once in a few days.
Please, mention in body of your reply when you add or remove recepients.
Jan Braun
2019-08-21 16:50:02 UTC
Permalink
Source: dh-runit
Followup-For: Bug #934500

Dear Maintainer,
I'd like to point out that moving the supervise directories to /run
means that they get wiped on reboot. Therefore the local admin can't
persistently change their permissions, to give certain users additional
rights.
Personally, I see little difference between fiddling with /etc/sudoers
or with permissions in /var/lib/runit/* , but sudoless operation is
touted by some as an advantage of daemontools-style supervision.

Just trying to make sure you've considered this aspect of the
transition before you pour more energy into it.

Thanks a lot for pushing runit into debian, much appreciated!
cheers,
Jan

-- System Information:
Debian Release: bullseye/sid
APT prefers testing
APT policy: (990, 'testing'), (650, 'testing-debug'), (550, 'unstable-debug'), (550, 'unstable'), (10, 'experimental-debug'), (10, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.19.0-5-amd64 (SMP w/4 CPU cores)
Locale: LANG=C.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8), LANGUAGE=C.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)
Dmitry Bogatov
2019-08-23 11:30:01 UTC
Permalink
Post by Jan Braun
Source: dh-runit
Followup-For: Bug #934500
Dear Maintainer,
I'd like to point out that moving the supervise directories to /run
means that they get wiped on reboot. Therefore the local admin can't
persistently change their permissions, to give certain users additional
rights.
Personally, I see little difference between fiddling with /etc/sudoers
or with permissions in /var/lib/runit/* , but sudoless operation is
touted by some as an advantage of daemontools-style supervision.
Just trying to make sure you've considered this aspect of the
transition before you pour more energy into it.
No, I did not consider this aspect. Thank you.
But since runscripts are conffiles, admin can add line

chown -R trusted_user:0 /run/runit/supervise/foo

into /etc/sv/foo/run, and they will be preserved during upgrade. Not
that staightforward, but still solution. Is it acceptable?
--
Note, that I send and fetch email in batch, once in a few days.
Please, mention in body of your reply when you add or remove recepients.
Jan Braun
2019-08-24 14:10:03 UTC
Permalink
Post by Dmitry Bogatov
No, I did not consider this aspect. Thank you.
But since runscripts are conffiles, admin can add line
chown -R trusted_user:0 /run/runit/supervise/foo
into /etc/sv/foo/run, and they will be preserved during upgrade. Not
that staightforward, but still solution. Is it acceptable?
That means they'll get a conffile prompt if/whe the maintainer changes
the run file. Personally, I'd prefer linking /etc/sv/foo/supervise to a
place of my choosing and expect that link to be preserved. Not sure how
others would feel, or how they'd try to deal with the issue.

<rambling = on>
I'm still trying to figure out the best way to split things for my
personal needs (which include service definitions being rsync()able
across machines), and in the middle of another reorganization. So I
clearly haven't found the ideal layout yet either. :(

FWIW, I really liked having a ramdisk on /etc/service and copying
/etc/sv/* there. Biggest issue with that was getting updates across
cleanly. I wish runsvdir had a mode to "kill -TERM your runsvdir
children, wait for them to exit, then exit yourself." Maybe the answer
will be implementing that. Or maybe a 2nd attempt at scaffolding to move
services out of the way, wait for them to stop, move the new definitions
in place. Or maybe have /etc/sv be the ramdisk. Aaargh. :D
</rambling>

Anyway, I clearly have no bronze bullet, sorry.
cheers,
Jan
Dmitry Bogatov
2019-08-26 21:30:02 UTC
Permalink
Post by Jan Braun
Post by Dmitry Bogatov
No, I did not consider this aspect. Thank you.
But since runscripts are conffiles, admin can add line
chown -R trusted_user:0 /run/runit/supervise/foo
into /etc/sv/foo/run, and they will be preserved during upgrade. Not
that staightforward, but still solution. Is it acceptable?
That means they'll get a conffile prompt if/whe the maintainer changes
the run file.
This can be solved in /lib/runit/invoke-run. Something like running
/etc/service/foo/run.pre before "run".

Feel free to file wishlist. With heavy heart, but I will implement it.
Post by Jan Braun
Personally, I'd prefer linking /etc/sv/foo/supervise to a place of my
choosing and expect that link to be preserved. Not sure how others
would feel, or how they'd try to deal with the issue.
Why would you need it? Content of 'supervise' directory is transient,
there is nothing valuable in it.

While I understand desire to make things as configurable as possible, it
will put burden of /properly/ purging things from dpkg to me, which I'd
rather avoid.
--
Note, that I send and fetch email in batch, once in a few days.
Please, mention in body of your reply when you add or remove recepients.
Jan Braun
2019-08-27 14:40:02 UTC
Permalink
Hi,
Post by Dmitry Bogatov
Post by Jan Braun
That means they'll get a conffile prompt if/whe the maintainer changes
the run file.
This can be solved in /lib/runit/invoke-run. Something like running
/etc/service/foo/run.pre before "run".
Feel free to file wishlist. With heavy heart, but I will implement it.
Eww, no.
Post by Dmitry Bogatov
Post by Jan Braun
Personally, I'd prefer linking /etc/sv/foo/supervise to a place of my
choosing and expect that link to be preserved. Not sure how others
would feel, or how they'd try to deal with the issue.
Why would you need it? Content of 'supervise' directory is transient,
there is nothing valuable in it.
Except the permissions, if non-default.
Post by Dmitry Bogatov
While I understand desire to make things as configurable as possible, it
will put burden of /properly/ purging things from dpkg to me, which I'd
rather avoid.
Are you saying "me" as in the Debian maintianier of runit, or "me" as in
the local admin of the machine in question?

When I add files in non-default locations, I expect dpkg/maintainer
scripts not to touch them, and possibly dpkg to message me "directory
/some/path not empty, not removed" when purging the package, alerting me
that I might need to clean up manually. I think that kind of behaviour
is required by debian policy, it has worked whenever I needed it in the
past, and AFAICT it works with the runit* packages right now. So there
should be no additional burden on you, the Debian maintainer.

If you, the local admin of your own machine(s), don't like that
behaviour, then just don't add files. :-P It's fine if you edit
/etc/sv/foo/run while I change the symlink /etc/sv/foo/supervise and
create /some/where/supervise-foo . Both will achieve the same result:
persisting non-default permissions of the supervise dir. You will get
bothered by dpkg when the runscript changes, I will need to manually
clean up when I purge foo. Valid tradeoff, and nothing that needs to be
resolved IMHO.


If you meant the purging of /var/lib/runit/supervise/foo in case the
runit* packages decide to put things there by default, that's an
unconditional "rm -rf" in the appropriate maintainer script and not much
of a burden. Or am I missing something there?


As to where Debian's runit should point the symlink by default:

/var/lib/runit/supervise/foo:
+ persists non-default supervise dir permissions at no cost to the
local admin.
- unneccessarily persists the rest of the supervise dir.
/run/runit/supervise/foo:
- requires admin effort to get persistent non-default supervise dir
permissions.
+ saves writes to a durable medium during operation.

I don't really have a preference here.


And I don't need to. :) Because on my machines, I'm back to a ramdisk on
/etc/service, which:
+ persists exactly what it's told to (by placing it in /etc/sv )
+ saves writes to durable storage
- requires a reboot or other admin action to apply their /etc/sv
changes to the ramdisk. :(

I doubt that's an acceptable default configuration for Debian, but it's
implemented as an option with very little effort:
* a few lines setting up the ramdisk in /etc/runit/2 (conditional on
/etc/service being a symlink to /run/service),
* a one-line change preventing /lib/runit/invoke-run from dying to
the changed path¹,
* a new script (or patch to update-service) to implement said
admin action without rebooting.
Holler if you're interested in adding it to Debian. (Which would be
nice, but I can live very well on my local modifications too.) Not sure
if having this as an option should tilt the /run vs. /var decision.

cheers,
Jan


¹I don't like that aspect of invoke-run at all. I'll probably file a
separate bug for it.
Dmitry Bogatov
2019-08-28 16:30:02 UTC
Permalink
Post by Jan Braun
Post by Dmitry Bogatov
Post by Jan Braun
Personally, I'd prefer linking /etc/sv/foo/supervise to a place of my
choosing and expect that link to be preserved. Not sure how others
would feel, or how they'd try to deal with the issue.
Why would you need it? Content of 'supervise' directory is transient,
there is nothing valuable in it.
Except the permissions, if non-default.
Okay.
Post by Jan Braun
Post by Dmitry Bogatov
While I understand desire to make things as configurable as possible, it
will put burden of /properly/ purging things from dpkg to me, which I'd
rather avoid.
Are you saying "me" as in the Debian maintianier of runit, or "me" as in
the local admin of the machine in question?
As Debian maintainer.
Post by Jan Braun
When I add files in non-default locations, I expect dpkg/maintainer
scripts not to touch them,
This. Maintainers scripts is my burden.
Post by Jan Braun
and possibly dpkg to message me "directory /some/path not empty, not
removed" when purging the package, alerting me that I might need to
clean up manually. I think that kind of behaviour is required by
debian policy, it has worked whenever I needed it in the past, and
AFAICT it works with the runit* packages right now. So there should be
no additional burden on you, the Debian maintainer.
That is the point. dpkg manages files that are part of package (as of
dpkg -L), that is why I want make /etc/sv/foo/service symlink part of
package.

If I create something in maintainer script, I have to purge it in
maintainer script, with all kinds of interesting corner cases (e.g
#848239 #848240)
Post by Jan Braun
If you meant the purging of /var/lib/runit/supervise/foo in case the
runit* packages decide to put things there by default, that's an
unconditional "rm -rf" in the appropriate maintainer script and not much
of a burden. Or am I missing something there?
If admin put something valuable in /var/lib/runit/supervise/foo I have
to re-implement (see runit-helper source) "directory not empty, not
removing" logic.
Post by Jan Braun
+ persists non-default supervise dir permissions at no cost to the
local admin.
- unneccessarily persists the rest of the supervise dir.
- requires admin effort to get persistent non-default supervise dir
permissions.
+ saves writes to a durable medium during operation.
I don't really have a preference here.
Thanks for this summary. I did not thought about saving writes.

As I pointed above I have strong preference for /run/runit. Actually, I
consider /var/lib/runit/supervise major design error of mine.
Post by Jan Braun
And I don't need to. :) Because on my machines, I'm back to a ramdisk on
+ persists exactly what it's told to (by placing it in /etc/sv )
+ saves writes to durable storage
- requires a reboot or other admin action to apply their /etc/sv
changes to the ramdisk. :(
I doubt that's an acceptable default configuration for Debian, but it's
* a few lines setting up the ramdisk in /etc/runit/2 (conditional on
/etc/service being a symlink to /run/service),
* a one-line change preventing /lib/runit/invoke-run from dying to
the changed path¹,
* a new script (or patch to update-service) to implement said
admin action without rebooting.
Holler if you're interested in adding it to Debian. (Which would be
nice, but I can live very well on my local modifications too.) Not sure
if having this as an option should tilt the /run vs. /var decision.
While I see advantages of your setup, I am not ready to support it
Debian-wide, in numerous Debian configurations. What I can offer is
following:

* I can bundle your /etc/runit/2 in /usr/share/doc/runit/contrib, with
clear notice that this setup is not officially supported by Debian,
but is succesfully used by some admins.

* You can propose patch for update-service. As long it still work
with default configuration and does not introduces too much
complexity, I am okay to apply it. Lorenzo knows better about
`update-service` than me.

* You can file bug on `invoke-run`, we will see what can be amended.
--
Note, that I send and fetch email in batch, once in a few days.
Please, mention in body of your reply when you add or remove recepients.
Dmitry Bogatov
2019-08-23 11:30:01 UTC
Permalink
Post by Dmitry Bogatov
Post by Dmitry Bogatov
if test ! -h "$svdir"/supervise; then
rm -rf "$svdir"/supervise
- ln -s /var/lib/runit/supervise/"$sv" "$svdir"/supervise
+ ln -s /run/runit/supervise/"$sv" "$svdir"/supervise
Will it handle both /var/lib and /run/runit location?
Mmm.. No
In my system i have a mix of
* supervise inside /etc/sv/foo that is not a symlink ( due to my own
experiment i believe)
* supervise that is a symlink to /var/lib/runit/supervise/foo (current
dh-runit)
* supervise that is a symlink to /var/lib/supervise/foo (update-service
before dh-runit)
if you prefer i can force a replace of /var/ with /run each time one types
'update-service --add /etc/sv/foo'
I do not have opinion on that, since I do not use 'update-service'.
new patch attached should handle all of the above cases, except it
won't replace a supervise dir of an already running service (of
course)
Nice, thank you.
About this, I forsee trouble during an upgrade of a package that already
ship a runscript, think of the following
[...]
I can test the opposite (switch from /run into /var) and it doesn't
end up good
[...]
Looks an acpid process managed by runsv survives but i can't send signal to
it!
I see and more or less, understand why it happens. What about making
in postinst symlink

/run/runit/supervise/foo -> /lib/runit/supervise/foo

if latter exists? It should resolve problem with overwriting symlink of
running process.
Maybe let runit-helper create the symlinks rather than shipping in the
package itself is a more flexible approach?
Maybe, but I hope to avoid it. Testing maintainer scripts is pain,
making sure files created by maintainer scripts are correctly
updated/purged is pain.

I want `runit-helper' getting small and trivial, not big and
complicated.
--
Note, that I send and fetch email in batch, once in a few days.
Please, mention in body of your reply when you add or remove recepients.
Lorenz
2019-08-24 13:10:01 UTC
Permalink
[...]What about making
in postinst symlink
/run/runit/supervise/foo -> /lib/runit/supervise/foo
if latter exists? It should resolve problem with overwriting symlink of
running process.
It's ok for me, thanks
Loading...