Alexey Izbyshev <izbys...@ispras.ru> added the comment:
Thank you for the review and your thoughts, Gregory. > With this in place we may want to make the _use_posix_spawn() logic in > subprocess.py stricter? That could be its own followup PR. Yes, I think we can always use vfork() on Linux unless we find some issues. (Victor Stinner doesn't seem to be against: https://github.com/python/cpython/pull/11668#issuecomment-457637207). Though C libraries are in a better position to provide safe vfork/exec, by using our implementation we will: * avoid breaking QEMU user-mode (and preserve existing subprocess behavior) * automatically support fast process creation on musl (which has a good posix_spawn, but doesn't have easy means (macros) on detect that we're on musl and thus avoid unacceptable posix_spawn). * avoid having different code paths (and thus potential surprises) depending on arguments of subprocess.Popen() > This is a scary issue. But I think a reasonable approach could be to never > use vfork when running as whatever we choose to define a "privileged user" to > be. > getuid() or geteuid() return 0? don't use vfork. I thought about something like this, but initially dismissed it because I (mistakenly) decided that an application may concurrently switch between arbitrary uids back and forth, so that checking getuid() becomes useless (if we're already talking about an exotic app that calls setuid() concurrently with spawning children, why stop there?). But now I realize that an app may use *arbitrary* uids only if one of its real, effective or saved uids is zero (that is, if we don't take capabilities into account), so at least we could call getresuid() to get all 3 uids in a race-free way and check whether the app is *definitely* privileged... > the concept of "privileged user" can obviously mean a lot more than that and > likely goes beyond what we should attempt to ascertain ourselves. ...but you're making a good point here. So I'm not sure that we want such checks, but if we do, we'd probably need to allow users to disable them -- what if some heavy privileged daemon wants a faster subprocess? > How about also providing a disable-only global setting so that someone > writing code they consider to have elevated privileges can prevent its use > entirely. subprocess.disable_use_of_vfork() and > subprocess.is_vfork_enabled() calls perhaps (just setting/reading a static > int vfork_disallowed = 0 flag within _posixsubprocess.c). I think it's reasonable. I'll look into it when I'm done with current issues. > True setuid vs vfork attack security would suggest code needs to opt-in to > vfork() or posix_spawn() rather than opt-out. Which would destroy the > benefit for most users (who won't bother) for the sake of an issue that just > is not what most code ever does (setuid/setgid/etc. calls are very uncommon > for most software). Agree, especially since we're not talking about "just" using setuid() but rather about using setuid() *in a multithreaded process in a racy manner while spawning children*. Honestly, I'm not sure such apps can blame Python if they get a security hole :) ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <https://bugs.python.org/issue35823> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
