diff --git a/changelog/99.fixed.md b/changelog/99.fixed.md new file mode 100644 index 0000000..a7f78e9 --- /dev/null +++ b/changelog/99.fixed.md @@ -0,0 +1 @@ +Fixed compatibility with master cluster mode diff --git a/src/saltext/vault/runners/vault.py b/src/saltext/vault/runners/vault.py index ffd3a92..5137e31 100644 --- a/src/saltext/vault/runners/vault.py +++ b/src/saltext/vault/runners/vault.py @@ -10,6 +10,7 @@ import logging import os from collections.abc import Mapping +from pathlib import Path import salt.cache import salt.crypt @@ -916,15 +917,18 @@ def _validate_signature(minion_id, signature, impersonated_by_master): Validate that either minion with id minion_id, or the master, signed the request """ - pki_dir = __opts__["pki_dir"] + if not impersonated_by_master and __opts__.get("cluster_id") is not None: + pki_dir = Path(__opts__["cluster_pki_dir"]) + else: + pki_dir = Path(__opts__["pki_dir"]) if impersonated_by_master: - public_key = f"{pki_dir}/master.pub" + public_key = pki_dir / "master.pub" else: - public_key = f"{pki_dir}/minions/{minion_id}" + public_key = pki_dir / "minions" / minion_id log.trace("Validating signature for %s", minion_id) signature = base64.b64decode(signature) - if not salt.crypt.verify_signature(public_key, minion_id, signature): + if not salt.crypt.verify_signature(str(public_key), minion_id, signature): raise salt.exceptions.AuthenticationError( f"Could not validate token request from {minion_id}" ) diff --git a/tests/integration/runners/conftest.py b/tests/integration/runners/conftest.py new file mode 100644 index 0000000..0a963d9 --- /dev/null +++ b/tests/integration/runners/conftest.py @@ -0,0 +1,13 @@ +import pytest + +from tests.support.vault import vault_delete_secret +from tests.support.vault import vault_write_secret + + +@pytest.fixture(scope="class") +def vault_testing_values(vault_container_version): # pylint: disable=unused-argument + vault_write_secret("secret/path/foo", success="yeehaaw") + try: + yield + finally: + vault_delete_secret("secret/path/foo") diff --git a/tests/integration/runners/test_vault.py b/tests/integration/runners/test_vault.py index dc20506..0912f5c 100644 --- a/tests/integration/runners/test_vault.py +++ b/tests/integration/runners/test_vault.py @@ -20,7 +20,6 @@ log = logging.getLogger(__name__) - pytestmark = [ pytest.mark.slow_test, pytest.mark.skip_if_binaries_missing("vault", "getent"), @@ -526,15 +525,6 @@ def vault_pillar_values_approle(vault_salt_minion): vault_delete_secret("salt/roles/foo") -@pytest.fixture(scope="class") -def vault_testing_values(vault_container_version): # pylint: disable=unused-argument - vault_write_secret("secret/path/foo", success="yeehaaw") - try: - yield - finally: - vault_delete_secret("secret/path/foo") - - @pytest.fixture def minion_conn_cachedir(vault_salt_call_cli): ret = vault_salt_call_cli.run("config.get", "cachedir") diff --git a/tests/integration/runners/test_vault_master_cluster.py b/tests/integration/runners/test_vault_master_cluster.py new file mode 100644 index 0000000..a5e5726 --- /dev/null +++ b/tests/integration/runners/test_vault_master_cluster.py @@ -0,0 +1,169 @@ +import logging +import subprocess + +import pytest +import salt.utils.platform +import salt.version + +pytest.importorskip("docker") + +log = logging.getLogger(__name__) +salt_version = int(salt.version.__version__.split(".")[0]) + +pytestmark = [ + pytest.mark.slow_test, + pytest.mark.skip_if_binaries_missing("vault", "getent"), + pytest.mark.skip_if(salt_version < 3007, reason="Master cluster requires Salt 3007+"), + pytest.mark.usefixtures("vault_container_version", "vault_testing_values"), + pytest.mark.parametrize("vault_container_version", ("latest",), indirect=True), +] + + +@pytest.fixture(scope="module") +def vault_master_config(vault_port): + return { + "open_mode": True, + "ext_pillar": [{"vault": "secret/path/foo"}], + "peer_run": { + ".*": [ + "vault.get_config", + "vault.generate_new_token", + ], + }, + "vault": { + "auth": {"token": "testsecret"}, + "cache": { + "backend": "file", + }, + "issue": { + "type": "token", + "token": { + "params": { + "num_uses": 0, + } + }, + }, + "policies": { + "assign": [ + "salt_minion", + "salt_minion_{minion}", + "salt_role_{pillar[roles]}", + ], + "cache_time": 0, + }, + "server": { + "url": f"http://127.0.0.1:{vault_port}", + }, + }, + "minion_data_cache": True, + } + + +@pytest.fixture(scope="module") +def cluster_shared_path(tmp_path_factory): + return tmp_path_factory.mktemp("cluster") + + +@pytest.fixture(scope="module") +def cluster_pki_path(cluster_shared_path): + path = cluster_shared_path / "pki" + path.mkdir() + (path / "peers").mkdir() + return path + + +@pytest.fixture(scope="module") +def cluster_cache_path(cluster_shared_path): + path = cluster_shared_path / "cache" + path.mkdir() + return path + + +@pytest.fixture(scope="module") +def cluster_master_1(salt_factories, cluster_pki_path, cluster_cache_path, vault_master_config): + config_overrides = { + "interface": "127.0.0.1", + "cluster_id": "master_cluster", + "cluster_peers": [ + "127.0.0.2", + "127.0.0.3", + ], + "cluster_pki_dir": str(cluster_pki_path), + "cache_dir": str(cluster_cache_path), + } + factory = salt_factories.salt_master_daemon( + "127.0.0.1", + defaults=vault_master_config, + overrides=config_overrides, + extra_cli_arguments_after_first_start_failure=["--log-level=info"], + ) + with factory.started(start_timeout=120): + yield factory + + +@pytest.fixture(scope="module") +def cluster_master_2(salt_factories, cluster_master_1, vault_master_config): + if salt.utils.platform.is_darwin() or salt.utils.platform.is_freebsd(): + subprocess.check_output(["ifconfig", "lo0", "alias", "127.0.0.2", "up"]) + + config_overrides = { + "interface": "127.0.0.2", + "cluster_id": "master_cluster", + "cluster_peers": [ + "127.0.0.1", + "127.0.0.3", + ], + "cluster_pki_dir": cluster_master_1.config["cluster_pki_dir"], + "cache_dir": cluster_master_1.config["cache_dir"], + } + + # Use the same ports for both masters, they are binding to different interfaces + for key in ( + "ret_port", + "publish_port", + ): + config_overrides[key] = cluster_master_1.config[key] + factory = salt_factories.salt_master_daemon( + "127.0.0.2", + defaults=vault_master_config, + overrides=config_overrides, + extra_cli_arguments_after_first_start_failure=["--log-level=info"], + ) + with factory.started(start_timeout=120): + yield factory + + +@pytest.fixture(scope="module") +def cluster_minion_1(cluster_master_1, vault_master_config): + port = cluster_master_1.config["ret_port"] + addr = cluster_master_1.config["interface"] + config_overrides = { + "master": f"{addr}:{port}", + } + factory = cluster_master_1.salt_minion_daemon( + "cluster-minion-1", + defaults=vault_master_config, + overrides=config_overrides, + extra_cli_arguments_after_first_start_failure=["--log-level=info"], + ) + with factory.started(start_timeout=120): + yield factory + + +@pytest.fixture +def salt_call_cli(cluster_minion_1): + return cluster_minion_1.salt_call_cli(timeout=120) + + +def test_minion_can_authenticate(salt_call_cli): + ret = salt_call_cli.run("vault.read_secret", "secret/path/foo") + assert ret.returncode == 0 + assert ret.data + assert ret.data.get("success") == "yeehaaw" + + +def test_minion_pillar_is_populated_as_expected(salt_call_cli): + ret = salt_call_cli.run("pillar.items") + assert ret.returncode == 0 + assert ret.data + assert ret.data.get("success") == "yeehaww" diff --git a/tests/unit/runners/vault/test_vault.py b/tests/unit/runners/vault/test_vault.py index 41aaf5f..eecd7dc 100644 --- a/tests/unit/runners/vault/test_vault.py +++ b/tests/unit/runners/vault/test_vault.py @@ -1,3 +1,4 @@ +from pathlib import Path from unittest.mock import ANY from unittest.mock import MagicMock from unittest.mock import Mock @@ -13,10 +14,11 @@ @pytest.fixture -def configure_loader_modules(): +def configure_loader_modules(master_opts): return { vault: { "__grains__": {"id": "test-master"}, + "__opts__": master_opts, } } @@ -1484,3 +1486,24 @@ def test_revoke_token_by_accessor(client): client.post.assert_called_once_with( "auth/token/revoke-accessor", payload={"accessor": "test-accessor"} ) + + +@pytest.mark.parametrize("cluster_id", (None, "test_cluster")) +@pytest.mark.parametrize("impersonated", (False, True)) +def test_validate_signature_pki_dir(cluster_id, impersonated, master_opts): + """ + Ensure we can validate minion signatures when running in + master cluster mode. + """ + master_opts["cluster_id"] = cluster_id + base = Path(master_opts["pki_dir"]) + if not impersonated and cluster_id: + base = base.parent / "cluster" / "pki" + master_opts["cluster_pki_dir"] = str(base) + if impersonated: + expected = base / "master.pub" + else: + expected = base / "minions" / "test-minion" + with patch("salt.crypt.verify_signature", autospec=True) as verify: + vault._validate_signature("test-minion", "", impersonated) + assert verify.call_args.args[0] == str(expected)