Repository: aurora Updated Branches: refs/heads/master c115ac6bc -> 50f47ccc9
Fix thermos killing heuristic to permit setuid(2). Previously this process killing heuristic would not allow killing of a process if the uid it was launched with differs from the real uid of the currently running process. The logic is too conservative because it doesn't factor in that a process launched as root can use `setuid(2)` to change it's real uid. This patch fixes the heuristic by permitting killing of a process launched as root but the real uid is now not root. Bugs closed: AURORA-1753 Reviewed at https://reviews.apache.org/r/51348/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/50f47ccc Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/50f47ccc Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/50f47ccc Branch: refs/heads/master Commit: 50f47ccc957381f101ecbffa91db0f6776fe19ad Parents: c115ac6 Author: Zameer Manji <[email protected]> Authored: Tue Aug 23 15:07:45 2016 -0700 Committer: Zameer Manji <[email protected]> Committed: Tue Aug 23 15:07:45 2016 -0700 ---------------------------------------------------------------------- src/main/python/apache/thermos/core/helper.py | 6 ++++++ src/test/python/apache/thermos/core/test_helper.py | 8 ++++++++ 2 files changed, 14 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/50f47ccc/src/main/python/apache/thermos/core/helper.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/thermos/core/helper.py b/src/main/python/apache/thermos/core/helper.py index dda40ed..68855e1 100644 --- a/src/main/python/apache/thermos/core/helper.py +++ b/src/main/python/apache/thermos/core/helper.py @@ -101,6 +101,12 @@ class TaskRunnerHelper(object): if process_uid == uid: return True + elif uid == 0: + # If the process was launched as root but is now not root, we should + # kill this because it could have called `setuid` on itself. + log.info("pid %s appears to be have launched by root but it's uid is now %s" % ( + process.pid, process_uid)) + return True else: log.info("Expected pid %s to be ours but the pid uid is %s and we're %s" % ( process.pid, process_uid, uid)) http://git-wip-us.apache.org/repos/asf/aurora/blob/50f47ccc/src/test/python/apache/thermos/core/test_helper.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/thermos/core/test_helper.py b/src/test/python/apache/thermos/core/test_helper.py index 35397ab..628b6ba 100644 --- a/src/test/python/apache/thermos/core/test_helper.py +++ b/src/test/python/apache/thermos/core/test_helper.py @@ -23,6 +23,7 @@ from apache.thermos.core.helper import TaskRunnerHelper as TRH from gen.apache.thermos.ttypes import ProcessStatus, RunnerHeader, RunnerState USER1 = 'user1' +ROOT_UID = 0 UID = 567 PID = 12345 CREATE_TIME = time.time() @@ -45,6 +46,13 @@ def mock_process(pid, username, uid=None): return process +def test_this_is_really_our_pid_root(): + # Test the case where a process has a non root uid but the uid in the header + # was root (meaning that it `setuid` itself) + process = mock_process(PID, USER1, uid=UID) + assert TRH.this_is_really_our_pid(process, ROOT_UID, USER1, process.create_time()) + + def test_this_is_really_our_pid(): process = mock_process(PID, USER1, uid=UID) assert TRH.this_is_really_our_pid(process, UID, USER1, process.create_time())
