Skip to content
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

start-stop-daemon: fix: pam_start before chrooting #517

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sijanec
Copy link

@sijanec sijanec commented Apr 17, 2022

pam_start(3) must be called before chroot(2) for chroots that do not have PAM-related files present.

related bug report: http://bugs.gentoo.org/show_bug.cgi?id=292632

Example with patched openrc (on Gentoo):

rc-service sear.c start
 * /var/run/sear.c: creating directory
 * Starting sear.c ...

Service starts successfully. See below to see how is start-stop-daemon being run when this is executed.

Example with unpatched openrc (on Gentoo):

strace --follow-forks -o /tmp/out.txt start-stop-daemon --start --exec sear.c --chroot /var/run/sear.c --pidfile /pid --user searc:searc --make-pidfile
 * start-stop-daemon: pam error: Critical error - immediate abort
 * start-stop-daemon: failed to start `sear.c'

strace output uploaded to http://1507103400/krneki/strace.txt.

Note that on Gentoo openrc is a bit modified. I hope this is still an upstream issue and I'm not falsely submitting a PR, although that may be the case.

@williamh
Copy link
Contributor

It looks like supervise-daemon has the same issue, can you fix it there also?

@sijanec
Copy link
Author

sijanec commented Apr 17, 2022

It looks like supervise-daemon has the same issue, can you fix it there also?

I did the same thing, but I did not test anything.

Another thing: stderr_process and redirect_stderr is also called after chrooting. It might be useful to execute the logger process before chrooting, to avoid including programs for logging and to avoid leaving logs in the chroot.

That would break setups that expect the current behavior, although I doubt that many people went through the hassle of setting up a logger in a chroot (?).

For example using --stderr-logger logger does not work in my case of a minimal chroot.

Perhaps four additional arguments could be added, --{stderr,stdout}{-logger,}-before-chroot for this.

EDIT: I made a simple modification that creates the above options. I tested --stderr-logger-before-chroot with logger on gentoo and it works: http://1507103400/krneki/start-stop-daemon.c

tail -fn3 /var/log/messages
Apr 17 22:52:03 varovalka root: [sear.c] SC_LOG_DEBUG sc_api()@src/api.c:58: contentType = application/xhtml+xml; charset=ISO-8859-1, redir = NULL
Apr 17 22:52:33 varovalka root: cleaning up!
Apr 17 22:52:33 varovalka root: [sear.c] SC_LOG_DEBUG sc_query_free()@src/structs.c:125: sc_query_free: gedxg

Chrooted service is able to output data to the syslog. I haven't commited this to patch-1 yet, because it might not have high enough quality to be merged (?). Shall I commit it?

@sijanec sijanec force-pushed the patch-1 branch 3 times, most recently from 0fd7d52 to f40c585 Compare April 18, 2022 12:51
pam_start(3) must be called before chroot(2), because chroots usually do
not have PAM-related files present.

New options --std{out,err}{-logger,}-before-chroot in start-stop-daemon
source and also described in the manpage.

Manpage start-stop-daemon.8: -background => --background, remove
duplicate --stderr entry.
@williamh
Copy link
Contributor

williamh commented Sep 2, 2022

This commit has too many unrelated changes.
Please rework it so that each commit has a single logical change -- for example, adding an option and documenting it on the man page. Also, doing the pam processing before chroot is called could be a separate commit.

Thanks,

William

@createyourpersonalaccount
Copy link
Contributor

@williamh
Is there still interest for this patch? I could try to rework it into a series of commits as you have asked.

@q3cpma
Copy link

q3cpma commented Aug 18, 2024

Well, that'd be nice in any form. Not to act as an entitled nagger, but for the fun anecdote, I lost a full afternoon trying to chroot the transmission-daemon init script (basically re-implementing parts of bwrap myself by doing RO bind mounts for runtime dependencies) only to get stuck on the same PAM error.

Think I'll continue to launch the daemon manually and bwrap it myself (though bwrap can't forward signals at the moment, so I'll have to use RPC to shut it down).

@createyourpersonalaccount
Copy link
Contributor

@q3cpma I don't think I'm going to work on this right now, I don't use OpenRC currently.

@q3cpma
Copy link

q3cpma commented Aug 18, 2024

@q3cpma I don't think I'm going to work on this right now, I don't use OpenRC currently.

Don't worry, I wasn't expecting anything of the sort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants