diff --git a/_lastrevision b/_lastrevision index 6a979f1..0936235 100644 --- a/_lastrevision +++ b/_lastrevision @@ -1 +1 @@ -e2ff2b8c17278bf750998f7619ae9b29fbe017a1 \ No newline at end of file +1aa01d4bf6fdffc46ba5ea6aa7beca9eb4f06bc0 \ No newline at end of file diff --git a/fix-user.list_groups-omits-remote-groups.patch b/fix-user.list_groups-omits-remote-groups.patch new file mode 100644 index 0000000..c1b5c4f --- /dev/null +++ b/fix-user.list_groups-omits-remote-groups.patch @@ -0,0 +1,265 @@ +From 70509ff67d4eb734c88032913134092257a0d35b Mon Sep 17 00:00:00 2001 +From: Flex Liu +Date: Tue, 2 Jul 2024 15:25:30 +0800 +Subject: [PATCH] Fix user.list_groups omits remote groups + +* fixes saltstack/salt#64953 user.list_groups omits remote groups + +* fixes saltstack/salt#65029 support for pysss can be removed + +* add changlog entries + +* add tests for _getgrall and local vs remote group handling + +* add negative tests for _getgrall + +* root can still read the file and tests run as root + +* remove permission check as its probably an unreachable edge case + +--------- + +Co-authored-by: nicholasmhughes +--- + changelog/64888.fixed.md | 1 + + changelog/64953.fixed.md | 1 + + changelog/65029.removed.md | 1 + + salt/auth/pam.py | 9 --- + salt/utils/user.py | 73 ++++++++++++------- + .../functional/utils/user/test__getgrall.py | 44 +++++++++++ + tests/pytests/unit/utils/test_user.py | 29 ++++++++ + 7 files changed, 122 insertions(+), 36 deletions(-) + create mode 100644 changelog/64888.fixed.md + create mode 100644 changelog/64953.fixed.md + create mode 100644 changelog/65029.removed.md + create mode 100644 tests/pytests/functional/utils/user/test__getgrall.py + create mode 100644 tests/pytests/unit/utils/test_user.py + +diff --git a/changelog/64888.fixed.md b/changelog/64888.fixed.md +new file mode 100644 +index 0000000000..08b2efd042 +--- /dev/null ++++ b/changelog/64888.fixed.md +@@ -0,0 +1 @@ ++Fixed grp.getgrall() in utils/user.py causing performance issues +diff --git a/changelog/64953.fixed.md b/changelog/64953.fixed.md +new file mode 100644 +index 0000000000..f0b1ed46f1 +--- /dev/null ++++ b/changelog/64953.fixed.md +@@ -0,0 +1 @@ ++Fix user.list_groups omits remote groups via sssd, etc. +diff --git a/changelog/65029.removed.md b/changelog/65029.removed.md +new file mode 100644 +index 0000000000..d09f10b4ba +--- /dev/null ++++ b/changelog/65029.removed.md +@@ -0,0 +1 @@ ++Tech Debt - support for pysss removed due to functionality addition in Python 3.3 +diff --git a/salt/auth/pam.py b/salt/auth/pam.py +index f0397c1062..12af29bbdb 100644 +--- a/salt/auth/pam.py ++++ b/salt/auth/pam.py +@@ -24,15 +24,6 @@ authenticated against. This defaults to `login` + + The Python interface to PAM does not support authenticating as ``root``. + +-.. note:: Using PAM groups with SSSD groups on python2. +- +- To use sssd with the PAM eauth module and groups the `pysss` module is +- needed. On RedHat/CentOS this is `python-sss`. +- +- This should not be needed with python >= 3.3, because the `os` modules has the +- `getgrouplist` function. +- +- + .. note:: This module executes itself in a subprocess in order to user the system python + and pam libraries. We do this to avoid openssl version conflicts when + running under a salt onedir build. +diff --git a/salt/utils/user.py b/salt/utils/user.py +index 2f1ca65cf9..3588b3804a 100644 +--- a/salt/utils/user.py ++++ b/salt/utils/user.py +@@ -31,13 +31,6 @@ try: + except ImportError: + HAS_GRP = False + +-try: +- import pysss +- +- HAS_PYSSS = True +-except ImportError: +- HAS_PYSSS = False +- + try: + import salt.utils.win_functions + +@@ -289,30 +282,35 @@ def get_group_list(user, include_default=True): + return [] + group_names = None + ugroups = set() +- if hasattr(os, "getgrouplist"): +- # Try os.getgrouplist, available in python >= 3.3 +- log.trace("Trying os.getgrouplist for '%s'", user) +- try: +- user_group_list = os.getgrouplist(user, pwd.getpwnam(user).pw_gid) +- group_names = [ +- _group.gr_name +- for _group in grp.getgrall() +- if _group.gr_gid in user_group_list +- ] +- except Exception: # pylint: disable=broad-except +- pass +- elif HAS_PYSSS: +- # Try pysss.getgrouplist +- log.trace("Trying pysss.getgrouplist for '%s'", user) +- try: +- group_names = list(pysss.getgrouplist(user)) +- except Exception: # pylint: disable=broad-except +- pass ++ # Try os.getgrouplist, available in python >= 3.3 ++ log.trace("Trying os.getgrouplist for '%s'", user) ++ try: ++ user_group_list = sorted(os.getgrouplist(user, pwd.getpwnam(user).pw_gid)) ++ local_grall = _getgrall() ++ local_gids = sorted(lgrp.gr_gid for lgrp in local_grall) ++ max_idx = -1 ++ local_max = local_gids[max_idx] ++ while local_max >= 65000: ++ max_idx -= 1 ++ local_max = local_gids[max_idx] ++ user_group_list_local = [lgrp for lgrp in user_group_list if lgrp <= local_max] ++ user_group_list_remote = [rgrp for rgrp in user_group_list if rgrp > local_max] ++ local_group_names = [ ++ _group.gr_name ++ for _group in local_grall ++ if _group.gr_gid in user_group_list_local ++ ] ++ remote_group_names = [ ++ grp.getgrgid(group_id).gr_name for group_id in user_group_list_remote ++ ] ++ group_names = local_group_names + remote_group_names ++ except Exception: # pylint: disable=broad-except ++ pass + + if group_names is None: + # Fall back to generic code + # Include the user's default group to match behavior of +- # os.getgrouplist() and pysss.getgrouplist() ++ # os.getgrouplist() + log.trace("Trying generic group list for '%s'", user) + group_names = [g.gr_name for g in grp.getgrall() if user in g.gr_mem] + try: +@@ -389,3 +387,24 @@ def get_gid(group=None): + return grp.getgrnam(group).gr_gid + except KeyError: + return None ++ ++ ++def _getgrall(root=None): ++ """ ++ Alternative implemetantion for getgrall, that uses only /etc/group ++ """ ++ ret = [] ++ root = "/" if not root else root ++ etc_group = os.path.join(root, "etc/group") ++ with salt.utils.files.fopen(etc_group) as fp_: ++ for line in fp_: ++ line = salt.utils.stringutils.to_unicode(line) ++ comps = line.strip().split(":") ++ # Generate a getgrall compatible output ++ comps[2] = int(comps[2]) ++ if comps[3]: ++ comps[3] = [mem.strip() for mem in comps[3].split(",")] ++ else: ++ comps[3] = [] ++ ret.append(grp.struct_group(comps)) ++ return ret +diff --git a/tests/pytests/functional/utils/user/test__getgrall.py b/tests/pytests/functional/utils/user/test__getgrall.py +new file mode 100644 +index 0000000000..db994019e6 +--- /dev/null ++++ b/tests/pytests/functional/utils/user/test__getgrall.py +@@ -0,0 +1,44 @@ ++from textwrap import dedent ++ ++import pytest ++ ++pytest.importorskip("grp") ++ ++import grp ++ ++import salt.utils.user ++ ++ ++@pytest.fixture(scope="function") ++def etc_group(tmp_path): ++ etcgrp = tmp_path / "etc" / "group" ++ etcgrp.parent.mkdir() ++ etcgrp.write_text( ++ dedent( ++ """games:x:50: ++ docker:x:959:debian,salt ++ salt:x:1000:""" ++ ) ++ ) ++ return etcgrp ++ ++ ++def test__getgrall(etc_group): ++ group_lines = [ ++ ["games", "x", 50, []], ++ ["docker", "x", 959, ["debian", "salt"]], ++ ["salt", "x", 1000, []], ++ ] ++ expected_grall = [grp.struct_group(comps) for comps in group_lines] ++ ++ grall = salt.utils.user._getgrall(root=str(etc_group.parent.parent)) ++ ++ assert grall == expected_grall ++ ++ ++def test__getgrall_bad_format(etc_group): ++ with etc_group.open("a") as _fp: ++ _fp.write("\n# some comment here\n") ++ ++ with pytest.raises(IndexError): ++ salt.utils.user._getgrall(root=str(etc_group.parent.parent)) +diff --git a/tests/pytests/unit/utils/test_user.py b/tests/pytests/unit/utils/test_user.py +new file mode 100644 +index 0000000000..17c6b1551f +--- /dev/null ++++ b/tests/pytests/unit/utils/test_user.py +@@ -0,0 +1,29 @@ ++from types import SimpleNamespace ++ ++import pytest ++ ++from tests.support.mock import MagicMock, patch ++ ++pytest.importorskip("grp") ++ ++import grp ++ ++import salt.utils.user ++ ++ ++def test_get_group_list(): ++ getpwname = SimpleNamespace(pw_gid=1000) ++ getgrgid = MagicMock(side_effect=[SimpleNamespace(gr_name="remote")]) ++ group_lines = [ ++ ["games", "x", 50, []], ++ ["salt", "x", 1000, []], ++ ] ++ getgrall = [grp.struct_group(comps) for comps in group_lines] ++ with patch("os.getgrouplist", MagicMock(return_value=[50, 1000, 12000])), patch( ++ "pwd.getpwnam", MagicMock(return_value=getpwname) ++ ), patch("salt.utils.user._getgrall", MagicMock(return_value=getgrall)), patch( ++ "grp.getgrgid", getgrgid ++ ): ++ group_list = salt.utils.user.get_group_list("salt") ++ assert group_list == ["games", "remote", "salt"] ++ getgrgid.assert_called_once() +-- +2.35.3 + diff --git a/salt.changes b/salt.changes index bc6a188..c7256f7 100644 --- a/salt.changes +++ b/salt.changes @@ -1,3 +1,11 @@ +------------------------------------------------------------------- +Wed Jul 3 11:13:00 UTC 2024 - Flex Liu + +- Fix performance of user.list_groups with many remote groups + +- Added: + * fix-user.list_groups-omits-remote-groups.patch + ------------------------------------------------------------------- Tue Jun 18 15:00:44 UTC 2024 - Pablo Suárez Hernández diff --git a/salt.spec b/salt.spec index 47a1224..814e2b2 100644 --- a/salt.spec +++ b/salt.spec @@ -402,6 +402,8 @@ Patch120: provide-systemd-timer-unit.patch Patch121: skip-certain-tests-if-necessary-and-mark-some-flaky-.patch # PATCH-FIX_UPSTREAM https://github.com/saltstack/salt/pull/66647 Patch122: fix-status.diskusage-and-exclude-some-tests-to-run-w.patch +# PATCH-FIX_UPSTREAM https://github.com/saltstack/salt/pull/65077 +Patch123: fix-user.list_groups-omits-remote-groups.patch ### IMPORTANT: The line below is used as a snippet marker. Do not touch it. ### SALT PATCHES LIST END