4aacd02544
- Added: * fix-user.list_groups-omits-remote-groups.patch OBS-URL: https://build.opensuse.org/package/show/systemsmanagement:saltstack/salt?expand=0&rev=247
266 lines
8.7 KiB
Diff
266 lines
8.7 KiB
Diff
From 70509ff67d4eb734c88032913134092257a0d35b Mon Sep 17 00:00:00 2001
|
|
From: Flex Liu <fliu@suse.com>
|
|
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 <nicholasmhughes@gmail.com>
|
|
---
|
|
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
|
|
|