Skip to content

Commit 2b2ead7

Browse files
patrick-mcleangpshead
patrick-mclean
authored and
gpshead
committed
bpo-36046: Add user and group parameters to subprocess (GH-11950)
* subprocess: Add user, group and extra_groups paremeters to subprocess.Popen This adds a `user` parameter to the Popen constructor that will call setreuid() in the child before calling exec(). This allows processes running as root to safely drop privileges before running the subprocess without having to use a preexec_fn. This also adds a `group` parameter that will call setregid() in the child process before calling exec(). Finally an `extra_groups` parameter was added that will call setgroups() to set the supplimental groups.
1 parent 57b7dbc commit 2b2ead7

File tree

7 files changed

+426
-19
lines changed

7 files changed

+426
-19
lines changed

‎Doc/library/subprocess.rst‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,9 @@ functions.
339339
stderr=None, preexec_fn=None, close_fds=True, shell=False, \
340340
cwd=None, env=None, universal_newlines=None, \
341341
startupinfo=None, creationflags=0, restore_signals=True, \
342-
start_new_session=False, pass_fds=(), *, \
343-
encoding=None, errors=None, text=None)
342+
start_new_session=False, pass_fds=(), *, group=None, \
343+
extra_groups=None, user=None, encoding=None, errors=None, \
344+
text=None)
344345
345346
Execute a child program in a new process. On POSIX, the class uses
346347
:meth:`os.execvp`-like behavior to execute the child program. On Windows,
@@ -544,6 +545,33 @@ functions.
544545
.. versionchanged:: 3.2
545546
*start_new_session* was added.
546547

548+
If *group* is not ``None``, the setregid() system call will be made in the
549+
child process prior to the execution of the subprocess. If the provided
550+
value is a string, it will be looked up via :func:`grp.getgrnam()` and
551+
the value in ``gr_gid`` will be used. If the value is an integer, it
552+
will be passed verbatim. (POSIX only)
553+
554+
.. availability:: POSIX
555+
.. versionadded:: 3.9
556+
557+
If *extra_groups* is not ``None``, the setgroups() system call will be
558+
made in the child process prior to the execution of the subprocess.
559+
Strings provided in *extra_groups* will be looked up via
560+
:func:`grp.getgrnam()` and the values in ``gr_gid`` will be used.
561+
Integer values will be passed verbatim. (POSIX only)
562+
563+
.. availability:: POSIX
564+
.. versionadded:: 3.9
565+
566+
If *user* is not ``None``, the setreuid() system call will be made in the
567+
child process prior to the execution of the subprocess. If the provided
568+
value is a string, it will be looked up via :func:`pwd.getpwnam()` and
569+
the value in ``pw_uid`` will be used. If the value is an integer, it will
570+
be passed verbatim. (POSIX only)
571+
572+
.. availability:: POSIX
573+
.. versionadded:: 3.9
574+
547575
If *env* is not ``None``, it must be a mapping that defines the environment
548576
variables for the new process; these are used instead of the default
549577
behavior of inheriting the current process' environment.

‎Lib/multiprocessing/util.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ def spawnv_passfds(path, args, passfds):
429429
return _posixsubprocess.fork_exec(
430430
args, [os.fsencode(path)], True, passfds, None, None,
431431
-1, -1, -1, -1, -1, -1, errpipe_read, errpipe_write,
432-
False, False, None)
432+
False, False, None, None, None, None)
433433
finally:
434434
os.close(errpipe_read)
435435
os.close(errpipe_write)

‎Lib/subprocess.py‎

Lines changed: 100 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,14 @@
5353
import contextlib
5454
from time import monotonic as _time
5555

56+
try:
57+
import pwd
58+
except ImportError:
59+
pwd = None
60+
try:
61+
import grp
62+
except ImportError:
63+
grp = None
5664

5765
__all__ = ["Popen", "PIPE", "STDOUT", "call", "check_call", "getstatusoutput",
5866
"getoutput", "check_output", "run", "CalledProcessError", "DEVNULL",
@@ -719,6 +727,12 @@ class Popen(object):
719727
720728
start_new_session (POSIX only)
721729
730+
group (POSIX only)
731+
732+
extra_groups (POSIX only)
733+
734+
user (POSIX only)
735+
722736
pass_fds (POSIX only)
723737
724738
encoding and errors: Text mode encoding and error handling to use for
@@ -735,7 +749,8 @@ def __init__(self, args, bufsize=-1, executable=None,
735749
shell=False, cwd=None, env=None, universal_newlines=None,
736750
startupinfo=None, creationflags=0,
737751
restore_signals=True, start_new_session=False,
738-
pass_fds=(), *, encoding=None, errors=None, text=None):
752+
pass_fds=(), *, user=None, group=None, extra_groups=None,
753+
encoding=None, errors=None, text=None):
739754
"""Create new Popen instance."""
740755
_cleanup()
741756
# Held while anything is calling waitpid before returncode has been
@@ -833,6 +848,78 @@ def __init__(self, args, bufsize=-1, executable=None,
833848
else:
834849
line_buffering = False
835850

851+
gid = None
852+
if group is not None:
853+
if not hasattr(os, 'setregid'):
854+
raise ValueError("The 'group' parameter is not supported on the "
855+
"current platform")
856+
857+
elif isinstance(group, str):
858+
if grp is None:
859+
raise ValueError("The group parameter cannot be a string "
860+
"on systems without the grp module")
861+
862+
gid = grp.getgrnam(group).gr_gid
863+
elif isinstance(group, int):
864+
gid = group
865+
else:
866+
raise TypeError("Group must be a string or an integer, not {}"
867+
.format(type(group)))
868+
869+
if gid < 0:
870+
raise ValueError(f"Group ID cannot be negative, got {gid}")
871+
872+
gids = None
873+
if extra_groups is not None:
874+
if not hasattr(os, 'setgroups'):
875+
raise ValueError("The 'extra_groups' parameter is not "
876+
"supported on the current platform")
877+
878+
elif isinstance(extra_groups, str):
879+
raise ValueError("Groups must be a list, not a string")
880+
881+
gids = []
882+
for extra_group in extra_groups:
883+
if isinstance(extra_group, str):
884+
if grp is None:
885+
raise ValueError("Items in extra_groups cannot be "
886+
"strings on systems without the "
887+
"grp module")
888+
889+
gids.append(grp.getgrnam(extra_group).gr_gid)
890+
elif isinstance(extra_group, int):
891+
gids.append(extra_group)
892+
else:
893+
raise TypeError("Items in extra_groups must be a string "
894+
"or integer, not {}"
895+
.format(type(extra_group)))
896+
897+
# make sure that the gids are all positive here so we can do less
898+
# checking in the C code
899+
for gid_check in gids:
900+
if gid_check < 0:
901+
raise ValueError(f"Group ID cannot be negative, got {gid_check}")
902+
903+
uid = None
904+
if user is not None:
905+
if not hasattr(os, 'setreuid'):
906+
raise ValueError("The 'user' parameter is not supported on "
907+
"the current platform")
908+
909+
elif isinstance(user, str):
910+
if pwd is None:
911+
raise ValueError("The user parameter cannot be a string "
912+
"on systems without the pwd module")
913+
914+
uid = pwd.getpwnam(user).pw_uid
915+
elif isinstance(user, int):
916+
uid = user
917+
else:
918+
raise TypeError("User must be a string or an integer")
919+
920+
if uid < 0:
921+
raise ValueError(f"User ID cannot be negative, got {uid}")
922+
836923
try:
837924
if p2cwrite != -1:
838925
self.stdin = io.open(p2cwrite, 'wb', bufsize)
@@ -857,7 +944,9 @@ def __init__(self, args, bufsize=-1, executable=None,
857944
p2cread, p2cwrite,
858945
c2pread, c2pwrite,
859946
errread, errwrite,
860-
restore_signals, start_new_session)
947+
restore_signals,
948+
gid, gids, uid,
949+
start_new_session)
861950
except:
862951
# Cleanup if the child failed starting.
863952
for f in filter(None, (self.stdin, self.stdout, self.stderr)):
@@ -1227,7 +1316,9 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
12271316
p2cread, p2cwrite,
12281317
c2pread, c2pwrite,
12291318
errread, errwrite,
1230-
unused_restore_signals, unused_start_new_session):
1319+
unused_restore_signals,
1320+
unused_gid, unused_gids, unused_uid,
1321+
unused_start_new_session):
12311322
"""Execute program (MS Windows version)"""
12321323

12331324
assert not pass_fds, "pass_fds not supported on Windows."
@@ -1553,7 +1644,9 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
15531644
p2cread, p2cwrite,
15541645
c2pread, c2pwrite,
15551646
errread, errwrite,
1556-
restore_signals, start_new_session):
1647+
restore_signals,
1648+
gid, gids, uid,
1649+
start_new_session):
15571650
"""Execute program (POSIX version)"""
15581651

15591652
if isinstance(args, (str, bytes)):
@@ -1641,7 +1734,9 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
16411734
p2cread, p2cwrite, c2pread, c2pwrite,
16421735
errread, errwrite,
16431736
errpipe_read, errpipe_write,
1644-
restore_signals, start_new_session, preexec_fn)
1737+
restore_signals, start_new_session,
1738+
gid, gids, uid,
1739+
preexec_fn)
16451740
self._child_created = True
16461741
finally:
16471742
# be sure the FD is closed no matter what

‎Lib/test/test_capi.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,15 @@ class Z(object):
9696
def __len__(self):
9797
return 1
9898
self.assertRaises(TypeError, _posixsubprocess.fork_exec,
99-
1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17)
99+
1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20)
100100
# Issue #15736: overflow in _PySequence_BytesToCharpArray()
101101
class Z(object):
102102
def __len__(self):
103103
return sys.maxsize
104104
def __getitem__(self, i):
105105
return b'x'
106106
self.assertRaises(MemoryError, _posixsubprocess.fork_exec,
107-
1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17)
107+
1,Z(),3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20)
108108

109109
@unittest.skipUnless(_posixsubprocess, '_posixsubprocess required for this test.')
110110
def test_subprocess_fork_exec(self):
@@ -114,7 +114,7 @@ def __len__(self):
114114

115115
# Issue #15738: crash in subprocess_fork_exec()
116116
self.assertRaises(TypeError, _posixsubprocess.fork_exec,
117-
Z(),[b'1'],3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17)
117+
Z(),[b'1'],3,(1, 2),5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20)
118118

119119
@unittest.skipIf(MISSING_C_DOCSTRINGS,
120120
"Signature information for builtins requires docstrings")

0 commit comments

Comments
 (0)