pager: also check for $SUDO_UID

This returns to the original approach proposed in
https://github.com/systemd/systemd/pull/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/user@1000.service/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 https://github.com/polkit-org/polkit/pull/562, which adds support for
$SUDO_UID/$SUDO_GID to pkexec.

(cherry picked from commit cd93478af8b9dc69478d5667f113b67d175090fa)
(cherry picked from commit b93f53c122124582fa80ae246343791063d65074)

diff --git a/man/common-variables.xml b/man/common-variables.xml
index 6a6e98d334d6519e0f74b8573915cb0bc2f9db85..991946f60df1843a5281a54135f02ecd4d48b4e1 100644 (file)
--- a/man/common-variables.xml
+++ b/man/common-variables.xml
@@ -205,9 +205,16 @@
       enabled if the effective UID is not the same as the owner of the login session, see
       <citerefentry project='man-pages'><refentrytitle>geteuid</refentrytitle><manvolnum>2</manvolnum></citerefentry>
       and
-      <citerefentry><refentrytitle>sd_pid_get_owner_uid</refentrytitle><manvolnum>3</manvolnum></citerefentry>.
-      In this case, <varname>SYSTEMD_PAGERSECURE=1</varname> will be set and pagers which are not known to
-      implement "secure mode" will not be used at all.</para>
+      <citerefentry><refentrytitle>sd_pid_get_owner_uid</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
+      or when running under
+      <citerefentry><refentrytitle>sudo</refentrytitle><manvolnum>8</manvolnum></citerefentry> or similar
+      tools (<varname>$SUDO_UID</varname> is set <footnote>
+      <para>It is recommended for other tools to set and check <varname>$SUDO_UID</varname> as appropriate,
+      treating it is a common interface.</para></footnote>). In those cases,
+      <varname>SYSTEMD_PAGERSECURE=1</varname> will be set and pagers which are not known to implement
+      "secure mode" will not be used at all. Note that this autodetection only covers the most common
+      mechanisms to elevate privileges and is intended as convenience. It is recommended to explicitly set
+      <varname>$SYSTEMD_PAGERSECURE</varname> or disable the pager.</para>
 
       <para>Note that if the <varname>$SYSTEMD_PAGER</varname> or <varname>$PAGER</varname> variables are to
       be honoured, other than to disable the pager, <varname>$SYSTEMD_PAGERSECURE</varname> must be set

diff --git a/src/shared/pager.c b/src/shared/pager.c
index 9b8ae76700537859bc9a97620d2325753aae2a92..f1043ec13261cd707b45a397d41b67128205b0e0 100644 (file)
--- a/src/shared/pager.c
+++ b/src/shared/pager.c
@@ -82,6 +82,22 @@ static int no_quit_on_interrupt(int exe_name_fd, const char *less_opts) {
         return r;
 }
 
+static bool running_with_escalated_privileges(void) {
+        int r;
+
+        if (getenv("SUDO_UID"))
+                return true;
+
+        uid_t uid;
+        r = sd_pid_get_owner_uid(0, &uid);
+        if (r < 0) {
+                log_debug_errno(r, "sd_pid_get_owner_uid() failed, enabling pager secure mode: %m");
+                return true;
+        }
+
+        return uid != geteuid();
+}
+
 void pager_open(PagerFlags flags) {
         _cleanup_close_pair_ int fd[2] = EBADF_PAIR, exe_name_pipe[2] = EBADF_PAIR;
         _cleanup_strv_free_ char **pager_args = NULL;
@@ -177,16 +193,9 @@ void pager_open(PagerFlags flags) {
                  * know to be good. */
                 int use_secure_mode = secure_getenv_bool("SYSTEMD_PAGERSECURE");
                 bool trust_pager = use_secure_mode >= 0;
-                if (use_secure_mode == -ENXIO) {
-                        uid_t uid;
-
-                        r = sd_pid_get_owner_uid(0, &uid);
-                        if (r < 0)
-                                log_debug_errno(r, "sd_pid_get_owner_uid() failed, enabling pager secure mode: %m");
-
-                        use_secure_mode = r < 0 || uid != geteuid();
-
-                } else if (use_secure_mode < 0) {
+                if (use_secure_mode == -ENXIO)
+                        use_secure_mode = running_with_escalated_privileges();
+                else if (use_secure_mode < 0) {
                         log_warning_errno(use_secure_mode, "Unable to parse $SYSTEMD_PAGERSECURE, assuming true: %m");
                         use_secure_mode = true;
                 }