Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default use of 'less' is a security concern #5666

Closed
1 of 2 tasks
stvhay opened this issue Mar 30, 2017 · 14 comments
Closed
1 of 2 tasks

Default use of 'less' is a security concern #5666

stvhay opened this issue Mar 30, 2017 · 14 comments

Comments

@stvhay
Copy link

stvhay commented Mar 30, 2017

Submission type

  • Bug report
  • Request for enhancement (RFE)

NOTE: Do not submit anything other than bug reports or RFEs via the issue tracker!

systemd version the issue has been seen with

ALL

NOTE: Do not submit bug reports about anything but the two most recently released systemd versions upstream!

Used distribution

ALL

In case of bug report: Expected behaviour you didn't see

adding this to sudoers: "%manager ALL=(root) NOPASSWD: /bin/systemctl status" would not provide a path to root shell

In case of bug report: Unexpected behaviour you saw

once the pager (less) is invoked, !/bin/bash drops you to root shell.

In case of bug report: Steps to reproduce the problem

(covered above)

I realize that this can be addressed by adding the --no-pager option and creating aliases, but is sure seems like invoking a pager that lets you drop shell on a utility run as root is just begging for trouble.

@Stebalien
Copy link
Contributor

Stebalien commented Mar 30, 2017

That sudoers rule is begging for trouble (and less is the standard linux pager). Why exactly do you need permission to run this as root? It works just fine as a normal user.

@poettering
Copy link
Member

Sorry, but this appears to be a limitation of your sudoers rule, but not systemctl. The auto-paging behaviour of systemctl is well-known, and can be disabled easily, including in a per-invocation mode the way you describe.

Sorry, but I don't think there's anything to fix here in systemd: the general should be to be very careful when writing sudo rules (or even better simply not use it at all).

I hope this makes sense, closing.

@stvhay
Copy link
Author

stvhay commented Mar 30, 2017

What you are saying makes sense, but I disagree with it. Your choice.

I'll only expand on what I said by saying this. Misconfiguration is a very common root cause of security vulnerabilities in live systems. For example, it is in the OWASP Top 10 (A5). It seems like a poor design choice to invoke a pager in a utility that will be frequently run as root, and may need access marshaled to it using tools like sudo. In fact, if you Google around to see what people are doing, you can see examples of people using sudo in this way to marshal access to systemctl without disabling the pager. I would think that the systemd project would want to prevent these kinds of common misconfigurations.

I agree that 'systemctl status' is not the best example, given that is works pretty much fully intact as root. That said, I've seen cases where status output varies a bit depending on level of access. In my own experience, I run systemctl status dhcpd.service as root, because doing it this way gives me a short tail of the dhcp log. Doing it as a user does not. There's enough lines there to invoke a pager, and now we have a potential path to root.

I also will concede that in this case, the best practice is to lock down the environment better. This can be done in sudoers. It is not difficult to create an environment for systemctl that will not invoke a pager. I just think its something that a lot of people are going to miss, and it seems like default behavior that is just asking for trouble.

@Stebalien
Copy link
Contributor

This is literally why policykit was invented. Allowing untrusted users to run interactive applications as root is just begging for trouble. If you, for example, need to allow a user start/stop a specific service, you can add a policykit rule to allow then to do so without allowing anyone to run systemctl as root.

because doing it this way gives me a short tail of the dhcp log

Add yourself to the systemd-journal group.

This can be done in sudoers. It is not difficult to create an environment for systemctl that will not invoke a pager.

You can also set the LESSSECURE environment variable to 1 (but make sure that the user can't reset it). See the less(1) manual for details. Unfortunately, I don't know of a way to automatically set this variable for all NOPASSWD sudoers entries. Regardless, I'd still argue that this is a "bad idea".

@robmv
Copy link

robmv commented Aug 27, 2020

As the original submitter of the bug on Red Hat's bugzilla, I have seen these sudo "misconfigurations" too much. You can't ask for people to know exactly what other processes are invoked when you use a tool like systemctl.

The user that installed the sudo rule could have tried it on a 20" monitor and never saw that a pager was activated, so it has no idea a pager needs to be disabled, then goes and writes a tool for less privileged IT staff that use systemctl status, and then some of those notice the pager is active because they are using a small monitor / window and take advantage of the less feature to invoke programs.

IMHO LESSSECURE should be added to all less invocations by systemd, and by the way, never use more as a fallback because it doesn't have a way to disable program invocation

@robmv
Copy link

robmv commented Aug 27, 2020

This is literally why policykit was invented. Allowing untrusted users to run interactive applications as root is just begging for trouble. If you, for example, need to allow a user start/stop a specific service, you can add a policykit rule to allow then to do so without allowing anyone to run systemctl as root.

policykit is an overkill for a sysadmin that just want to give their IT staff a tool to check if a system is running or not via sudo.

@poettering
Copy link
Member

IMHO LESSSECURE should be added to all less invocations by systemd, and by the way, never use more as a fallback because it doesn't have a way to disable program invocation

That makes sense I guess. Can you file an RFE about exactly this?

@poettering
Copy link
Member

policykit is an overkill for a sysadmin that just want to give their IT staff a tool to check if a system is running or not via sudo.

You don't need privs to check that... You only need privs if you actually want to change state of something, introspection of the objects systemd manages is unrestricted.

@poettering
Copy link
Member

That makes sense I guess. Can you file an RFE about exactly this?

Or even better file a PR that adds that?

@robmv
Copy link

robmv commented Aug 31, 2020

Or even better file a PR that adds that?

Cloning right now, it will probably take more time for me to learn the testing infrastructure.

poettering added a commit to poettering/systemd that referenced this issue Aug 31, 2020
Some extra safety when invoked via "sudo". With this we address a
genuine design flaw of sudo, and we shouldn't need to deal with this.
But it's still a good idea to disable this surface given how exotic it
is.

Prompted by systemd#5666
@poettering
Copy link
Member

@robmv hmm, so i figure it's quicker if i just prep the PR for that, please see #16916.

@keszybz keszybz changed the title Default use of 'less' is a security concern. Default use of 'less' is a security concern Sep 1, 2020
@keszybz keszybz reopened this Sep 1, 2020
@vcaputo
Copy link
Member

vcaputo commented Sep 1, 2020

I think the automagic pager invocation is just a misfeature plain and simple. The tools should just print to stdout, and a caller may redirect stdout into a pager if they wanted one.

If it weren't launching the pager from the privileged context during this magic, we wouldn't be having this discussion. There's zero security problem when the user pipes stdout of sudo into a pager.

@topimiettinen
Copy link
Contributor

systemctl and other tools should allow (opt-in/opt-out) fully sandboxing the helper applications. I know the life cycle is not expected to be long for udevd helpers, but it could still be an opt-in feature. In this specific case, less should not have any need for capabilities (Capabilities=), network access (IPAddressDeny=, RestrictAddressFamilies=), writable or even accessible ABIFS (InaccessiblePaths=/sys /proc, MountAPIVFS=no etc), namespaces (RestrictNamespaces=), access to block devices (DeviceAllow=char:* rw etc) and lots of system calls could be blocked (SystemCallFilter=@systemd-helper, SystemCallArchitectures=native, MemoryDenyWriteExecute=yes, LockPersonality=yes etc).

Those extreme systems with weird NSS LDAP NFS etc. stuff where anything of the above needs could be expected from puny less tool could opt out.

poettering added a commit to poettering/systemd that referenced this issue Sep 2, 2020
Some extra safety when invoked via "sudo". With this we address a
genuine design flaw of sudo, and we shouldn't need to deal with this.
But it's still a good idea to disable this surface given how exotic it
is.

Prompted by systemd#5666
poettering added a commit to poettering/systemd that referenced this issue Sep 3, 2020
Some extra safety when invoked via "sudo". With this we address a
genuine design flaw of sudo, and we shouldn't need to deal with this.
But it's still a good idea to disable this surface given how exotic it
is.

Prompted by systemd#5666
poettering added a commit to poettering/systemd that referenced this issue Sep 3, 2020
Some extra safety when invoked via "sudo". With this we address a
genuine design flaw of sudo, and we shouldn't need to deal with this.
But it's still a good idea to disable this surface given how exotic it
is.

Prompted by systemd#5666
poettering added a commit to poettering/systemd that referenced this issue Sep 4, 2020
Some extra safety when invoked via "sudo". With this we address a
genuine design flaw of sudo, and we shouldn't need to deal with this.
But it's still a good idea to disable this surface given how exotic it
is.

Prompted by systemd#5666
keszybz pushed a commit to keszybz/systemd that referenced this issue Oct 7, 2020
Some extra safety when invoked via "sudo". With this we address a
genuine design flaw of sudo, and we shouldn't need to deal with this.
But it's still a good idea to disable this surface given how exotic it
is.

Prompted by systemd#5666
keszybz added a commit to keszybz/systemd that referenced this issue Oct 7, 2020
The variable is renamed to SYSTEMD_PAGERSECURE (because it's not just about
less now), and we automatically enable secure mode under sudo, but not otherwise.

This approach is more nuanced, but should provide a better experience for
users:

- Previusly we would set LESSSECURE=1 and trust the pager to make use of
  it. But this has an effect only on less. We need to not start pagers which
  are insecure when in secure mode. In particular more is like that and is a
  very popular pager.

- We don't enable secure mode always, which means that those other pagers can
  reasonably used.

- We do the right thing by default, but the user has ultimate control by
  setting SYSTEMD_PAGERSECURE.

Fixes systemd#5666.
keszybz added a commit to keszybz/systemd that referenced this issue Oct 8, 2020
…ested

The variable is renamed to SYSTEMD_PAGERSECURE (because it's not just about
less now), and we automatically enable secure mode under sudo, but not otherwise.

This approach is more nuanced, but should provide a better experience for
users:

- Previusly we would set LESSSECURE=1 and trust the pager to make use of
  it. But this has an effect only on less. We need to not start pagers which
  are insecure when in secure mode. In particular more is like that and is a
  very popular pager.

- We don't enable secure mode always, which means that those other pagers can
  reasonably used.

- We do the right thing by default, but the user has ultimate control by
  setting SYSTEMD_PAGERSECURE.

Fixes systemd#5666.

v2:
- also check $PKEXEC_UID
keszybz added a commit to keszybz/systemd that referenced this issue Oct 12, 2020
…uested

The variable is renamed to SYSTEMD_PAGERSECURE (because it's not just about
less now), and we automatically enable secure mode in certain cases, but not
otherwise.

This approach is more nuanced, but should provide a better experience for
users:

- Previusly we would set LESSSECURE=1 and trust the pager to make use of
  it. But this has an effect only on less. We need to not start pagers which
  are insecure when in secure mode. In particular more is like that and is a
  very popular pager.

- We don't enable secure mode always, which means that those other pagers can
  reasonably used.

- We do the right thing by default, but the user has ultimate control by
  setting SYSTEMD_PAGERSECURE.

Fixes systemd#5666.

v2:
- also check $PKEXEC_UID

v3:
- use 'sd_pid_get_owner_uid() != geteuid()' as the condition
keszybz added a commit to keszybz/systemd that referenced this issue Oct 13, 2020
…uested

The variable is renamed to SYSTEMD_PAGERSECURE (because it's not just about
less now), and we automatically enable secure mode in certain cases, but not
otherwise.

This approach is more nuanced, but should provide a better experience for
users:

- Previusly we would set LESSSECURE=1 and trust the pager to make use of
  it. But this has an effect only on less. We need to not start pagers which
  are insecure when in secure mode. In particular more is like that and is a
  very popular pager.

- We don't enable secure mode always, which means that those other pagers can
  reasonably used.

- We do the right thing by default, but the user has ultimate control by
  setting SYSTEMD_PAGERSECURE.

Fixes systemd#5666.

v2:
- also check $PKEXEC_UID

v3:
- use 'sd_pid_get_owner_uid() != geteuid()' as the condition
keszybz added a commit to keszybz/systemd that referenced this issue Oct 13, 2020
…uested

The variable is renamed to SYSTEMD_PAGERSECURE (because it's not just about
less now), and we automatically enable secure mode in certain cases, but not
otherwise.

This approach is more nuanced, but should provide a better experience for
users:

- Previusly we would set LESSSECURE=1 and trust the pager to make use of
  it. But this has an effect only on less. We need to not start pagers which
  are insecure when in secure mode. In particular more is like that and is a
  very popular pager.

- We don't enable secure mode always, which means that those other pagers can
  reasonably used.

- We do the right thing by default, but the user has ultimate control by
  setting SYSTEMD_PAGERSECURE.

Fixes systemd#5666.

v2:
- also check $PKEXEC_UID

v3:
- use 'sd_pid_get_owner_uid() != geteuid()' as the condition
lqb pushed a commit to lqb/systemd that referenced this issue Oct 29, 2020
Some extra safety when invoked via "sudo". With this we address a
genuine design flaw of sudo, and we shouldn't need to deal with this.
But it's still a good idea to disable this surface given how exotic it
is.

Prompted by systemd#5666
lqb pushed a commit to lqb/systemd that referenced this issue Oct 29, 2020
…uested

The variable is renamed to SYSTEMD_PAGERSECURE (because it's not just about
less now), and we automatically enable secure mode in certain cases, but not
otherwise.

This approach is more nuanced, but should provide a better experience for
users:

- Previusly we would set LESSSECURE=1 and trust the pager to make use of
  it. But this has an effect only on less. We need to not start pagers which
  are insecure when in secure mode. In particular more is like that and is a
  very popular pager.

- We don't enable secure mode always, which means that those other pagers can
  reasonably used.

- We do the right thing by default, but the user has ultimate control by
  setting SYSTEMD_PAGERSECURE.

Fixes systemd#5666.

v2:
- also check $PKEXEC_UID

v3:
- use 'sd_pid_get_owner_uid() != geteuid()' as the condition
mikhailnov pushed a commit to mikhailnov/systemd that referenced this issue Jan 16, 2021
Some extra safety when invoked via "sudo". With this we address a
genuine design flaw of sudo, and we shouldn't need to deal with this.
But it's still a good idea to disable this surface given how exotic it
is.

Prompted by systemd#5666

(cherry picked from commit 612ebf6)
mikhailnov pushed a commit to mikhailnov/systemd that referenced this issue Jan 16, 2021
…uested

The variable is renamed to SYSTEMD_PAGERSECURE (because it's not just about
less now), and we automatically enable secure mode in certain cases, but not
otherwise.

This approach is more nuanced, but should provide a better experience for
users:

- Previusly we would set LESSSECURE=1 and trust the pager to make use of
  it. But this has an effect only on less. We need to not start pagers which
  are insecure when in secure mode. In particular more is like that and is a
  very popular pager.

- We don't enable secure mode always, which means that those other pagers can
  reasonably used.

- We do the right thing by default, but the user has ultimate control by
  setting SYSTEMD_PAGERSECURE.

Fixes systemd#5666.

v2:
- also check $PKEXEC_UID

v3:
- use 'sd_pid_get_owner_uid() != geteuid()' as the condition

(cherry picked from commit 0a42426)
@Zenmovie
Copy link

Cve.mitre assigned vulnerability as CVE-2023-26604. Security mechanisms referred to above appeared in systemd 247. While ubuntu 20 and rhel 8.x use systemd 245.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

8 participants