Skip to content

Commit

Permalink
Merge branch '3006.x' into refix_64572
Browse files Browse the repository at this point in the history
  • Loading branch information
dmurphy18 authored Sep 21, 2023
2 parents 19ab315 + 8fbb5d3 commit 2aa7b65
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 36 deletions.
1 change: 1 addition & 0 deletions changelog/64888.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed grp.getgrall() in utils/user.py causing performance issues
1 change: 1 addition & 0 deletions changelog/64953.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix user.list_groups omits remote groups via sssd, etc.
1 change: 1 addition & 0 deletions changelog/65029.removed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Tech Debt - support for pysss removed due to functionality addition in Python 3.3
9 changes: 0 additions & 9 deletions salt/auth/pam.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,6 @@
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.
Expand Down
73 changes: 46 additions & 27 deletions salt/utils/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,6 @@
except ImportError:
HAS_GRP = False

try:
import pysss

HAS_PYSSS = True
except ImportError:
HAS_PYSSS = False

try:
import salt.utils.win_functions

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -385,3 +383,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
44 changes: 44 additions & 0 deletions tests/pytests/functional/utils/user/test__getgrall.py
Original file line number Diff line number Diff line change
@@ -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))
29 changes: 29 additions & 0 deletions tests/pytests/unit/utils/test_user.py
Original file line number Diff line number Diff line change
@@ -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()

0 comments on commit 2aa7b65

Please sign in to comment.