Opened 4 years ago

Closed 4 years ago

#6443 defect closed fixed (fixed)

setuid should be called in the child after forking the process in spawnProcess

Reported by: Jonathan Stoppani Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/only-child-setuid-6443
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

Currently the Process class sets the euid in the parent process before forking (http://twistedmatrix.com/trac/browser/trunk/twisted/internet/process.py#L380)

There seem to be no reason this can't be more properly done in the forked child.

Attachments (1)

procmontest.tac (367 bytes) - added by Jean-Paul Calderone 4 years ago.
sudo PYTHONPATH=... twistd --euid --uid nobody --gid nogroup -ny procmontest.tac

Download all attachments as: .zip

Change History (12)

comment:1 Changed 4 years ago by Jean-Paul Calderone

Owner: set to Jean-Paul Calderone
Status: newassigned

comment:2 Changed 4 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/only-child-setuid-6443

(In [38200]) Branching to 'only-child-setuid-6443'

comment:3 Changed 4 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jean-Paul Calderone deleted
Status: assignednew

Build results. Some slaves are broken right now, unfortunately.

Also worth noting that the test suite doesn't actually try to run a process with another uid/gid, since that requires root which our slaves don't generally have. There are some tests with mocks, as apparent from the changes in the branch. I did some manual testing with the attached tac.

Changed 4 years ago by Jean-Paul Calderone

Attachment: procmontest.tac added

sudo PYTHONPATH=... twistd --euid --uid nobody --gid nogroup -ny procmontest.tac

comment:4 Changed 4 years ago by Jean-Paul Calderone

Build results link above is wrong, sorry. Here's a working one.

comment:5 Changed 4 years ago by Tom Prince

Keywords: review removed
  1. The docstrings for test_mockSetUidInParent and test_mockPTYSetUidInParent are no longer accurate.
    • It appears that
  2. If we know how to test this, but it requires root, why don't we include the test but skipped.
    • twisted.python.test.test_fakepwd.SPwdModuleTests is an existing example of doing this.

Please merge after updating the docstring and adding a real test.

comment:6 Changed 4 years ago by Jean-Paul Calderone

(In [38541]) Fix the docstrings of two tests which had their expectations modified.

refs #6443

comment:7 Changed 4 years ago by Jean-Paul Calderone

  • It appears that

Was this the start of an important comment, or an item you meant to delete?

If we know how to test this, but it requires root, why don't we include the test but skipped.

Hm. Okay, if we set up a slave that runs as root.

comment:8 Changed 4 years ago by Jean-Paul Calderone

(In [38549]) pyflakes fixes

refs #6443

comment:9 Changed 4 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [38550]) Merge only-child-setuid-6443

Author: exarkun Reviewer: tom.prince Fixes: #6443

Avoid changing the process UID or GID when launching a child process which is to run with a different UID or GID.

comment:10 Changed 4 years ago by Jean-Paul Calderone

Resolution: fixed
Status: closedreopened

(In [38552]) Revert r38550 - test suite regression

New tests fail on Windows, which lacks UIDs and GIDs.

Reopens: #6443

comment:11 Changed 4 years ago by Jean-Paul Calderone

Resolution: fixed
Status: reopenedclosed

(In [38568]) Merge only-child-setuid-6443

Author: exarkun Reviewer: tom.prince Fixes: #6443

Avoid changing the process UID or GID when launching a child process which is to run with a different UID or GID.

Note: See TracTickets for help on using tickets.