aurora git commit: Fix thermos killing heuristic to permit setuid(2).

[zm...@apache.org]

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 <zm...@apache.org>
Authored: Tue Aug 23 15:07:45 2016 -0700
Committer: Zameer Manji <zm...@apache.org>
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())