This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/131/

On leden 20th, 2015, 8:48 odp. CET, Miloslav Trmac wrote:

src/rolekit/async.py (Diff revision 1)
def input_handler(unused_fd, condition, unused_data):
294
    def demote(user_uid, user_gid):

AFAICS calling the demote() function is superfluous and, to me, confusing; we only need to pass the set_ids callable.

On leden 21st, 2015, 4:21 odp. CET, Stephen Gallagher wrote:

I actually looked up how to do this in several places and all the examples I could find insisted that it had to be done this way or else the calling application would also be affected by the permission drop. I'm choosing to trust them.

Testing disagrees:

import os, subprocess def set_ids(): os.setregid(1,1); os.setreuid(1,1) ... p = subprocess.Popen(['/bin/id', '-a'], preexec_fn = set_ids) uid=1(bin) gid=1(bin) skupiny=1(bin),0(root) kontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023

print os.geteuid() 0 print os.getuid() 0 print os.getegid() 0 print os.getgid() 0

(and this also shows that supplementary groups need to be dropped. Will raise both separately in the new patch.)


On leden 20th, 2015, 8:48 odp. CET, Miloslav Trmac wrote:

src/rolekit/async.py (Diff revision 1)
def input_handler(unused_fd, condition, unused_data):
298
            os.setgid(user_gid)
299
            os.setuid(user_uid)

setre[ug]id() to make it explicit that both are changed?

On leden 21st, 2015, 4:21 odp. CET, Stephen Gallagher wrote:

Seems excessive, but ok, sure.

Specifically setRE[ug]id would be better because set[ug]id also affects the e[ug]id in not-quite-intuitive ways (and in platform-dependent ways, which does not matter for Linux but makes readability worse). os.setreuid and os.setregid are available.


On leden 20th, 2015, 8:48 odp. CET, Miloslav Trmac wrote:

src/rolekit/async.py (Diff revision 1)
def input_handler(unused_fd, condition, unused_data):
301
        if user_uid and user_gid:

One way to fix: move this check inside set_ids(), and then call Popen(… preexec_fn=set_ids)

Another way, more similar to the current code:

if (user_uid is not None or user_gid is not None): # minimal cleanup related to 0/none and being paranoid preexec_fn = set_ids else: preexec_fn = None … and then call Popen(…, preexec_fn=preexec_fn) (change names as you like)

Oops, let me try to fix the line wraps:

if (user_uid is not None or user_gid is not None): # minimal cleanup related to 0/none and being paranoid preexec_fn = set_ids else: preexec_fn = None

… and then call Popen(…, preexec_fn=preexec_fn) (change names as you like)


- Miloslav


On leden 21st, 2015, 4:49 odp. CET, Stephen Gallagher wrote:

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, and Thomas Woerner.
By Stephen Gallagher.

Updated Led. 21, 2015, 4:49 odp.

Repository: rolekit

Description

Allow impersonating a different UID/GID in subprocesses

Diffs

  • src/rolekit/async.py (0f9ddaac1beb27cebdf41ca0383a62a807c4fcb6)

View Diff