Return-Path: X-Original-To: apmail-aurora-commits-archive@minotaur.apache.org Delivered-To: apmail-aurora-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 4DED7106E5 for ; Mon, 10 Feb 2014 22:24:02 +0000 (UTC) Received: (qmail 31543 invoked by uid 500); 10 Feb 2014 22:24:01 -0000 Delivered-To: apmail-aurora-commits-archive@aurora.apache.org Received: (qmail 31503 invoked by uid 500); 10 Feb 2014 22:24:01 -0000 Mailing-List: contact commits-help@aurora.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@aurora.incubator.apache.org Delivered-To: mailing list commits@aurora.incubator.apache.org Received: (qmail 31496 invoked by uid 99); 10 Feb 2014 22:24:00 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 10 Feb 2014 22:24:00 +0000 X-ASF-Spam-Status: No, hits=-2000.6 required=5.0 tests=ALL_TRUSTED,RP_MATCHES_RCVD X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO mail.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with SMTP; Mon, 10 Feb 2014 22:23:57 +0000 Received: (qmail 31330 invoked by uid 99); 10 Feb 2014 22:23:35 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 10 Feb 2014 22:23:35 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id A520F922B48; Mon, 10 Feb 2014 22:23:35 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: wickman@apache.org To: commits@aurora.incubator.apache.org Message-Id: <69f48ab171274a159ce19921ee80439a@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: git commit: AURORA-214: Make sandboxes safer. Date: Mon, 10 Feb 2014 22:23:35 +0000 (UTC) X-Virus-Checked: Checked by ClamAV on apache.org Updated Branches: refs/heads/master e6f415085 -> 1d1bf9d9f AURORA-214: Make sandboxes safer. Testing Done: ./pants src/test/python/apache/aurora/executor/common:directory_sandbox -v Bugs closed: AURORA-204 Reviewed at https://reviews.apache.org/r/17913/ Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/1d1bf9d9 Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/1d1bf9d9 Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/1d1bf9d9 Branch: refs/heads/master Commit: 1d1bf9d9f4f7f26969a41c4183e834e792ea1fce Parents: e6f4150 Author: Brian Wickman Authored: Mon Feb 10 14:23:16 2014 -0800 Committer: Brian Wickman Committed: Mon Feb 10 14:23:16 2014 -0800 ---------------------------------------------------------------------- .../apache/aurora/executor/common/sandbox.py | 32 ++++++++++++---- .../apache/aurora/executor/gc_executor.py | 5 ++- .../executor/common/test_directory_sandbox.py | 40 +++++++++++++++++++- 3 files changed, 67 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/1d1bf9d9/src/main/python/apache/aurora/executor/common/sandbox.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/executor/common/sandbox.py b/src/main/python/apache/aurora/executor/common/sandbox.py index f1fe4d5..c1beb1e 100644 --- a/src/main/python/apache/aurora/executor/common/sandbox.py +++ b/src/main/python/apache/aurora/executor/common/sandbox.py @@ -76,13 +76,29 @@ class DirectorySandbox(SandboxInterface): def create(self): log.debug('DirectorySandbox: mkdir %s' % self.root) - safe_mkdir(self.root) - pwent = pwd.getpwnam(self._user) - grent = grp.getgrgid(pwent.pw_gid) - log.debug('DirectorySandbox: chown %s:%s %s' % (self._user, grent.gr_name, self.root)) - os.chown(self.root, pwent.pw_uid, pwent.pw_gid) - log.debug('DirectorySandbox: chmod 700 %s' % self.root) - os.chmod(self.root, 0700) + + try: + safe_mkdir(self.root) + except (IOError, OSError) as e: + raise self.CreationError('Failed to create the sandbox: %s' % e) + + try: + pwent = pwd.getpwnam(self._user) + grent = grp.getgrgid(pwent.pw_gid) + except KeyError: + raise self.CreationError( + 'Could not create sandbox because user does not exist: %s' % self._user) + + try: + log.debug('DirectorySandbox: chown %s:%s %s' % (self._user, grent.gr_name, self.root)) + os.chown(self.root, pwent.pw_uid, pwent.pw_gid) + log.debug('DirectorySandbox: chmod 700 %s' % self.root) + os.chmod(self.root, 0700) + except (IOError, OSError) as e: + raise self.CreationError('Failed to chown/chmod the sandbox: %s' % e) def destroy(self): - safe_rmtree(self.root) + try: + safe_rmtree(self.root) + except (IOError, OSError) as e: + raise self.DeletionError('Failed to destroy sandbox: %s' % e) http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/1d1bf9d9/src/main/python/apache/aurora/executor/gc_executor.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/executor/gc_executor.py b/src/main/python/apache/aurora/executor/gc_executor.py index cc4e517..b8b5026 100644 --- a/src/main/python/apache/aurora/executor/gc_executor.py +++ b/src/main/python/apache/aurora/executor/gc_executor.py @@ -343,7 +343,10 @@ class ThermosGCExecutor(ThermosExecutorBase, ExceptionalThread, Observable): directory_sandbox = DirectorySandbox(header_sandbox) if header_sandbox else None if directory_sandbox and directory_sandbox.exists(): self.log('Destroying DirectorySandbox for %s' % task_id) - directory_sandbox.destroy() + try: + directory_sandbox.destroy() + except DirectorySandbox.Error as e: + self.log('Failed to destroy DirectorySandbox: %s' % e) else: self.log('Found no sandboxes for %s' % task_id) http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/1d1bf9d9/src/test/python/apache/aurora/executor/common/test_directory_sandbox.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/executor/common/test_directory_sandbox.py b/src/test/python/apache/aurora/executor/common/test_directory_sandbox.py index f8231fb..d896aed 100644 --- a/src/test/python/apache/aurora/executor/common/test_directory_sandbox.py +++ b/src/test/python/apache/aurora/executor/common/test_directory_sandbox.py @@ -16,10 +16,12 @@ import os -from apache.aurora.executor.common.sandbox import DirectorySandbox +from apache.aurora.executor.common.sandbox import DirectorySandbox, SandboxInterface + from twitter.common.contextutil import temporary_dir import mock +import pytest def test_directory_sandbox(): @@ -56,3 +58,39 @@ def test_create(chmod, chown, getpwnam, getgrgid): getgrgid.assert_called_with(123) chown.assert_called_with(real_path, 456, 123) chmod.assert_called_with(real_path, 0700) + + +@mock.patch('pwd.getpwnam') +def test_user_does_not_exist(getpwnam): + getpwnam.side_effect = KeyError('johndoe') + + with temporary_dir() as d: + real_path = os.path.join(d, 'sandbox') + ds = DirectorySandbox(real_path, 'cletus') + with pytest.raises(DirectorySandbox.CreationError): + ds.create() + + getpwnam.assert_called_with('cletus') + + +@mock.patch('os.chown') +def test_create_ioerror(chown): + chown.side_effect = IOError('Disk is borked') + + with temporary_dir() as d: + real_path = os.path.join(d, 'sandbox') + ds = DirectorySandbox(real_path) + with pytest.raises(DirectorySandbox.CreationError): + ds.create() + + +def test_destroy_ioerror(): + with temporary_dir() as d: + real_path = os.path.join(d, 'sandbox') + ds = DirectorySandbox(real_path) + ds.create() + + with mock.patch('shutil.rmtree') as shutil_rmtree: + shutil_rmtree.side_effect = IOError('What even are you doing?') + with pytest.raises(DirectorySandbox.DeletionError): + ds.destroy()