This looks like an intentional configuration in Arch Linux. See this for discussion with links to solutions.
The best tip there seems to be adding "DISPLAY XAUTHORITY" to to the "env_keep" defaults in /etc/sudoers
.
Fedora has in /etc/sudoers
the following and this allows sudo somexapp to succeed.
Defaults env_reset
Defaults env_keep = "COLORS DISPLAY HOSTNAME HISTSIZE INPUTRC KDEDIR LS_COLORS"
Defaults env_keep += "MAIL PS1 PS2 QTDIR USERNAME LANG LC_ADDRESS LC_CTYPE"
Defaults env_keep += "LC_COLLATE LC_IDENTIFICATION LC_MEASUREMENT LC_MESSAGES"
Defaults env_keep += "LC_MONETARY LC_NAME LC_NUMERIC LC_PAPER LC_TELEPHONE"
Defaults env_keep += "LC_TIME LC_ALL LANGUAGE LINGUAS _XKB_CHARSET XAUTHORITY"
Then answer is that sudo
has a bug. First, the workaround: I put this in my /etc/sudoers.d/zabbix file
:
zabbix ALL=(root) NOPASSWD: /bin/env SHELL=/bin/sh /usr/local/bin/zabbix_raid_discovery
and now subcommands called from zabbix_raid_discovery
work.
A patch to fix this will be in sudo 1.8.15. From the maintainer, Todd Miller:
This is just a case of "it's always been like that". There's not
really a good reason for it. The diff below should make the behavior
match the documentation.
- todd
diff -r adb927ad5e86 plugins/sudoers/env.c
--- a/plugins/sudoers/env.c Tue Oct 06 09:33:27 2015 -0600
+++ b/plugins/sudoers/env.c Tue Oct 06 10:04:03 2015 -0600
@@ -939,8 +939,6 @@
CHECK_SETENV2("USERNAME", runas_pw->pw_name,
ISSET(didvar, DID_USERNAME), true);
} else {
- if (!ISSET(didvar, DID_SHELL))
- CHECK_SETENV2("SHELL", sudo_user.pw->pw_shell, false, true);
/* We will set LOGNAME later in the def_set_logname case. */
if (!def_set_logname) {
if (!ISSET(didvar, DID_LOGNAME))
@@ -984,6 +982,8 @@
if (!env_should_delete(*ep)) {
if (strncmp(*ep, "SUDO_PS1=", 9) == 0)
ps1 = *ep + 5;
+ else if (strncmp(*ep, "SHELL=", 6) == 0)
+ SET(didvar, DID_SHELL);
else if (strncmp(*ep, "PATH=", 5) == 0)
SET(didvar, DID_PATH);
else if (strncmp(*ep, "TERM=", 5) == 0)
@@ -1039,7 +1039,9 @@
if (reset_home)
CHECK_SETENV2("HOME", runas_pw->pw_dir, true, true);
- /* Provide default values for $TERM and $PATH if they are not set. */
+ /* Provide default values for $SHELL, $TERM and $PATH if not set. */
+ if (!ISSET(didvar, DID_SHELL))
+ CHECK_SETENV2("SHELL", runas_pw->pw_shell, false, false);
if (!ISSET(didvar, DID_TERM))
CHECK_PUTENV("TERM=unknown", false, false);
if (!ISSET(didvar, DID_PATH))
Best Answer
The only bug I see with your code is that you're running the user's command unnecessarily through
sh -c
when you should just run it directly. Running it throughsh -c
buys you nothing but it destroys quoting that the user originally put into the command. For example, try this:should list a file called
a b
inside the context of the namespace but instead it lists two files calleda
andb
.should list a file called
*
(literal asterisk), fails for the same reason.So it should look like this instead:
Makes the script simpler to boot!
Another point I can make is that although in this case the bug was just a functionality bug, not a security bug, one is always suspicious when auditing security-sensitive code and finding that it runs things through shells because that's almost always a problem.
Finally, the user's supplementary groups will not be propagated into the namespace (they get only their uid and main gid), but that doesn't seem like a huge problem and fixing that is not trivial.
Other than that, it looks good to me.