From b5bf92918192e7528d203d62f1c2b36700cb25a8 Mon Sep 17 00:00:00 2001 From: Lucas Meneghel Rodrigues Date: Mon, 15 Dec 2014 15:04:52 -0200 Subject: [PATCH 1/2] avocado.utils: Introduce data_structures module Introduce a module with useful classes for use inside avocado core code or plugins. We start with the Borg class, a design pattern considered to be better than the Singleton [1] [1] http://www.aleax.it/5ep.html Signed-off-by: Lucas Meneghel Rodrigues --- avocado/utils/data_structures.py | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 avocado/utils/data_structures.py diff --git a/avocado/utils/data_structures.py b/avocado/utils/data_structures.py new file mode 100644 index 0000000000..b99c1579c4 --- /dev/null +++ b/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 +# Lucas Meneghel Rodrigues +# +""" +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 From 7015aa321636cb253354b00843a88e3d292f1d5a Mon Sep 17 00:00:00 2001 From: Lucas Meneghel Rodrigues Date: Mon, 15 Dec 2014 10:35:10 -0200 Subject: [PATCH 2/2] avocado: Create and keep track of avocado tmp dir safely Currently avocado uses /var/tmp/avocado as the default location for the avocado tmp dir. This is a bug from the security standpoint, since a malicious user could create a /var/tmp/avocado symlink pointing to an important directory of the system. So, instead of using /var/tmp/avocado, remove the ability to configure the base tmp directory (other than the traditional $TMPDIR environment variable), and make the data_dir.get_tmp_dir() API to return a safe, non predictable temporary directory. Recommended reading: https://kurt.seifried.org/2012/03/14/creating-temporary-files-securely/ http://www.linuxsecurity.com/content/view/115462/151/ Signed-off-by: Lucas Meneghel Rodrigues --- avocado/core/data_dir.py | 21 +++++++++++++++---- avocado/plugins/config.py | 9 ++++---- avocado/test.py | 6 +----- docs/source/Configuration.rst | 13 +++++++----- docs/source/WritingTests.rst | 4 ++-- etc/avocado/avocado.conf | 1 - .../all/unit/avocado/datadir_unittest.py | 6 ++---- 7 files changed, 34 insertions(+), 26 deletions(-) diff --git a/avocado/core/data_dir.py b/avocado/core/data_dir.py index 621aed54f8..c4e5222a02 100755 --- a/avocado/core/data_dir.py +++ b/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,7 +47,6 @@ 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: @@ -54,13 +54,13 @@ 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(): diff --git a/avocado/plugins/config.py b/avocado/plugins/config.py index 8f5345e9ce..7405661df2 100644 --- a/avocado/plugins/config.py +++ b/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()) diff --git a/avocado/test.py b/avocado/test.py index 36e3d7616f..d8405dadae 100644 --- a/avocado/test.py +++ b/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) diff --git a/docs/source/Configuration.rst b/docs/source/Configuration.rst index 81c2ae1356..4a576f7b85 100644 --- a/docs/source/Configuration.rst +++ b/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. diff --git a/docs/source/WritingTests.rst b/docs/source/WritingTests.rst index 4e19ac4db2..3702522526 100644 --- a/docs/source/WritingTests.rst +++ b/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 | +-------------------------+---------------------------------------+-----------------------------------------------------------------------------------------------------+ diff --git a/etc/avocado/avocado.conf b/etc/avocado/avocado.conf index 0e00a51a31..a9a0e6ec8a 100644 --- a/etc/avocado/avocado.conf +++ b/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/ diff --git a/selftests/all/unit/avocado/datadir_unittest.py b/selftests/all/unit/avocado/datadir_unittest.py index 33a574c2ec..5860d7fb2e 100755 --- a/selftests/all/unit/avocado/datadir_unittest.py +++ b/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()