Fix malformed shadow entries

If a shadow entry for a user has too many (or too few) fields, due to
having been hand-edited for some silly reason, this will cause Salt not
to be able to successfully run a user.present state (with password
argument) against it, as spwd will not be able to retrieve a hash for
the user, causing Salt to always think that the password hash still
needs changes.

This fixes these two cases.
This commit is contained in:
Erik Johnson 2020-09-10 10:53:41 -05:00 committed by Megan Wilhite
parent 327664a375
commit d8ed6b2a2d
2 changed files with 124 additions and 18 deletions

View file

@ -15,7 +15,6 @@ import os
import salt.utils.data
import salt.utils.files
import salt.utils.stringutils
from salt.exceptions import CommandExecutionError
try:
@ -390,11 +389,15 @@ def set_password(name, password, use_usermod=False, root=None):
lines = []
user_found = False
lstchg = str((datetime.datetime.today() - datetime.datetime(1970, 1, 1)).days)
with salt.utils.files.fopen(s_file, "rb") as fp_:
with salt.utils.files.fopen(s_file, "r") as fp_:
for line in fp_:
line = salt.utils.stringutils.to_unicode(line)
comps = line.strip().split(":")
# Fix malformed entry by first ignoring extra fields, then
# adding missing fields.
comps = line.strip().split(":")[:9]
if comps[0] == name:
num_missing = 9 - len(comps)
if num_missing:
comps.extend([""] * num_missing)
user_found = True
comps[1] = password
comps[2] = lstchg
@ -410,7 +413,6 @@ def set_password(name, password, use_usermod=False, root=None):
)
else:
with salt.utils.files.fopen(s_file, "w+") as fp_:
lines = [salt.utils.stringutils.to_str(_l) for _l in lines]
fp_.writelines(lines)
uinfo = info(name, root=root)
return uinfo["passwd"] == password
@ -531,7 +533,6 @@ def _getspnam(name, root=None):
passwd = os.path.join(root, "etc/shadow")
with salt.utils.files.fopen(passwd) as fp_:
for line in fp_:
line = salt.utils.stringutils.to_unicode(line)
comps = line.strip().split(":")
if comps[0] == name:
# Generate a getspnam compatible output
@ -549,7 +550,6 @@ def _getspall(root=None):
passwd = os.path.join(root, "etc/shadow")
with salt.utils.files.fopen(passwd) as fp_:
for line in fp_:
line = salt.utils.stringutils.to_unicode(line)
comps = line.strip().split(":")
# Generate a getspall compatible output
for i in range(2, 9):

View file

@ -1,13 +1,11 @@
"""
:codeauthor: Erik Johnson <erik@saltstack.com>
"""
import textwrap
import pytest
import salt.utils.platform
import salt.utils.stringutils
from tests.support.mixins import LoaderModuleMockMixin
from tests.support.mock import DEFAULT, MagicMock, mock_open, patch
from tests.support.unit import TestCase, skipIf
@ -64,15 +62,16 @@ class LinuxShadowTest(TestCase, LoaderModuleMockMixin):
Test the corner case in which shadow.set_password is called for a user
that has an entry in /etc/passwd but not /etc/shadow.
"""
original_file = textwrap.dedent(
"""\
foo:orighash:17955::::::
bar:somehash:17955::::::
"""
)
original_lines = original_file.splitlines(True)
data = {
"/etc/shadow": salt.utils.stringutils.to_bytes(
textwrap.dedent(
"""\
foo:orighash:17955::::::
bar:somehash:17955::::::
"""
)
),
"/etc/shadow": original_file,
"*": Exception("Attempted to open something other than /etc/shadow"),
}
isfile_mock = MagicMock(
@ -114,7 +113,7 @@ class LinuxShadowTest(TestCase, LoaderModuleMockMixin):
# Should only have the same two users in the file
assert len(lines) == 2
# The first line should be unchanged
assert lines[0] == "foo:orighash:17955::::::\n"
assert lines[0] == original_lines[0]
# The second line should have the new password hash
assert lines[1].split(":")[:2] == [user, password]
@ -199,6 +198,113 @@ class LinuxShadowTest(TestCase, LoaderModuleMockMixin):
expected_result, sorted(result.items(), key=lambda x: x[0])
)
@pytest.mark.skip_if_not_root
def test_set_password_malformed_shadow_entry(self):
"""
Test that Salt will repair a malformed shadow entry (that is, one that
doesn't have the correct number of fields).
"""
original_file = textwrap.dedent(
"""\
valid:s00persekr1thash:17955::::::
tooshort:orighash:17955:::::
toolong:orighash:17955:::::::
"""
)
original_lines = original_file.splitlines(True)
data = {
"/etc/shadow": original_file,
"*": Exception("Attempted to open something other than /etc/shadow"),
}
isfile_mock = MagicMock(
side_effect=lambda x: True if x == "/etc/shadow" else DEFAULT
)
password = "newhash"
shadow_info_mock = MagicMock(return_value={"passwd": password})
#
# CASE 1: Fix an entry with too few fields
#
user = "tooshort"
user_exists_mock = MagicMock(
side_effect=lambda x, **y: 0 if x == ["id", user] else DEFAULT
)
with patch(
"salt.utils.files.fopen", mock_open(read_data=data)
) as shadow_mock, patch("os.path.isfile", isfile_mock), patch.object(
shadow, "info", shadow_info_mock
), patch.dict(
shadow.__salt__, {"cmd.retcode": user_exists_mock}
), patch.dict(
shadow.__grains__, {"os": "CentOS"}
):
result = shadow.set_password(user, password, use_usermod=False)
assert result
filehandles = shadow_mock.filehandles["/etc/shadow"]
# We should only have opened twice, once to read the contents and once
# to write.
assert len(filehandles) == 2
# We're rewriting the entire file
assert filehandles[1].mode == "w+"
# We should be calling writelines instead of write, to rewrite the
# entire file.
assert len(filehandles[1].writelines_calls) == 1
# Make sure we wrote the correct info
lines = filehandles[1].writelines_calls[0]
# Should only have the same three users in the file
assert len(lines) == 3
# The first and third line should be unchanged
assert lines[0] == original_lines[0]
assert lines[2] == original_lines[2]
# The second line should have the new password hash, and it should have
# gotten "fixed" by adding back in the missing hash.
fixed = lines[1].split(":")
assert fixed[:2] == [user, password]
assert len(fixed) == 9
#
# CASE 2: Fix an entry with too many fields
#
user = "toolong"
user_exists_mock = MagicMock(
side_effect=lambda x, **y: 0 if x == ["id", user] else DEFAULT
)
with patch(
"salt.utils.files.fopen", mock_open(read_data=data)
) as shadow_mock, patch("os.path.isfile", isfile_mock), patch.object(
shadow, "info", shadow_info_mock
), patch.dict(
shadow.__salt__, {"cmd.retcode": user_exists_mock}
), patch.dict(
shadow.__grains__, {"os": "CentOS"}
):
result = shadow.set_password(user, password, use_usermod=False)
assert result
filehandles = shadow_mock.filehandles["/etc/shadow"]
# We should only have opened twice, once to read the contents and once
# to write.
assert len(filehandles) == 2
# We're rewriting the entire file
assert filehandles[1].mode == "w+"
# We should be calling writelines instead of write, to rewrite the
# entire file.
assert len(filehandles[1].writelines_calls) == 1
# Make sure we wrote the correct info
lines = filehandles[1].writelines_calls[0]
# Should only have the same three users in the file
assert len(lines) == 3
# The first and second line should be unchanged
assert lines[0] == original_lines[0]
assert lines[1] == original_lines[1]
# The third line should have the new password hash, and it should have
# gotten "fixed" by adding back in the missing hash.
fixed = lines[2].split(":")
assert fixed[:2] == [user, password]
assert len(fixed) == 9
@pytest.mark.skip_if_not_root
def test_list_users(self):
"""