From ba941b08016e9931eeaef5063a0fdc879d85f234 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 23 Sep 2017 19:04:46 +0200 Subject: [PATCH 1/6] refactor/fix subprocess env preparation refactor: make a generally usable function fix: remove support code for ancient pyinstaller the "else" branch was needed for pyinstaller < 20160820 because it did not have the LD_LIBRARY_PATH_ORIG env var, so we just killed LDLP because we had no better way. but with borg tests running under fakeroot, this is troublesome as fakeroot uses this also and can't find its library without it. so, just remove it, we do not need to support old pyinstaller. --- src/borg/helpers/process.py | 28 ++++++++++++++++++++++++++++ src/borg/remote.py | 16 ++++------------ 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/borg/helpers/process.py b/src/borg/helpers/process.py index 14209c5b5..59b454b2b 100644 --- a/src/borg/helpers/process.py +++ b/src/borg/helpers/process.py @@ -6,6 +6,8 @@ import signal import subprocess import sys +from .. import __version__ + from ..logger import create_logger logger = create_logger() @@ -115,3 +117,29 @@ def popen_with_error_handling(cmd_line: str, log_prefix='', **kwargs): def is_terminal(fd=sys.stdout): return hasattr(fd, 'isatty') and fd.isatty() and (sys.platform != 'win32' or 'ANSICON' in os.environ) + + +def prepare_subprocess_env(system, env=None): + """ + Prepare the environment for a subprocess we are going to create. + + :param system: True for preparing to invoke system-installed binaries, + False for stuff inside the pyinstaller environment (like borg, python). + :param env: optionally give a environment dict here. if not given, default to os.environ. + :return: a modified copy of the environment + """ + env = dict(env if env is not None else os.environ) + if system: + # a pyinstaller binary's bootloader modifies LD_LIBRARY_PATH=/tmp/_ME..., + # but we do not want that system binaries (like ssh or other) pick up + # (non-matching) libraries from there. + # thus we install the original LDLP, before pyinstaller has modified it: + lp_key = 'LD_LIBRARY_PATH' + lp_orig = env.get(lp_key + '_ORIG') # pyinstaller >= 20160820 has this + if lp_orig is not None: + env[lp_key] = lp_orig + # security: do not give secrets to subprocess + env.pop('BORG_PASSPHRASE', None) + # for information, give borg version to the subprocess + env['BORG_VERSION'] = __version__ + return env diff --git a/src/borg/remote.py b/src/borg/remote.py index 277f388c6..e02eca0dc 100644 --- a/src/borg/remote.py +++ b/src/borg/remote.py @@ -29,6 +29,7 @@ from .helpers import replace_placeholders from .helpers import sysinfo from .helpers import format_file_size from .helpers import truncate_and_unlink +from .helpers import prepare_subprocess_env from .logger import create_logger, setup_logging from .repository import Repository from .version import parse_version, format_version @@ -530,21 +531,12 @@ class RemoteRepository: self.server_version = parse_version('1.0.8') # fallback version if server is too old to send version information self.p = None testing = location.host == '__testsuite__' + # when testing, we invoke and talk to a borg process directly (no ssh). + # when not testing, we invoke the system-installed ssh binary to talk to a remote borg. + env = prepare_subprocess_env(system=not testing) borg_cmd = self.borg_cmd(args, testing) - env = dict(os.environ) if not testing: borg_cmd = self.ssh_cmd(location) + borg_cmd - # pyinstaller binary modifies LD_LIBRARY_PATH=/tmp/_ME... but we do not want - # that the system's ssh binary picks up (non-matching) libraries from there. - # thus we install the original LDLP, before pyinstaller has modified it: - lp_key = 'LD_LIBRARY_PATH' - lp_orig = env.get(lp_key + '_ORIG') # pyinstaller >= 20160820 has this - if lp_orig is not None: - env[lp_key] = lp_orig - else: - env.pop(lp_key, None) - env.pop('BORG_PASSPHRASE', None) # security: do not give secrets to subprocess - env['BORG_VERSION'] = __version__ logger.debug('SSH command line: %s', borg_cmd) self.p = Popen(borg_cmd, bufsize=0, stdin=PIPE, stdout=PIPE, stderr=PIPE, env=env) self.stdin_fd = self.p.stdin.fileno() From 6a6fd318045ceea254871de8042944875781bb7f Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 23 Sep 2017 20:04:47 +0200 Subject: [PATCH 2/6] use prepared env for calling BORG_PASSCOMMAND, fixes #3050 --- src/borg/crypto/key.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index 25aae6cf7..7fc1a5a7a 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -23,6 +23,7 @@ from ..helpers import yes from ..helpers import get_keys_dir, get_security_dir from ..helpers import get_limited_unpacker from ..helpers import bin_to_hex +from ..helpers import prepare_subprocess_env from ..item import Key, EncryptedKey from ..platform import SaveFile @@ -425,8 +426,10 @@ class Passphrase(str): def env_passcommand(cls, default=None): passcommand = os.environ.get('BORG_PASSCOMMAND', None) if passcommand is not None: + # passcommand is a system command (not inside pyinstaller env) + env = prepare_subprocess_env(system=True) try: - passphrase = subprocess.check_output(shlex.split(passcommand), universal_newlines=True) + passphrase = subprocess.check_output(shlex.split(passcommand), universal_newlines=True, env=env) except (subprocess.CalledProcessError, FileNotFoundError) as e: raise PasscommandFailure(e) return cls(passphrase.rstrip('\n')) From 6da5bf4b850fe1e197e8e58b46e9a6a9ca4e4770 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 23 Sep 2017 22:35:10 +0200 Subject: [PATCH 3/6] use prepared env for borg with-lock --- src/borg/archiver.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index 2dd290875..ce1b2b9c7 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -64,7 +64,7 @@ from .helpers import ProgressIndicatorPercent from .helpers import basic_json_data, json_print from .helpers import replace_placeholders from .helpers import ChunkIteratorFileWrapper -from .helpers import popen_with_error_handling +from .helpers import popen_with_error_handling, prepare_subprocess_env from .helpers import dash_open from .helpers import umount from .nanorst import rst_to_terminal @@ -1480,9 +1480,10 @@ class Archiver: # we can only do this for local repositories (with .io), though: if hasattr(repository, 'io'): repository.io.close_segment() + env = prepare_subprocess_env(system=True) try: # we exit with the return code we get from the subprocess - return subprocess.call([args.command] + args.args) + return subprocess.call([args.command] + args.args, env=env) finally: # we need to commit the "no change" operation we did to the manifest # because it created a new segment file in the repository. if we would From b88da1064155f159163a78cfbafa1adc0eb5ec3a Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 23 Sep 2017 22:38:26 +0200 Subject: [PATCH 4/6] use prepared env for borg umount --- src/borg/helpers/fs.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/borg/helpers/fs.py b/src/borg/helpers/fs.py index be05d0faa..783c14d7a 100644 --- a/src/borg/helpers/fs.py +++ b/src/borg/helpers/fs.py @@ -6,6 +6,8 @@ import subprocess import sys import textwrap +from .process import prepare_subprocess_env + from ..constants import * # NOQA @@ -154,7 +156,8 @@ def dash_open(path, mode): def umount(mountpoint): + env = prepare_subprocess_env(system=True) try: - return subprocess.call(['fusermount', '-u', mountpoint]) + return subprocess.call(['fusermount', '-u', mountpoint], env=env) except FileNotFoundError: - return subprocess.call(['umount', mountpoint]) + return subprocess.call(['umount', mountpoint], env=env) From cf59f653e5821ddee7af2c4d4fc8806744fab017 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 23 Sep 2017 22:54:52 +0200 Subject: [PATCH 5/6] use prepared env for borg export-tar --tar-filter subprocess --- src/borg/archiver.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/borg/archiver.py b/src/borg/archiver.py index ce1b2b9c7..394d802b7 100644 --- a/src/borg/archiver.py +++ b/src/borg/archiver.py @@ -771,10 +771,12 @@ class Archiver: # The decision whether to close that or not remains the same. filterout = tarstream filterout_close = tarstream_close + env = prepare_subprocess_env(system=True) # There is no deadlock potential here (the subprocess docs warn about this), because # communication with the process is a one-way road, i.e. the process can never block # for us to do something while we block on the process for something different. - filterproc = popen_with_error_handling(filter, stdin=subprocess.PIPE, stdout=filterout, log_prefix='--tar-filter: ') + filterproc = popen_with_error_handling(filter, stdin=subprocess.PIPE, stdout=filterout, + log_prefix='--tar-filter: ', env=env) if not filterproc: return EXIT_ERROR # Always close the pipe, otherwise the filter process would not notice when we are done. From a57e23fdb35096c40b4849849c3752ae83090f06 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 23 Sep 2017 23:27:14 +0200 Subject: [PATCH 6/6] use prepared env for xattr module's fakeroot version check --- src/borg/xattr.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/borg/xattr.py b/src/borg/xattr.py index 03c5eae26..c1f331465 100644 --- a/src/borg/xattr.py +++ b/src/borg/xattr.py @@ -10,7 +10,7 @@ from ctypes import CDLL, create_string_buffer, c_ssize_t, c_size_t, c_char_p, c_ from ctypes.util import find_library from distutils.version import LooseVersion -from .helpers import Buffer +from .helpers import Buffer, prepare_subprocess_env try: @@ -88,7 +88,9 @@ if sys.platform.startswith('linux'): preloads = re.split("[ :]", LD_PRELOAD) for preload in preloads: if preload.startswith("libfakeroot"): - fakeroot_version = LooseVersion(subprocess.check_output(['fakeroot', '-v']).decode('ascii').split()[-1]) + env = prepare_subprocess_env(system=True) + fakeroot_output = subprocess.check_output(['fakeroot', '-v'], env=env) + fakeroot_version = LooseVersion(fakeroot_output.decode('ascii').split()[-1]) if fakeroot_version >= LooseVersion("1.20.2"): # 1.20.2 has been confirmed to have xattr support # 1.18.2 has been confirmed not to have xattr support