Merge pull request #309 from lmr/fix-var-tmp_v2

[V2] Fix avocado tmp directory security issue

avocado/core/data_dir.py

@@ -35,6 +35,7 @@
 
 from avocado.core import job_id
 from avocado.utils import path
+from avocado.utils.data_structures import Borg
 from avocado.settings import settings
 
 
@@ -46,21 +47,20 @@
 SETTINGS_TEST_DIR = os.path.expanduser(settings.get_value('runner', 'test_dir'))
 SETTINGS_DATA_DIR = os.path.expanduser(settings.get_value('runner', 'data_dir'))
 SETTINGS_LOG_DIR = os.path.expanduser(settings.get_value('runner', 'logs_dir'))
-SETTINGS_TMP_DIR = os.path.expanduser(settings.get_value('runner', 'tmp_dir'))
 
 SYSTEM_BASE_DIR = '/var/lib/avocado'
 if 'VIRTUAL_ENV' in os.environ:
     SYSTEM_BASE_DIR = os.environ['VIRTUAL_ENV']
 SYSTEM_TEST_DIR = os.path.join(SYSTEM_BASE_DIR, 'tests')
 SYSTEM_DATA_DIR = os.path.join(SYSTEM_BASE_DIR, 'data')
 SYSTEM_LOG_DIR = os.path.join(SYSTEM_BASE_DIR, 'job-results')
-SYSTEM_TMP_DIR = '/var/tmp/avocado'
 
 USER_BASE_DIR = os.path.expanduser('~/avocado')
 USER_TEST_DIR = os.path.join(USER_BASE_DIR, 'tests')
 USER_DATA_DIR = os.path.join(USER_BASE_DIR, 'data')
 USER_LOG_DIR = os.path.join(USER_BASE_DIR, 'job-results')
-USER_TMP_DIR = '/var/tmp/avocado'
+
+BASE_TMP_DIR = os.environ.get('TMPDIR', '/var/tmp')
 
 
 def _usable_rw_dir(directory):
@@ -233,6 +233,19 @@ def get_job_logs_dir(args=None, unique_id=None):
     return debugdir
 
 
+class _TmpDirTracker(Borg):
+
+    def __init__(self):
+        Borg.__init__(self)
+        if not hasattr(self, 'tmp_dir'):
+            self.tmp_dir = tempfile.mkdtemp(prefix='avocado_', dir=BASE_TMP_DIR)
+
+    def get(self):
+        return self.tmp_dir
+
+_tmp_tracker = _TmpDirTracker()
+
+
 def get_tmp_dir():
     """
     Get the most appropriate tmp dir location.
@@ -243,7 +256,7 @@ def get_tmp_dir():
         * Copies of a test suite source code
         * Compiled test suite source code
     """
-    return _get_rw_dir(SETTINGS_TMP_DIR, SYSTEM_TMP_DIR, USER_TMP_DIR)
+    return _tmp_tracker.get()
 
 
 def clean_tmp_files():

avocado/plugins/config.py

@@ -67,8 +67,7 @@ def run(self, args):
             view.notify(event="minor", msg="file to customize values")
             view.notify(event="message", msg='')
             view.notify(event="message", msg='Avocado Data Directories:')
-            view.notify(event="minor", msg='    base  ' + data_dir.get_base_dir())
-            view.notify(event="minor", msg='    tests ' + data_dir.get_test_dir())
-            view.notify(event="minor", msg='    data  ' + data_dir.get_data_dir())
-            view.notify(event="minor", msg='    logs  ' + data_dir.get_logs_dir())
-            view.notify(event="minor", msg='    tmp   ' + data_dir.get_tmp_dir())
+            view.notify(event="minor", msg='    base     ' + data_dir.get_base_dir())
+            view.notify(event="minor", msg='    tests    ' + data_dir.get_test_dir())
+            view.notify(event="minor", msg='    data     ' + data_dir.get_data_dir())
+            view.notify(event="minor", msg='    logs     ' + data_dir.get_logs_dir())

avocado/test.py

@@ -118,11 +118,7 @@ def __init__(self, methodName='runTest', name=None, params=None,
 
         basename = os.path.basename(self.name)
 
-        if job is not None:
-            tmpdir = tempfile.mkdtemp(dir=data_dir.get_tmp_dir(),
-                                      prefix='job-%s-' % job.unique_id)
-        else:
-            tmpdir = tempfile.mkdtemp(dir=data_dir.get_tmp_dir())
+        tmpdir = data_dir.get_tmp_dir()
 
         self.basedir = os.path.dirname(inspect.getfile(self.__class__))
         self.datadir = os.path.join(self.basedir, '%s.data' % basename)

avocado/utils/data_structures.py

@@ -0,0 +1,38 @@
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+#
+# See LICENSE for more details.
+#
+# Copyright: Red Hat Inc. 2014
+#
+# Authors: Ruda Moura <[email protected]>
+#          Lucas Meneghel Rodrigues <[email protected]>
+#
+"""
+This module contains handy classes that can be used inside
+avocado core code or plugins.
+"""
+
+
+class Borg:
+
+    """
+    Multiple instances of this class will share the same state.
+
+    This is considered a better design pattern in Python than
+    more popular patterns, such as the Singleton. Inspired by
+    Alex Martelli's article mentioned below:
+
+    :see: http://www.aleax.it/5ep.html
+    """
+
+    __shared_state = {}
+
+    def __init__(self):
+        self.__dict__ = self.__shared_state

docs/source/Configuration.rst

@@ -16,13 +16,12 @@ that contain a number of `keys` and `values`. Take for example a basic avocado c
     test_dir = /$HOME/Code/avocado/examples/tests
     data_dir = /usr/share/avocado/data
     logs_dir = ~/avocado/job-results
-    tmp_dir = /var/tmp/avocado
 
 The ``runner`` section contains a number of keys, all of them related to directories used by
 the test runner. The ``base_dir`` is the base directory to other important avocado directories, such
 as log, data and test directories. You can also choose to set those other important directories by
-means of the variables ``test_dir``, ``data_dir``, ``logs_dir`` and ``tmp_dir``. You can do this by
-simply editing the config files available.
+means of the variables ``test_dir``, ``data_dir`` and ``logs_dir``. You can do this by simply editing
+the config files available.
 
 
 Config file parsing order
@@ -82,7 +81,6 @@ configuration, after all the files are parsed in their correct resolution order.
         runner.test_dir $HOME/Code/avocado/examples/tests
         runner.data_dir /usr/share/avocado/data
         runner.logs_dir ~/avocado/job-results
-        runner.tmp_dir  /var/tmp/avocado
 
 The command also shows the order in which your config files were parsed, giving you a better understanding of
 what's going on. The Section.Key nomenclature was inspired in ``git config --list`` output.
@@ -118,7 +116,6 @@ it will give you an output similar to the one seen below::
         tests $HOME/Code/avocado/examples/tests
         data  $HOME/avocado/data
         logs  $HOME/avocado/job-results
-        tmp   /var/tmp/avocado
 
 Note that, while avocado will do its best to use the config values you
 provide in the config file, if it can't write values to the locations
@@ -129,6 +126,12 @@ The relevant API documentation and meaning of each of those data directories
 is in :mod:`avocado.core.data_dir`, so it's higly recommended you take a look.
 
 You may set your preferred data dirs by setting them in the avocado config files.
+The only exception for important data dirs here is the avocado tmp dir, used to
+place temporary files used by tests. That directory will be in normal circumstances
+`/var/tmp/avocado_XXXXX`, (where `XXXXX` is in actuality a random string) securely
+created on `/var/tmp/`, unless the user has the `$TMPDIR` environment variable set,
+since that is customary among unix programs.
+
 The next section of the documentation explains how you can see and set config
 values that modify the behavior for the avocado utilities and plugins.
 

docs/source/WritingTests.rst

@@ -662,9 +662,9 @@ Here are the current variables that Avocado exports to the tests:
 +-------------------------+---------------------------------------+-----------------------------------------------------------------------------------------------------+
 | AVOCADO_TEST_DATADIR    | Data directory for the test           | $AVOCADO_TEST_BASEDIR/my_test.sh.data                                                               |
 +-------------------------+---------------------------------------+-----------------------------------------------------------------------------------------------------+
-| AVOCADO_TEST_WORKDIR    | Work directory for the test           | /var/tmp/avocado/my_test.sh                                                                         |
+| AVOCADO_TEST_WORKDIR    | Work directory for the test           | /var/tmp/avocado_Bjr_rd/my_test.sh                                                                  |
 +-------------------------+---------------------------------------+-----------------------------------------------------------------------------------------------------+
-| AVOCADO_TEST_SRCDIR     | Source directory for the test         | /var/tmp/avocado/my-test.sh/src                                                                     |
+| AVOCADO_TEST_SRCDIR     | Source directory for the test         | /var/tmp/avocado_Bjr_rd/my-test.sh/src                                                              |
 +-------------------------+---------------------------------------+-----------------------------------------------------------------------------------------------------+
 | AVOCADO_TEST_LOGDIR     | Log directory for the test            | $HOME/logs/job-results/job-2014-09-16T14.38-ac332e6/test-results/$HOME/my_test.sh.1                 |
 +-------------------------+---------------------------------------+-----------------------------------------------------------------------------------------------------+

etc/avocado/avocado.conf

@@ -3,4 +3,3 @@ base_dir = /usr/share/avocado
 test_dir = /usr/share/avocado/tests
 data_dir = /usr/share/avocado/data
 logs_dir = ~/avocado/job-results
-tmp_dir = /var/tmp/

selftests/all/unit/avocado/datadir_unittest.py

@@ -22,8 +22,7 @@ def _get_bogus_settings(args):
             'base_dir = %(base_dir)s\n'
             'test_dir = %(test_dir)s\n'
             'data_dir = %(data_dir)s\n'
-            'logs_dir = %(logs_dir)s\n'
-            'tmp_dir = %(tmp_dir)s\n') % args
+            'logs_dir = %(logs_dir)s\n') % args
 
 
 class DataDirTest(unittest.TestCase):
@@ -33,9 +32,8 @@ def setUp(self):
         tdir = os.path.join(tbase, 'tests')
         tdata = os.path.join(tbase, 'data')
         tlogs = os.path.join(tbase, 'logs')
-        ttmp = os.path.join(tbase, 'tmp')
         self.mapping = {'base_dir': tbase, 'test_dir': tdir, 'data_dir': tdata,
-                        'logs_dir': tlogs, 'tmp_dir': ttmp}
+                        'logs_dir': tlogs}
         self.config_file = tempfile.NamedTemporaryFile(delete=False)
         self.config_file.write(_get_bogus_settings(self.mapping))
         self.config_file.close()