saltstack/salt
1
0
Fork 0
mirror of https://github.com/saltstack/salt.git synced 2025-04-10 14:51:40 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Fix stacktrace with unmapped account names

Browse source
This commit is contained in:
Twangboy 2025-02-21 09:51:35 -07:00 • committed by Daniel Wozniak
parent 2b41383e2b
commit 680f27f9b8
4 changed files with 71 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
changelog/66637.fixed.md Normal file
View file

@ -0,0 +1,4 @@
Fixes an issue when getting account names using the get_name function in the
win_dacl.py salt util. Capability SIDs return ``None``. SIDs for deleted
accounts return the SID. SIDs for domain accounts where the system is not
connected to the domain return the SID.

5
salt/modules/win_file.py
View file

@ -494,13 +494,14 @@ def get_group(path, follow_symlinks=True):
def uid_to_user(uid):
"""
Convert a uid to a user name
Convert a User ID (uid) to a username
Args:
uid (str): The user id to lookup
Returns:
str: The name of the user
str: The name of the user. The ``uid`` will be returned if there is no
corresponding username
CLI Example:

78
salt/utils/win_dacl.py
View file

@ -125,7 +125,7 @@ should match what you see when you look at the properties for an object.
- subfolders_only: Applies to all containers beneath this object
- files_only: Applies to all file objects beneath this object
.. NOTE::
.. note::
'applies to' properties can only be modified on directories. Files
will always be ``this_folder_only``.
@ -883,10 +883,7 @@ def dacl(obj_name=None, obj_type="file"):
"""
# Get the principal from the sid (object sid)
sid = win32security.ConvertSidToStringSid(ace[2])
try:
principal = get_name(sid)
except CommandExecutionError:
principal = sid
principal = get_name(sid)
# Get the ace type
ace_type = self.ace_type[ace[0][0]]
@ -1194,14 +1191,17 @@ def get_name(principal):
principal (str):
Find the Normalized name based on this. Can be a PySID object, a SID
string, or a user name in any capitalization.
string, or a username in any capitalization.
.. note::
Searching based on the user name can be slow on hosts connected
Searching based on the username can be slow on hosts connected
to large Active Directory domains.
Returns:
str: The name that corresponds to the passed principal
str: The username that corresponds to the passed principal. If there is
no corresponding username, the string SID will be returned.
Capability SIDs will return ``None``.
Usage:
@ -1246,7 +1246,7 @@ def get_name(principal):
name = f"NT Service\\{name}"
return name
except (pywintypes.error, TypeError) as exc:
except pywintypes.error as exc:
# Microsoft introduced the concept of Capability SIDs in Windows 8
# https://docs.microsoft.com/en-us/windows/security/identity-protection/access-control/security-identifiers#capability-sids
# https://support.microsoft.com/en-us/help/4502539/some-sids-do-not-resolve-into-friendly-names
@ -1254,11 +1254,26 @@ def get_name(principal):
# These types of SIDs do not resolve, so we'll just ignore them for this
# All capability SIDs begin with `S-1-15-3`, so we'll only throw an
# error when the sid does not begin with `S-1-15-3`
if not str_sid.startswith("S-1-15-3"):
message = f'Error resolving "{principal}"'
if type(exc) == pywintypes.error:
win_error = win32api.FormatMessage(exc.winerror).rstrip("\n")
message = f"{message}: {win_error}"
# 1332: No mapping between account names and security IDs was done
if exc.winerror == 1332:
# Capability SID, return None
if str_sid.startswith("S-1-15-3"):
log.debug("Name mapping not available for capability SID: %s", str_sid)
return None
# User does not exist on the system or is on a disconnected domain
# Return the SID
else:
log.debug(
f"Could not resolve SID: %s\nThe user has either been removed"
" from the system or is a domain user and the system is not"
" connected to the domain"
)
return str_sid
# Some other unknown error
else:
message = f'Error resolving "{principal}: {exc.strerror}"'
log.exception(message)
raise CommandExecutionError(message, exc)
@ -2242,13 +2257,19 @@ def _check_perms(obj_name, obj_type, new_perms, access_mode, ret, test_mode=Fals
cur_perms = get_permissions(obj_name=obj_name, obj_type=obj_type)
changes = {}
for user in new_perms:
applies_to_text = ""
# Check that user exists:
try:
user_name = get_name(principal=user)
except CommandExecutionError:
user_name = get_name(principal=user)
# username will be the SID if there is no corresponding username
if user_name == get_sid_string(principal=user):
ret["comment"].append(
'{} Perms: User "{}" missing from Target System'.format(
"{} Perms: Could not find a corresponding username for: {}".format(
access_mode.capitalize(), user
)
)
continue
if user_name is None:
ret["comment"].append(
"{} Perms: Skipping Capability SID: {}".format(
access_mode.capitalize(), user
)
)
@ -2471,7 +2492,7 @@ def check_perms(
log.debug("Resetting permissions for %s", obj_name)
cur_perms = get_permissions(obj_name=obj_name, obj_type=obj_type)
for user_name in cur_perms["Not Inherited"]:
# case insensitive dictionary search
# case-insensitive dictionary search
if user_name not in {get_name(k) for k in (grant_perms or {})}:
if "grant" in cur_perms["Not Inherited"][user_name]:
ret["changes"].setdefault("remove_perms", {})
@ -2489,7 +2510,7 @@ def check_perms(
ret["changes"]["remove_perms"].update(
{user_name: cur_perms["Not Inherited"][user_name]}
)
# case insensitive dictionary search
# case-insensitive dictionary search
if user_name not in {get_name(k) for k in (deny_perms or {})}:
if "deny" in cur_perms["Not Inherited"][user_name]:
ret["changes"].setdefault("remove_perms", {})
@ -2541,7 +2562,7 @@ def check_perms(
log.debug("Resetting permissions for %s", obj_name)
cur_perms = get_permissions(obj_name=obj_name, obj_type=obj_type)
for user_name in cur_perms["Not Inherited"]:
# case insensitive dictionary search
# case-insensitive dictionary search
if user_name not in {get_name(k) for k in (grant_perms or {})}:
if "grant" in cur_perms["Not Inherited"][user_name]:
rm_permissions(
@ -2550,7 +2571,7 @@ def check_perms(
ace_type="grant",
obj_type=obj_type,
)
# case insensitive dictionary search
# case-insensitive dictionary search
if user_name not in {get_name(k) for k in (deny_perms or {})}:
if "deny" in cur_perms["Not Inherited"][user_name]:
rm_permissions(
@ -2582,14 +2603,9 @@ def _set_perms(obj_dacl, obj_type, new_perms, cur_perms, access_mode):
ret = {}
for user in new_perms:
# Check that user exists:
try:
user_name = get_name(user)
except CommandExecutionError:
log.debug(
'%s Perms: User "%s" missing from Target System',
access_mode.capitalize(),
user,
)
user_name = get_name(user)
# We want to skip unmapped usernames
if user_name == get_sid_string(user):
continue
# Get applies_to

22
tests/pytests/functional/utils/win_dacl/test_get_name.py
View file

@ -9,9 +9,11 @@ import pytest
import salt.exceptions
import salt.utils.win_dacl
from tests.support.mock import patch
# Third-party libs
try:
import pywintypes
import win32security
HAS_WIN32 = True
@ -84,12 +86,22 @@ def test_get_name_capability_sid():
assert salt.utils.win_dacl.get_name(sid_obj) is None
def test_get_name_error():
def test_get_name_unmapped_sid():
"""
Test get_name with an un mapped SID, should throw a CommandExecutionError
Test get_name with an un mapped SID, should return the passed sid
"""
test_sid = "S-1-2-3-4"
sid_obj = win32security.ConvertStringSidToSid(test_sid)
with pytest.raises(salt.exceptions.CommandExecutionError) as exc:
salt.utils.win_dacl.get_name(sid_obj)
assert "No mapping between account names" in exc.value.message
assert salt.utils.win_dacl.get_name(sid_obj) == test_sid
def test_get_name_error():
"""
Test get_name with an unexpected error, should throw a CommandExecutionError
"""
test_sid = "S-1-2-3-4"
sid_obj = win32security.ConvertStringSidToSid(test_sid)
with patch("win32security.LookupAccountSid", side_effect=pywintypes.error):
with pytest.raises(salt.exceptions.CommandExecutionError) as exc:
salt.utils.win_dacl.get_name(sid_obj)
assert "Error resolving" in exc.value.message