Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Leap:15.5:Update
salt.27393
make-pass-renderer-configurable-other-fixes-532...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File make-pass-renderer-configurable-other-fixes-532.patch of Package salt.27393
From 7b4f5007b7e6a35386d197afe53d02c8d7b41d53 Mon Sep 17 00:00:00 2001 From: Daniel Mach <daniel.mach@gmail.com> Date: Thu, 6 Oct 2022 11:58:23 +0200 Subject: [PATCH] Make pass renderer configurable & other fixes (#532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * pass: Use a secure way of handling pass arguments The original code would fail on pass paths with spaces, because they would be split into multiple arguments. * pass: Strip only trailing newline characters from the secret * pass: Do not modify $HOME env globally Just set $HOME for calling the pass binary to avoid affecting anything outside the pass renderer. * pass: Use pass executable path from _get_pass_exec() * Make the pass renderer more configurable 1. Allow us to make the pass renderer fail during pillar rendering when a secret corresponding with a pass path cannot be fetched. For this we add a master config variable pass_strict_fetch. 2. Allow to have prefix for variables that should be processed with the pass renderer. For this we add a master config variable pass_variable_prefix. 3. Allow us to configure pass' GNUPGHOME and PASSWORD_STORE_DIR environmental variables. For this we add master config variables pass_gnupghome and pass_dir. * Add tests for the pass renderer * pass: Handle FileNotFoundError when pass binary is not available Co-authored-by: Marcus Rückert <darix@nordisch.org> --- changelog/62120.added | 4 + changelog/62120.fixed | 4 + salt/config/__init__.py | 12 ++ salt/renderers/pass.py | 104 ++++++++++++-- tests/pytests/unit/renderers/test_pass.py | 164 ++++++++++++++++++++++ 5 files changed, 274 insertions(+), 14 deletions(-) create mode 100644 changelog/62120.added create mode 100644 changelog/62120.fixed create mode 100644 tests/pytests/unit/renderers/test_pass.py diff --git a/changelog/62120.added b/changelog/62120.added new file mode 100644 index 0000000000..4303d124f0 --- /dev/null +++ b/changelog/62120.added @@ -0,0 +1,4 @@ +Config option pass_variable_prefix allows to distinguish variables that contain paths to pass secrets. +Config option pass_strict_fetch allows to error out when a secret cannot be fetched from pass. +Config option pass_dir allows setting the PASSWORD_STORE_DIR env for pass. +Config option pass_gnupghome allows setting the $GNUPGHOME env for pass. diff --git a/changelog/62120.fixed b/changelog/62120.fixed new file mode 100644 index 0000000000..22a9711383 --- /dev/null +++ b/changelog/62120.fixed @@ -0,0 +1,4 @@ +Pass executable path from _get_path_exec() is used when calling the program. +The $HOME env is no longer modified globally. +Only trailing newlines are stripped from the fetched secret. +Pass process arguments are handled in a secure way. diff --git a/salt/config/__init__.py b/salt/config/__init__.py index 2c42290598..9e72a5b4b7 100644 --- a/salt/config/__init__.py +++ b/salt/config/__init__.py @@ -960,6 +960,14 @@ VALID_OPTS = immutabletypes.freeze( # Use Adler32 hashing algorithm for server_id (default False until Sodium, "adler32" after) # Possible values are: False, adler32, crc32 "server_id_use_crc": (bool, str), + # pass renderer: Fetch secrets only for the template variables matching the prefix + "pass_variable_prefix": str, + # pass renderer: Whether to error out when unable to fetch a secret + "pass_strict_fetch": bool, + # pass renderer: Set GNUPGHOME env for Pass + "pass_gnupghome": str, + # pass renderer: Set PASSWORD_STORE_DIR env for Pass + "pass_dir": str, } ) @@ -1601,6 +1609,10 @@ DEFAULT_MASTER_OPTS = immutabletypes.freeze( "fips_mode": False, "detect_remote_minions": False, "remote_minions_port": 22, + "pass_variable_prefix": "", + "pass_strict_fetch": False, + "pass_gnupghome": "", + "pass_dir": "", } ) diff --git a/salt/renderers/pass.py b/salt/renderers/pass.py index 71b1021b96..ba0f152c23 100644 --- a/salt/renderers/pass.py +++ b/salt/renderers/pass.py @@ -45,6 +45,34 @@ Install pass binary pass: pkg.installed + +Salt master configuration options + +.. code-block:: yaml + + # If the prefix is *not* set (default behavior), all template variables are + # considered for fetching secrets from Pass. Those that cannot be resolved + # to a secret are passed through. + # + # If the prefix is set, only the template variables with matching prefix are + # considered for fetching the secrets, other variables are passed through. + # + # For ease of use it is recommended to set the following options as well: + # renderer: 'jinja|yaml|pass' + # pass_strict_fetch: true + # + pass_variable_prefix: 'pass:' + + # If set to 'true', error out when unable to fetch a secret for a template variable. + pass_strict_fetch: true + + # Set GNUPGHOME env for Pass. + # Defaults to: ~/.gnupg + pass_gnupghome: <path> + + # Set PASSWORD_STORE_DIR env for Pass. + # Defaults to: ~/.password-store + pass_dir: <path> """ @@ -54,7 +82,7 @@ from os.path import expanduser from subprocess import PIPE, Popen import salt.utils.path -from salt.exceptions import SaltRenderError +from salt.exceptions import SaltConfigurationError, SaltRenderError log = logging.getLogger(__name__) @@ -75,18 +103,71 @@ def _fetch_secret(pass_path): Fetch secret from pass based on pass_path. If there is any error, return back the original pass_path value """ - cmd = "pass show {}".format(pass_path.strip()) - log.debug("Fetching secret: %s", cmd) + pass_exec = _get_pass_exec() + + # Make a backup in case we want to return the original value without stripped whitespaces + original_pass_path = pass_path + + # Remove the optional prefix from pass path + pass_prefix = __opts__["pass_variable_prefix"] + if pass_prefix: + # If we do not see our prefix we do not want to process this variable + # and we return the unmodified pass path + if not pass_path.startswith(pass_prefix): + return pass_path + + # strip the prefix from the start of the string + pass_path = pass_path[len(pass_prefix) :] + + # The pass_strict_fetch option must be used with pass_variable_prefix + pass_strict_fetch = __opts__["pass_strict_fetch"] + if pass_strict_fetch and not pass_prefix: + msg = "The 'pass_strict_fetch' option requires 'pass_variable_prefix' option enabled" + raise SaltConfigurationError(msg) + + # Remove whitespaces from the pass_path + pass_path = pass_path.strip() - proc = Popen(cmd.split(" "), stdout=PIPE, stderr=PIPE) - pass_data, pass_error = proc.communicate() + cmd = [pass_exec, "show", pass_path] + log.debug("Fetching secret: %s", " ".join(cmd)) + + # Make sure environment variable HOME is set, since Pass looks for the + # password-store under ~/.password-store. + env = os.environ.copy() + env["HOME"] = expanduser("~") + + pass_dir = __opts__["pass_dir"] + if pass_dir: + env["PASSWORD_STORE_DIR"] = pass_dir + + pass_gnupghome = __opts__["pass_gnupghome"] + if pass_gnupghome: + env["GNUPGHOME"] = pass_gnupghome + + try: + proc = Popen(cmd, stdout=PIPE, stderr=PIPE, env=env) + pass_data, pass_error = proc.communicate() + pass_returncode = proc.returncode + except OSError as e: + pass_data, pass_error = "", str(e) + pass_returncode = 1 # The version of pass used during development sent output to # stdout instead of stderr even though its returncode was non zero. - if proc.returncode or not pass_data: - log.warning("Could not fetch secret: %s %s", pass_data, pass_error) - pass_data = pass_path - return pass_data.strip() + if pass_returncode or not pass_data: + try: + pass_error = pass_error.decode("utf-8") + except (AttributeError, ValueError): + pass + msg = "Could not fetch secret '{}' from the password store: {}".format( + pass_path, pass_error + ) + if pass_strict_fetch: + raise SaltRenderError(msg) + else: + log.warning(msg) + return original_pass_path + return pass_data.rstrip("\r\n") def _decrypt_object(obj): @@ -108,9 +189,4 @@ def render(pass_info, saltenv="base", sls="", argline="", **kwargs): """ Fetch secret from pass based on pass_path """ - _get_pass_exec() - - # Make sure environment variable HOME is set, since Pass looks for the - # password-store under ~/.password-store. - os.environ["HOME"] = expanduser("~") return _decrypt_object(pass_info) diff --git a/tests/pytests/unit/renderers/test_pass.py b/tests/pytests/unit/renderers/test_pass.py new file mode 100644 index 0000000000..74e822c7ec --- /dev/null +++ b/tests/pytests/unit/renderers/test_pass.py @@ -0,0 +1,164 @@ +import importlib + +import pytest + +import salt.config +import salt.exceptions +from tests.support.mock import MagicMock, patch + +# "pass" is a reserved keyword, we need to import it differently +pass_ = importlib.import_module("salt.renderers.pass") + + +@pytest.fixture +def configure_loader_modules(): + return { + pass_: { + "__opts__": salt.config.DEFAULT_MASTER_OPTS.copy(), + "_get_pass_exec": MagicMock(return_value="/usr/bin/pass"), + } + } + + +# The default behavior is that if fetching a secret from pass fails, +# the value is passed through. Even the trailing newlines are preserved. +def test_passthrough(): + pass_path = "secret\n" + expected = pass_path + result = pass_.render(pass_path) + + assert result == expected + + +# Fetch a secret in the strict mode. +def test_strict_fetch(): + config = { + "pass_variable_prefix": "pass:", + "pass_strict_fetch": True, + } + + popen_mock = MagicMock(spec=pass_.Popen) + popen_mock.return_value.communicate.return_value = ("password123456\n", "") + popen_mock.return_value.returncode = 0 + + mocks = { + "Popen": popen_mock, + } + + pass_path = "pass:secret" + expected = "password123456" + with patch.dict(pass_.__opts__, config), patch.dict(pass_.__dict__, mocks): + result = pass_.render(pass_path) + + assert result == expected + + +# Fail to fetch a secret in the strict mode. +def test_strict_fetch_fail(): + config = { + "pass_variable_prefix": "pass:", + "pass_strict_fetch": True, + } + + popen_mock = MagicMock(spec=pass_.Popen) + popen_mock.return_value.communicate.return_value = ("", "Secret not found") + popen_mock.return_value.returncode = 1 + + mocks = { + "Popen": popen_mock, + } + + pass_path = "pass:secret" + with patch.dict(pass_.__opts__, config), patch.dict(pass_.__dict__, mocks): + with pytest.raises(salt.exceptions.SaltRenderError): + pass_.render(pass_path) + + +# Passthrough a value that doesn't have a pass prefix. +def test_strict_fetch_passthrough(): + config = { + "pass_variable_prefix": "pass:", + "pass_strict_fetch": True, + } + + pass_path = "variable-without-pass-prefix\n" + expected = pass_path + with patch.dict(pass_.__opts__, config): + result = pass_.render(pass_path) + + assert result == expected + + +# Fetch a secret in the strict mode. The pass path contains spaces. +def test_strict_fetch_pass_path_with_spaces(): + config = { + "pass_variable_prefix": "pass:", + "pass_strict_fetch": True, + } + + popen_mock = MagicMock(spec=pass_.Popen) + popen_mock.return_value.communicate.return_value = ("password123456\n", "") + popen_mock.return_value.returncode = 0 + + mocks = { + "Popen": popen_mock, + } + + pass_path = "pass:se cr et" + with patch.dict(pass_.__opts__, config), patch.dict(pass_.__dict__, mocks): + pass_.render(pass_path) + + call_args, call_kwargs = popen_mock.call_args_list[0] + assert call_args[0] == ["/usr/bin/pass", "show", "se cr et"] + + +# Fetch a secret in the strict mode. The secret contains leading and trailing whitespaces. +def test_strict_fetch_secret_with_whitespaces(): + config = { + "pass_variable_prefix": "pass:", + "pass_strict_fetch": True, + } + + popen_mock = MagicMock(spec=pass_.Popen) + popen_mock.return_value.communicate.return_value = (" \tpassword123456\t \r\n", "") + popen_mock.return_value.returncode = 0 + + mocks = { + "Popen": popen_mock, + } + + pass_path = "pass:secret" + expected = " \tpassword123456\t " # only the trailing newlines get striped + with patch.dict(pass_.__opts__, config), patch.dict(pass_.__dict__, mocks): + result = pass_.render(pass_path) + + assert result == expected + + +# Test setting env variables based on config values: +# - pass_gnupghome -> GNUPGHOME +# - pass_dir -> PASSWORD_STORE_DIR +def test_env(): + config = { + "pass_variable_prefix": "pass:", + "pass_strict_fetch": True, + "pass_gnupghome": "/path/to/gnupghome", + "pass_dir": "/path/to/secretstore", + } + + popen_mock = MagicMock(spec=pass_.Popen) + popen_mock.return_value.communicate.return_value = ("password123456\n", "") + popen_mock.return_value.returncode = 0 + + mocks = { + "Popen": popen_mock, + } + + pass_path = "pass:secret" + expected = " \tpassword123456\t " # only the trailing newlines get striped + with patch.dict(pass_.__opts__, config), patch.dict(pass_.__dict__, mocks): + result = pass_.render(pass_path) + + call_args, call_kwargs = popen_mock.call_args_list[0] + assert call_kwargs["env"]["GNUPGHOME"] == config["pass_gnupghome"] + assert call_kwargs["env"]["PASSWORD_STORE_DIR"] == config["pass_dir"] -- 2.37.3
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
