diff --git a/man/less-variables.xml b/man/less-variables.xml
index c52511ca8e..049e9f75d7 100644
--- a/man/less-variables.xml
+++ b/man/less-variables.xml
@@ -65,12 +65,30 @@
- $SYSTEMD_LESSSECURE
+ $SYSTEMD_PAGERSECURE
- Takes a boolean argument. Overrides the $LESSSECURE environment
- variable when invoking the pager, which controls the "secure" mode of less (which disables commands
- such as | which allow to easily shell out to external command lines). By default
- less secure mode is enabled, with this setting it may be disabled.
+ Takes a boolean argument. When true, the "secure" mode of the pager is enabled; if
+ false, disabled. If $SYSTEMD_PAGERSECURE is not set at all, secure mode is enabled
+ if the effective UID is not the same as the owner of the login session, see geteuid2 and
+ sd_pid_get_owner_uid3.
+ In secure mode, will be set when invoking the pager, and the pager shall
+ disable commands that open or create new files or start new subprocesses. When
+ $SYSTEMD_PAGERSECURE is not set at all, pagers which are not known to implement
+ secure mode will not be used. (Currently only
+ less1 implements
+ secure mode.)
+
+ Note: when commands are invoked with elevated privileges, for example under sudo8 or
+ pkexec1, care
+ must be taken to ensure that unintended interactive features are not enabled. "Secure" mode for the
+ pager may be enabled automatically as describe above. Setting SYSTEMD_PAGERSECURE=0
+ or not removing it from the inherited environment allows the user to invoke arbitrary commands. Note
+ that if the $SYSTEMD_PAGER or $PAGER variables are to be
+ honoured, $SYSTEMD_PAGERSECURE must be set too. It might be reasonable to completly
+ disable the pager using instead.
diff --git a/src/shared/pager.c b/src/shared/pager.c
index 9c21881241..9a14d44d69 100644
--- a/src/shared/pager.c
+++ b/src/shared/pager.c
@@ -8,6 +8,8 @@
#include
#include
+#include "sd-login.h"
+
#include "copy.h"
#include "env-util.h"
#include "fd-util.h"
@@ -165,25 +167,42 @@ int pager_open(PagerFlags flags) {
}
/* People might invoke us from sudo, don't needlessly allow less to be a way to shell out
- * privileged stuff. */
- r = getenv_bool("SYSTEMD_LESSSECURE");
- if (r == 0) { /* Remove env var if off */
- if (unsetenv("LESSSECURE") < 0) {
- log_error_errno(errno, "Failed to uset environment variable LESSSECURE: %m");
- _exit(EXIT_FAILURE);
- }
- } else {
- /* Set env var otherwise */
- if (r < 0)
- log_warning_errno(r, "Unable to parse $SYSTEMD_LESSSECURE, ignoring: %m");
+ * privileged stuff. If the user set $SYSTEMD_PAGERSECURE, trust their configuration of the
+ * pager. If they didn't, use secure mode when under euid is changed. If $SYSTEMD_PAGERSECURE
+ * wasn't explicitly set, and we autodetect the need for secure mode, only use the pager we
+ * know to be good. */
+ int use_secure_mode = getenv_bool("SYSTEMD_PAGERSECURE");
+ bool trust_pager = use_secure_mode >= 0;
+ if (use_secure_mode == -ENXIO) {
+ uid_t uid;
- if (setenv("LESSSECURE", "1", 1) < 0) {
- log_error_errno(errno, "Failed to set environment variable LESSSECURE: %m");
- _exit(EXIT_FAILURE);
- }
+ 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) {
+ log_warning_errno(use_secure_mode, "Unable to parse $SYSTEMD_PAGERSECURE, assuming true: %m");
+ use_secure_mode = true;
}
- if (pager_args) {
+ /* We generally always set variables used by less, even if we end up using a different pager.
+ * They shouldn't hurt in any case, and ideally other pagers would look at them too. */
+ if (use_secure_mode)
+ r = setenv("LESSSECURE", "1", 1);
+ else
+ r = unsetenv("LESSSECURE");
+ if (r < 0) {
+ log_error_errno(errno, "Failed to adjust environment variable LESSSECURE: %m");
+ _exit(EXIT_FAILURE);
+ }
+
+ if (trust_pager && pager_args) { /* The pager config might be set globally, and we cannot
+ * know if the user adjusted it to be appropriate for the
+ * secure mode. Thus, start the pager specified through
+ * envvars only when $SYSTEMD_PAGERSECURE was explicitly set
+ * as well. */
r = loop_write(exe_name_pipe[1], pager_args[0], strlen(pager_args[0]) + 1, false);
if (r < 0) {
log_error_errno(r, "Failed to write pager name to socket: %m");
@@ -195,13 +214,14 @@ int pager_open(PagerFlags flags) {
"Failed to execute '%s', using fallback pagers: %m", pager_args[0]);
}
- /* Debian's alternatives command for pagers is
- * called 'pager'. Note that we do not call
- * sensible-pagers here, since that is just a
- * shell script that implements a logic that
- * is similar to this one anyway, but is
- * Debian-specific. */
+ /* Debian's alternatives command for pagers is called 'pager'. Note that we do not call
+ * sensible-pagers here, since that is just a shell script that implements a logic that is
+ * similar to this one anyway, but is Debian-specific. */
FOREACH_STRING(exe, "pager", "less", "more") {
+ /* Only less implements secure mode right now. */
+ if (use_secure_mode && !streq(exe, "less"))
+ continue;
+
r = loop_write(exe_name_pipe[1], exe, strlen(exe) + 1, false);
if (r < 0) {
log_error_errno(r, "Failed to write pager name to socket: %m");
@@ -212,6 +232,7 @@ int pager_open(PagerFlags flags) {
"Failed to execute '%s', using next fallback pager: %m", exe);
}
+ /* Our builtin is also very secure. */
r = loop_write(exe_name_pipe[1], "(built-in)", strlen("(built-in)") + 1, false);
if (r < 0) {
log_error_errno(r, "Failed to write pager name to socket: %m");