Merge branch 'master' into add-keyvalue-create_if_missing

changelog/63278.added.md

@@ -0,0 +1 @@
+Added match runner

changelog/64226.fixed.md

@@ -0,0 +1,2 @@
+Fixed issue in mac_user.enable_auto_login that caused the user's keychain
+to be reset at each boot

doc/ref/runners/all/index.rst

@@ -30,6 +30,7 @@ runner modules
     launchd
     lxc
     manage
+    match
     mattermost
     mine
     nacl

doc/ref/runners/all/salt.runners.match.rst

@@ -0,0 +1,5 @@
+salt.runners.match
+===================
+
+.. automodule:: salt.runners.match
+    :members:

salt/modules/mac_user.py

@@ -529,10 +529,10 @@ def _kcpassword(password):
     # The magic 11 bytes - these are just repeated
     # 0x7D 0x89 0x52 0x23 0xD2 0xBC 0xDD 0xEA 0xA3 0xB9 0x1F
     key = [125, 137, 82, 35, 210, 188, 221, 234, 163, 185, 31]
-    key_len = len(key)
+    key_len = len(key) + 1  # macOS adds an extra byte for the trailing null
 
-    # Convert each character to a byte
-    password = list(map(ord, password))
+    # Convert each character to a byte and add a trailing null
+    password = list(map(ord, password)) + [0]
 
     # pad password length out to an even multiple of key length
     remainder = len(password) % key_len
@@ -554,9 +554,8 @@ def _kcpassword(password):
             password[password_index] = password[password_index] ^ key[key_index]
             key_index += 1
 
-    # Convert each byte back to a character
-    password = list(map(chr, password))
-    return b"".join(salt.utils.data.encode(password))
+    # Return the raw bytes
+    return bytes(password)
 
 
 def enable_auto_login(name, password):

salt/runners/match.py

@@ -0,0 +1,73 @@
+"""
+Run matchers from the master context.
+
+.. versionadded:: 3007.0
+"""
+import logging
+
+import salt.utils.minions
+import salt.utils.verify
+from salt.defaults import DEFAULT_TARGET_DELIM
+
+log = logging.getLogger(__name__)
+
+
+def compound_matches(expr, minion_id):
+    """
+    Check whether a minion is matched by a given compound match expression.
+    On success, this function will return the minion ID, otherwise False.
+
+    .. note::
+
+        Pillar values will be matched literally only since this function is intended
+        for remote calling. This also applies to node groups defined on the master.
+        Custom matchers are not respected.
+
+    .. note::
+
+        If a module calls this runner from a minion, you will need to explicitly
+        allow the remote call. See :conf_master:`peer_run`.
+
+    CLI Example:
+
+    .. code-block:: bash
+
+        salt-run match.compound_matches 'I@foo:bar and G@os:Deb* and not db*' myminion
+
+    expr
+        The :term:`Compound Matcher` expression to validate against.
+
+    minion_id
+        The minion ID of the minion to check the match for.
+
+    """
+    try:
+        # Ensure that if the minion data cache is disabled, we always return
+        # False. This is because the matcher will return a list of all minions
+        # in that case (assumption is greedy).
+        if not __opts__.get("minion_data_cache", True):
+            log.warning(
+                "Minion data cache is disabled. Cannot evaluate compound matcher expression."
+            )
+            return {"res": False}
+        # Ensure the passed minion ID is valid.
+        if not salt.utils.verify.valid_id(__opts__, minion_id):
+            log.warning("Got invalid minion ID.")
+            return {"res": False}
+        log.debug("Evaluating if minion '%s' is matched by '%s'.", minion_id, expr)
+        ckminions = salt.utils.minions.CkMinions(__opts__)
+        # Compound expressions are usually evaluated in greedy mode since you
+        # want to make sure the executing user has privileges to run a command on
+        # any possibly matching minion, including those with uncached data.
+        # This function has the opposite requirements, we want to make absolutely
+        # sure the minion is matched by the expression.
+        # Thus we do not include minions whose data has not been cached (greedy=False).
+        # Also, allow exact pillar matches only to make enumeration attacks harder.
+        minions = ckminions._check_compound_pillar_exact_minions(
+            expr, DEFAULT_TARGET_DELIM, greedy=False
+        )
+        if minion_id in minions["minions"]:
+            return {"res": minion_id}
+    except Exception:  # pylint: disable=broad-except
+        pass
+    return {"res": False}

tests/integration/modules/test_mac_user.py

@@ -157,7 +157,7 @@ class MacUserModuleTest(ModuleCase):
             self.assertTrue(os.path.exists("/etc/kcpassword"))
 
             # Are the contents of the file correct
-            test_data = b".\xc3\xb8'B\xc2\xa0\xc3\x99\xc2\xad\xc2\x8b\xc3\x8d\xc3\x8dl"
+            test_data = bytes.fromhex("2e f8 27 42 a0 d9 ad 8b cd cd 6c 7d")
             with salt.utils.files.fopen("/etc/kcpassword", "rb") as f:
                 file_data = f.read()
             self.assertEqual(test_data, file_data)

tests/pytests/README.md

@@ -11,4 +11,4 @@ shall be used, neither our [customizations to it](../support/case.py).
 While [PyTest](https://docs.pytest.org) can happily run unittest tests(withough taking advantage of most of PyTest's strengths),
 this new path in the tests directory was created to provide a clear separation between the two approaches to writing tests.
 Some(hopefully all) of the existing unittest tests might get ported to PyTest's style of writing tests, new tests should be added under
-this directory tree, and, in the long run, this directoy shall become the top level tests directoy.
+this directory tree, and, in the long run, this directory shall become the top level tests directory.

tests/pytests/integration/runners/test_match.py

@@ -0,0 +1,318 @@
+"""
+Integration tests for the match runner
+"""
+import logging
+
+import pytest
+
+log = logging.getLogger(__name__)
+
+pytestmark = [
+    pytest.mark.slow_test,
+]
+
+
+@pytest.fixture(scope="class")
+def match_master_config():
+    return {
+        "open_mode": True,
+        "peer_run": {
+            "match-minion-bob": [
+                "match.compound_matches",
+            ],
+        },
+        "nodegroups": {
+            "alice_eve": "I@name:ali*",
+            "alice": "L@match-minion-alice",
+        },
+        "minion_data_cache": True,
+    }
+
+
+@pytest.fixture(scope="class", autouse=True)
+def pillar_tree(match_salt_master, match_salt_minion_alice, match_salt_minion_eve):
+    top_file = f"""
+    base:
+      '{match_salt_minion_alice.id}':
+        - alice
+      '{match_salt_minion_eve.id}':
+        - eve
+    """
+    alice_pillar_file = """
+    name: alice
+    """
+    eve_pillar_file = """
+    name: alice_whoops_sorry_eve_hrhr
+    """
+    top_tempfile = match_salt_master.pillar_tree.base.temp_file("top.sls", top_file)
+    alice_tempfile = match_salt_master.pillar_tree.base.temp_file(
+        "alice.sls", alice_pillar_file
+    )
+    eve_tempfile = match_salt_master.pillar_tree.base.temp_file(
+        "eve.sls", eve_pillar_file
+    )
+
+    with top_tempfile, alice_tempfile, eve_tempfile:
+        ret = match_salt_minion_alice.salt_call_cli().run(
+            "saltutil.refresh_pillar", wait=True
+        )
+        assert ret.returncode == 0
+        assert ret.data is True
+        ret = match_salt_minion_eve.salt_call_cli().run(
+            "saltutil.refresh_pillar", wait=True
+        )
+        assert ret.returncode == 0
+        assert ret.data is True
+        yield
+
+
+@pytest.fixture(scope="class")
+def match_salt_master(salt_factories, match_master_config):
+    factory = salt_factories.salt_master_daemon(
+        "match-master", defaults=match_master_config
+    )
+    with factory.started():
+        yield factory
+
+
+@pytest.fixture(scope="class")
+def match_salt_minion_alice(match_salt_master):
+    assert match_salt_master.is_running()
+    factory = match_salt_master.salt_minion_daemon(
+        "match-minion-alice",
+        defaults={"open_mode": True, "grains": {"role": "alice"}},
+    )
+    with factory.started():
+        # Sync All
+        salt_call_cli = factory.salt_call_cli()
+        ret = salt_call_cli.run("saltutil.sync_all", _timeout=120)
+        assert ret.returncode == 0, ret
+        yield factory
+
+
+@pytest.fixture(scope="class")
+def match_salt_minion_eve(match_salt_master):
+    assert match_salt_master.is_running()
+    factory = match_salt_master.salt_minion_daemon(
+        "match-minion-eve",
+        defaults={"open_mode": True, "grains": {"role": "eve"}},
+    )
+    with factory.started():
+        # Sync All
+        salt_call_cli = factory.salt_call_cli()
+        ret = salt_call_cli.run("saltutil.sync_all", _timeout=120)
+        assert ret.returncode == 0, ret
+        yield factory
+
+
+@pytest.fixture(scope="class")
+def match_salt_minion_bob(match_salt_master):
+    assert match_salt_master.is_running()
+    factory = match_salt_master.salt_minion_daemon(
+        "match-minion-bob",
+        defaults={"open_mode": True},
+    )
+    with factory.started():
+        # Sync All
+        salt_call_cli = factory.salt_call_cli()
+        ret = salt_call_cli.run("saltutil.sync_all", _timeout=120)
+        assert ret.returncode == 0, ret
+        yield factory
+
+
+@pytest.fixture(scope="class")
+def match_salt_call_cli(match_salt_minion_bob):
+    return match_salt_minion_bob.salt_call_cli()
+
+
+@pytest.fixture(scope="class")
+def match_salt_run_cli(match_salt_master):
+    return match_salt_master.salt_run_cli()
+
+
+class TestMatchCompoundRunner:
+    @pytest.fixture
+    def alice_uncached(self, match_salt_minion_alice, match_salt_run_cli):
+        ret = match_salt_run_cli.run("cache.clear_all", "match-minion-alice")
+        assert ret.returncode == 0
+        yield
+        match_salt_minion_alice.salt_call_cli().run("pillar.items")
+
+    @pytest.fixture
+    def eve_cached(self, match_salt_minion_eve):
+        ret = match_salt_minion_eve.salt_call_cli().run("pillar.items")
+        assert ret.returncode == 0
+        yield
+
+    @pytest.fixture
+    def alice_down(self, match_salt_minion_alice):
+        with match_salt_minion_alice.stopped():
+            yield
+
+    @pytest.mark.parametrize(
+        "expr,expected",
+        [
+            ("G@role:alice", True),
+            ("G@role:ali*", True),
+            (r"E@match\-minion\-(alice|bob)", True),
+            ("P@role:^(alice|bob)$", True),
+            ("L@match-minion-alice,match-minion-bob", True),
+            ("I@name:alice", True),
+            ("I@name:ali*", False),
+            ("J@name:alice", True),
+            ("J@name:^(alice|bob)$", False),
+            ("N@alice", True),
+            ("N@alice_eve", False),
+            ("G@role:ali* and I@name:alice", True),
+            ("G@role:ali* and I@name:ali*", False),
+            ("G@role:ali* or I@name:ali*", True),
+            ("G@role:ali* and not I@name:alice", False),
+        ],
+    )
+    def test_match_compound_matches(self, match_salt_run_cli, expr, expected):
+        if expected:
+            expected = "match-minion-alice"
+        ret = match_salt_run_cli.run(
+            "match.compound_matches", expr, "match-minion-alice"
+        )
+        assert ret.returncode == 0
+        assert ret.data
+        assert "res" in ret.data
+        assert ret.data["res"] == expected
+
+    @pytest.mark.usefixtures("eve_cached")
+    def test_match_compound_matches_only_allows_exact_pillar_matching(
+        self, match_salt_run_cli
+    ):
+        """
+        This check is mostly redundant, but better check explicitly with the scenario
+        to prevent because it is security-critical.
+        """
+        ret = match_salt_run_cli.run(
+            "match.compound_matches", "I@name:alic*", "match-minion-eve"
+        )
+        assert ret.returncode == 0
+        assert ret.data
+        assert "res" in ret.data
+        assert ret.data["res"] is False
+
+    @pytest.mark.parametrize(
+        "expr,expected",
+        [
+            ("G@role:alice", False),
+            ("G@role:ali*", False),
+            ("I@name:alice", False),
+            ("I@name:ali*", False),
+            ("G@role:ali* and I@name:alice", False),
+            ("L@match-minion-alice,match-minion-bob", True),
+            ("L@match-minion-alice,match-minion-bob and G@role:alice", False),
+            ("L@match-minion-alice,match-minion-bob and I@name:alice", False),
+        ],
+    )
+    @pytest.mark.usefixtures("alice_uncached")
+    def test_match_compound_matches_with_uncached_minion_data(
+        self, match_salt_run_cli, expr, expected
+    ):
+        if expected:
+            expected = "match-minion-alice"
+        ret = match_salt_run_cli.run(
+            "match.compound_matches", expr, "match-minion-alice"
+        )
+        assert ret.returncode == 0
+        assert ret.data
+        assert "res" in ret.data
+        assert ret.data["res"] == expected
+
+    @pytest.mark.parametrize(
+        "expr,expected",
+        [
+            ("G@role:alice", True),
+            ("G@role:ali*", True),
+            ("I@name:alice", True),
+            ("I@name:ali*", False),
+            ("G@role:ali* and I@name:alice", True),
+            ("L@match-minion-alice,match-minion-bob", True),
+        ],
+    )
+    @pytest.mark.usefixtures("alice_down")
+    def test_match_compound_matches_when_minion_is_down(
+        self, match_salt_run_cli, expr, expected
+    ):
+        if expected:
+            expected = "match-minion-alice"
+        ret = match_salt_run_cli.run(
+            "match.compound_matches", expr, "match-minion-alice"
+        )
+        assert ret.returncode == 0
+        assert ret.data
+        assert "res" in ret.data
+        assert ret.data["res"] == expected
+
+    @pytest.mark.parametrize(
+        "minion_id",
+        [
+            "hi\\there",
+            "my/minion",
+            "../../../../../../../../../etc/shadow",
+        ],
+    )
+    def test_match_compound_matches_with_invalid_minion_id(
+        self, minion_id, match_salt_run_cli
+    ):
+        ret = match_salt_run_cli.run("match.compound_matches", "*", minion_id)
+        assert ret.returncode == 0
+        assert ret.data
+        assert "res" in ret.data
+        assert ret.data["res"] is False
+
+    @pytest.mark.parametrize(
+        "expr,expected",
+        [
+            ("G@role:alice", True),
+            ("G@role:ali*", True),
+            ("I@name:alice", True),
+            ("I@name:ali*", False),
+            ("G@role:ali* and I@name:alice", True),
+            ("L@match-minion-alice,match-minion-bob", True),
+        ],
+    )
+    def test_match_compound_matches_as_peer_run(
+        self, match_salt_call_cli, expr, expected
+    ):
+        if expected:
+            expected = "match-minion-alice"
+        ret = match_salt_call_cli.run(
+            "publish.runner", "match.compound_matches", [expr, "match-minion-alice"]
+        )
+        assert ret.returncode == 0
+        assert ret.data
+        assert "res" in ret.data
+        assert ret.data["res"] == expected
+
+
+class TestMatchCompoundRunnerWithoutMinionDataCache:
+    @pytest.fixture(scope="class")
+    def match_master_config(self):
+        return {
+            "open_mode": True,
+            "minion_data_cache": False,
+        }
+
+    @pytest.mark.parametrize(
+        "expr",
+        [
+            "G@role:alice",
+            "G@role:ali*",
+            "I@name:alice",
+            "I@name:ali*",
+            "G@role:ali* and I@name:alice",
+        ],
+    )
+    def test_match_compound_matches(self, match_salt_run_cli, expr):
+        ret = match_salt_run_cli.run(
+            "match.compound_matches", expr, "match-minion-alice"
+        )
+        assert ret.returncode == 0
+        assert ret.data
+        assert "res" in ret.data
+        assert ret.data["res"] is False

tests/pytests/unit/modules/test_mac_user.py

@@ -420,3 +420,24 @@ def test_list_users():
         mac_user.__salt__, {"cmd.run_all": mock_run}
     ):
         assert mac_user.list_users() == expected
+
+
+def test_kcpassword():
+    hashes = {
+        # Actual hashes from macOS, since reference implementation didn't account for trailing null
+        "0": "4d 89 f9 91 1f 7a 46 5e f7 a8 11 ff",
+        "password": "0d e8 21 50 a5 d3 af 8e a3 de d9 14",
+        "shorterpwd": "0e e1 3d 51 a6 d9 af 9a d4 dd 1f 27",
+        "Squarepants": "2e f8 27 42 a0 d9 ad 8b cd cd 6c 7d",
+        "longerpasswd": "11 e6 3c 44 b7 ce ad 8b d0 ca 68 19 89 b1 65 ae 7e 89 12 b8 51 f8 f0 ff",
+        "ridiculouslyextendedpass": "0f e0 36 4a b1 c9 b1 85 d6 ca 73 04 ec 2a 57 b7 d2 b9 8f c7 c9 7e 0e fa 52 7b 71 e6 f8 b7 a6 ae 47 94 d7 86",
+    }
+    for password, hash in hashes.items():
+        kcpass = mac_user._kcpassword(password)
+        hash = bytes.fromhex(hash)
+
+        # macOS adds a trailing null and pads the rest with random data
+        length = len(password) + 1
+
+        assert kcpass[:length] == hash[:length]
+        assert len(kcpass) == len(hash)