Skip to content

Commit a897e45

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)
1 parent 661ffe6 commit a897e45

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
@@ -196,9 +196,16 @@
196196
enabled if the effective UID is not the same as the owner of the login session, see
197197
<citerefentry project='man-pages'><refentrytitle>geteuid</refentrytitle><manvolnum>2</manvolnum></citerefentry>
198198
and
199-
<citerefentry><refentrytitle>sd_pid_get_owner_uid</refentrytitle><manvolnum>3</manvolnum></citerefentry>.
200-
In this case, <varname>SYSTEMD_PAGERSECURE=1</varname> will be set and pagers which are not known to
201-
implement "secure mode" will not be used at all.</para>
199+
<citerefentry><refentrytitle>sd_pid_get_owner_uid</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
200+
or when running under
201+
<citerefentry><refentrytitle>sudo</refentrytitle><manvolnum>8</manvolnum></citerefentry> or similar
202+
tools (<varname>$SUDO_UID</varname> is set <footnote>
203+
<para>It is recommended for other tools to set and check <varname>$SUDO_UID</varname> as appropriate,
204+
treating it is a common interface.</para></footnote>). In those cases,
205+
<varname>SYSTEMD_PAGERSECURE=1</varname> will be set and pagers which are not known to implement
206+
"secure mode" will not be used at all. Note that this autodetection only covers the most common
207+
mechanisms to elevate privileges and is intended as convenience. It is recommended to explicitly set
208+
<varname>$SYSTEMD_PAGERSECURE</varname> or disable the pager.</para>
202209

203210
<para>Note that if the <varname>$SYSTEMD_PAGER</varname> or <varname>$PAGER</varname> variables are to
204211
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
@@ -82,6 +82,22 @@ static int no_quit_on_interrupt(int exe_name_fd, const char *less_opts) {
8282
return r;
8383
}
8484

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

0 commit comments

Comments
 (0)