systemctl: add possibility to skip the chroot check#4374
systemctl: add possibility to skip the chroot check#4374keszybz merged 1 commit intosystemd:masterfrom
Conversation
lnykryn
force-pushed
the
systemctl_allow_chroot
branch
from
ce6188c to
c8343c8
Compare
src/systemctl/systemctl.c
Outdated
| const char *e; | ||
| int r; | ||
|
|
||
| e = getenv("SYSTEMCTL_ALLOW_CHROOT"); |
There was a problem hiding this comment.
please use getenv_bool() for this, it makes this call a bit simpler...
src/systemctl/systemctl.c
Outdated
| goto finish; | ||
|
|
||
| if (arg_action != ACTION_SYSTEMCTL && running_in_chroot() > 0) { | ||
| if (arg_action != ACTION_SYSTEMCTL && running_in_chroot() > 0 && !arg_allow_chroot) { |
There was a problem hiding this comment.
there are a couple of more cases where we check for chroots in systemctl.c, what about those? There's also the VERB_NOCHROOT logic used all over the place, what about that?
Maybe the env var check should be added to running_in_chroot() itself, so that it applies everywhere? (and then probably be renamed to $SYSTEMD_IGNORE_CHROOT or so?
There was a problem hiding this comment.
Yeah it probably make sense, I will rewrite that.
poettering
added
systemctl
reviewed/needs-rework 🔨
lnykryn
force-pushed
the
systemctl_allow_chroot
branch
2 times, most recently
from
989494d to
8e45a05
Compare
lnykryn
removed
the
reviewed/needs-rework 🔨
|
I have force pushed the branch with the SYSTEMD_IGNORE_CHROOT stuff. I am just not sure if getenv_bool should not use secure_getenv instead. |
lnykryn
force-pushed
the
systemctl_allow_chroot
branch
from
8e45a05 to
dbc827a
Compare
LGTM. |
|
But we are using secure_getenv i environment_parse_log functions, which looks to me as a similar case now, |
secure_getenv(3) says: The secure_getenv() function is intended for use in general-purpose libraries to avoid vulnerabilities that could occur if set-user-ID or set-group-ID programs accidentally trusted the environment. log_parse_environment is only used internally in our daemons and is not exported as a library function, or called by any of the library functions. And it should not be: it uses systemd specific variable names. So using secure_getenv() is not necessary. Our daemons are not written to be used suid, and we should not pretend that anything like that is supported. While at it, improve the error messages to mention what we were trying to parse. Follow-up for systemd#4374.
|
Hmm, the getenv() vs. secure_getenv() thing is a pretty good question. So far I mostly treated src/shared and in particular src/basic as library code, as it is exposed indirectly (at least partially) in libsystemd.so, hence I ended up using secure_getenv() for most of that code, except for some obvious cases. I have the suspicion that secure_getenv_bool() probably wouldn't be the worst of the ideas... That said, I figure in this specific case it really doesn't matter, dunno if we should really bother... |
3 participants
https://bugzilla.redhat.com/show_bug.cgi?id=1379852