From 409918f9ba83e45e9bc5cc0b6c589e2fc8ae9b60 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Tue, 29 Aug 2017 09:59:20 -0400 Subject: Use /run/cloud-init for tempfile operations. During boot, the usage of /tmp is not safe. In systemd systems, systemd-tmpfiles-clean may run at any point and clear out a temp file while cloud-init is using it. The solution here is to use /run/cloud-init/tmp. LP: #1707222 --- cloudinit/config/cc_bootcmd.py | 3 +- cloudinit/config/cc_chef.py | 3 +- cloudinit/config/cc_snappy.py | 4 +- cloudinit/net/dhcp.py | 3 +- cloudinit/sources/helpers/azure.py | 4 +- cloudinit/temp_utils.py | 93 ++++++++++++++++++++++ cloudinit/util.py | 36 +-------- packages/bddeb | 5 +- .../unittests/test_datasource/test_azure_helper.py | 4 +- tests/unittests/test_net.py | 3 +- 10 files changed, 112 insertions(+), 46 deletions(-) create mode 100644 cloudinit/temp_utils.py diff --git a/cloudinit/config/cc_bootcmd.py b/cloudinit/config/cc_bootcmd.py index 604f93b0..9c0476af 100644 --- a/cloudinit/config/cc_bootcmd.py +++ b/cloudinit/config/cc_bootcmd.py @@ -37,6 +37,7 @@ specified either as lists or strings. For invocation details, see ``runcmd``. import os from cloudinit.settings import PER_ALWAYS +from cloudinit import temp_utils from cloudinit import util frequency = PER_ALWAYS @@ -49,7 +50,7 @@ def handle(name, cfg, cloud, log, _args): " no 'bootcmd' key in configuration"), name) return - with util.ExtendedTemporaryFile(suffix=".sh") as tmpf: + with temp_utils.ExtendedTemporaryFile(suffix=".sh") as tmpf: try: content = util.shellify(cfg["bootcmd"]) tmpf.write(util.encode_text(content)) diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py index 02c70b10..c192dd32 100644 --- a/cloudinit/config/cc_chef.py +++ b/cloudinit/config/cc_chef.py @@ -71,6 +71,7 @@ import itertools import json import os +from cloudinit import temp_utils from cloudinit import templater from cloudinit import url_helper from cloudinit import util @@ -303,7 +304,7 @@ def install_chef(cloud, chef_cfg, log): "omnibus_url_retries", default=OMNIBUS_URL_RETRIES)) content = url_helper.readurl(url=url, retries=retries).contents - with util.tempdir() as tmpd: + with temp_utils.tempdir() as tmpd: # Use tmpdir over tmpfile to avoid 'text file busy' on execute tmpf = "%s/chef-omnibus-install" % tmpd util.write_file(tmpf, content, mode=0o700) diff --git a/cloudinit/config/cc_snappy.py b/cloudinit/config/cc_snappy.py index a9682f19..eecb8178 100644 --- a/cloudinit/config/cc_snappy.py +++ b/cloudinit/config/cc_snappy.py @@ -63,11 +63,11 @@ is ``auto``. Options are: from cloudinit import log as logging from cloudinit.settings import PER_INSTANCE +from cloudinit import temp_utils from cloudinit import util import glob import os -import tempfile LOG = logging.getLogger(__name__) @@ -183,7 +183,7 @@ def render_snap_op(op, name, path=None, cfgfile=None, config=None): # config # Note, however, we do not touch config files on disk. nested_cfg = {'config': {shortname: config}} - (fd, cfg_tmpf) = tempfile.mkstemp() + (fd, cfg_tmpf) = temp_utils.mkstemp() os.write(fd, util.yaml_dumps(nested_cfg).encode()) os.close(fd) cfgfile = cfg_tmpf diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index c7febc57..c842c839 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -9,6 +9,7 @@ import os import re from cloudinit.net import find_fallback_nic, get_devicelist +from cloudinit import temp_utils from cloudinit import util LOG = logging.getLogger(__name__) @@ -47,7 +48,7 @@ def maybe_perform_dhcp_discovery(nic=None): if not dhclient_path: LOG.debug('Skip dhclient configuration: No dhclient command found.') return {} - with util.tempdir(prefix='cloud-init-dhcp-') as tmpdir: + with temp_utils.tempdir(prefix='cloud-init-dhcp-') as tmpdir: return dhcp_discovery(dhclient_path, nic, tmpdir) diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index e22409d1..28ed0ae2 100644 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -6,10 +6,10 @@ import os import re import socket import struct -import tempfile import time from cloudinit import stages +from cloudinit import temp_utils from contextlib import contextmanager from xml.etree import ElementTree @@ -111,7 +111,7 @@ class OpenSSLManager(object): } def __init__(self): - self.tmpdir = tempfile.mkdtemp() + self.tmpdir = temp_utils.mkdtemp() self.certificate = None self.generate_certificate() diff --git a/cloudinit/temp_utils.py b/cloudinit/temp_utils.py new file mode 100644 index 00000000..0355f19d --- /dev/null +++ b/cloudinit/temp_utils.py @@ -0,0 +1,93 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +import contextlib +import errno +import os +import shutil +import tempfile + +_TMPDIR = None +_ROOT_TMPDIR = "/run/cloud-init/tmp" + + +def _tempfile_dir_arg(odir=None): + """Return the proper 'dir' argument for tempfile functions. + + When root, cloud-init will use /run/cloud-init/tmp to avoid + any cleaning that a distro boot might do on /tmp (such as + systemd-tmpfiles-clean). + + If the caller of this function (mkdtemp or mkstemp) was provided + with a 'dir' argument, then that is respected. + + @param odir: original 'dir' arg to 'mkdtemp' or other.""" + + if odir is not None: + return odir + + global _TMPDIR + if _TMPDIR: + return _TMPDIR + + if os.getuid() == 0: + tdir = _ROOT_TMPDIR + else: + tdir = os.environ.get('TMPDIR', '/tmp') + if not os.path.isdir(tdir): + os.makedirs(tdir) + os.chmod(tdir, 0o1777) + + _TMPDIR = tdir + return tdir + + +def ExtendedTemporaryFile(**kwargs): + kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + fh = tempfile.NamedTemporaryFile(**kwargs) + # Replace its unlink with a quiet version + # that does not raise errors when the + # file to unlink has been unlinked elsewhere.. + + def _unlink_if_exists(path): + try: + os.unlink(path) + except OSError as e: + if e.errno != errno.ENOENT: + raise e + + fh.unlink = _unlink_if_exists + + # Add a new method that will unlink + # right 'now' but still lets the exit + # method attempt to remove it (which will + # not throw due to our del file being quiet + # about files that are not there) + def unlink_now(): + fh.unlink(fh.name) + + setattr(fh, 'unlink_now', unlink_now) + return fh + + +@contextlib.contextmanager +def tempdir(**kwargs): + # This seems like it was only added in python 3.2 + # Make it since its useful... + # See: http://bugs.python.org/file12970/tempdir.patch + tdir = mkdtemp(**kwargs) + try: + yield tdir + finally: + shutil.rmtree(tdir) + + +def mkdtemp(**kwargs): + kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + return tempfile.mkdtemp(**kwargs) + + +def mkstemp(**kwargs): + kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + return tempfile.mkstemp(**kwargs) + +# vi: ts=4 expandtab diff --git a/cloudinit/util.py b/cloudinit/util.py index 609e94c8..ae5cda8d 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -30,7 +30,6 @@ import stat import string import subprocess import sys -import tempfile import time from errno import ENOENT, ENOEXEC @@ -45,6 +44,7 @@ from cloudinit import importer from cloudinit import log as logging from cloudinit import mergers from cloudinit import safeyaml +from cloudinit import temp_utils from cloudinit import type_utils from cloudinit import url_helper from cloudinit import version @@ -349,26 +349,6 @@ class DecompressionError(Exception): pass -def ExtendedTemporaryFile(**kwargs): - fh = tempfile.NamedTemporaryFile(**kwargs) - # Replace its unlink with a quiet version - # that does not raise errors when the - # file to unlink has been unlinked elsewhere.. - LOG.debug("Created temporary file %s", fh.name) - fh.unlink = del_file - - # Add a new method that will unlink - # right 'now' but still lets the exit - # method attempt to remove it (which will - # not throw due to our del file being quiet - # about files that are not there) - def unlink_now(): - fh.unlink(fh.name) - - setattr(fh, 'unlink_now', unlink_now) - return fh - - def fork_cb(child_cb, *args, **kwargs): fid = os.fork() if fid == 0: @@ -790,18 +770,6 @@ def umask(n_msk): os.umask(old) -@contextlib.contextmanager -def tempdir(**kwargs): - # This seems like it was only added in python 3.2 - # Make it since its useful... - # See: http://bugs.python.org/file12970/tempdir.patch - tdir = tempfile.mkdtemp(**kwargs) - try: - yield tdir - finally: - del_dir(tdir) - - def center(text, fill, max_len): return '{0:{fill}{align}{size}}'.format(text, fill=fill, align="^", size=max_len) @@ -1587,7 +1555,7 @@ def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True): mtypes = [''] mounted = mounts() - with tempdir() as tmpd: + with temp_utils.tempdir() as tmpd: umount = False if os.path.realpath(device) in mounted: mountpoint = mounted[os.path.realpath(device)]['mountpoint'] diff --git a/packages/bddeb b/packages/bddeb index 7c123548..4f2e2ddf 100755 --- a/packages/bddeb +++ b/packages/bddeb @@ -21,8 +21,9 @@ def find_root(): if "avoid-pep8-E402-import-not-top-of-file": # Use the util functions from cloudinit sys.path.insert(0, find_root()) - from cloudinit import templater from cloudinit import util + from cloudinit import temp_utils + from cloudinit import templater DEBUILD_ARGS = ["-S", "-d"] @@ -148,7 +149,7 @@ def main(): capture = False templ_data = {'debian_release': args.release} - with util.tempdir() as tdir: + with temp_utils.tempdir() as tdir: # output like 0.7.6-1022-g36e92d3 ver_data = read_version() diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py index 80ce003d..44b99eca 100644 --- a/tests/unittests/test_datasource/test_azure_helper.py +++ b/tests/unittests/test_datasource/test_azure_helper.py @@ -275,7 +275,7 @@ class TestOpenSSLManager(TestCase): mock.patch('builtins.open')) @mock.patch.object(azure_helper, 'cd', mock.MagicMock()) - @mock.patch.object(azure_helper.tempfile, 'mkdtemp') + @mock.patch.object(azure_helper.temp_utils, 'mkdtemp') def test_openssl_manager_creates_a_tmpdir(self, mkdtemp): manager = azure_helper.OpenSSLManager() self.assertEqual(mkdtemp.return_value, manager.tmpdir) @@ -292,7 +292,7 @@ class TestOpenSSLManager(TestCase): manager.clean_up() @mock.patch.object(azure_helper, 'cd', mock.MagicMock()) - @mock.patch.object(azure_helper.tempfile, 'mkdtemp', mock.MagicMock()) + @mock.patch.object(azure_helper.temp_utils, 'mkdtemp', mock.MagicMock()) @mock.patch.object(azure_helper.util, 'del_dir') def test_clean_up(self, del_dir): manager = azure_helper.OpenSSLManager() diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index c10ef905..f2496151 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -9,6 +9,7 @@ from cloudinit.net import network_state from cloudinit.net import renderers from cloudinit.net import sysconfig from cloudinit.sources.helpers import openstack +from cloudinit import temp_utils from cloudinit import util from cloudinit.tests.helpers import CiTestCase @@ -2150,7 +2151,7 @@ class TestCmdlineConfigParsing(CiTestCase): static['mac_address'] = macs['eth1'] expected = {'version': 1, 'config': [dhcp, static]} - with util.tempdir() as tmpd: + with temp_utils.tempdir() as tmpd: for fname, content in pairs: fp = os.path.join(tmpd, fname) files.append(fp) -- cgit v1.2.3