[SCM] Samba Shared Repository - branch master updated
Jeremy Allison
jra at samba.org
Mon Nov 21 22:06:01 UTC 2022
The branch, master has been updated
via 59b5abbe8ce gp: Test PAM Access with DENY_ALL
via ca5f8072a4c gp: PAM Access should implicitly deny ALL w/ allow
via 9f6cf276e22 gp: samba-tool manage gpo access add don't fail w/out upn
via 8d0d79ba3b3 gp: Make samba-tool gpo manage sudoers remove backward compatible
via d0c4aebb0ef gp: Test that samba-tool gpo manage removes gpme sudoers
via cc0c784d3ab gp: Make samba-tool gpo manage sudoers list backward compatible
via 4c2b418882e gp: Test that samba-tool gpo manage lists gpme sudoers
from f03665bb7e8 s3:rpc_server: Fix include directive substitution when enumerating shares
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 59b5abbe8cec52d7cf1197a91f32d832670284d5
Author: David Mulder <dmulder at samba.org>
Date: Fri Nov 18 11:42:15 2022 -0700
gp: Test PAM Access with DENY_ALL
Signed-off-by: David Mulder <dmulder at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Autobuild-User(master): Jeremy Allison <jra at samba.org>
Autobuild-Date(master): Mon Nov 21 22:05:01 UTC 2022 on sn-devel-184
commit ca5f8072a4c7be6fdebef494664a27bbd73340ff
Author: David Mulder <dmulder at samba.org>
Date: Thu Nov 17 16:33:24 2022 -0700
gp: PAM Access should implicitly deny ALL w/ allow
If an allow entry is specified, the PAM Access
CSE should implicitly deny ALL (everyone other
than the explicit allow entries).
Signed-off-by: David Mulder <dmulder at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit 9f6cf276e22b82601a81286fabeae5303f58339c
Author: David Mulder <dmulder at samba.org>
Date: Thu Nov 17 12:37:20 2022 -0700
gp: samba-tool manage gpo access add don't fail w/out upn
The search response for the user could possibly
not include a upn (this happens with Administrator
for example).
Signed-off-by: David Mulder <dmulder at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit 8d0d79ba3b317401cfe089c0df871189bb79c9dc
Author: David Mulder <dmulder at samba.org>
Date: Wed Nov 16 15:04:16 2022 -0700
gp: Make samba-tool gpo manage sudoers remove backward compatible
Ensure `samba-tool gpo manage sudoers remove` is
backward compatible with the GPME sudo rules.
Signed-off-by: David Mulder <dmulder at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit d0c4aebb0eff59716cfc51d86eec26a52f6913c5
Author: David Mulder <dmulder at samba.org>
Date: Wed Nov 16 15:03:18 2022 -0700
gp: Test that samba-tool gpo manage removes gpme sudoers
The file format for storing the sudo rules
changed in samba-tool, but these can still be
added via the GPME. We should still include them
here.
Signed-off-by: David Mulder <dmulder at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit cc0c784d3ab914593356b4b1a0ca924c9dc4b9fa
Author: David Mulder <dmulder at samba.org>
Date: Wed Nov 16 10:46:11 2022 -0700
gp: Make samba-tool gpo manage sudoers list backward compatible
Ensure `samba-tool gpo manage sudoers list` is
backward compatible with the GPME sudo rules.
Signed-off-by: David Mulder <dmulder at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
commit 4c2b418882ecdb6293cc1d033c33685ada684c2e
Author: David Mulder <dmulder at samba.org>
Date: Wed Nov 16 10:44:22 2022 -0700
gp: Test that samba-tool gpo manage lists gpme sudoers
The file format for storing the sudo rules
changed in samba-tool, but these can still be
added via the GPME. We should still include them
here.
Signed-off-by: David Mulder <dmulder at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
-----------------------------------------------------------------------
Summary of changes:
python/samba/gp/vgp_access_ext.py | 54 +++++++++++---
python/samba/netcmd/gpo.py | 135 ++++++++++++++++++++++++-----------
python/samba/tests/gpo.py | 7 +-
python/samba/tests/samba_tool/gpo.py | 53 ++++++++++++++
4 files changed, 197 insertions(+), 52 deletions(-)
Changeset truncated at 500 lines:
diff --git a/python/samba/gp/vgp_access_ext.py b/python/samba/gp/vgp_access_ext.py
index efd91ef93fb..474d5e278ee 100644
--- a/python/samba/gp/vgp_access_ext.py
+++ b/python/samba/gp/vgp_access_ext.py
@@ -19,6 +19,7 @@ from samba.gp.gpclass import gp_xml_ext
from hashlib import blake2b
from tempfile import NamedTemporaryFile
from samba.common import get_bytes
+from samba.gp.util.logging import log
intro = '''
### autogenerated by samba
@@ -31,11 +32,34 @@ intro = '''
'''
+# The deny all file is implicit any time an allow entry is used
+DENY_BOUND = 9000000000
+DENY_FILE = '_gp_DENY_ALL.conf'
+
+# Each policy MUST create it's own DENY_ALL file if an allow entry exists,
+# otherwise policies will conflict and one could remove a DENY_ALL when another
+# one still requires it.
+def deny_file(access):
+ deny_filename = os.path.join(access,
+ '%d%s' % (select_next_deny(access), DENY_FILE))
+ with NamedTemporaryFile(delete=False, dir=access) as f:
+ with open(f.name, 'w') as w:
+ w.write(intro)
+ w.write('-:ALL:ALL')
+ os.chmod(f.name, 0o644)
+ os.rename(f.name, deny_filename)
+ return deny_filename
+
+def select_next_deny(directory):
+ configs = [re.match(r'(\d+)', f) for f in os.listdir(directory) if DENY_FILE in f]
+ return max([int(m.group(1)) for m in configs if m]+[DENY_BOUND])+1
+
# Access files in /etc/security/access.d are read in the order of the system
# locale. Here we number the conf files to ensure they are read in the correct
-# order.
+# order. Ignore the deny file, since allow entries should always come before
+# the implicit deny ALL.
def select_next_conf(directory):
- configs = [re.match(r'(\d+)', f) for f in os.listdir(directory)]
+ configs = [re.match(r'(\d+)', f) for f in os.listdir(directory) if DENY_FILE not in f]
return max([int(m.group(1)) for m in configs if m]+[0])+1
class vgp_access_ext(gp_xml_ext):
@@ -47,9 +71,10 @@ class vgp_access_ext(gp_xml_ext):
for guid, settings in deleted_gpo_list:
self.gp_db.set_guid(guid)
if str(self) in settings:
- for attribute, access_file in settings[str(self)].items():
- if os.path.exists(access_file):
- os.unlink(access_file)
+ for attribute, policy_files in settings[str(self)].items():
+ for access_file in policy_files.split(':'):
+ if os.path.exists(access_file):
+ os.unlink(access_file)
self.gp_db.delete(str(self), attribute)
self.gp_db.commit()
@@ -63,14 +88,20 @@ class vgp_access_ext(gp_xml_ext):
path = os.path.join(gpo.file_sys_path, deny)
deny_conf = self.parse(path)
entries = []
+ policy_files = []
if allow_conf:
policy = allow_conf.find('policysetting')
data = policy.find('data')
- for listelement in data.findall('listelement'):
+ allow_listelements = data.findall('listelement')
+ for listelement in allow_listelements:
adobject = listelement.find('adobject')
name = adobject.find('name').text
domain = adobject.find('domain').text
entries.append('+:%s\\%s:ALL' % (domain, name))
+ if len(allow_listelements) > 0:
+ log.info('Adding an implicit deny ALL because an allow'
+ ' entry is present')
+ policy_files.append(deny_file(access))
if deny_conf:
policy = deny_conf.find('policysetting')
data = policy.find('data')
@@ -79,10 +110,14 @@ class vgp_access_ext(gp_xml_ext):
name = adobject.find('name').text
domain = adobject.find('domain').text
entries.append('-:%s\\%s:ALL' % (domain, name))
+ if len(allow_listelements) > 0:
+ log.warn("Deny entry '%s' is meaningless with "
+ "allow present" % entries[-1])
if len(entries) == 0:
continue
conf_id = select_next_conf(access)
access_file = os.path.join(access, '%010d_gp.conf' % conf_id)
+ policy_files.append(access_file)
access_contents = '\n'.join(entries)
attribute = blake2b(get_bytes(access_contents)).hexdigest()
old_val = self.gp_db.retrieve(str(self), attribute)
@@ -96,7 +131,7 @@ class vgp_access_ext(gp_xml_ext):
w.write(access_contents)
os.chmod(f.name, 0o644)
os.rename(f.name, access_file)
- self.gp_db.store(str(self), attribute, access_file)
+ self.gp_db.store(str(self), attribute, ':'.join(policy_files))
self.gp_db.commit()
def rsop(self, gpo):
@@ -113,13 +148,16 @@ class vgp_access_ext(gp_xml_ext):
if allow_conf:
policy = allow_conf.find('policysetting')
data = policy.find('data')
- for listelement in data.findall('listelement'):
+ allow_listelements = data.findall('listelement')
+ for listelement in allow_listelements:
adobject = listelement.find('adobject')
name = adobject.find('name').text
domain = adobject.find('domain').text
if str(self) not in output.keys():
output[str(self)] = []
output[str(self)].append('+:%s\\%s:ALL' % (name, domain))
+ if len(allow_listelements) > 0:
+ output[str(self)].append('-:ALL:ALL')
if deny_conf:
policy = deny_conf.find('policysetting')
data = policy.find('data')
diff --git a/python/samba/netcmd/gpo.py b/python/samba/netcmd/gpo.py
index eefce9d8d0e..9cd08273aaa 100644
--- a/python/samba/netcmd/gpo.py
+++ b/python/samba/netcmd/gpo.py
@@ -1847,6 +1847,42 @@ samba-tool gpo manage sudoers list {31B2F340-016D-11D2-945F-00C04FB984F9}
'SudoersConfiguration\\manifest.xml'])
try:
xml_data = ET.fromstring(conn.loadfile(vgp_xml))
+ except NTSTATUSError as e:
+ # STATUS_OBJECT_NAME_INVALID, STATUS_OBJECT_NAME_NOT_FOUND,
+ # STATUS_OBJECT_PATH_NOT_FOUND
+ if e.args[0] in [0xC0000033, 0xC0000034, 0xC000003A]:
+ # The file doesn't exist, so there is nothing to list
+ xml_data = None
+ elif e.args[0] == 0xC0000022: # STATUS_ACCESS_DENIED
+ raise CommandError("The authenticated user does "
+ "not have sufficient privileges")
+ else:
+ raise
+
+ if xml_data is not None:
+ policy = xml_data.find('policysetting')
+ data = policy.find('data')
+ for entry in data.findall('sudoers_entry'):
+ command = entry.find('command').text
+ user = entry.find('user').text
+ listelements = entry.findall('listelement')
+ principals = []
+ for listelement in listelements:
+ principals.extend(listelement.findall('principal'))
+ if len(principals) > 0:
+ uname = ','.join([u.text if u.attrib['type'] == 'user' \
+ else '%s%%' % u.text for u in principals])
+ else:
+ uname = 'ALL'
+ nopassword = entry.find('password') is None
+ np_entry = ' NOPASSWD:' if nopassword else ''
+ p = '%s ALL=(%s)%s %s' % (uname, user, np_entry, command)
+ self.outf.write('%s\n' % p)
+
+ pol_file = '\\'.join([realm.lower(), 'Policies', gpo,
+ 'MACHINE\\Registry.pol'])
+ try:
+ pol_data = ndr_unpack(preg.file, conn.loadfile(pol_file))
except NTSTATUSError as e:
# STATUS_OBJECT_NAME_INVALID, STATUS_OBJECT_NAME_NOT_FOUND,
# STATUS_OBJECT_PATH_NOT_FOUND
@@ -1857,24 +1893,12 @@ samba-tool gpo manage sudoers list {31B2F340-016D-11D2-945F-00C04FB984F9}
"not have sufficient privileges")
raise
- policy = xml_data.find('policysetting')
- data = policy.find('data')
- for entry in data.findall('sudoers_entry'):
- command = entry.find('command').text
- user = entry.find('user').text
- listelements = entry.findall('listelement')
- principals = []
- for listelement in listelements:
- principals.extend(listelement.findall('principal'))
- if len(principals) > 0:
- uname = ','.join([u.text if u.attrib['type'] == 'user' \
- else '%s%%' % u.text for u in principals])
- else:
- uname = 'ALL'
- nopassword = entry.find('password') is None
- np_entry = ' NOPASSWD:' if nopassword else ''
- p = '%s ALL=(%s)%s %s' % (uname, user, np_entry, command)
- self.outf.write('%s\n' % p)
+ # Also list the policies set from the GPME
+ keyname = b'Software\\Policies\\Samba\\Unix Settings\\Sudo Rights'
+ for entry in pol_data.entries:
+ if get_bytes(entry.keyname) == keyname and \
+ get_string(entry.data).strip():
+ self.outf.write('%s\n' % entry.data)
class cmd_remove_sudoers(Command):
"""Removes a Samba Sudoers Group Policy from the sysvol
@@ -1931,14 +1955,30 @@ samba-tool gpo manage sudoers remove {31B2F340-016D-11D2-945F-00C04FB984F9} 'fak
# STATUS_OBJECT_NAME_INVALID, STATUS_OBJECT_NAME_NOT_FOUND,
# STATUS_OBJECT_PATH_NOT_FOUND
if e.args[0] in [0xC0000033, 0xC0000034, 0xC000003A]:
- raise CommandError("The specified entry does not exist")
+ data = None
elif e.args[0] == 0xC0000022: # STATUS_ACCESS_DENIED
raise CommandError("The authenticated user does "
"not have sufficient privileges")
- raise
+ else:
+ raise
+
+ pol_file = '\\'.join([realm.lower(), 'Policies', gpo,
+ 'MACHINE\\Registry.pol'])
+ try:
+ pol_data = ndr_unpack(preg.file, conn.loadfile(pol_file))
+ except NTSTATUSError as e:
+ # STATUS_OBJECT_NAME_INVALID, STATUS_OBJECT_NAME_NOT_FOUND,
+ # STATUS_OBJECT_PATH_NOT_FOUND
+ if e.args[0] in [0xC0000033, 0xC0000034, 0xC000003A]:
+ pol_data = None
+ elif e.args[0] == 0xC0000022: # STATUS_ACCESS_DENIED
+ raise CommandError("The authenticated user does "
+ "not have sufficient privileges")
+ else:
+ raise
entries = {}
- for e in data.findall('sudoers_entry'):
+ for e in data.findall('sudoers_entry') if data else []:
command = e.find('command').text
user = e.find('user').text
listelements = e.findall('listelement')
@@ -1955,23 +1995,35 @@ samba-tool gpo manage sudoers remove {31B2F340-016D-11D2-945F-00C04FB984F9} 'fak
p = '%s ALL=(%s)%s %s' % (uname, user, np_entry, command)
entries[p] = e
- if entry not in entries.keys():
- raise CommandError("Cannot remove '%s' because it does not exist" %
- entry)
+ if entry in entries.keys():
+ data.remove(entries[entry])
- data.remove(entries[entry])
+ out = BytesIO()
+ xml_data.write(out, encoding='UTF-8', xml_declaration=True)
+ out.seek(0)
+ try:
+ create_directory_hier(conn, vgp_dir)
+ conn.savefile(vgp_xml, out.read())
+ except NTSTATUSError as e:
+ if e.args[0] == 0xC0000022: # STATUS_ACCESS_DENIED
+ raise CommandError("The authenticated user does "
+ "not have sufficient privileges")
+ raise
+ elif entry in ([e.data for e in pol_data.entries] if pol_data else []):
+ entries = [e for e in pol_data.entries if e.data != entry]
+ pol_data.num_entries = len(entries)
+ pol_data.entries = entries
- out = BytesIO()
- xml_data.write(out, encoding='UTF-8', xml_declaration=True)
- out.seek(0)
- try:
- create_directory_hier(conn, vgp_dir)
- conn.savefile(vgp_xml, out.read())
- except NTSTATUSError as e:
- if e.args[0] == 0xC0000022: # STATUS_ACCESS_DENIED
- raise CommandError("The authenticated user does "
- "not have sufficient privileges")
- raise
+ try:
+ conn.savefile(pol_file, ndr_pack(pol_data))
+ except NTSTATUSError as e:
+ if e.args[0] == 0xC0000022: # STATUS_ACCESS_DENIED
+ raise CommandError("The authenticated user does "
+ "not have sufficient privileges")
+ raise
+ else:
+ raise CommandError("Cannot remove '%s' because it does not exist" %
+ entry)
class cmd_sudoers(SuperCommand):
"""Manage Sudoers Group Policy Objects"""
@@ -3763,7 +3815,8 @@ class cmd_add_access(Command):
"""Adds a VGP Host Access Group Policy to the sysvol
This command adds a host access setting to the sysvol for applying to winbind
-clients.
+clients. Any time an allow entry is detected by the client, an implicit deny
+ALL will be assumed.
Example:
samba-tool gpo manage access add {31B2F340-016D-11D2-945F-00C04FB984F9} allow goodguy example.com
@@ -3864,13 +3917,11 @@ samba-tool gpo manage access add {31B2F340-016D-11D2-945F-00C04FB984F9} allow go
etype = ET.SubElement(listelement, 'type')
etype.text = objectclass.upper()
entry = ET.SubElement(listelement, 'entry')
- if objectclass == 'user':
- entry.text = get_string(res[0]['userPrincipalName'][-1])
- else:
+ entry.text = '%s\\%s' % (samdb.domain_netbios_name(),
+ get_string(res[0]['samaccountname'][-1]))
+ if objectclass == 'group':
groupattr = ET.SubElement(data, 'groupattr')
groupattr.text = 'samAccountName'
- entry.text = '%s\\%s' % (domain,
- get_string(res[0]['samaccountname'][-1]))
adobject = ET.SubElement(listelement, 'adobject')
name = ET.SubElement(adobject, 'name')
name.text = get_string(res[0]['samaccountname'][-1])
diff --git a/python/samba/tests/gpo.py b/python/samba/tests/gpo.py
index 7ba193884a1..9873f13c1e3 100644
--- a/python/samba/tests/gpo.py
+++ b/python/samba/tests/gpo.py
@@ -8543,8 +8543,11 @@ class GPOTests(tests.TestCase):
with TemporaryDirectory() as dname:
ext.process_group_policy([], gpos, dname)
conf = os.listdir(dname)
- self.assertEquals(len(conf), 1, 'The conf file was not created')
- gp_cfg = os.path.join(dname, conf[0])
+ # There will be 2 files, the policy file and the deny file
+ self.assertEquals(len(conf), 2, 'The conf file was not created')
+ # Ignore the DENY_ALL conf file
+ gp_cfg = os.path.join(dname,
+ [c for c in conf if '_gp_DENY_ALL.conf' not in c][0])
# Check the access config for the correct access.conf entries
print('Config file %s found' % gp_cfg)
diff --git a/python/samba/tests/samba_tool/gpo.py b/python/samba/tests/samba_tool/gpo.py
index d60e5b96c34..78ed5d493af 100644
--- a/python/samba/tests/samba_tool/gpo.py
+++ b/python/samba/tests/samba_tool/gpo.py
@@ -730,6 +730,24 @@ class GpoCmdTestCase(SambaToolCmdTest):
self.assertFalse(inf_data.has_section('Kerberos Policy'))
def test_sudoers_add(self):
+ lp = LoadParm()
+ lp.load(os.environ['SERVERCONFFILE'])
+ local_path = lp.get('path', 'sysvol')
+ reg_pol = os.path.join(local_path, lp.get('realm').lower(), 'Policies',
+ self.gpo_guid, 'Machine/Registry.pol')
+
+ # Stage the Registry.pol file with test data
+ stage = preg.file()
+ e = preg.entry()
+ e.keyname = b'Software\\Policies\\Samba\\Unix Settings\\Sudo Rights'
+ e.valuename = b'Software\\Policies\\Samba\\Unix Settings'
+ e.type = 1
+ e.data = b'fakeu ALL=(ALL) NOPASSWD: ALL'
+ stage.num_entries = 1
+ stage.entries = [e]
+ ret = stage_file(reg_pol, ndr_pack(stage))
+ self.assertTrue(ret, 'Could not create the target %s' % reg_pol)
+
(result, out, err) = self.runsublevelcmd("gpo", ("manage",
"sudoers", "add"),
self.gpo_guid, 'ALL', 'ALL',
@@ -751,6 +769,7 @@ class GpoCmdTestCase(SambaToolCmdTest):
(os.environ["USERNAME"],
os.environ["PASSWORD"]))
self.assertIn(sudoer, out, 'The test entry was not found!')
+ self.assertIn(get_string(e.data), out, 'The test entry was not found!')
(result, out, err) = self.runsublevelcmd("gpo", ("manage",
"sudoers", "remove"),
@@ -762,6 +781,17 @@ class GpoCmdTestCase(SambaToolCmdTest):
os.environ["PASSWORD"]))
self.assertCmdSuccess(result, out, err, 'Sudoers remove failed')
+ (result, out, err) = self.runsublevelcmd("gpo", ("manage",
+ "sudoers", "remove"),
+ self.gpo_guid,
+ get_string(e.data),
+ "-H", "ldap://%s" %
+ os.environ["SERVER"],
+ "-U%s%%%s" %
+ (os.environ["USERNAME"],
+ os.environ["PASSWORD"]))
+ self.assertCmdSuccess(result, out, err, 'Sudoers remove failed')
+
(result, out, err) = self.runsublevelcmd("gpo", ("manage",
"sudoers", "list"),
self.gpo_guid, "-H",
@@ -771,6 +801,11 @@ class GpoCmdTestCase(SambaToolCmdTest):
(os.environ["USERNAME"],
os.environ["PASSWORD"]))
self.assertNotIn(sudoer, out, 'The test entry was still found!')
+ self.assertNotIn(get_string(e.data), out,
+ 'The test entry was still found!')
+
+ # Unstage the Registry.pol file
+ unstage_file(reg_pol)
def test_sudoers_list(self):
lp = LoadParm()
@@ -825,6 +860,21 @@ class GpoCmdTestCase(SambaToolCmdTest):
ret = stage_file(vgp_xml, etree.tostring(stage, 'utf-8'))
self.assertTrue(ret, 'Could not create the target %s' % vgp_xml)
+ reg_pol = os.path.join(local_path, lp.get('realm').lower(), 'Policies',
+ self.gpo_guid, 'Machine/Registry.pol')
+
+ # Stage the Registry.pol file with test data
+ stage = preg.file()
+ e = preg.entry()
+ e.keyname = b'Software\\Policies\\Samba\\Unix Settings\\Sudo Rights'
+ e.valuename = b'Software\\Policies\\Samba\\Unix Settings'
+ e.type = 1
+ e.data = b'fakeu3 ALL=(ALL) NOPASSWD: ALL'
+ stage.num_entries = 1
+ stage.entries = [e]
+ ret = stage_file(reg_pol, ndr_pack(stage))
+ self.assertTrue(ret, 'Could not create the target %s' % reg_pol)
+
sudoer = 'fakeu ALL=(ALL) NOPASSWD: ALL'
sudoer2 = 'fakeu2,fakeg2% ALL=(ALL) NOPASSWD: ALL'
sudoer_no_principal = 'ALL ALL=(ALL) NOPASSWD: ALL'
@@ -839,6 +889,7 @@ class GpoCmdTestCase(SambaToolCmdTest):
self.assertCmdSuccess(result, out, err, 'Sudoers list failed')
self.assertIn(sudoer, out, 'The test entry was not found!')
self.assertIn(sudoer2, out, 'The test entry was not found!')
+ self.assertIn(get_string(e.data), out, 'The test entry was not found!')
self.assertIn(sudoer_no_principal, out,
'The test entry was not found!')
@@ -877,6 +928,8 @@ class GpoCmdTestCase(SambaToolCmdTest):
# Unstage the manifest.xml file
unstage_file(vgp_xml)
+ # Unstage the Registry.pol file
+ unstage_file(reg_pol)
def test_symlink_list(self):
lp = LoadParm()
--
Samba Shared Repository
More information about the samba-cvs
mailing list