Skip to content

Commit 79c337e

Browse files
keszybzbluca
authored andcommitted
pager: also check for $SUDO_UID
This returns to the original approach proposed in systemd/systemd#17270. After review, the approach was changed to use sd_pid_get_owner_uid() instead. Back then, when running in a typical graphical session, sd_pid_get_owner_uid() would usually return the user UID, and when running under sudo, geteuid() would return 0, so we'd trigger the secure path. sudo may allocate a new session if is invoked outside of a session (depending on the PAM config). Since nowadays desktop environments usually start the user shell through user units, the typical shell in a terminal emulator is not part of a session, and when sudo is invoked, a new session is allocated, and sd_pid_get_owner_uid() returns 0 too. Technically, the code still works as documented in the man page, but in the common case, it doesn't do the expected thing. $ build/test-sd-login |& rg 'get_(owner_uid|cgroup|session)' sd_pid_get_session(0) → No data available sd_pid_get_owner_uid(0) → 1000 sd_pid_get_cgroup(0) → /user.slice/user-1000.slice/[email protected]/app.slice/app-ghostty-transient-5088.scope/surfaces/556FAF50BA40.scope $ sudo build/test-sd-login |& rg 'get_(owner_uid|cgroup|session)' sd_pid_get_session(0) → c289 sd_pid_get_owner_uid(0) → 0 sd_pid_get_cgroup(0) → /user.slice/user-0.slice/session-c289.scope I think it's worth checking for sudo because it is a common case used by users. There obviously are other mechanims, so the man page is extended to say that only some common mechanisms are supported, and to (again) recommend setting SYSTEMD_LESSSECURE explicitly. The other option would be to set "secure mode" by default. But this would create an inconvenience for users doing the right thing, running systemctl and other tools directly, because then they can't run privileged commands from the pager, e.g. to save the output to a file. (Or the user would need to explicitly set SYSTEMD_LESSSECURE. One option would be to set it always in the environment and to rely on sudo and other tools stripping it from the environment before running privileged code. But that is also fairly fragile and it obviously relies on the user doing a complicated setup to support a fairly common use case. I think this decreases usability of the system quite a bit. I don't think we should build solutions that work in priniciple, but are painfully inconvenient in common cases.) Fixes https://yeswehack.com/vulnerability-center/reports/346802. Also see polkit-org/polkit#562, which adds support for $SUDO_UID/$SUDO_GID to pkexec. (cherry picked from commit cd93478) (cherry picked from commit b93f53c122124582fa80ae246343791063d65074) (cherry picked from commit f3a13eca4ed6b4852153179a2197ee797bbbe898) (cherry picked from commit df9bf67) (cherry picked from commit a897e45) (cherry picked from commit cce5535)
1 parent 945d948 commit 79c337e

File tree

2 files changed

+29
-13
lines changed

2 files changed

+29
-13
lines changed

man/common-variables.xml

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,16 @@
188188
enabled if the effective UID is not the same as the owner of the login session, see
189189
<citerefentry project='man-pages'><refentrytitle>geteuid</refentrytitle><manvolnum>2</manvolnum></citerefentry>
190190
and
191-
<citerefentry><refentrytitle>sd_pid_get_owner_uid</refentrytitle><manvolnum>3</manvolnum></citerefentry>.
192-
In this case, <varname>SYSTEMD_PAGERSECURE=1</varname> will be set and pagers which are not known to
193-
implement "secure mode" will not be used at all.</para>
191+
<citerefentry><refentrytitle>sd_pid_get_owner_uid</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
192+
or when running under
193+
<citerefentry><refentrytitle>sudo</refentrytitle><manvolnum>8</manvolnum></citerefentry> or similar
194+
tools (<varname>$SUDO_UID</varname> is set <footnote>
195+
<para>It is recommended for other tools to set and check <varname>$SUDO_UID</varname> as appropriate,
196+
treating it is a common interface.</para></footnote>). In those cases,
197+
<varname>SYSTEMD_PAGERSECURE=1</varname> will be set and pagers which are not known to implement
198+
"secure mode" will not be used at all. Note that this autodetection only covers the most common
199+
mechanisms to elevate privileges and is intended as convenience. It is recommended to explicitly set
200+
<varname>$SYSTEMD_PAGERSECURE</varname> or disable the pager.</para>
194201

195202
<para>Note that if the <varname>$SYSTEMD_PAGER</varname> or <varname>$PAGER</varname> variables are to
196203
be honoured, other than to disable the pager, <varname>$SYSTEMD_PAGERSECURE</varname> must be set

src/shared/pager.c

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,22 @@ static int no_quit_on_interrupt(int exe_name_fd, const char *less_opts) {
8383
return r;
8484
}
8585

86+
static bool running_with_escalated_privileges(void) {
87+
int r;
88+
89+
if (getenv("SUDO_UID"))
90+
return true;
91+
92+
uid_t uid;
93+
r = sd_pid_get_owner_uid(0, &uid);
94+
if (r < 0) {
95+
log_debug_errno(r, "sd_pid_get_owner_uid() failed, enabling pager secure mode: %m");
96+
return true;
97+
}
98+
99+
return uid != geteuid();
100+
}
101+
86102
void pager_open(PagerFlags flags) {
87103
_cleanup_close_pair_ int fd[2] = { -1, -1 }, exe_name_pipe[2] = { -1, -1 };
88104
_cleanup_strv_free_ char **pager_args = NULL;
@@ -178,16 +194,9 @@ void pager_open(PagerFlags flags) {
178194
* know to be good. */
179195
int use_secure_mode = getenv_bool_secure("SYSTEMD_PAGERSECURE");
180196
bool trust_pager = use_secure_mode >= 0;
181-
if (use_secure_mode == -ENXIO) {
182-
uid_t uid;
183-
184-
r = sd_pid_get_owner_uid(0, &uid);
185-
if (r < 0)
186-
log_debug_errno(r, "sd_pid_get_owner_uid() failed, enabling pager secure mode: %m");
187-
188-
use_secure_mode = r < 0 || uid != geteuid();
189-
190-
} else if (use_secure_mode < 0) {
197+
if (use_secure_mode == -ENXIO)
198+
use_secure_mode = running_with_escalated_privileges();
199+
else if (use_secure_mode < 0) {
191200
log_warning_errno(use_secure_mode, "Unable to parse $SYSTEMD_PAGERSECURE, assuming true: %m");
192201
use_secure_mode = true;
193202
}

0 commit comments

Comments
 (0)