auth_keep: do not ask for reauth if new process shares same UID/parent/cgroup/tty #533
Conversation
| gint uid; | ||
| gint pidfd; | ||
| gint ppidfd; |
There was a problem hiding this comment.
i don't think looking at the ppid is a good idea, because that means subshells (some of which are somewhat hidden in shells) would not qualify anymore.
instead: check the posix session id (i.e. the one managed by setsid()/getsid(), not the audit session id, nor the logind session id, which are three distinct things). the posix session id should be unique for each shell, and is necessary for correct shell sesson mgmt (it means that if the shell goes away all forked of processes get SIGHUP).
the session leader has pid=sid btw, and if it exits the session becomes orphaned. (thus the sid will exist for longer, until all processes with it actually exited, but now without a leader). but that's good enough, it allows tracking sids via pidfds for this purpose, and thus get a proper pidfd inode id for the sid, that is safe regarding recycling
There was a problem hiding this comment.
i don't think looking at the ppid is a good idea, because that means subshells (some of which are somewhat hidden in shells) would not qualify anymore.
That was intentional: right now auth is checked on every single invocation. This PR relaxes this somewhat, by allowing multiple commands in the same shell to be done without checking auth again. Letting scripts running in the same shell also run without auth is a step too far for now. We can think about it again later, but for now I prefer to keep it very explicit, so that if you do:
$ pkexec foo
$ pkexec bar
You only get auth'ed once, but if you do:
$ pkexec foo
$ ./bar.sh
and bar.sh itself calls pkexec, auth is checked again. This was it's very very explicit, or at least it tries to be. We can consider further relaxing it more down the line. There are other things to consider in the future too, to match sudo's behaviour: whether to increase the default window to 15m, and whether to "refresh" the token on each invocation. But for now let's keep it nice and tight.
the posix session id should be unique for each shell
- it should be, but it's not mandatory, it depends on the shell implementation itself
- it is 32bit so it can wrap around, and the only way to make that safe is to use the super-new pidfd filesystem (introduced this year) which is 64bit only and requires a disabled-by-default kconfig to be enabled
so it's riskier, and cuts off basically all existing Linux systems, and might be usable in a year or two, so it's too restrictive, so for now I'll keep checking the tty
There was a problem hiding this comment.
it should be, but it's not mandatory, it depends on the shell implementation itself
no it does not? the sid is always initialized from the pid of the caller. you don't get to choose your sid?
ptys are tightly packed (i.e. you always get lowest free pty), and aggressively recycled. otoh, sids are just like pids, i.e. allocated form a counter basically.
in other words, there is no rational rason why you think checking ptys is fine but checking sids is not.
sids change between user sessions on the same ttys, but tty numbers are super stable, hence not suitable for authentication that shall be context bound.
There was a problem hiding this comment.
no it does not? the sid is always initialized from the pid of the caller. you don't get to choose your sid?
But that means, for all intents and purposes, sid == pid, including the fact that it wraps around and doesn't require privileges to get a new one, and hit a collision.
sids change between user sessions on the same ttys, but tty numbers are super stable, hence not suitable for authentication that shall be context bound.
sudo uses ttys to cache
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
bluca
force-pushed
the
keep_by_ppid
branch
3 times, most recently
from
99d89bc to
6aff8c0
Compare
|
you shouldn't do those pty shenanigans, that helps noone, it just breaks the concept on non-pty ttys, such as the VT, serial, hvc, and so on. and use the sid anyway, not the other stuff. note that /proc/$PID/stat is a linux thing. as i understand you care a lot about solaris, so why not check the sid instead, that's posix after all... |
…t/cgroup/tty sudo keeps a record of authenticated processes via either the controlling TTY (default) or the parent process id. Implement the same caching behaviour, but stricter: if a process is authenticated for auth_keep, do not expunge it when it exits if it was tracked via PID FD (to make it safe against reuse attacks). Then, if another process comes along, skip re-auth and allow it if it shared the same UID, parent process id, cgroup id and controlling terminal (and all processes are newer than the controlling terminal ctime). PID FDs must be used all the way through, otherwise there's no caching. This is much stricter than sudo, as all conditions must be met. But it still allows to fulfill the main use case, which is to run multiple commands on the same terminal without being asked for the password again and again. Unlike sudo, we also do not refresh the countdown on each use. Fixes polkit-org#472
Serial/hvc do not work anyway, as they are not local.
That's not enough, as it can wrap, it's a 32bit number |
This seems to be a pretty annoying pointless restriction. Whether a seat is available or not shouldn't matter for authentication. It should be fine to restric scope to the sid of a session (i.e. to one specific ssh session for example) |
so is the tty, but with much worse properties. you cannot use small range set as excuse for using tty over sid, because the sid range is much larger and not tightly packed near the bottom. |
That may or may not be, but it's how it works now and how it always did, and I will not change it here, as it's out of scope |
There is one crucial difference, and it's likely the reason sudo uses it: you can check the mtime of the tty, and ensure it is older than both processes, which is what I am doing here, and what sudo also does. |
|
uh? mtime of the tty? that's updated on every write() to the tty, from every command you invoke that writes to stdou/stderr. we use that in logind for example to do idle detection of tty sections. mtime is under full user control, should not be used for security decisions whatsoever. anyone can do |
|
Check the diff, meant ctime, typing from phone |
sudo keeps a record of authenticated processes via either the controlling TTY (default) or the parent process id.
Implement the same caching behaviour, but stricter: if a process is authenticated for auth_keep, do not expunge it when it exits if it was tracked via PID FD (to make it safe against reuse attacks).
Then, if another process comes along, skip re-auth and allow it if it shared the same UID, parent process id, cgroup id and controlling terminal (and all processes are newer than the controlling terminal ctime). PID FDs must be used all the way through, otherwise there's no caching.
This is much stricter than sudo, as all conditions must be met. But it still allows to fulfill the main use case, which is to run multiple commands on the same terminal without being asked for the password again and again.
Unlike sudo, we also do not refresh the countdown on each use.
Fixes #472
This allows running
run0orsystemctland friends multiple times from the same terminal without having to re-enter the password multiple times, if within the default temporary auth_keep time window (5 minutes)