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

Please consider enabling option use_pty by default (security) #258

Closed
hartwork opened this issue Mar 28, 2023 · 19 comments
Closed

Please consider enabling option use_pty by default (security) #258

hartwork opened this issue Mar 28, 2023 · 19 comments

Comments

@hartwork
Copy link

Hello! πŸ‘‹

I became aware recently that sudo needs non-default option use_pty in /etc/sudoers β€” a line Defaults use_pty β€” to no longer be vulnerable to privilege escalation via TIOCSTI and/or lesser-known TIOCLINUX command injection by default. For anyone curious to see the attack with their own eyes, ttyjack can be used.

While Linux >=6.2.0 introduced a config switch CONFIG_LEGACY_TIOCSTI (that is default y upstream, n in Arch Linux) and a related sysctl variable dev.tty.legacy_tiocsti to disable TIOCSTI, there is no equivalent for TIOCLINUX available and use of a PTY may be the best protection for both attacks and similar future ones in sudo and other software, because the code under execution no longer gets access to the outer controlling terminal.

This timeline and CVE list potentially indicates how little known the attack vector is to this day, 12+ years after the introduction of use_pty by commit 6f05b56 for sudo 1.8.0 and ~18 years after the first(?) public report about security aspects of TIOCSTI in 2005.

As of today, sudo documents the issue in man 5 sudoers near option use_pty saying:

A malicious program run under sudo may be capable of injecting commands into the user's terminal or running a background process that retains access to the user's terminal device even after the main program has finished executing. By running the command in a separate pseudo-terminal, this attack is no longer possible. This flag is off by default.

Is there a chance that option use_pty could be enabled by default in sudo?

Thanks for your consideration! πŸ™

Best, Sebastian

@millert
Copy link
Collaborator

millert commented Mar 30, 2023

It has been my intention to enable use_pty by default once the behavior of sudo with a pty is as close as possible to the behavior without one. With some recent bug fixes we may have reached that point. Some behavior will always be different, for example a command run in a pty will consume terminal input even if the command being run does not need it. There are probably still some minor issues (proxying signals and job control from one terminal to another adds complexity) but those bugs will be fixed as they are found. I will tentatively plan for sudo 1.9.14 to enable use_pty by default unless there is a major problem found in the meantime.

@hartwork
Copy link
Author

@millert cool!
I wonder how much need there is for a new option no_pty or some other way to temporarily disable use_pty once use_pty is enabled by default. How do you feel about that?

@millert
Copy link
Collaborator

millert commented Mar 31, 2023

There is no need for a new option, users can just negate use_pty, for example:

Defaults !use_pty

@hartwork
Copy link
Author

@millert I wasn't aware, great, thanks! πŸ‘

@millert
Copy link
Collaborator

millert commented Jun 27, 2023

Sudo 1.9.14 is out and sets use_pty by default.

@millert millert closed this as completed Jun 27, 2023
@hartwork
Copy link
Author

@millert thanks a lot β€” awesome!

For anyone looking for all related commits, this is what I found (in reverse order as git log would do):

  • 5d2b176 β€” Clarify that use_pty is on by default starting with 1.9.14.
  • afb09e0 β€” Sudo runs the command in a pty by default in 1.9.14 and above.
  • 4da1f37 β€” Add commented out example for disabling use_pty.
  • c7070b0 β€” sudo 1.9.14 (the change log part)
  • 894daa8 β€” Enable the use_pty option by default for sudo 1.9.14.

mtremer pushed a commit to ipfire/ipfire-2.x that referenced this issue Jul 1, 2023
- Update from version 1.9.13p3 to 1.9.14
- Update of rootfile not required
- Changelog
   Significant change is that use_pty is now defined as the default setting.
   This parameter was made available back in version 1.8.0 but not as default.
   It was implemented in response to a variety of CVE's related to being vulnerable to
    privilege escalation via TIOCSTI and/or lesser-known TIOCLINUX command injection.
   Apparently it was not made default as that would change the way that sudo worked.
   As various existing bugs have been resolved it has now been declared by the sudo devs
    that now sudo with a pseudo terminal works close to the same as with the users terminal
    Hence in this version the use of the pseudo terminal is now default.
   See sudo-project/sudo#258 for more details.
1.9.14
    Fixed a bug where if the intercept or log_subcmds sudoers option was enabled and a
     sub-command was run where the first entry of the argument vector didn't match the
     command being run. This resulted in commands like sudo su - being killed due to the
     mismatch. Bug #1050.
    The sudoers plugin now canonicalizes command path names before matching (where
     possible). This fixes a bug where sudo could execute the wrong path if there are
     multiple symbolic links with the same target and the same base name in sudoers that a
     user is allowed to run. GitHub issue #228.
    Improved command matching when a chroot is specified in sudoers. The sudoers plugin
     will now change the root directory id needed before performing command matching.
     Previously, the root directory was simply prepended to the path that was being
     processed.
    When NETGROUP_BASE is set in the ldap.conf file, sudo will now perform its own
     netgroup lookups of the host name instead of using the system innetgr(3) function.
     This guarantees that user and host netgroup lookups are performed using the same LDAP
     server (or servers).
    Fixed a bug introduced in sudo 1.9.13 that resulted in a missing " ; " separator
     between environment variables and the command in log entries.
    The visudo utility now displays a warning when it ignores a file in an include dir
     such as /etc/sudoers.d.
    When running a command in a pseudo-terminal, sudo will initialize the terminal
     settings even if it is the background process. Previously, sudo only initialized the
     pseudo-terminal when running in the foreground. This fixes an issue where a program
     that checks the window size would read the wrong value when sudo was running in the
     background.
    Fixed a bug where only the first two digits of the TSID field being was logged.
     Bug #1046.
    The use_pty sudoers option is now enabled by default. To restore the historic behavior
     where a command is run in the user's terminal, add Defaults !use_pty to the sudoers
     file. GitHub issue #258.
    Sudo's -b option now works when the command is run in a pseudo-terminal.
    When disabling core dumps, sudo now only modifies the soft limit and leaves the hard
     limit as-is. This avoids problems on Linux when sudo does not have CAP_SYS_RESOURCE,
     which may be the case when run inside a container. GitHub issue #42.
    Sudo configuration file paths have been converted to colon-separated lists of paths.
     This makes it possible to have configuration files on a read-only file system while
     still allowing for local modifications in a different (writable) directory. The new
     --enable-adminconf configure option can be used to specify a directory that is
     searched for configuration files in preference to the sysconfdir (which is usually
     /etc).
    The intercept_verify sudoers option is now only applied when the intercept option is
     set in sudoers. Previously, it was also applied when log_subcmds was enabled.
    The NETGROUP_QUERY ldap.conf parameter can now be disabled for LDAP servers that do
     not support querying the nisNetgroup object by its nisNetgroupTriple attribute, while
     still allowing sudo to query the LDAP server directly to determine netgroup
     membership.
    Fixed a long-standing bug where a sudoers rule without an explicit runas list allowed
     the user to run a command as root and any group instead of just one of the groups
     that root is a member of. For example, a rule such as myuser ALL = ALL would permit
     sudo -u root -g othergroup even if root did not belong to othergroup.
    Fixed a bug where a sudoers rule with an explicit runas list allowed a user to run
     sudo commands as themselves. For example, a rule such as myuser ALL = (root) ALL,
     myuser should only allow commands to be run as root (optionally using one of root's
     groups). However, the rule also allowed the user to run sudo -u myuser -g myuser
     command.
    Fixed a bug that prevented the user from specifying a group on the command line via
     sudo -g if the rule's Runas_Spec contained a Runas_Alias.
    Sudo now requires a C99 compiler due to the use of flexible array members.

Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
Reviewed-by: Peter MΓΌller <peter.mueller@ipfire.org>
@Toolybird
Copy link

This seems to have caused a regression on Arch Linux. The following command produces a "staircase" effect under 1.9.14.p1:

$ sudo pacman -S wget | tee wget.log
resolving dependencies...
looking for conflicting packages...

Packages (1) wget-1.21.4-1

Total Installed Size:  3.18 MiB

:: Proceed with installation? [Y/n] 
checking keyring...
                   checking package integrity...
                                                loading package files...
                                                                        checking for file conflicts...
                                                                                                      checking available disk space...
                                                                                                                                      :: Processing package changes...
                    installing wget...
                                      Optional dependencies for wget
                                                                        ca-certificates: HTTPS downloads [installed]
                                                                                                                    :: Running post-transaction hooks...
      (1/1) Arming ConditionNeedsUpdate...

There is no problem when rolling back to 1.9.13.p3. The problem also goes away when setting Defaults !use_pty in sudoers.

I haven't found any other way to trigger it (yet). Maybe it's a bug in pacman? Any thoughts? Thanks.

@millert
Copy link
Collaborator

millert commented Jul 13, 2023

That indicates that the terminal is not set to convert newlines into return + newline pairs. Specifically, that the ONLCR flag is not set in the c_oflag field of struct termios (or post-processing is disabled). You can see the value of this flag by running stty -a from the shell and looking for onlcr and opost.

The most likely cause is that either 1) sudo is being run from a terminal with the ONLCR or OPOST flags off, or 2) pacman is changing the terminal mode itself. When sudo allocates a pty it copies most of the terminal modes from the user's terminal to the pty.

@Toolybird
Copy link

Thanks for the hints. onlcr and opost are both definitely on. That seems to point the finger at pacman. Although, tee might also be playing a role here...will investigate.

@millert
Copy link
Collaborator

millert commented Jul 14, 2023

@Toolybird Does it happen with just sudo pacman or only with tee? If only with tee, does the same thing happen if you pipe the output to cat?

@Toolybird
Copy link

It doesn't happen with just sudo pacman. It also doesn't happen with cat, but in this case the pacman command doesn't execute properly (possibly because of the [Y/n] prompt)

@millert
Copy link
Collaborator

millert commented Jul 14, 2023

I can reproduce this running arch in docker (actually podman). It also happens for:

sudo podman -S wget | cat

So it is not specific to tee. It only happens after Proceed with installation is displayed, the earlier output is OK. The terminal settings on the pty look fine too. I'll try to debug this further tomorrow.

@millert
Copy link
Collaborator

millert commented Jul 14, 2023

I've confirmed this is a sudo problem and just committed a fix which is dependent on a few other commits from today. Basically, when sudo runs a command that has its output piped or redirected it leaves the user's terminal settings unchanged until the program being run needs to read from the terminal. At that point, sudo sets the user's terminal (not the pty running the command) into raw mode so it can proxy input from the user's terminal to the pty. However, this has the effect of turning off output post-processing, leading to the stair-step output. The fix is to preserve the terminal's output flags if output is not to a terminal.

@pallaswept
Copy link

pallaswept commented Aug 2, 2023

Just putting this out there so someone can find the answer without having to figure it out for themselves - this breaks kdesu/yast

I discovered this when trying to run Yast software management, I'd get a bouncing icon and then nothing.

Running the program from the CLI returns the following:

> /usr/lib/YaST2/bin/sw_single_wrapper
Don't need password!!

kf.su: [ /home/abuild/rpmbuild/BUILD/kdesu-5.108.0/src/stubprocess.cpp : 215 ]  Unknown request: "ok"

I have run kwriteconfig5 --file kdesurc --group super-user-command --key super-user-command sudo so that kdesu will use sudo rather than su, so that sudoers will be respected by KDE.
KDE frameworks got an update around the same time as this change rolled out, so the natural thought was that kdesu was broken, but it turns out to be this option.

Running echo 'Defaults !use_pty' | sudo tee -a /etc/sudoers and then repeating the above instantly fixes it:

> /usr/lib/YaST2/bin/sw_single_wrapper
Don't need password!!

and the GUI appears and works as intended.

I intended to upstream this to KDE but the issue already exists there: https://bugs.kde.org/show_bug.cgi?id=452532

I will provide them this information also and encourage them to fix kdesu so that it it works properly with the secure option enabled.

Edit: A patch has already been merged to kdesu so that it will work with sudo's new default. Hooray! and thanks to Fabian Vogt for fixing it up for us all. Edit2: Merged already! Thanks again Fabian!

@koenigbj
Copy link

Since use_pty is the default, I have noticed some strange behavior using FreeBSD 13 on single-core systems. Sometimes (i.e. 1 out of 10 times) sudo seems to hang. top shows two sudo processes that together generate 100% load. It takes about a minute for the system to become responsive again and for sudo to continue running properly. I don't have this problem on systems with multiple cores.

The simplest solution is obviously to set "Default !use_pty". But it didn't leave me alone, which is why I spent some time debugging. Apparently there can be situations where the while loop to wait for the parent process in the exec_cmnd_pty function (exec_monitor.c) drives the child process to full load, thus blocking the parent process or rather making it very slow. I changed the wait time of nanosleep from 1Β΅s to 10Β΅s and now the problem doesn't seem to occur anymore.

Do you have any thoughts on this?

@millert
Copy link
Collaborator

millert commented Sep 18, 2023

@koenigbj It's possible that FreeBSD doesn't really support such short sleep times. I just committed 0cb3e33 which replaces that loop with a read from the other end of a socketpair. That should remove the possibility of a tight CPU loop.

@koenigbj
Copy link

@millert Thanks a lot! 0cb3e33 works like a charm for me. I have now been able to invoke sudo a thousand times without it getting stuck.

In principle, FreeBSD supports such short sleep times, but the hardware must of course also play along. In my case we are talking about virtual machines and therefore there is a virtual clock driver, which could be the cause.

@sylv-io
Copy link

sylv-io commented Oct 9, 2023

Hello there.
I noticed a regression due to enabling this option by default. (See #312)
In summary, if you run sudo in parallel with use_pty enabled, it may break tty. go-task/task#1369 has an aciinema and sample code that shows this behavior.

While I now understand the security implications that lead to this decision, it would be great if we could somehow fix this regression. Any suggestions are welcome.

Thanks!

@sylv-io
Copy link

sylv-io commented Oct 20, 2023

Hello there. I noticed a regression due to enabling this option by default. (See #312) In summary, if you run sudo in parallel with use_pty enabled, it may break tty. go-task/task#1369 has an aciinema and sample code that shows this behavior.

While I now understand the security implications that lead to this decision, it would be great if we could somehow fix this regression. Any suggestions are welcome.

Thanks!

Fixed by fabb626

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

No branches or pull requests

6 participants