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.

Sign up for GitHub

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

Jump to bottom

Bug: deny_remote seems to prevent non-tty-sessions on Ubuntu #8

Closed
mcdope opened this issue Aug 21, 2020 · 7 comments · Fixed by #43 or #45
Closed

Bug: deny_remote seems to prevent non-tty-sessions on Ubuntu #8

mcdope opened this issue Aug 21, 2020 · 7 comments · Fixed by #43 or #45
Assignees
@mcdope
Labels
bug Something isn't working

Comments

@mcdope
Copy link
Owner

mcdope commented Aug 21, 2020 

* Authentication request for user "mcdope" (pamusb-check)
[src/local.c:037] Checking whether the caller is local or not...
[src/local.c:046] Authentication request from tty pts/0
[src/local.c:053] No utmp entry found for tty "pts/0"
* Access denied.
@mcdope mcdope added the bug Something isn't working label Aug 21, 2020
@mcdope
Copy link
Owner Author

mcdope commented Aug 22, 2020

Seems to be related to virtual terminal / pts devices.

This happens for local sessions on pts/* devices. But if the session tied to the pts/* is an OpenSSH session this doesn't occur. Maybe change the behaviour to default to success in sessions coming from pts/* with no getutline() result? What would be the implications?

Behaviour was different before, too. See aa63b60

@mcdope
Copy link
Owner Author

mcdope commented Aug 22, 2020

Can't think of any scenario that would allow an attacker to launch a pseudo-terminal without being already logged in... Since the remote check works except for local pts/* it should be safe to change the behaviour for such cases. But should add an option for it...

@mcdope mcdope self-assigned this Aug 22, 2020
@mcdope mcdope changed the title Bug: deny_remote seems to prevent login on Ubuntu Bug: deny_remote seems to prevent sudo on Ubuntu Aug 22, 2020
mcdope added a commit that referenced this issue Aug 22, 2020
@mcdope
Configuration: introduce new option 'unknown_pts_as_local' to allow s…
5075672 
…udo usage on local non-login-pseudoterminals - closes issue #8
@mcdope mcdope closed this as completed Aug 22, 2020
@mcdope
Copy link
Owner Author

mcdope commented Aug 23, 2020

aa63b60#diff-62be85f8fd64a755fae6ec9226c8865eL42 still causing issues when auth'ing from virtual stuff. Currently in gdm.

Aug 23 12:28:02 dope-seiner pam_usb[29185]: Authentication request for user "mcdope" (gdm-password)
Aug 23 12:28:02 dope-seiner pam_usb[29185]: Checking whether the caller is local or not...
Aug 23 12:28:02 dope-seiner pam_usb[29185]: Couldn't retrieve the tty name, aborting.
Aug 23 12:28:02 dope-seiner pam_usb[29185]: Access denied.
Aug 23 12:28:05 dope-seiner gdm-password][29185]: pam_unix(gdm-password:auth): authentication failure; logname= uid=0 euid=0 tty=/dev/tty1 ruser= rhost=  user=mcdope

@mcdope mcdope reopened this Aug 23, 2020
@mcdope
Copy link
Owner Author

mcdope commented Aug 23, 2020

According to https://man7.org/linux/man-pages/man3/ttyname.3.html#NOTES this is expected behaviour. Seems there is no real solution to this, see https://stackoverflow.com/a/41043948. So I guess we must default to "pass", like it was before aa63b60

mcdope added a commit that referenced this issue Aug 23, 2020
@mcdope
local.c: #8: Fix local detection once more, this time for gdm etc
9766b13
@mcdope
Copy link
Owner Author

mcdope commented Aug 23, 2020

With 9766b13 the original behaviour is now fully restored when unknown_pts_as_local is enabled. Guess this should fix it totally now.

@mcdope mcdope closed this as completed Aug 23, 2020
@mcdope mcdope changed the title Bug: deny_remote seems to prevent sudo on Ubuntu Bug: deny_remote seems to prevent non-tty-sessions on Ubuntu Aug 23, 2020
This was referenced Feb 12, 2021
Closed
Merged
mcdope added a commit that referenced this issue Feb 12, 2021
@mcdope
Merge pull request #43 from mcdope/issue-39-remote-sudo-password-bypa…
c6fde1b 
…ss-via-vim

#39: remote sudo password bypass via vim

Back when we fixed #8 we introduced a sideeffect, allowing sudo bypass if connected to remote host as userX when userX has his already configured media connected.

This changes the default value for the then introduced option unknown_pts_as_local to false to fix this.

Closes #39
@mcdope
Copy link
Owner Author

mcdope commented Feb 13, 2021

Well, #39 is fixed now - but now the original bug is back, GDM Login not working...

Need to find a proper way to check this, maybe some kernel api?

@mcdope mcdope reopened this Feb 13, 2021
@mcdope
Copy link
Owner Author

mcdope commented Feb 13, 2021 •
edited
Loading

This script snippet (source: https://serverfault.com/a/773890) so far seems to be able to reliable be able to detect "remoteness" based on checking the parent process tree.

Guess it would be the best to rework deny_remote to use that approach instead of utmp checking.

`#!/bin/bash

if pstree -p | egrep --quiet --extended-regexp ".sshd.($$)"; then
echo "I am remote."
else
echo "I am local."
fi`

mcdope added a commit that referenced this issue Feb 13, 2021
@mcdope
#8/#39: Rework to use process based checking, renders useless - to be…
20bef8e 
… removed
This was referenced Feb 13, 2021
Merged
Open
mcdope added a commit that referenced this issue Feb 13, 2021
@mcdope
Remove unknown_pts_as_local option (#8/#39)
f9e90cc
mcdope added a commit that referenced this issue Feb 14, 2021
@mcdope
Merge pull request #45 from mcdope/issue-8-rework-deny_remote-option
ac725b6 
#8/#39: Rework deny_remote / remove unknown_pts_as_local

This reworks deny_remote handling to use process based checking.

Instead of checking utmp and hoping there is an entry for the current session, which there often isn't for virtual terminals, we now check the chain of parent processes. If any parent process is sshd or telnetd we deny authentication.

This renders unknown_pts_as_local obsolete - removed.

Closes #8 (again)
See #39
This was referenced Mar 1, 2021
Merged
Open
Closed
mcdope added a commit that referenced this issue Aug 31, 2021
@mcdope
#51: fix terminal multiplexers bypass remote sudo password (#54)
b5ec9eb 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mcdope mcdope

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
1 participant
@mcdope