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

terminal multiplexers bypass remote sudo password #51

Closed
Fuseteam opened this issue Feb 20, 2021 · 95 comments · Fixed by #54
Closed

terminal multiplexers bypass remote sudo password #51

Fuseteam opened this issue Feb 20, 2021 · 95 comments · Fixed by #54
Assignees
Labels
bug Something isn't working security
Milestone

Comments

@Fuseteam
Copy link

Fuseteam commented Feb 20, 2021

hi me again, thanks for recent update, i just happen to be running version 0.7.1 and version 0.7.3 at the same time, so I can confirm this bug also exists on version 0.7.1
basically the steps to reproduce it are these:

  • ssh into a remote system
  • run tmux/byobu
  • run sudo su

deny remote is set to true and it indeed denies access outside of byobu, it also denies the 'cup of tee' access aswell in the case of 0.7.3(as expected), but inside of byobu or tmux it happily grants access

@Fuseteam Fuseteam changed the title terminal multiplexers bypass remote deny terminal multiplexers bypass remote sudo password Feb 20, 2021
@mcdope
Copy link
Owner

mcdope commented Feb 20, 2021

Thanks for reporting!

I'm slightly irritated because we now walk the process hierarchy and as long sshd (or telnetd) is found in the parent chain it should be denied. I have the feeling it's somehow related to the detaching most multiplexers provide. In that case I could slap the previously used utmp check on it - i hope - to catch those cases.

But well possible that we will have to accept that it either has some holes, or some cases where the auth fails incorrectly. Which would mean an option again :D

Will investigate next week.

In case you're motivated to test some things: it would be interesting to see if screen is also affected (wanna bet yes). Also interesting would be the output with debug set to true globally (or per user).

@mcdope mcdope self-assigned this Feb 20, 2021
@mcdope mcdope added bug Something isn't working security labels Feb 20, 2021
@mcdope
Copy link
Owner

mcdope commented Feb 20, 2021

Note to myself: check if possible to find spawning process in case there is no parent found (or left)

@Fuseteam
Copy link
Author

Thanks for reporting!

I'm slightly irritated because we now walk the process hierarchy and as long sshd (or telnetd) is found in the parent chain it should be denied. I have the feeling it's somehow related to the detaching most multiplexers provide. In that case I could slap the previously used utmp check on it - i hope - to catch those cases.

But well possible that we will have to accept that it either has some holes, or some cases where the auth fails incorrectly. Which would mean an option again :D

Will investigate next week.

In case you're motivated to test some things: it would be interesting to see if screen is also affected (wanna bet yes). Also interesting would be the output with debug set to true globally (or per user).

haha yeah but considering it was also bypassed in version 0.7.1 i kinda doubt the utmp will help much

i will test screen for sure and i'll paste the output from debug true as soon as i got it x
D

@mcdope
Copy link
Owner

mcdope commented Feb 21, 2021

haha yeah but considering it was also bypassed in version 0.7.1 i kinda doubt the utmp will help much

utmp wasn't used in default config because of "unknown_pts_as_local" (well, it was used - but a not-found resulted in "assume it will be local".

That option was introduced with default=true because else i.e gdm3 failed to auth (at least on Ubuntu). But I have the hope that maybe a detached session (which I assume is the cause here) will get a proper utmp entry. In that case we could use the current approach, and if no remote daemon is found (like it seems in this case/issue) use utmp additionally to verify.

Of course that requires that we have a proper utmp entry for that session, if not - get rekt.

Which btw is another thing you could check if you want - after you used that bypass method you could run w to show active sessions. Would be interesting if there is a session for the bypass and if it shows an IP (or Hostname).

Edit: Also, a small disclaimer: I'm a more or less veteran coder, and avid linux user, but not very familiar with the inner workings in all cases on such deep levels. So please bear with me while I work such stuff out. Thanks for your help!

@Fuseteam
Copy link
Author

Fuseteam commented Feb 21, 2021

sure no problem just checked out screen it is being denied xD
this the debug log for tmux:

* Authentication request for user "fuseteam" (sudo)                             
[src/local.c:033] Checking whether the caller is local or not...                
[src/local.c:039]     Checking pid  18382 (sudo)...                             
[src/local.c:039]     Checking pid  18106 (/bin/bash)...                        
[src/local.c:039]     Checking pid  13778 (tmux)...                             
[src/local.c:048] No remote daemons found in parent process list, seems to be local request - allowing.                                                         
[src/device.c:038] Searching for "fuseteam" in the hardware database...
* Authentication device "fuseteam" is connected.
* Performing one time pad verification...
[src/volume.c:097] Searching for volume with uuid 1398-7F06.
[src/volume.c:121] Found mount points: /media/fuseteam/KINGSTON
[src/volume.c:134] Found volume 1398-7F06.
[src/volume.c:158] Volume 1398-7F06 is already mounted.
[src/pad.c:256] Loading device pad...
[src/pad.c:258] Loading system pad...
[src/pad.c:264] Pad match.
[src/pad.c:153] Checking whether pads are expired or not...
[src/pad.c:182] Pads were generated 2667 seconds ago, not updating.
* Access granted.

and here's the one for screen

* Authentication request for user "fuseteam" (sudo)
[src/local.c:033] Checking whether the caller is local or not...
[src/local.c:039]     Checking pid  24316 (sudo)...
[src/local.c:039]     Checking pid  24213 (/bin/bash)...
[src/local.c:039]     Checking pid  24212 (SCREEN)...
[src/local.c:039]     Checking pid  24211 (screen)...
[src/local.c:039]     Checking pid  15448 (-bash)...
[src/local.c:039]     Checking pid  15447 (sshd: fuseteam@pts/1)...
* One of the parent processes found to be a remote access daemon, denying.
* Access denied.

and the output of w does not seem to change before or after the bypass but it does show the IP from the local device used to ssh in

12:16:24 up  2:51,  3 users,  load average: 0.38, 0.29, 0.21
USER     TTY      FROM             LOGIN@   IDLE   JCPU   PCPU WHAT
fuseteam tty7     :0               09:26    2:51m  3:13   0.01s /bin/sh
fuseteam pts/1    192.168.1.67     12:05    0.00s  0.03s  0.00s tmux
fuseteam pts/2    tmux(26479).%0   12:15    0.00s  0.04s  0.01s tmux

@mcdope
Copy link
Owner

mcdope commented Feb 21, 2021

Really surprised screen isn't affected. Interesting.

But for tmux the "utmp on top" approach would seem to work since there is clearly a remote session for it in utmp, that's nice to know.

@Fuseteam
Copy link
Author

yeah i'm surprised by screen as well xD

@Fuseteam
Copy link
Author

Fuseteam commented Feb 21, 2021

i did a couple more tests and it appears there's always 2 sessions for tmux/byobu in utmp when I'm ssh'ed in regardless whether I'm attached to the same session on both devices or not
also disclaimer: I'm not a veteran coder but I'm an avid linux user that not very familiar with the inner workings in all cases on such deep levels. I'm happy to help

@mcdope
Copy link
Owner

mcdope commented Feb 24, 2021

For the records: tmux having seemingly no parent process is explained here: https://superuser.com/a/1472783

@Fuseteam
Copy link
Author

For the records: tmux having seemingly no parent process is explained here: https://superuser.com/a/1472783

nice find! based on this piece

So the server gets PPID of 1 almost immediately. Every shell or another process you run within tmux is a descendant of the server. After you (re-)attach, you see what the client shows you. The client itself is a child of the shell you invoked it in (or sshd or whatever). It talks to the server, passes keystrokes to it, receives information on how the window should look like and prints characters accordingly.

bolding mine I suppose this means we're walking the process tree of the server rather of the client perhaps? 🤔

@mcdope
Copy link
Owner

mcdope commented Feb 24, 2021

The issue is: trough this "Quake 3 like re-spawning" tmux isn't connected to the host process (sshd) anymore. So we can't detect it by the current "walk parents and check name" approach.

I'm right now working on an approach that still walks the process tree, but also checks if stdin of parent process is attached to a remote session in utmp. If this works like intended it would be "throwing a nuke onto cockroaches" and should solve all similar cases.

I've opened a local ssh session and ran tmux to verify with shell commands that my approach should work. One of tmuxs file descriptors in proc (/proc/<pid>/fd) seems to always point to some pts (pseudo terminal). And that pts should be available with IP in utmp, at least according to w. So by simply walking /proc/<pid>/fd for symlinks pointing to /dev, then checking utmp for them it should be possible to reliable detect remote sessions. If this turns out to work like I suppose we can remove the name based code at all.

@Fuseteam
Copy link
Author

cool that sounds like quite a robust approach, hopefully this hammers the hole shut once and for all

mcdope added a commit that referenced this issue Feb 24, 2021
mcdope added a commit that referenced this issue Feb 24, 2021
@mcdope
Copy link
Owner

mcdope commented Feb 26, 2021

Since tmux redirects stdin, stdout and stderr my planned approach isn't working.

Current new plan: understand the libprocps api and use it to get the tty of given process - which would work very likely. That's what's used by ps and top to get informations, including the correct tty. But that will take a while because I couldn't find a nice example of how to just get the info for a given pid 😅

@Fuseteam
Copy link
Author

yeah no problem, thanks for the update

@mcdope
Copy link
Owner

mcdope commented Feb 27, 2021

Know what, I hate tmux lol. Turn's out, my approach works nicely - but: we are running in the context of the server (in tmux terms), which yields the wrong tty, which again results in a non working check 🤦 Still need to check if this also applies to byobu.

But I've found that tmux sets an env var containing the client id, which again can be used with tmux list-clients to grep the correct tty which can the be used to check utmp for "remoteness" of the sesssion. You can run CLIENTID=${TMUX: -1}; tmux list-clients | grep ": $CLIENTID \[" | awk 'BEGIN { FS = ":" } ; {print $1}' under tmux to get your login tty as a POC. (Disclaimer: only works if less then 10 clients are connected :P just a poc)

image

Of course this could be circumvented by just deleting or modifing that envvar. For deleting I would fallback to denying the request. Modification are still a potential attack vector, but since this would stilll require to gain login at first I don't think it's an issue.

You happen to know any other multiplexers I should test?

@Fuseteam
Copy link
Author

Fuseteam commented Feb 27, 2021

lol tmux does things quite weirdly, sadly I only know of byobu, tmux and screen, afaict byobu is just a front end for a terminal multiplexer. by default the backend on ubuntuis tmux

i now found two more terminal multiplexers we could test: https://fedoramagazine.org/4-cool-terminal-multiplexers/

@mcdope
Copy link
Owner

mcdope commented Feb 27, 2021

Oh that's nice to know, then I should check what backends it supports.

@Fuseteam
Copy link
Author

Fuseteam commented Feb 27, 2021

hmm found a list of 7 terminal multiplexers: https://www.linuxlinks.com/terminalmultiplexers/ but I have a feeling the additional two are just forked from tmux

@mcdope
Copy link
Owner

mcdope commented Feb 28, 2021

Local tmux (in bash, in terminator, under gnome-shell with gdm3): https://asciinema.org/a/4TYiAyxIAwUHt3AmGHqs5eGhw
Remote tmux (well, ssh to localhost): https://asciinema.org/a/ZonJgePgeGyHCFYv7ejwavzPD
Local bash (in clion, under gs/gdm again): https://asciinema.org/a/fIKFRFQkZDxAaQ0H86aO7v7Io
Local console tty: https://asciinema.org/a/C5dpgxYQke9z1sd5cdm7ysmJW

😁

Seems this works quite well, but need to clean up and shuffle some stuff around before I will provide a testing release. Guess you can expect a deb sometime next week.

@Fuseteam
Copy link
Author

cool 🤩 great work!

@mcdope
Copy link
Owner

mcdope commented Aug 9, 2021

Seems the screen backend case is handled already lol

image

@mcdope
Copy link
Owner

mcdope commented Aug 19, 2021

Seems tmux backend is fixed too. To cite Britney Spears: Oops I did it again! (Yes, I'm THAT old)

image

Updated deb at the usual link, https://www.dropbox.com/s/af1ghfgheyp7cfn/libpam-usb_0.7.4_amd64.deb?dl=0

@Fuseteam
Copy link
Author

Fuseteam commented Aug 19, 2021

hmm weird stuff happening here:
i also managed to still get through if i do
launch byobu
ssh in from another machine
attach to byobu
image

but if i also open a new byobu window from the remote machine in the same session to get the above i get this:
image
image

i installed with apt install --reinstall

@mcdope
Copy link
Owner

mcdope commented Aug 19, 2021

I smell a botched if again...

@mcdope
Copy link
Owner

mcdope commented Aug 19, 2021

Should be fixed, deb is updated again https://www.dropbox.com/s/af1ghfgheyp7cfn/libpam-usb_0.7.4_amd64.deb?dl=0

btw, we are "network siblings" hehe - same address space XD

@Fuseteam
Copy link
Author

Fuseteam commented Aug 19, 2021

oho~ touché
image
now just the first case left xD
seems new byobu windows are caught but since the existing one originates from the local user that one passes
image

btw, we are "network siblings" hehe - same address space XD

haha how fun xDDD

@mcdope
Copy link
Owner

mcdope commented Aug 19, 2021

Yeah, I can see how that happens... That will be easy to fix I guess.

@Fuseteam
Copy link
Author

cool xD

@mcdope
Copy link
Owner

mcdope commented Aug 20, 2021

Since you seem to know byobu and surrounding stuff quite well, could you test this with new-window too? Guess that would pass too...

Edit: split-window is maybe interesting too, but not sure.

Edit 2: nvm, i dont think either one matters. Deb updated, https://www.dropbox.com/s/af1ghfgheyp7cfn/libpam-usb_0.7.4_amd64.deb?dl=0

@Fuseteam
Copy link
Author

oho~ seems to work
image

@mcdope
Copy link
Owner

mcdope commented Aug 23, 2021

lmao i literally opened the issue right as you commented, nice :D

@Fuseteam
Copy link
Author

great timing xDDD

@mcdope
Copy link
Owner

mcdope commented Aug 23, 2021

if you still find a way to break it before 29. August 23:59 we should start talking about bug bounties, else it will be released after that datetime.

huge thanks for all your testing efforts, i owe you one. If you ever need a hand with one of your issues feel free to tag me.

@Fuseteam
Copy link
Author

Fuseteam commented Aug 23, 2021

so far so good, awesome work 👍 if i don't find a wayt to break it by that datetime i'll close the issue too xD

happy to help, and thanks will do

@mcdope
Copy link
Owner

mcdope commented Aug 26, 2021

I like your silence 🤣

@Fuseteam
Copy link
Author

Fuseteam commented Aug 27, 2021

🤣 🤣 🤣 🤣 🤣 🤣

@Fuseteam
Copy link
Author

heh so far so good

@mcdope
Copy link
Owner

mcdope commented Aug 31, 2021

Let's leave it open for now, since the fix is not merged yet. I still need to clean some stuff in the PR up before merging, issue will be autoclosed by merge then.

@mcdope mcdope reopened this Aug 31, 2021
mcdope added a commit that referenced this issue Aug 31, 2021
mcdope added a commit that referenced this issue Aug 31, 2021
mcdope added a commit that referenced this issue Aug 31, 2021
The tty only approach had downsides (see #8), its fix had downsides too (see #39), and the name approach had downsides (see #51) too, let's try all together plus some additional magic.

This modifies local check to:
* check for sshd/telnetd in process chain - for the obvious cases
* check for tmux in process chain, if found parse its environment to determine tmux client id to determine session tty for utmp check
* if tmux found, but session tty not, check for remote clients attached to tmux
* check for DISPLAY, if found use that for utmp check
* in case no remote daemon was found, tmux wasn't detected, and DISPLAY is not set - fall back to good ol' ttyname() which should now be safe since we handled all edge cases before

Closes #51 


* #51: process:c add get_process_tty()

* #51: local.c: Re-add utmp code, to be used by parent pid using process.c [WIP]

* #51: Add @todo

* #51: [WIP] Rework get_process_tty(), check for X session, add more debug logging

* #51: [WIP] If tmux detected use it to detect the login tty

* #51: local.c use new tmux/display/tty approach in all cases

* #51: process.c: remove get_process_tty(), local.c: rename from to session_tty

* #51: [Debian] [Packaging] Re-add 'Standards-Version', got lost somehow

* #51: local.c: replace 4-spaces with tabs to keep uniform formatting

* #51: remove libprocps depency again

* #51: local.c: remove current_tty - used only for logging / making the code order nicer

* local.c: spaces...

* local.c: fix alt-tab-typo and some formatting

* #51: Test for open udp port 177 (XDMCP negotiation), if not found allow (when display manager is found)

* #51: Remove port check stuff again, XDMCP is a pain to setup for testing and is insecure anyway

* #51: local.c: whitelist graphical logins by service tag, remove xdmcp leftovers

* #51: Make ttyname() approach default fallback for all cases

* #51: local.c: extract tmux magic to tmux.c

* #51: local.c/Makefile: make use of tmux.c, adjust to new chain

* #51: Iterate all tty methods, add 'tmux var from parent proc', cleanup formatting

* #51: local.c: add pusb_get_tty_by_xorg_display(), used to get tty by DISPLAY var (for SDDM sessions)

* #51: Fix DISPLAY fallback, add more debug, expect console and pts

* #51: Add pusb_ prefix to new functions

* #51: Add pusb_ prefix to new functions 2nd edition

* #51: Whitelist sddm too

* #51: [WIP] [deb} Update news and changelog

* #51: Fix incorrect return handling reported in #51 (comment)

* #51: Fix derp

* actions: make sure no previously build debs are installed again...

* #51: Check for remotely connected clients to local tmux sessions

* #51: This and that

* #51: Fix v6 detection of connected tmux sessions

* #51: Cleanup

* #51: 'Fix' debug output

* #51: Fix warning unitialized for tmux_pid

* #51/#64: Replace utmp with utmpx stuff / posix compliance

* #51: tmux.c: extend regex to capture full 'attach' argument too

* #51: Fix last case of byobu/tmux

* #51: Remove version update, will be done in seperate PR

* #51: Cleanup
@mcdope
Copy link
Owner

mcdope commented Sep 1, 2021

Released as v0.8.0

@Fuseteam
Copy link
Author

Fuseteam commented Sep 3, 2021

wooohooo thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants