The branch, master has been updated via 0bd4bc40f4a samba-tool: Check specified domain and realm against our own via 3dccf63e82b samba-tool: Return correct result for _get_user_realm_domain() via 52f9629408e samba-tool delegation: Clarify msDS-AllowedToDelegateTo delegation command documentation via 9a480f274b6 samba-tool delegation: Add commands to add/remove principals for RBCD via 572f90bdefc samba-tool delegation show: Display information for RBCD via e4ea06ec242 samba-tool delegation: Add function to display security descriptor for RBCD via bd1fd3de5cc s4:selftest: Remove ad_dc_ntvfs env from several tests from 67294a23b97 testprogs: A PKINIT PAC test which runs against Heimdal and MIT Kerberos
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 0bd4bc40f4ad29446577d23e84e059e5bb1e5de5 Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Thu Feb 24 11:05:57 2022 +1300 samba-tool: Check specified domain and realm against our own Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Mon Mar 28 03:11:51 UTC 2022 on sn-devel-184 commit 3dccf63e82b38988828001a1d7f3a5a7b24a73de Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Thu Feb 24 10:07:35 2022 +1300 samba-tool: Return correct result for _get_user_realm_domain() We were returning the realm and the domain in the wrong order. Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 52f9629408e7674f28a90d030c475178d644e192 Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Mon Feb 21 14:58:47 2022 +1300 samba-tool delegation: Clarify msDS-AllowedToDelegateTo delegation command documentation This makes the difference between msDS-AllowedToDelegateTo and msDS-AllowedToActOnBehalfOfOtherIdentity more clear. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14954 Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 9a480f274b62d8e491bcd54bfd189099729ff57a Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Mon Feb 21 14:58:30 2022 +1300 samba-tool delegation: Add commands to add/remove principals for RBCD These commands allow updating the msDS-AllowedToActOnBehalfOfOtherIdentity attribute with principals allowed to delegate to an account. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14954 Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 572f90bdefcde13611fe50b7a5228fd6e3db2117 Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Mon Feb 21 15:07:50 2022 +1300 samba-tool delegation show: Display information for RBCD BUG: https://bugzilla.samba.org/show_bug.cgi?id=14954 Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit e4ea06ec242e6f26a5d997d0ba992bc0d2437cba Author: Joseph Sutton <josephsut...@catalyst.net.nz> Date: Mon Feb 21 14:56:45 2022 +1300 samba-tool delegation: Add function to display security descriptor for RBCD We also check some features of the security descriptor, and display warnings if they are not as expected. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14954 Signed-off-by: Joseph Sutton <josephsut...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit bd1fd3de5cc1ee83bb5164277de714a61b0fd544 Author: Andreas Schneider <a...@samba.org> Date: Sat Mar 26 08:42:21 2022 +0100 s4:selftest: Remove ad_dc_ntvfs env from several tests It doesn't make sense to run tests against ad_dc and ad_dc_ntvfs in those cases. Signed-off-by: Andreas Schneider <a...@samba.org> Reviewed-by: Andrew Bartlett <abart...@samba.org> ----------------------------------------------------------------------- Summary of changes: python/samba/netcmd/common.py | 29 ++- python/samba/netcmd/delegation.py | 393 +++++++++++++++++++++++++++++++++++++- python/samba/netcmd/spn.py | 4 +- source4/selftest/tests.py | 6 +- 4 files changed, 413 insertions(+), 19 deletions(-) Changeset truncated at 500 lines: diff --git a/python/samba/netcmd/common.py b/python/samba/netcmd/common.py index bb17bfa10f2..4cdccd073ba 100644 --- a/python/samba/netcmd/common.py +++ b/python/samba/netcmd/common.py @@ -20,6 +20,7 @@ import re from samba.dcerpc import nbt from samba.net import Net +from samba.netcmd import CommandError import ldb @@ -27,26 +28,44 @@ import ldb NEVER_TIMESTAMP = int(-0x8000000000000000) -def _get_user_realm_domain(user): +def _get_user_realm_domain(user, sam=None): r""" get the realm or the domain and the base user from user like: * username * DOMAIN\username * username@REALM + + A SamDB object can also be passed in to check + our domain or realm against the obtained ones. """ baseuser = user - realm = "" - domain = "" m = re.match(r"(\w+)\\(\w+$)", user) if m: domain = m.group(1) baseuser = m.group(2) - return (baseuser.lower(), domain.upper(), realm) + + if sam is not None: + our_domain = sam.domain_netbios_name() + if domain.lower() != our_domain.lower(): + raise CommandError(f"Given domain '{domain}' does not match " + f"our domain '{our_domain}'!") + + return (baseuser.lower(), "", domain.upper()) + + realm = "" m = re.match(r"(\w+)@(\w+)", user) if m: baseuser = m.group(1) realm = m.group(2) - return (baseuser.lower(), domain, realm.upper()) + + if sam is not None: + our_realm = sam.domain_dns_name() + our_realm_initial = our_realm.split('.', 1)[0] + if realm.lower() != our_realm_initial.lower(): + raise CommandError(f"Given realm '{realm}' does not match our " + f"realm '{our_realm}'!") + + return (baseuser.lower(), realm.upper(), "") def netcmd_dnsname(lp): diff --git a/python/samba/netcmd/delegation.py b/python/samba/netcmd/delegation.py index 54ba71bdf41..35a91aca458 100644 --- a/python/samba/netcmd/delegation.py +++ b/python/samba/netcmd/delegation.py @@ -24,6 +24,8 @@ from samba import provision from samba import dsdb from samba.samdb import SamDB from samba.auth import system_session +from samba.dcerpc import security +from samba.ndr import ndr_pack, ndr_unpack from samba.netcmd.common import _get_user_realm_domain from samba.netcmd import ( Command, @@ -51,6 +53,89 @@ class cmd_delegation_show(Command): takes_args = ["accountname"] + def show_security_descriptor(self, sam, security_descriptor): + dacl = security_descriptor.dacl + desc_type = security_descriptor.type + + warning_info = ('Security Descriptor of attribute ' + 'msDS-AllowedToActOnBehalfOfOtherIdentity') + + if dacl is None or not desc_type & security.SEC_DESC_DACL_PRESENT: + self.errf.write(f'Warning: DACL not present in {warning_info}!\n') + return + + if not desc_type & security.SEC_DESC_SELF_RELATIVE: + self.errf.write(f'Warning: DACL in {warning_info} lacks ' + f'SELF_RELATIVE flag!\n') + return + + first = True + + for ace in dacl.aces: + trustee = ace.trustee + + # Convert the trustee SID into a DN if we can. + try: + res = sam.search(f'<SID={trustee}>', + scope=ldb.SCOPE_BASE) + except ldb.LdbError as err: + num, _ = err.args + if num != ldb.ERR_NO_SUCH_OBJECT: + raise + else: + if len(res) == 1: + trustee = res[0].dn + + ignore = False + + if (ace.type == security.SEC_ACE_TYPE_ACCESS_DENIED + or ace.type == security.SEC_ACE_TYPE_ACCESS_DENIED_OBJECT): + self.errf.write(f'Warning: ACE in {warning_info} denies ' + f'access for trustee {trustee}!\n') + # Ignore the ACE if it denies access + ignore = True + elif (ace.type != security.SEC_ACE_TYPE_ACCESS_ALLOWED + and ace.type != security.SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT): + # Ignore the ACE if it doesn't explicitly allow access + ignore = True + + inherit_only = ace.flags & security.SEC_ACE_FLAG_INHERIT_ONLY + object_inherit = ace.flags & security.SEC_ACE_FLAG_OBJECT_INHERIT + container_inherit = ( + ace.flags & security.SEC_ACE_FLAG_CONTAINER_INHERIT) + inherited_ace = ace.flags & security.SEC_ACE_FLAG_INHERITED_ACE + + if inherit_only and not object_inherit and not container_inherit: + # Ignore the ACE if it is propagated only to child objects, but + # neither of the object and container inherit flags are set. + ignore = True + else: + if container_inherit: + self.errf.write(f'Warning: ACE for trustee {trustee} has ' + f'unexpected CONTAINER_INHERIT flag set in ' + f'{warning_info}!\n') + ignore = True + + if inherited_ace: + self.errf.write(f'Warning: ACE for trustee {trustee} has ' + f'unexpected INHERITED_ACE flag set in ' + f'{warning_info}!\n') + ignore = True + + if not ace.access_mask: + # Ignore the ACE if it doesn't grant any permissions. + ignore = True + + if not ignore: + if first: + self.outf.write(f' Principals that may delegate to this ' + f'account:\n') + first = False + + self.outf.write(f'msDS-AllowedToActOnBehalfOfOtherIdentity: ' + f'{trustee}\n') + + def run(self, accountname, H=None, credopts=None, sambaopts=None, versionopts=None): lp = sambaopts.get_loadparm() creds = credopts.get_credentials(lp) @@ -65,18 +150,21 @@ class cmd_delegation_show(Command): credentials=creds, lp=lp) # TODO once I understand how, use the domain info to naildown # to the correct domain - (cleanedaccount, realm, domain) = _get_user_realm_domain(accountname) + (cleanedaccount, realm, domain) = _get_user_realm_domain(accountname, + sam) res = sam.search(expression="sAMAccountName=%s" % ldb.binary_encode(cleanedaccount), scope=ldb.SCOPE_SUBTREE, - attrs=["userAccountControl", "msDS-AllowedToDelegateTo"]) + attrs=["userAccountControl", "msDS-AllowedToDelegateTo", + "msDS-AllowedToActOnBehalfOfOtherIdentity"]) if len(res) == 0: raise CommandError("Unable to find account name '%s'" % accountname) assert(len(res) == 1) uac = int(res[0].get("userAccountControl")[0]) allowed = res[0].get("msDS-AllowedToDelegateTo") + allowed_from = res[0].get("msDS-AllowedToActOnBehalfOfOtherIdentity", idx=0) self.outf.write("Account-DN: %s\n" % str(res[0].dn)) self.outf.write("UF_TRUSTED_FOR_DELEGATION: %s\n" @@ -84,9 +172,19 @@ class cmd_delegation_show(Command): self.outf.write("UF_TRUSTED_TO_AUTHENTICATE_FOR_DELEGATION: %s\n" % bool(uac & dsdb.UF_TRUSTED_TO_AUTHENTICATE_FOR_DELEGATION)) - if allowed is not None: + if allowed: + self.outf.write(" Services this account may delegate to:\n") for a in allowed: self.outf.write("msDS-AllowedToDelegateTo: %s\n" % a) + if allowed_from is not None: + try: + security_descriptor = ndr_unpack(security.descriptor, allowed_from) + except RuntimeError: + self.errf.write("Warning: Security Descriptor of attribute " + "msDS-AllowedToActOnBehalfOfOtherIdentity " + "could not be unmarshalled!\n") + else: + self.show_security_descriptor(sam, security_descriptor) class cmd_delegation_for_any_service(Command): @@ -130,7 +228,8 @@ class cmd_delegation_for_any_service(Command): credentials=creds, lp=lp) # TODO once I understand how, use the domain info to naildown # to the correct domain - (cleanedaccount, realm, domain) = _get_user_realm_domain(accountname) + (cleanedaccount, realm, domain) = _get_user_realm_domain(accountname, + sam) search_filter = "sAMAccountName=%s" % ldb.binary_encode(cleanedaccount) flag = dsdb.UF_TRUSTED_FOR_DELEGATION @@ -183,7 +282,8 @@ class cmd_delegation_for_any_protocol(Command): credentials=creds, lp=lp) # TODO once I understand how, use the domain info to naildown # to the correct domain - (cleanedaccount, realm, domain) = _get_user_realm_domain(accountname) + (cleanedaccount, realm, domain) = _get_user_realm_domain(accountname, + sam) search_filter = "sAMAccountName=%s" % ldb.binary_encode(cleanedaccount) flag = dsdb.UF_TRUSTED_TO_AUTHENTICATE_FOR_DELEGATION @@ -196,7 +296,7 @@ class cmd_delegation_for_any_protocol(Command): class cmd_delegation_add_service(Command): - """Add a service principal as msDS-AllowedToDelegateTo.""" + """Add a service principal to msDS-AllowedToDelegateTo so that an account may delegate to it.""" synopsis = "%prog <accountname> <principal> [options]" @@ -228,7 +328,8 @@ class cmd_delegation_add_service(Command): credentials=creds, lp=lp) # TODO once I understand how, use the domain info to naildown # to the correct domain - (cleanedaccount, realm, domain) = _get_user_realm_domain(accountname) + (cleanedaccount, realm, domain) = _get_user_realm_domain(accountname, + sam) res = sam.search(expression="sAMAccountName=%s" % ldb.binary_encode(cleanedaccount), @@ -250,7 +351,7 @@ class cmd_delegation_add_service(Command): class cmd_delegation_del_service(Command): - """Delete a service principal as msDS-AllowedToDelegateTo.""" + """Delete a service principal from msDS-AllowedToDelegateTo so that an account may no longer delegate to it.""" synopsis = "%prog <accountname> <principal> [options]" @@ -282,7 +383,8 @@ class cmd_delegation_del_service(Command): credentials=creds, lp=lp) # TODO once I understand how, use the domain info to naildown # to the correct domain - (cleanedaccount, realm, domain) = _get_user_realm_domain(accountname) + (cleanedaccount, realm, domain) = _get_user_realm_domain(accountname, + sam) res = sam.search(expression="sAMAccountName=%s" % ldb.binary_encode(cleanedaccount), @@ -303,6 +405,277 @@ class cmd_delegation_del_service(Command): raise CommandError(err) +class cmd_delegation_add_principal(Command): + """Add a principal to msDS-AllowedToActOnBehalfOfOtherIdentity that may delegate to an account.""" + + synopsis = "%prog <accountname> <principal> [options]" + + takes_optiongroups = { + "sambaopts": options.SambaOptions, + "credopts": options.CredentialsOptions, + "versionopts": options.VersionOptions, + } + + takes_options = [ + Option("-H", "--URL", help="LDB URL for database or target server", + type=str, metavar="URL", dest="H"), + ] + + takes_args = ["accountname", "principal"] + + def run(self, accountname, principal, H=None, credopts=None, sambaopts=None, + versionopts=None): + + lp = sambaopts.get_loadparm() + creds = credopts.get_credentials(lp) + paths = provision.provision_paths_from_lp(lp, lp.get("realm")) + if H is None: + path = paths.samdb + else: + path = H + + sam = SamDB(path, session_info=system_session(), + credentials=creds, lp=lp) + # TODO once I understand how, use the domain info to naildown + # to the correct domain + cleanedaccount, _, _ = _get_user_realm_domain(accountname, sam) + + account_res = sam.search( + expression="sAMAccountName=%s" % + ldb.binary_encode(cleanedaccount), + scope=ldb.SCOPE_SUBTREE, + attrs=["msDS-AllowedToActOnBehalfOfOtherIdentity"]) + if len(account_res) == 0: + raise CommandError(f"Unable to find account name '{accountname}'") + assert(len(account_res) == 1) + + data = account_res[0].get( + "msDS-AllowedToActOnBehalfOfOtherIdentity", idx=0) + if data is None: + # Create the security descriptor if it is not present. + owner_sid = security.dom_sid(security.SID_BUILTIN_ADMINISTRATORS) + + security_desc = security.descriptor() + security_desc.revision = security.SD_REVISION + security_desc.type = (security.SEC_DESC_DACL_PRESENT | + security.SEC_DESC_SELF_RELATIVE) + security_desc.owner_sid = owner_sid + + dacl = None + else: + try: + security_desc = ndr_unpack(security.descriptor, data) + except RuntimeError: + raise CommandError(f"Security Descriptor of attribute " + f"msDS-AllowedToActOnBehalfOfOtherIdentity " + f"for account '{accountname}' could not be " + f"unmarshalled!") + + dacl = security_desc.dacl + + if dacl is None: + # Create the DACL if it is not present. + dacl = security.acl() + dacl.revision = security.SECURITY_ACL_REVISION_ADS + dacl.num_aces = 0 + + # TODO once I understand how, use the domain info to naildown + # to the correct domain + cleanedprinc, _, _ = _get_user_realm_domain(principal, sam) + + princ_res = sam.search(expression="sAMAccountName=%s" % + ldb.binary_encode(cleanedprinc), + scope=ldb.SCOPE_SUBTREE, + attrs=["objectSid"]) + if len(princ_res) == 0: + raise CommandError(f"Unable to find principal name '{principal}'") + assert(len(princ_res) == 1) + + princ_sid = security.dom_sid( + sam.schema_format_value( + "objectSID", + princ_res[0].get("objectSID", idx=0)).decode("utf-8")) + + aces = dacl.aces + + # Check that there is no existing ACE for this principal. + if any(ace.trustee == princ_sid for ace in aces): + raise CommandError( + f"ACE for principal '{principal}' already present in Security " + f"Descriptor of attribute " + f"msDS-AllowedToActOnBehalfOfOtherIdentity for account " + f"'{accountname}'.") + + # Create the new ACE. + ace = security.ace() + ace.type = security.SEC_ACE_TYPE_ACCESS_ALLOWED + ace.flags = 0 + ace.access_mask = security.SEC_ADS_GENERIC_ALL + ace.trustee = princ_sid + + aces.append(ace) + + dacl.aces = aces + dacl.num_aces += 1 + + security_desc.dacl = dacl + + new_data = ndr_pack(security_desc) + + # Set the new security descriptor. First, delete the original value to + # detect a race condition if someone else updates the attribute at the + # same time. + msg = ldb.Message() + msg.dn = account_res[0].dn + if data is not None: + msg["0"] = ldb.MessageElement( + data, ldb.FLAG_MOD_DELETE, + "msDS-AllowedToActOnBehalfOfOtherIdentity") + msg["1"] = ldb.MessageElement( + new_data, ldb.FLAG_MOD_ADD, + "msDS-AllowedToActOnBehalfOfOtherIdentity") + try: + sam.modify(msg) + except ldb.LdbError as err: + num, _ = err.args + if num == ldb.ERR_NO_SUCH_ATTRIBUTE: + raise CommandError( + f"Refused to update attribute " + f"msDS-AllowedToActOnBehalfOfOtherIdentity for account " + f"'{accountname}': a conflicting attribute update " + f"occurred simultaneously.") + else: + raise CommandError(err) + + +class cmd_delegation_del_principal(Command): + """Delete a principal from msDS-AllowedToActOnBehalfOfOtherIdentity that may no longer delegate to an account.""" + + synopsis = "%prog <accountname> <principal> [options]" + + takes_optiongroups = { + "sambaopts": options.SambaOptions, + "credopts": options.CredentialsOptions, + "versionopts": options.VersionOptions, + } + + takes_options = [ + Option("-H", "--URL", help="LDB URL for database or target server", + type=str, metavar="URL", dest="H"), + ] + + takes_args = ["accountname", "principal"] + + def run(self, accountname, principal, H=None, credopts=None, sambaopts=None, + versionopts=None): + + lp = sambaopts.get_loadparm() + creds = credopts.get_credentials(lp) + paths = provision.provision_paths_from_lp(lp, lp.get("realm")) + if H is None: + path = paths.samdb + else: + path = H + + sam = SamDB(path, session_info=system_session(), + credentials=creds, lp=lp) + # TODO once I understand how, use the domain info to naildown + # to the correct domain + cleanedaccount, _, _ = _get_user_realm_domain(accountname, sam) + + account_res = sam.search( + expression="sAMAccountName=%s" % + ldb.binary_encode(cleanedaccount), + scope=ldb.SCOPE_SUBTREE, + attrs=["msDS-AllowedToActOnBehalfOfOtherIdentity"]) + if len(account_res) == 0: + raise CommandError("Unable to find account name '%s'" % accountname) + assert(len(account_res) == 1) + + data = account_res[0].get( + "msDS-AllowedToActOnBehalfOfOtherIdentity", idx=0) + if data is None: + raise CommandError(f"Attribute " + f"msDS-AllowedToActOnBehalfOfOtherIdentity for " + f"account '{accountname}' not present!") + + try: + security_desc = ndr_unpack(security.descriptor, data) + except RuntimeError: + raise CommandError(f"Security Descriptor of attribute " + f"msDS-AllowedToActOnBehalfOfOtherIdentity for " + f"account '{accountname}' could not be " + f"unmarshalled!") + + dacl = security_desc.dacl + if dacl is None: + raise CommandError(f"DACL not present on Security Descriptor of " + f"attribute " + f"msDS-AllowedToActOnBehalfOfOtherIdentity for " + f"account '{accountname}'!") + + # TODO once I understand how, use the domain info to naildown + # to the correct domain + cleanedprinc, _, _ = _get_user_realm_domain(principal, sam) + + princ_res = sam.search(expression="sAMAccountName=%s" % + ldb.binary_encode(cleanedprinc), + scope=ldb.SCOPE_SUBTREE, + attrs=["objectSid"]) + if len(princ_res) == 0: + raise CommandError(f"Unable to find principal name '{principal}'") + assert(len(princ_res) == 1) + + princ_sid = security.dom_sid( + sam.schema_format_value( + "objectSID", + princ_res[0].get("objectSID", idx=0)).decode("utf-8")) + + old_aces = dacl.aces + -- Samba Shared Repository