Version in base suite: 4.17.11+dfsg-0+deb12u1 Base version: samba_4.17.11+dfsg-0+deb12u1 Target version: samba_4.17.12+dfsg-0+deb12u1 Base file: /srv/ftp-master.debian.org/ftp/pool/main/s/samba/samba_4.17.11+dfsg-0+deb12u1.dsc Target file: /srv/ftp-master.debian.org/policy/pool/main/s/samba/samba_4.17.12+dfsg-0+deb12u1.dsc VERSION | 2 WHATSNEW.txt | 87 +++ debian/changelog | 23 docs-xml/smbdotconf/protocol/dcerpcendpointservers.xml | 2 lib/param/loadparm.c | 2 lib/replace/replace.h | 15 libcli/security/security_descriptor.c | 121 ++++ libcli/security/security_descriptor.h | 10 python/samba/ndr.py | 19 python/samba/sd_utils.py | 153 +++++ selftest/knownfail | 2 selftest/knownfail.d/dirsync | 13 selftest/target/Samba4.pm | 2 source3/param/loadparm.c | 2 source3/rpc_client/local_np.c | 13 source3/rpc_server/rpc_host.c | 154 ----- source3/rpc_server/rpcd_classic.c | 45 + source3/rpc_server/rpcd_epmapper.c | 33 + source3/rpc_server/rpcd_lsad.c | 21 source3/rpc_server/rpcd_rpcecho.c | 33 + source3/rpc_server/wscript_build | 1 source3/selftest/tests.py | 15 source3/smbd/open.c | 4 source3/torture/proto.h | 1 source3/torture/test_smb2.c | 105 +++ source3/torture/torture.c | 4 source4/dsdb/samdb/ldb_modules/dirsync.c | 33 - source4/dsdb/samdb/samdb.h | 1 source4/dsdb/tests/python/acl.py | 12 source4/dsdb/tests/python/ad_dc_search_performance.py | 2 source4/dsdb/tests/python/confidential_attr.py | 212 ++----- source4/dsdb/tests/python/dirsync.py | 473 ++++++++++++++--- source4/dsdb/tests/python/ldap.py | 14 source4/dsdb/tests/python/ldap_modify_order.py | 4 source4/dsdb/tests/python/ldap_syntaxes.py | 4 source4/dsdb/tests/python/login_basics.py | 2 source4/dsdb/tests/python/password_settings.py | 4 source4/dsdb/tests/python/passwords.py | 4 source4/dsdb/tests/python/sam.py | 2 source4/dsdb/tests/python/sec_descriptor.py | 14 source4/dsdb/tests/python/token_group.py | 4 source4/dsdb/tests/python/user_account_control.py | 2 source4/librpc/ndr/py_security.c | 62 ++ source4/rpc_server/wscript_build | 3 source4/torture/smb2/acls.c | 143 +++++ 45 files changed, 1434 insertions(+), 443 deletions(-) diff -Nru samba-4.17.11+dfsg/VERSION samba-4.17.12+dfsg/VERSION --- samba-4.17.11+dfsg/VERSION 2023-09-07 08:59:49.021451000 +0000 +++ samba-4.17.12+dfsg/VERSION 2023-10-10 08:44:10.129233600 +0000 @@ -25,7 +25,7 @@ ######################################################## SAMBA_VERSION_MAJOR=4 SAMBA_VERSION_MINOR=17 -SAMBA_VERSION_RELEASE=11 +SAMBA_VERSION_RELEASE=12 ######################################################## # If a official release has a serious bug # diff -Nru samba-4.17.11+dfsg/WHATSNEW.txt samba-4.17.12+dfsg/WHATSNEW.txt --- samba-4.17.11+dfsg/WHATSNEW.txt 2023-09-07 08:59:49.021451000 +0000 +++ samba-4.17.12+dfsg/WHATSNEW.txt 2023-10-10 08:41:03.820061200 +0000 @@ -1,4 +1,88 @@ =============================== + Release Notes for Samba 4.17.12 + October 10, 2023 + =============================== + + +This is a security release in order to address the following defects: + + +o CVE-2023-3961: Unsanitized pipe names allow SMB clients to connect as root to + existing unix domain sockets on the file system. + https://www.samba.org/samba/security/CVE-2023-3961.html + +o CVE-2023-4091: SMB client can truncate files to 0 bytes by opening files with + OVERWRITE disposition when using the acl_xattr Samba VFS + module with the smb.conf setting + "acl_xattr:ignore system acls = yes" + https://www.samba.org/samba/security/CVE-2023-4091.html + +o CVE-2023-4154: An RODC and a user with the GET_CHANGES right can view all + attributes, including secrets and passwords. Additionally, + the access check fails open on error conditions. + https://www.samba.org/samba/security/CVE-2023-4154.html + +o CVE-2023-42669: Calls to the rpcecho server on the AD DC can request that the + server block for a user-defined amount of time, denying + service. + https://www.samba.org/samba/security/CVE-2023-42669.html + +o CVE-2023-42670: Samba can be made to start multiple incompatible RPC + listeners, disrupting service on the AD DC. + https://www.samba.org/samba/security/CVE-2023-42670.html + + +Changes since 4.17.11 +--------------------- + +o Jeremy Allison + * BUG 15422: CVE-2023-3961. + +o Andrew Bartlett + * BUG 15424: CVE-2023-4154. + * BUG 15473: CVE-2023-42670. + * BUG 15474: CVE-2023-42669. + +o Ralph Boehme + * BUG 15439: CVE-2023-4091. + +o Christian Merten + * BUG 15424: CVE-2023-4154. + +o Stefan Metzmacher + * BUG 15424: CVE-2023-4154. + +o Andreas Schneider + * BUG 15424: CVE-2023-4154. + +o Joseph Sutton + * BUG 15424: CVE-2023-4154. + + +####################################### +Reporting bugs & Development Discussion +####################################### + +Please discuss this release on the samba-technical mailing list or by +joining the #samba-technical:matrix.org matrix room, or +#samba-technical IRC channel on irc.libera.chat. + +If you do report problems then please try to send high quality +feedback. If you don't provide vital information to help us track down +the problem then you will probably be ignored. All bug reports should +be filed under the Samba 4.1 and newer product in the project's Bugzilla +database (https://bugzilla.samba.org/). + + +====================================================================== +== Our Code, Our Bugs, Our Responsibility. +== The Samba Team +====================================================================== + + +Release notes for older releases follow: +---------------------------------------- + =============================== Release Notes for Samba 4.17.11 September 07, 2023 =============================== @@ -85,8 +169,7 @@ ====================================================================== -Release notes for older releases follow: ----------------------------------------- +---------------------------------------------------------------------- =============================== Release Notes for Samba 4.17.10 July 19, 2023 diff -Nru samba-4.17.11+dfsg/debian/changelog samba-4.17.12+dfsg/debian/changelog --- samba-4.17.11+dfsg/debian/changelog 2023-09-12 12:55:41.000000000 +0000 +++ samba-4.17.12+dfsg/debian/changelog 2023-10-10 15:17:19.000000000 +0000 @@ -1,3 +1,26 @@ +samba (2:4.17.12+dfsg-0+deb12u1) bookworm-security; urgency=medium + + * new stable security bugfix release: + o CVE-2023-3961: https://www.samba.org/samba/security/CVE-2023-3961.html + Unsanitized pipe names allow SMB clients to connect as root + to existing unix domain sockets on the file system. + o CVE-2023-4091: https://www.samba.org/samba/security/CVE-2023-4091.html + SMB client can truncate files to 0 bytes by opening files with OVERWRITE + disposition when using the acl_xattr Samba VFS module with the smb.conf + setting "acl_xattr:ignore system acls = yes" + o CVE-2023-4154: https://www.samba.org/samba/security/CVE-2023-4154.html + An RODC and a user with the GET_CHANGES right can view all attributes, + including secrets and passwords. Additionally, the access check fails + open on error conditions. + o CVE-2023-42669: https://www.samba.org/samba/security/CVE-2023-42669.html + Calls to the rpcecho server on the AD DC can request that the server + block for a user-defined amount of time, denying service. + o CVE-2023-42670: https://www.samba.org/samba/security/CVE-2023-42670.html + Samba can be made to start multiple incompatible RPC listeners, + disrupting service on the AD DC. + + -- Michael Tokarev Tue, 10 Oct 2023 18:17:19 +0300 + samba (2:4.17.11+dfsg-0+deb12u1) bookworm; urgency=medium * new upstream stable/bugfix release 4.17.11, including: diff -Nru samba-4.17.11+dfsg/docs-xml/smbdotconf/protocol/dcerpcendpointservers.xml samba-4.17.12+dfsg/docs-xml/smbdotconf/protocol/dcerpcendpointservers.xml --- samba-4.17.11+dfsg/docs-xml/smbdotconf/protocol/dcerpcendpointservers.xml 2022-08-08 14:15:39.024189500 +0000 +++ samba-4.17.12+dfsg/docs-xml/smbdotconf/protocol/dcerpcendpointservers.xml 2023-10-09 20:16:08.451592000 +0000 @@ -6,6 +6,6 @@ Specifies which DCE/RPC endpoint servers should be run. -epmapper, wkssvc, rpcecho, samr, netlogon, lsarpc, drsuapi, dssetup, unixinfo, browser, eventlog6, backupkey, dnsserver +epmapper, wkssvc, samr, netlogon, lsarpc, drsuapi, dssetup, unixinfo, browser, eventlog6, backupkey, dnsserver rpcecho diff -Nru samba-4.17.11+dfsg/lib/param/loadparm.c samba-4.17.12+dfsg/lib/param/loadparm.c --- samba-4.17.11+dfsg/lib/param/loadparm.c 2023-03-29 14:28:07.064394500 +0000 +++ samba-4.17.12+dfsg/lib/param/loadparm.c 2023-10-09 20:16:08.455592000 +0000 @@ -2732,7 +2732,7 @@ lpcfg_do_global_parameter(lp_ctx, "ntvfs handler", "unixuid default"); lpcfg_do_global_parameter(lp_ctx, "max connections", "0"); - lpcfg_do_global_parameter(lp_ctx, "dcerpc endpoint servers", "epmapper wkssvc rpcecho samr netlogon lsarpc drsuapi dssetup unixinfo browser eventlog6 backupkey dnsserver"); + lpcfg_do_global_parameter(lp_ctx, "dcerpc endpoint servers", "epmapper wkssvc samr netlogon lsarpc drsuapi dssetup unixinfo browser eventlog6 backupkey dnsserver"); lpcfg_do_global_parameter(lp_ctx, "server services", "s3fs rpc nbt wrepl ldap cldap kdc drepl winbindd ntp_signd kcc dnsupdate dns"); lpcfg_do_global_parameter(lp_ctx, "kccsrv:samba_kcc", "true"); /* the winbind method for domain controllers is for both RODC diff -Nru samba-4.17.11+dfsg/lib/replace/replace.h samba-4.17.12+dfsg/lib/replace/replace.h --- samba-4.17.11+dfsg/lib/replace/replace.h 2022-10-19 12:14:56.000196000 +0000 +++ samba-4.17.12+dfsg/lib/replace/replace.h 2023-10-09 20:16:08.131589700 +0000 @@ -890,6 +890,21 @@ if((i)<((n)-1)){memmove(&((a)[(i)]),&((a)[(i)+1]),(sizeof(*(a))*((n)-(i)-1)));} /** + * Insert an array element by moving the rest one up + * + */ +#define ARRAY_INSERT_ELEMENT(__array,__old_last_idx,__new_elem,__new_idx) do { \ + if ((__new_idx) < (__old_last_idx)) { \ + const void *__src = &((__array)[(__new_idx)]); \ + void *__dst = &((__array)[(__new_idx)+1]); \ + size_t __num = (__old_last_idx)-(__new_idx); \ + size_t __len = sizeof(*(__array)) * __num; \ + memmove(__dst, __src, __len); \ + } \ + (__array)[(__new_idx)] = (__new_elem); \ +} while(0) + +/** * Pointer difference macro */ #define PTR_DIFF(p1,p2) ((ptrdiff_t)(((const char *)(p1)) - (const char *)(p2))) diff -Nru samba-4.17.11+dfsg/libcli/security/security_descriptor.c samba-4.17.12+dfsg/libcli/security/security_descriptor.c --- samba-4.17.11+dfsg/libcli/security/security_descriptor.c 2022-08-08 14:15:39.188190700 +0000 +++ samba-4.17.12+dfsg/libcli/security/security_descriptor.c 2023-10-09 20:16:08.167590000 +0000 @@ -267,9 +267,11 @@ static NTSTATUS security_descriptor_acl_add(struct security_descriptor *sd, bool add_to_sacl, - const struct security_ace *ace) + const struct security_ace *ace, + ssize_t _idx) { struct security_acl *acl = NULL; + ssize_t idx; if (add_to_sacl) { acl = sd->sacl; @@ -288,15 +290,28 @@ acl->aces = NULL; } + if (_idx < 0) { + idx = (acl->num_aces + 1) + _idx; + } else { + idx = _idx; + } + + if (idx < 0) { + return NT_STATUS_ARRAY_BOUNDS_EXCEEDED; + } else if (idx > acl->num_aces) { + return NT_STATUS_ARRAY_BOUNDS_EXCEEDED; + } + acl->aces = talloc_realloc(acl, acl->aces, struct security_ace, acl->num_aces+1); if (acl->aces == NULL) { return NT_STATUS_NO_MEMORY; } - acl->aces[acl->num_aces] = *ace; + ARRAY_INSERT_ELEMENT(acl->aces, acl->num_aces, *ace, idx); + acl->num_aces++; - switch (acl->aces[acl->num_aces].type) { + switch (acl->aces[idx].type) { case SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT: case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT: case SEC_ACE_TYPE_SYSTEM_AUDIT_OBJECT: @@ -307,8 +322,6 @@ break; } - acl->num_aces++; - if (add_to_sacl) { sd->sacl = acl; sd->type |= SEC_DESC_SACL_PRESENT; @@ -327,7 +340,21 @@ NTSTATUS security_descriptor_sacl_add(struct security_descriptor *sd, const struct security_ace *ace) { - return security_descriptor_acl_add(sd, true, ace); + return security_descriptor_acl_add(sd, true, ace, -1); +} + +/* + insert an ACE at a given index to the SACL of a security_descriptor + + idx can be negative, which means it's related to the new size from the + end, so -1 means the ace is appended at the end. +*/ + +NTSTATUS security_descriptor_sacl_insert(struct security_descriptor *sd, + const struct security_ace *ace, + ssize_t idx) +{ + return security_descriptor_acl_add(sd, true, ace, idx); } /* @@ -337,7 +364,21 @@ NTSTATUS security_descriptor_dacl_add(struct security_descriptor *sd, const struct security_ace *ace) { - return security_descriptor_acl_add(sd, false, ace); + return security_descriptor_acl_add(sd, false, ace, -1); +} + +/* + insert an ACE at a given index to the DACL of a security_descriptor + + idx can be negative, which means it's related to the new size from the + end, so -1 means the ace is appended at the end. +*/ + +NTSTATUS security_descriptor_dacl_insert(struct security_descriptor *sd, + const struct security_ace *ace, + ssize_t idx) +{ + return security_descriptor_acl_add(sd, false, ace, idx); } /* @@ -420,6 +461,72 @@ } /* + delete the given ACE in the SACL or DACL of a security_descriptor +*/ +static NTSTATUS security_descriptor_acl_del_ace(struct security_descriptor *sd, + bool sacl_del, + const struct security_ace *ace) +{ + uint32_t i; + bool found = false; + struct security_acl *acl = NULL; + + if (sacl_del) { + acl = sd->sacl; + } else { + acl = sd->dacl; + } + + if (acl == NULL) { + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } + + for (i=0;inum_aces;i++) { + if (security_ace_equal(ace, &acl->aces[i])) { + ARRAY_DEL_ELEMENT(acl->aces, i, acl->num_aces); + acl->num_aces--; + if (acl->num_aces == 0) { + acl->aces = NULL; + } + found = true; + i--; + } + } + + if (!found) { + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } + + acl->revision = SECURITY_ACL_REVISION_NT4; + + for (i=0;inum_aces;i++) { + switch (acl->aces[i].type) { + case SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT: + case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT: + case SEC_ACE_TYPE_SYSTEM_AUDIT_OBJECT: + case SEC_ACE_TYPE_SYSTEM_ALARM_OBJECT: + acl->revision = SECURITY_ACL_REVISION_ADS; + return NT_STATUS_OK; + default: + break; /* only for the switch statement */ + } + } + + return NT_STATUS_OK; +} + +NTSTATUS security_descriptor_dacl_del_ace(struct security_descriptor *sd, + const struct security_ace *ace) +{ + return security_descriptor_acl_del_ace(sd, false, ace); +} + +NTSTATUS security_descriptor_sacl_del_ace(struct security_descriptor *sd, + const struct security_ace *ace) +{ + return security_descriptor_acl_del_ace(sd, true, ace); +} +/* compare two security ace structures */ bool security_ace_equal(const struct security_ace *ace1, diff -Nru samba-4.17.11+dfsg/libcli/security/security_descriptor.h samba-4.17.12+dfsg/libcli/security/security_descriptor.h --- samba-4.17.11+dfsg/libcli/security/security_descriptor.h 2022-08-08 14:15:39.188190700 +0000 +++ samba-4.17.12+dfsg/libcli/security/security_descriptor.h 2023-10-09 20:16:08.167590000 +0000 @@ -33,12 +33,22 @@ struct security_descriptor **_csd); NTSTATUS security_descriptor_sacl_add(struct security_descriptor *sd, const struct security_ace *ace); +NTSTATUS security_descriptor_sacl_insert(struct security_descriptor *sd, + const struct security_ace *ace, + ssize_t idx); NTSTATUS security_descriptor_dacl_add(struct security_descriptor *sd, const struct security_ace *ace); +NTSTATUS security_descriptor_dacl_insert(struct security_descriptor *sd, + const struct security_ace *ace, + ssize_t idx); NTSTATUS security_descriptor_dacl_del(struct security_descriptor *sd, const struct dom_sid *trustee); NTSTATUS security_descriptor_sacl_del(struct security_descriptor *sd, const struct dom_sid *trustee); +NTSTATUS security_descriptor_dacl_del_ace(struct security_descriptor *sd, + const struct security_ace *ace); +NTSTATUS security_descriptor_sacl_del_ace(struct security_descriptor *sd, + const struct security_ace *ace); bool security_ace_equal(const struct security_ace *ace1, const struct security_ace *ace2); bool security_acl_equal(const struct security_acl *acl1, diff -Nru samba-4.17.11+dfsg/python/samba/ndr.py samba-4.17.12+dfsg/python/samba/ndr.py --- samba-4.17.11+dfsg/python/samba/ndr.py 2022-08-08 14:15:39.260191200 +0000 +++ samba-4.17.12+dfsg/python/samba/ndr.py 2023-10-09 20:16:08.095589400 +0000 @@ -56,6 +56,25 @@ return ndr_print() +def ndr_deepcopy(object): + """Create a deep copy of a NDR object, using pack/unpack + + :param object: Object to copy + :return: The object copy + """ + ndr_pack = getattr(object, "__ndr_pack__", None) + if ndr_pack is None: + raise TypeError("%r is not a NDR object" % object) + data = ndr_pack() + cls = type(object) + copy = cls() + ndr_unpack = getattr(copy, "__ndr_unpack__", None) + if ndr_unpack is None: + raise TypeError("%r is not a NDR object" % copy) + ndr_unpack(data, allow_remaining=False) + return copy + + def ndr_pack_in(object, bigendian=False, ndr64=False): """Pack the input of an NDR function object. diff -Nru samba-4.17.11+dfsg/python/samba/sd_utils.py samba-4.17.12+dfsg/python/samba/sd_utils.py --- samba-4.17.11+dfsg/python/samba/sd_utils.py 2022-08-08 14:15:39.268191300 +0000 +++ samba-4.17.12+dfsg/python/samba/sd_utils.py 2023-10-09 20:16:07.963588500 +0000 @@ -21,8 +21,11 @@ import samba from ldb import Message, MessageElement, Dn from ldb import FLAG_MOD_REPLACE, SCOPE_BASE -from samba.ndr import ndr_pack, ndr_unpack +from samba.ndr import ndr_pack, ndr_unpack, ndr_deepcopy from samba.dcerpc import security +from samba.ntstatus import ( + NT_STATUS_OBJECT_NAME_NOT_FOUND, +) class SDUtils(object): @@ -63,19 +66,145 @@ res = self.ldb.search(object_dn) return ndr_unpack(security.dom_sid, res[0]["objectSid"][0]) + def update_aces_in_dacl(self, dn, del_aces=None, add_aces=None, + sddl_attr=None, controls=None): + if del_aces is None: + del_aces=[] + if add_aces is None: + add_aces=[] + + def ace_from_sddl(ace_sddl): + ace_sd = security.descriptor.from_sddl("D:" + ace_sddl, self.domain_sid) + assert(len(ace_sd.dacl.aces)==1) + return ace_sd.dacl.aces[0] + + if sddl_attr is None: + if controls is None: + controls=["sd_flags:1:%d" % security.SECINFO_DACL] + sd = self.read_sd_on_dn(dn, controls=controls) + if not sd.type & security.SEC_DESC_DACL_PROTECTED: + # if the DACL is not protected remove all + # inherited aces, as they will be re-inherited + # on the server, we need a ndr_deepcopy in order + # to avoid reference problems while deleting + # the aces while looping over them + dacl_copy = ndr_deepcopy(sd.dacl) + for ace in dacl_copy.aces: + if ace.flags & security.SEC_ACE_FLAG_INHERITED_ACE: + try: + sd.dacl_del_ace(ace) + except samba.NTSTATUSError as err: + if err.args[0] != NT_STATUS_OBJECT_NAME_NOT_FOUND: + raise err + # dacl_del_ace may remove more than + # one ace, so we may not find it anymore + pass + else: + if controls is None: + controls=[] + res = self.ldb.search(dn, SCOPE_BASE, None, + [sddl_attr], controls=controls) + old_sddl = str(res[0][sddl_attr][0]) + sd = security.descriptor.from_sddl(old_sddl, self.domain_sid) + + num_changes = 0 + del_ignored = [] + add_ignored = [] + inherited_ignored = [] + + for ace in del_aces: + if isinstance(ace, str): + ace = ace_from_sddl(ace) + assert(isinstance(ace, security.ace)) + + if ace.flags & security.SEC_ACE_FLAG_INHERITED_ACE: + inherited_ignored.append(ace) + continue + + if ace not in sd.dacl.aces: + del_ignored.append(ace) + continue + + sd.dacl_del_ace(ace) + num_changes += 1 + + for ace in add_aces: + add_idx = -1 + if isinstance(ace, dict): + if "idx" in ace: + add_idx = ace["idx"] + ace = ace["ace"] + if isinstance(ace, str): + ace = ace_from_sddl(ace) + assert(isinstance(ace, security.ace)) + + if ace.flags & security.SEC_ACE_FLAG_INHERITED_ACE: + inherited_ignored.append(ace) + continue + + if ace in sd.dacl.aces: + add_ignored.append(ace) + continue + + sd.dacl_add(ace, add_idx) + num_changes += 1 + + if num_changes == 0: + return del_ignored, add_ignored, inherited_ignored + + if sddl_attr is None: + self.modify_sd_on_dn(dn, sd, controls=controls) + else: + new_sddl = sd.as_sddl(self.domain_sid) + m = Message() + m.dn = dn + m[sddl_attr] = MessageElement(new_sddl.encode('ascii'), + FLAG_MOD_REPLACE, + sddl_attr) + self.ldb.modify(m, controls=controls) + + return del_ignored, add_ignored, inherited_ignored + + def dacl_prepend_aces(self, object_dn, aces, controls=None): + """Prepend an ACE (or more) to an objects security descriptor + """ + ace_sd = security.descriptor.from_sddl("D:" + aces, self.domain_sid) + add_aces = [] + add_idx = 0 + for ace in ace_sd.dacl.aces: + add_aces.append({"idx": add_idx, "ace": ace}) + add_idx += 1 + _,ai,ii = self.update_aces_in_dacl(object_dn, add_aces=add_aces, + controls=controls) + return ai, ii + def dacl_add_ace(self, object_dn, ace): - """Add an ACE to an objects security descriptor + """Add an ACE (or more) to an objects security descriptor """ - desc = self.read_sd_on_dn(object_dn, ["show_deleted:1"]) - desc_sddl = desc.as_sddl(self.domain_sid) - if ace in desc_sddl: - return - if desc_sddl.find("(") >= 0: - desc_sddl = (desc_sddl[:desc_sddl.index("(")] + ace + - desc_sddl[desc_sddl.index("("):]) - else: - desc_sddl = desc_sddl + ace - self.modify_sd_on_dn(object_dn, desc_sddl, ["show_deleted:1"]) + _,_ = self.dacl_prepend_aces(object_dn, ace, + controls=["show_deleted:1"]) + + def dacl_append_aces(self, object_dn, aces, controls=None): + """Append an ACE (or more) to an objects security descriptor + """ + ace_sd = security.descriptor.from_sddl("D:" + aces, self.domain_sid) + add_aces = [] + for ace in ace_sd.dacl.aces: + add_aces.append(ace) + _,ai,ii = self.update_aces_in_dacl(object_dn, add_aces=add_aces, + controls=controls) + return ai, ii + + def dacl_delete_aces(self, object_dn, aces, controls=None): + """Delete an ACE (or more) to an objects security descriptor + """ + del_sd = security.descriptor.from_sddl("D:" + aces, self.domain_sid) + del_aces = [] + for ace in del_sd.dacl.aces: + del_aces.append(ace) + di,_,ii = self.update_aces_in_dacl(object_dn, del_aces=del_aces, + controls=controls) + return di, ii def get_sd_as_sddl(self, object_dn, controls=[]): """Return object nTSecutiryDescriptor in SDDL format diff -Nru samba-4.17.11+dfsg/selftest/knownfail samba-4.17.12+dfsg/selftest/knownfail --- samba-4.17.11+dfsg/selftest/knownfail 2023-01-26 17:45:01.649668500 +0000 +++ samba-4.17.12+dfsg/selftest/knownfail 2023-10-09 20:16:08.187590000 +0000 @@ -151,7 +151,7 @@ ^samba4.smb2.acls.*.inheritflags ^samba4.smb2.acls.*.owner ^samba4.smb2.acls.*.ACCESSBASED -^samba4.ldap.dirsync.python.ad_dc_ntvfs..__main__.ExtendedDirsyncTests.test_dirsync_deleted_items +^samba4.ldap.dirsync.python.ad_dc_ntvfs..__main__.SimpleDirsyncTests.test_dirsync_deleted_items_OBJECT_SECURITY #^samba4.ldap.dirsync.python.ad_dc_ntvfs..__main__.ExtendedDirsyncTests.* ^samba4.libsmbclient.opendir.(NT1|SMB3).opendir # This requires netbios browsing ^samba4.rpc.drsuapi.*.drsuapi.DsGetDomainControllerInfo\(.*\)$ diff -Nru samba-4.17.11+dfsg/selftest/knownfail.d/dirsync samba-4.17.12+dfsg/selftest/knownfail.d/dirsync --- samba-4.17.11+dfsg/selftest/knownfail.d/dirsync 1970-01-01 00:00:00.000000000 +0000 +++ samba-4.17.12+dfsg/selftest/knownfail.d/dirsync 2023-10-09 20:16:08.435592000 +0000 @@ -0,0 +1,13 @@ +^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_OBJECT_SECURITY_insist_on_empty_element\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_OBJ_SEC_insist_on_empty_element\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_with_GET_CHANGES_OBJ_SEC_insist_on_empty_element\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_unicodePwd_with_GET_CHANGES_insist_on_empty_element\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_with_GET_CHANGES_OBJECT_SECURITY_insist_on_empty_element\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialDirsyncTests.test_dirsync_with_GET_CHANGES\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialFilteredDirsyncTests.test_dirsync_OBJECT_SECURITY_insist_on_empty_element\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialFilteredDirsyncTests.test_dirsync_OBJECT_SECURITY_with_GET_CHANGES_insist_on_empty_element\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialFilteredDirsyncTests.test_dirsync_with_GET_CHANGES_attr\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.ConfidentialFilteredDirsyncTests.test_dirsync_with_GET_CHANGES_insist_on_empty_element\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.FilteredDirsyncTests.test_dirsync_with_GET_CHANGES\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.FilteredDirsyncTests.test_dirsync_with_GET_CHANGES_attr\(.*\) +^samba4.ldap.dirsync.python\(.*\).__main__.FilteredDirsyncTests.test_dirsync_with_GET_CHANGES_insist_on_empty_element\(.*\) diff -Nru samba-4.17.11+dfsg/selftest/target/Samba4.pm samba-4.17.12+dfsg/selftest/target/Samba4.pm --- samba-4.17.11+dfsg/selftest/target/Samba4.pm 2022-12-15 16:09:31.733236600 +0000 +++ samba-4.17.12+dfsg/selftest/target/Samba4.pm 2023-10-09 20:16:08.455592000 +0000 @@ -782,7 +782,7 @@ wins support = yes server role = $ctx->{server_role} server services = +echo $services - dcerpc endpoint servers = +winreg +srvsvc + dcerpc endpoint servers = +winreg +srvsvc +rpcecho notify:inotify = false ldb:nosync = true ldap server require strong auth = yes diff -Nru samba-4.17.11+dfsg/source3/param/loadparm.c samba-4.17.12+dfsg/source3/param/loadparm.c --- samba-4.17.11+dfsg/source3/param/loadparm.c 2023-03-29 14:28:07.068394400 +0000 +++ samba-4.17.12+dfsg/source3/param/loadparm.c 2023-10-09 20:16:08.455592000 +0000 @@ -883,7 +883,7 @@ Globals.server_services = str_list_make_v3_const(NULL, "s3fs rpc nbt wrepl ldap cldap kdc drepl winbindd ntp_signd kcc dnsupdate dns", NULL); - Globals.dcerpc_endpoint_servers = str_list_make_v3_const(NULL, "epmapper wkssvc rpcecho samr netlogon lsarpc drsuapi dssetup unixinfo browser eventlog6 backupkey dnsserver", NULL); + Globals.dcerpc_endpoint_servers = str_list_make_v3_const(NULL, "epmapper wkssvc samr netlogon lsarpc drsuapi dssetup unixinfo browser eventlog6 backupkey dnsserver", NULL); Globals.tls_enabled = true; Globals.tls_verify_peer = TLS_VERIFY_PEER_AS_STRICT_AS_POSSIBLE; diff -Nru samba-4.17.11+dfsg/source3/rpc_client/local_np.c samba-4.17.12+dfsg/source3/rpc_client/local_np.c --- samba-4.17.11+dfsg/source3/rpc_client/local_np.c 2023-07-06 13:42:43.020797000 +0000 +++ samba-4.17.12+dfsg/source3/rpc_client/local_np.c 2023-10-09 20:16:07.403584200 +0000 @@ -542,6 +542,19 @@ return tevent_req_post(req, ev); } + /* + * Ensure we cannot process a path that exits + * the socket_dir. + */ + if (ISDOTDOT(lower_case_pipename) || + (strchr(lower_case_pipename, '/')!=NULL)) + { + DBG_DEBUG("attempt to connect to invalid pipe pathname %s\n", + lower_case_pipename); + tevent_req_error(req, ENOENT); + return tevent_req_post(req, ev); + } + state->socketpath = talloc_asprintf( state, "%s/np/%s", socket_dir, lower_case_pipename); if (tevent_req_nomem(state->socketpath, req)) { diff -Nru samba-4.17.11+dfsg/source3/rpc_server/rpc_host.c samba-4.17.12+dfsg/source3/rpc_server/rpc_host.c --- samba-4.17.11+dfsg/source3/rpc_server/rpc_host.c 2023-07-06 13:42:43.020797000 +0000 +++ samba-4.17.12+dfsg/source3/rpc_server/rpc_host.c 2023-10-09 20:16:08.515592300 +0000 @@ -214,7 +214,6 @@ char **argl; char *ncalrpc_endpoint; enum dcerpc_transport_t only_transport; - struct dcerpc_binding **existing_bindings; struct rpc_host_iface_name *iface_names; struct rpc_host_endpoint **endpoints; @@ -235,7 +234,6 @@ * @param[in] ev Event context to run this on * @param[in] rpc_server_exe Binary to ask with --list-interfaces * @param[in] only_transport Filter out anything but this - * @param[in] existing_bindings Filter out endpoints served by "samba" * @return The tevent_req representing this process */ @@ -243,8 +241,7 @@ TALLOC_CTX *mem_ctx, struct tevent_context *ev, const char *rpc_server_exe, - enum dcerpc_transport_t only_transport, - struct dcerpc_binding **existing_bindings) + enum dcerpc_transport_t only_transport) { struct tevent_req *req = NULL, *subreq = NULL; struct rpc_server_get_endpoints_state *state = NULL; @@ -256,7 +253,6 @@ return NULL; } state->only_transport = only_transport; - state->existing_bindings = existing_bindings; progname = strrchr(rpc_server_exe, '/'); if (progname != NULL) { @@ -417,37 +413,17 @@ * In member mode, we only serve named pipes. Indicated by NCACN_NP * passed in via "only_transport". * - * In AD mode, the "samba" process already serves many endpoints, - * passed in via "existing_binding". Don't serve those from - * samba-dcerpcd. - * * @param[in] binding Which binding is in question? * @param[in] only_transport Exclusive transport to serve - * @param[in] existing_bindings Endpoints served by "samba" already * @return Do we want to serve "binding" from samba-dcerpcd? */ static bool rpc_host_serve_endpoint( struct dcerpc_binding *binding, - enum dcerpc_transport_t only_transport, - struct dcerpc_binding **existing_bindings) + enum dcerpc_transport_t only_transport) { enum dcerpc_transport_t transport = dcerpc_binding_get_transport(binding); - size_t i, num_existing_bindings; - - num_existing_bindings = talloc_array_length(existing_bindings); - - for (i=0; ibinding, state->only_transport, state->existing_bindings); + ep->binding, state->only_transport); if (!serve_this) { goto fail; } @@ -1607,7 +1583,6 @@ TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct rpc_host *host, - struct dcerpc_binding **existing_bindings, const char *rpc_server_exe) { struct tevent_req *req = NULL, *subreq = NULL; @@ -1639,8 +1614,7 @@ state, ev, rpc_server_exe, - host->np_helper ? NCACN_NP : NCA_UNKNOWN, - existing_bindings); + host->np_helper ? NCACN_NP : NCA_UNKNOWN); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } @@ -2344,7 +2318,6 @@ TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct messaging_context *msg_ctx, - struct dcerpc_binding **existing_bindings, char *servers, int ready_signal_fd, const char *daemon_ready_progname, @@ -2465,7 +2438,6 @@ state, ev, host, - existing_bindings, exe); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); @@ -2648,117 +2620,6 @@ return EAGAIN; } -/* - * Find which interfaces are already being served by the samba AD - * DC so we know not to serve them. Some interfaces like netlogon - * are served by "samba", some like srvsvc will be served by the - * source3 based RPC servers. - */ -static NTSTATUS rpc_host_epm_lookup( - TALLOC_CTX *mem_ctx, - struct dcerpc_binding ***pbindings) -{ - struct rpc_pipe_client *cli = NULL; - struct pipe_auth_data *auth = NULL; - struct policy_handle entry_handle = { .handle_type = 0 }; - struct dcerpc_binding **bindings = NULL; - NTSTATUS status = NT_STATUS_UNSUCCESSFUL; - - status = rpc_pipe_open_ncalrpc(mem_ctx, &ndr_table_epmapper, &cli); - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("rpc_pipe_open_ncalrpc failed: %s\n", - nt_errstr(status)); - goto fail; - } - status = rpccli_ncalrpc_bind_data(cli, &auth); - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("rpccli_ncalrpc_bind_data failed: %s\n", - nt_errstr(status)); - goto fail; - } - status = rpc_pipe_bind(cli, auth); - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("rpc_pipe_bind failed: %s\n", nt_errstr(status)); - goto fail; - } - - for (;;) { - size_t num_bindings = talloc_array_length(bindings); - struct dcerpc_binding **tmp = NULL; - uint32_t num_entries = 0; - struct epm_entry_t *entry = NULL; - struct dcerpc_binding *binding = NULL; - uint32_t result; - - entry = talloc(cli, struct epm_entry_t); - if (entry == NULL) { - goto fail; - } - - status = dcerpc_epm_Lookup( - cli->binding_handle, /* binding_handle */ - cli, /* mem_ctx */ - 0, /* rpc_c_ep_all */ - NULL, /* object */ - NULL, /* interface id */ - 0, /* rpc_c_vers_all */ - &entry_handle, /* entry_handle */ - 1, /* max_ents */ - &num_entries, /* num_ents */ - entry, /* entries */ - &result); /* result */ - if (!NT_STATUS_IS_OK(status)) { - DBG_DEBUG("dcerpc_epm_Lookup failed: %s\n", - nt_errstr(status)); - goto fail; - } - - if (result == EPMAPPER_STATUS_NO_MORE_ENTRIES) { - break; - } - - if (result != EPMAPPER_STATUS_OK) { - DBG_DEBUG("dcerpc_epm_Lookup returned %"PRIu32"\n", - result); - break; - } - - if (num_entries != 1) { - DBG_DEBUG("epm_Lookup returned %"PRIu32" " - "entries, expected one\n", - num_entries); - break; - } - - status = dcerpc_binding_from_tower( - mem_ctx, &entry->tower->tower, &binding); - if (!NT_STATUS_IS_OK(status)) { - break; - } - - tmp = talloc_realloc( - mem_ctx, - bindings, - struct dcerpc_binding *, - num_bindings+1); - if (tmp == NULL) { - status = NT_STATUS_NO_MEMORY; - goto fail; - } - bindings = tmp; - - bindings[num_bindings] = talloc_move(bindings, &binding); - - TALLOC_FREE(entry); - } - - *pbindings = bindings; - status = NT_STATUS_OK; -fail: - TALLOC_FREE(cli); - return status; -} - static void samba_dcerpcd_stdin_handler( struct tevent_context *ev, struct tevent_fd *fde, @@ -2788,7 +2649,6 @@ struct tevent_context *ev_ctx = NULL; struct messaging_context *msg_ctx = NULL; struct tevent_req *req = NULL; - struct dcerpc_binding **existing_bindings = NULL; char *servers = NULL; const char *arg = NULL; size_t num_servers; @@ -2995,11 +2855,6 @@ exit(1); } - status = rpc_host_epm_lookup(frame, &existing_bindings); - DBG_DEBUG("rpc_host_epm_lookup returned %s, %zu bindings\n", - nt_errstr(status), - talloc_array_length(existing_bindings)); - ret = rpc_host_pidfile_create(msg_ctx, progname, ready_signal_fd); if (ret != 0) { DBG_DEBUG("rpc_host_pidfile_create failed: %s\n", @@ -3013,7 +2868,6 @@ ev_ctx, ev_ctx, msg_ctx, - existing_bindings, servers, ready_signal_fd, cmdline_daemon_cfg->fork ? NULL : progname, diff -Nru samba-4.17.11+dfsg/source3/rpc_server/rpcd_classic.c samba-4.17.12+dfsg/source3/rpc_server/rpcd_classic.c --- samba-4.17.11+dfsg/source3/rpc_server/rpcd_classic.c 2022-08-08 14:15:39.448192600 +0000 +++ samba-4.17.12+dfsg/source3/rpc_server/rpcd_classic.c 2023-10-09 20:16:08.495592400 +0000 @@ -42,14 +42,34 @@ static const struct ndr_interface_table *ifaces[] = { &ndr_table_srvsvc, &ndr_table_netdfs, - &ndr_table_wkssvc, + &ndr_table_initshutdown, &ndr_table_svcctl, &ndr_table_ntsvcs, &ndr_table_eventlog, - &ndr_table_initshutdown, + /* + * This last item is truncated from the list by the + * num_ifaces -= 1 below. Take care when adding new + * services. + */ + &ndr_table_wkssvc, }; + size_t num_ifaces = ARRAY_SIZE(ifaces); + + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC wkssvc is provided by the 'samba' + * binary from source4/ + */ + num_ifaces -= 1; + break; + default: + break; + } + *pifaces = ifaces; - return ARRAY_SIZE(ifaces); + return num_ifaces; + } static size_t classic_servers( @@ -58,15 +78,28 @@ void *private_data) { static const struct dcesrv_endpoint_server *ep_servers[7] = { NULL }; + size_t num_servers = ARRAY_SIZE(ep_servers); bool ok; ep_servers[0] = srvsvc_get_ep_server(); ep_servers[1] = netdfs_get_ep_server(); - ep_servers[2] = wkssvc_get_ep_server(); + ep_servers[2] = initshutdown_get_ep_server(); ep_servers[3] = svcctl_get_ep_server(); ep_servers[4] = ntsvcs_get_ep_server(); ep_servers[5] = eventlog_get_ep_server(); - ep_servers[6] = initshutdown_get_ep_server(); + ep_servers[6] = wkssvc_get_ep_server(); + + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC wkssvc is provided by the 'samba' + * binary from source4/ + */ + num_servers -= 1; + break; + default: + break; + } ok = secrets_init(); if (!ok) { @@ -85,7 +118,7 @@ mangle_reset_cache(); *_ep_servers = ep_servers; - return ARRAY_SIZE(ep_servers); + return num_servers; } int main(int argc, const char *argv[]) diff -Nru samba-4.17.11+dfsg/source3/rpc_server/rpcd_epmapper.c samba-4.17.12+dfsg/source3/rpc_server/rpcd_epmapper.c --- samba-4.17.11+dfsg/source3/rpc_server/rpcd_epmapper.c 2022-08-08 14:15:39.448192600 +0000 +++ samba-4.17.12+dfsg/source3/rpc_server/rpcd_epmapper.c 2023-10-09 20:16:08.495592400 +0000 @@ -19,6 +19,8 @@ #include "rpc_worker.h" #include "librpc/gen_ndr/ndr_epmapper.h" #include "librpc/gen_ndr/ndr_epmapper_scompat.h" +#include "param/loadparm.h" +#include "libds/common/roles.h" static size_t epmapper_interfaces( const struct ndr_interface_table ***pifaces, @@ -27,8 +29,22 @@ static const struct ndr_interface_table *ifaces[] = { &ndr_table_epmapper, }; + size_t num_ifaces = ARRAY_SIZE(ifaces); + + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC epmapper is provided by the 'samba' + * binary from source4/ + */ + num_ifaces = 0; + break; + default: + break; + } + *pifaces = ifaces; - return ARRAY_SIZE(ifaces); + return num_ifaces; } static size_t epmapper_servers( @@ -37,11 +53,24 @@ void *private_data) { static const struct dcesrv_endpoint_server *ep_servers[] = { NULL }; + size_t num_servers = ARRAY_SIZE(ep_servers); ep_servers[0] = epmapper_get_ep_server(); + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC epmapper is provided by the 'samba' + * binary from source4/ + */ + num_servers = 0; + break; + default: + break; + } + *_ep_servers = ep_servers; - return ARRAY_SIZE(ep_servers); + return num_servers; } int main(int argc, const char *argv[]) diff -Nru samba-4.17.11+dfsg/source3/rpc_server/rpcd_lsad.c samba-4.17.12+dfsg/source3/rpc_server/rpcd_lsad.c --- samba-4.17.11+dfsg/source3/rpc_server/rpcd_lsad.c 2022-08-08 14:15:39.448192600 +0000 +++ samba-4.17.12+dfsg/source3/rpc_server/rpcd_lsad.c 2023-10-09 20:16:08.495592400 +0000 @@ -36,6 +36,11 @@ &ndr_table_lsarpc, &ndr_table_samr, &ndr_table_dssetup, + /* + * This last item is truncated from the list by the + * num_ifaces -= 1 below for the fileserver. Take + * care when adding new services. + */ &ndr_table_netlogon, }; size_t num_ifaces = ARRAY_SIZE(ifaces); @@ -46,6 +51,14 @@ /* no netlogon for non-dc */ num_ifaces -= 1; break; + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * All these services are provided by the 'samba' + * binary from source4, not this code which is the + * source3 / NT4-like "classic" DC implementation + */ + num_ifaces = 0; + break; default: break; } @@ -80,6 +93,14 @@ /* no netlogon for non-dc */ num_servers -= 1; break; + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * All these services are provided by the 'samba' + * binary from source4, not this code which is the + * source3 / NT4-like "classic" DC implementation + */ + num_servers = 0; + break; default: break; } diff -Nru samba-4.17.11+dfsg/source3/rpc_server/rpcd_rpcecho.c samba-4.17.12+dfsg/source3/rpc_server/rpcd_rpcecho.c --- samba-4.17.11+dfsg/source3/rpc_server/rpcd_rpcecho.c 2022-08-08 14:15:39.448192600 +0000 +++ samba-4.17.12+dfsg/source3/rpc_server/rpcd_rpcecho.c 2023-10-09 20:16:08.495592400 +0000 @@ -19,6 +19,8 @@ #include "rpc_worker.h" #include "librpc/gen_ndr/ndr_echo.h" #include "librpc/gen_ndr/ndr_echo_scompat.h" +#include "param/loadparm.h" +#include "libds/common/roles.h" static size_t rpcecho_interfaces( const struct ndr_interface_table ***pifaces, @@ -27,8 +29,22 @@ static const struct ndr_interface_table *ifaces[] = { &ndr_table_rpcecho, }; + size_t num_ifaces = ARRAY_SIZE(ifaces); + + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC rpcecho is provided by the 'samba' + * binary from source4/ + */ + num_ifaces = 0; + break; + default: + break; + } + *pifaces = ifaces; - return ARRAY_SIZE(ifaces); + return num_ifaces; } static size_t rpcecho_servers( @@ -37,11 +53,24 @@ void *private_data) { static const struct dcesrv_endpoint_server *ep_servers[1] = { NULL }; + size_t num_servers = ARRAY_SIZE(ep_servers); ep_servers[0] = rpcecho_get_ep_server(); + switch(lp_server_role()) { + case ROLE_ACTIVE_DIRECTORY_DC: + /* + * On the AD DC rpcecho is provided by the 'samba' + * binary from source4/ + */ + num_servers = 0; + break; + default: + break; + } + *_ep_servers = ep_servers; - return ARRAY_SIZE(ep_servers); + return num_servers; } int main(int argc, const char *argv[]) diff -Nru samba-4.17.11+dfsg/source3/rpc_server/wscript_build samba-4.17.12+dfsg/source3/rpc_server/wscript_build --- samba-4.17.11+dfsg/source3/rpc_server/wscript_build 2023-01-26 17:45:01.653668600 +0000 +++ samba-4.17.12+dfsg/source3/rpc_server/wscript_build 2023-10-09 20:16:08.475592100 +0000 @@ -38,6 +38,7 @@ RPC_WORKER RPC_RPCECHO ''', + for_selftest=True, install_path='${SAMBA_LIBEXECDIR}') bld.SAMBA3_BINARY('rpcd_classic', diff -Nru samba-4.17.11+dfsg/source3/selftest/tests.py samba-4.17.12+dfsg/source3/selftest/tests.py --- samba-4.17.11+dfsg/source3/selftest/tests.py 2023-09-07 08:59:49.037451500 +0000 +++ samba-4.17.12+dfsg/source3/selftest/tests.py 2023-10-09 20:16:07.375584100 +0000 @@ -264,6 +264,21 @@ "-f msdfs-src1"]) # +# BUG: https://bugzilla.samba.org/show_bug.cgi?id=15422 +# Prevent bad pipenames. +# +plantestsuite("samba3.smbtorture_s3.smb2.SMB2-INVALID-PIPENAME", + "fileserver", + [os.path.join(samba3srcdir, + "script/tests/test_smbtorture_s3.sh"), + 'SMB2-INVALID-PIPENAME', + '//$SERVER_IP/tmp', + '$USERNAME', + '$PASSWORD', + smbtorture3, + "-mSMB2"]) + +# # SMB2-STREAM-ACL needs to run against a special share - vfs_wo_fruit # plantestsuite("samba3.smbtorture_s3.plain.%s" % "SMB2-STREAM-ACL", diff -Nru samba-4.17.11+dfsg/source3/smbd/open.c samba-4.17.12+dfsg/source3/smbd/open.c --- samba-4.17.11+dfsg/source3/smbd/open.c 2023-05-11 07:07:19.598420900 +0000 +++ samba-4.17.12+dfsg/source3/smbd/open.c 2023-10-09 20:16:07.455584500 +0000 @@ -1442,7 +1442,7 @@ dirfsp, fsp, false, - access_mask); + open_access_mask); if (!NT_STATUS_IS_OK(status)) { DBG_DEBUG("smbd_check_access_rights_fsp" @@ -1633,7 +1633,7 @@ status = smbd_check_access_rights_fsp(dirfsp, fsp, false, - access_mask); + open_access_mask); if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && posix_open && diff -Nru samba-4.17.11+dfsg/source3/torture/proto.h samba-4.17.12+dfsg/source3/torture/proto.h --- samba-4.17.11+dfsg/source3/torture/proto.h 2023-03-09 09:18:38.357810700 +0000 +++ samba-4.17.12+dfsg/source3/torture/proto.h 2023-10-09 20:16:07.375584100 +0000 @@ -120,6 +120,7 @@ bool run_smb2_sacl(int dummy); bool run_smb2_quota1(int dummy); bool run_smb2_stream_acl(int dummy); +bool run_smb2_invalid_pipename(int dummy); bool run_list_dir_async_test(int dummy); bool run_delete_on_close_non_empty(int dummy); bool run_delete_on_close_nonwrite_delete_yes_test(int dummy); diff -Nru samba-4.17.11+dfsg/source3/torture/test_smb2.c samba-4.17.12+dfsg/source3/torture/test_smb2.c --- samba-4.17.11+dfsg/source3/torture/test_smb2.c 2022-08-08 14:15:39.500193000 +0000 +++ samba-4.17.12+dfsg/source3/torture/test_smb2.c 2023-10-09 20:16:07.375584100 +0000 @@ -3608,3 +3608,108 @@ } return ret; } + +bool run_smb2_invalid_pipename(int dummy) +{ + struct cli_state *cli = NULL; + NTSTATUS status; + uint64_t fid_persistent = 0; + uint64_t fid_volatile = 0; + const char *unknown_pipe = "badpipe"; + const char *invalid_pipe = "../../../../../../../../../badpipe"; + + printf("Starting SMB2-INVALID-PIPENAME\n"); + + if (!torture_init_connection(&cli)) { + return false; + } + + status = smbXcli_negprot(cli->conn, + cli->timeout, + PROTOCOL_SMB2_02, + PROTOCOL_SMB3_11); + if (!NT_STATUS_IS_OK(status)) { + printf("smbXcli_negprot returned %s\n", nt_errstr(status)); + return false; + } + + status = cli_session_setup_creds(cli, torture_creds); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_session_setup returned %s\n", nt_errstr(status)); + return false; + } + + status = cli_tree_connect(cli, "IPC$", "?????", NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_tree_connect returned %s\n", nt_errstr(status)); + return false; + } + + /* Try and connect to an unknown pipename. */ + status = smb2cli_create(cli->conn, + cli->timeout, + cli->smb2.session, + cli->smb2.tcon, + unknown_pipe, + SMB2_OPLOCK_LEVEL_NONE, /* oplock_level, */ + SMB2_IMPERSONATION_IMPERSONATION, /* impersonation_level, */ + SEC_STD_SYNCHRONIZE| + SEC_FILE_READ_DATA| + SEC_FILE_WRITE_DATA| + SEC_FILE_READ_ATTRIBUTE, /* desired_access, */ + FILE_ATTRIBUTE_NORMAL, /* file_attributes, */ + FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access, */ + FILE_CREATE, /* create_disposition, */ + 0, /* create_options, */ + NULL, /* smb2_create_blobs *blobs */ + &fid_persistent, + &fid_volatile, + NULL, /* struct smb_create_returns * */ + talloc_tos(), /* mem_ctx. */ + NULL); /* struct smb2_create_blobs */ + /* We should get NT_STATUS_OBJECT_NAME_NOT_FOUND */ + if (!NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { + printf("%s:%d smb2cli_create on name %s returned %s\n", + __FILE__, + __LINE__, + unknown_pipe, + nt_errstr(status)); + return false; + } + + /* Try and connect to an invalid pipename containing unix separators. */ + status = smb2cli_create(cli->conn, + cli->timeout, + cli->smb2.session, + cli->smb2.tcon, + invalid_pipe, + SMB2_OPLOCK_LEVEL_NONE, /* oplock_level, */ + SMB2_IMPERSONATION_IMPERSONATION, /* impersonation_level, */ + SEC_STD_SYNCHRONIZE| + SEC_FILE_READ_DATA| + SEC_FILE_WRITE_DATA| + SEC_FILE_READ_ATTRIBUTE, /* desired_access, */ + FILE_ATTRIBUTE_NORMAL, /* file_attributes, */ + FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access, */ + FILE_CREATE, /* create_disposition, */ + 0, /* create_options, */ + NULL, /* smb2_create_blobs *blobs */ + &fid_persistent, + &fid_volatile, + NULL, /* struct smb_create_returns * */ + talloc_tos(), /* mem_ctx. */ + NULL); /* struct smb2_create_blobs */ + /* + * We should still get NT_STATUS_OBJECT_NAME_NOT_FOUND + * (tested against Windows 2022). + */ + if (!NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { + printf("%s:%d smb2cli_create on name %s returned %s\n", + __FILE__, + __LINE__, + invalid_pipe, + nt_errstr(status)); + return false; + } + return true; +} diff -Nru samba-4.17.11+dfsg/source3/torture/torture.c samba-4.17.12+dfsg/source3/torture/torture.c --- samba-4.17.11+dfsg/source3/torture/torture.c 2023-09-07 08:59:49.045451900 +0000 +++ samba-4.17.12+dfsg/source3/torture/torture.c 2023-10-09 20:16:07.379584000 +0000 @@ -15764,6 +15764,10 @@ .fn = run_oplock_cancel, }, { + .name = "SMB2-INVALID-PIPENAME", + .fn = run_smb2_invalid_pipename, + }, + { .name = "SMB1-TRUNCATED-SESSSETUP", .fn = run_smb1_truncated_sesssetup, }, diff -Nru samba-4.17.11+dfsg/source4/dsdb/samdb/ldb_modules/dirsync.c samba-4.17.12+dfsg/source4/dsdb/samdb/ldb_modules/dirsync.c --- samba-4.17.11+dfsg/source4/dsdb/samdb/ldb_modules/dirsync.c 2022-08-08 14:15:39.552193400 +0000 +++ samba-4.17.12+dfsg/source4/dsdb/samdb/ldb_modules/dirsync.c 2023-10-09 20:16:08.435592000 +0000 @@ -56,7 +56,6 @@ bool linkIncrVal; bool localonly; bool partial; - bool assystem; int functional_level; const struct GUID *our_invocation_id; const struct dsdb_schema *schema; @@ -872,10 +871,6 @@ DSDB_SEARCH_SHOW_DELETED | DSDB_SEARCH_SHOW_EXTENDED_DN; - if (dsc->assystem) { - flags = flags | DSDB_FLAG_AS_SYSTEM; - } - ret = dsdb_module_search_tree(dsc->module, dsc, &res, dn, LDB_SCOPE_BASE, req->op.search.tree, @@ -1005,7 +1000,6 @@ struct dirsync_context *dsc; struct ldb_context *ldb; struct ldb_parse_tree *new_tree = req->op.search.tree; - uint32_t flags = 0; enum ndr_err_code ndr_err; DATA_BLOB blob; const char **attrs; @@ -1103,27 +1097,27 @@ return LDB_ERR_OPERATIONS_ERROR; } objectclass = dsdb_get_structural_oc_from_msg(schema, acl_res->msgs[0]); + + /* + * While we never use the answer to this for access + * control (after CVE-2023-4154), we return a + * different error message depending on if the user + * was granted GUID_DRS_GET_CHANGES to provide a closer + * emulation and keep some tests passing. + * + * (Samba's ACL logic is not well suited to redacting + * only the secret and RODC filtered attributes). + */ ret = acl_check_extended_right(dsc, module, req, objectclass, sd, acl_user_token(module), GUID_DRS_GET_CHANGES, SEC_ADS_CONTROL_ACCESS, sid); - if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) { - return ret; - } - dsc->assystem = true; - ret = ldb_request_add_control(req, LDB_CONTROL_AS_SYSTEM_OID, false, NULL); - if (ret != LDB_SUCCESS) { return ret; } talloc_free(acl_res); - } else { - flags |= DSDB_ACL_CHECKS_DIRSYNC_FLAG; - - if (ret != LDB_SUCCESS) { - return ret; - } - + } else if (ret != LDB_SUCCESS) { + return ret; } dsc->functional_level = dsdb_functional_level(ldb); @@ -1394,7 +1388,6 @@ req->controls, dsc, dirsync_search_callback, req); - ldb_req_set_custom_flags(down_req, flags); LDB_REQ_SET_LOCATION(down_req); if (ret != LDB_SUCCESS) { return ret; diff -Nru samba-4.17.11+dfsg/source4/dsdb/samdb/samdb.h samba-4.17.12+dfsg/source4/dsdb/samdb/samdb.h --- samba-4.17.11+dfsg/source4/dsdb/samdb/samdb.h 2023-05-11 07:07:19.606421000 +0000 +++ samba-4.17.12+dfsg/source4/dsdb/samdb/samdb.h 2023-10-09 20:16:07.891587700 +0000 @@ -362,7 +362,6 @@ #define DSDB_FULL_JOIN_REPLICATION_COMPLETED_OPAQUE_NAME "DSDB_FULL_JOIN_REPLICATION_COMPLETED" -#define DSDB_ACL_CHECKS_DIRSYNC_FLAG 0x1 #define DSDB_SAMDB_MINIMUM_ALLOWED_RID 1000 #define DSDB_METADATA_SCHEMA_SEQ_NUM "SCHEMA_SEQ_NUM" diff -Nru samba-4.17.11+dfsg/source4/dsdb/tests/python/acl.py samba-4.17.12+dfsg/source4/dsdb/tests/python/acl.py --- samba-4.17.11+dfsg/source4/dsdb/tests/python/acl.py 2022-08-08 14:29:11.377506700 +0000 +++ samba-4.17.12+dfsg/source4/dsdb/tests/python/acl.py 2023-10-09 20:16:07.675586200 +0000 @@ -174,7 +174,7 @@ self.assertEqual(len(res), 0) def test_add_u1(self): - """Testing OU with the rights of Doman Admin not creator of the OU """ + """Testing OU with the rights of Domain Admin not creator of the OU """ self.assert_top_ou_deleted() # Change descriptor for top level OU self.ldb_owner.create_ou("OU=test_add_ou1," + self.base_dn) @@ -187,7 +187,7 @@ self.ldb_notowner.newgroup("test_add_group1", groupou="OU=test_add_ou2,OU=test_add_ou1", grouptype=samba.dsdb.GTYPE_DISTRIBUTION_DOMAIN_LOCAL_GROUP) # Make sure we HAVE created the two objects -- user and group - # !!! We should not be able to do that, but however beacuse of ACE ordering our inherited Deny ACE + # !!! We should not be able to do that, but however because of ACE ordering our inherited Deny ACE # !!! comes after explicit (A;;RPWPCRCCDCLCLORCWOWDSDDTSW;;;DA) that comes from somewhere res = self.ldb_admin.search(self.base_dn, expression="(distinguishedName=%s,%s)" % ("CN=test_add_user1,OU=test_add_ou2,OU=test_add_ou1", self.base_dn)) self.assertTrue(len(res) > 0) @@ -248,7 +248,7 @@ self.assertEqual(len(res), 0) def test_add_u4(self): - """ 4 Testing OU with the rights of Doman Admin creator of the OU""" + """ 4 Testing OU with the rights of Domain Admin creator of the OU""" self.assert_top_ou_deleted() self.ldb_owner.create_ou("OU=test_add_ou1," + self.base_dn) self.ldb_owner.create_ou("OU=test_add_ou2,OU=test_add_ou1," + self.base_dn) @@ -1890,7 +1890,7 @@ self.assertEqual(len(res), 0) def test_delete_u3(self): - """User indentified by SID has RIGHT_DELETE to another User object""" + """User identified by SID has RIGHT_DELETE to another User object""" user_dn = self.get_user_dn("test_delete_user1") # Create user that we try to delete self.ldb_admin.newuser("test_delete_user1", self.user_pass) @@ -2181,7 +2181,7 @@ minPwdAge = self.ldb_admin.get_minPwdAge() # Reset the "minPwdAge" as it was before self.addCleanup(self.ldb_admin.set_minPwdAge, minPwdAge) - # Set it temporarely to "0" + # Set it temporarily to "0" self.ldb_admin.set_minPwdAge("0") self.user_with_wp = "acl_car_user1" @@ -2637,7 +2637,7 @@ self.sd_utils.dacl_add_ace(self.deleted_dn2, mod) self.undelete_deleted(self.deleted_dn2, self.testuser2_dn) - # attempt undelete with simultanious addition of url, WP to which is denied + # attempt undelete with simultaneous addition of url, WP to which is denied mod = "(OD;;WP;9a9a0221-4a5b-11d1-a9c3-0000f80367c1;;%s)" % str(self.sid) self.sd_utils.dacl_add_ace(self.deleted_dn3, mod) try: diff -Nru samba-4.17.11+dfsg/source4/dsdb/tests/python/ad_dc_search_performance.py samba-4.17.12+dfsg/source4/dsdb/tests/python/ad_dc_search_performance.py --- samba-4.17.11+dfsg/source4/dsdb/tests/python/ad_dc_search_performance.py 2022-08-08 14:15:39.564193500 +0000 +++ samba-4.17.12+dfsg/source4/dsdb/tests/python/ad_dc_search_performance.py 2023-10-09 20:16:07.675586200 +0000 @@ -180,7 +180,7 @@ maybe_not = ['!(', ''] joiners = ['&', '|'] - # The number of permuations is 18432, which is not huge but + # The number of permutations is 18432, which is not huge but # would take hours to search. So we take a sample. all_permutations = list(itertools.product(joiners, classes, classes, diff -Nru samba-4.17.11+dfsg/source4/dsdb/tests/python/confidential_attr.py samba-4.17.12+dfsg/source4/dsdb/tests/python/confidential_attr.py --- samba-4.17.11+dfsg/source4/dsdb/tests/python/confidential_attr.py 2023-03-29 14:28:07.076394600 +0000 +++ samba-4.17.12+dfsg/source4/dsdb/tests/python/confidential_attr.py 2023-10-09 20:16:08.391591500 +0000 @@ -24,20 +24,21 @@ sys.path.insert(0, "bin/python") import samba -import os import random import statistics import time from samba.tests.subunitrun import SubunitOptions, TestProgram import samba.getopt as options from ldb import SCOPE_BASE, SCOPE_SUBTREE -from samba.dsdb import SEARCH_FLAG_CONFIDENTIAL, SEARCH_FLAG_PRESERVEONDELETE +from samba.dsdb import SEARCH_FLAG_CONFIDENTIAL, SEARCH_FLAG_RODC_ATTRIBUTE, SEARCH_FLAG_PRESERVEONDELETE from ldb import Message, MessageElement, Dn from ldb import FLAG_MOD_REPLACE, FLAG_MOD_ADD from samba.auth import system_session from samba import gensec, sd_utils from samba.samdb import SamDB from samba.credentials import Credentials, DONT_USE_KERBEROS +from samba.dcerpc import security + import samba.tests import samba.dsdb @@ -70,20 +71,6 @@ creds = credopts.get_credentials(lp) creds.set_gensec_features(creds.get_gensec_features() | gensec.FEATURE_SEAL) -# When a user does not have access rights to view the objects' attributes, -# Windows and Samba behave slightly differently. -# A windows DC will always act as if the hidden attribute doesn't exist AT ALL -# (for an unprivileged user). So, even for a user that lacks access rights, -# the inverse/'!' queries should return ALL objects. This is similar to the -# kludgeaclredacted behaviour on Samba. -# However, on Samba (for implementation simplicity) we never return a matching -# result for an unprivileged user. -# Either approach is OK, so long as it gets applied consistently and we don't -# disclose any sensitive details by varying what gets returned by the search. -DC_MODE_RETURN_NONE = 0 -DC_MODE_RETURN_ALL = 1 - - # # Tests start here # @@ -113,7 +100,9 @@ userou = "OU=conf-attr-test" self.ou = "{0},{1}".format(userou, self.base_dn) + samba.tests.delete_force(self.ldb_admin, self.ou, controls=['tree_delete:1']) self.ldb_admin.create_ou(self.ou) + self.addCleanup(samba.tests.delete_force, self.ldb_admin, self.ou, controls=['tree_delete:1']) # use a common username prefix, so we can use sAMAccountName=CATC-* as # a search filter to only return the users we're interested in @@ -149,14 +138,12 @@ # sanity-check the flag is not already set (this'll cause problems if # previous test run didn't clean up properly) - search_flags = self.get_attr_search_flags(self.attr_dn) - self.assertTrue(int(search_flags) & SEARCH_FLAG_CONFIDENTIAL == 0, - "{0} searchFlags already {1}".format(self.conf_attr, - search_flags)) - - def tearDown(self): - super(ConfidentialAttrCommon, self).tearDown() - self.ldb_admin.delete(self.ou, ["tree_delete:1"]) + search_flags = int(self.get_attr_search_flags(self.attr_dn)) + if search_flags & SEARCH_FLAG_CONFIDENTIAL|SEARCH_FLAG_RODC_ATTRIBUTE: + self.set_attr_search_flags(self.attr_dn, str(search_flags &~ (SEARCH_FLAG_CONFIDENTIAL|SEARCH_FLAG_RODC_ATTRIBUTE))) + search_flags = int(self.get_attr_search_flags(self.attr_dn)) + self.assertEqual(0, search_flags & (SEARCH_FLAG_CONFIDENTIAL|SEARCH_FLAG_RODC_ATTRIBUTE), + f"{self.conf_attr} searchFlags did not reset to omit SEARCH_FLAG_CONFIDENTIAL and SEARCH_FLAG_RODC_ATTRIBUTE ({search_flags})") def add_attr(self, dn, attr, value): m = Message() @@ -193,25 +180,6 @@ # reset the value after the test completes self.addCleanup(self.set_attr_search_flags, self.attr_dn, old_value) - # The behaviour of the DC can differ in some cases, depending on whether - # we're talking to a Windows DC or a Samba DC - def guess_dc_mode(self): - # if we're in selftest, we can be pretty sure it's a Samba DC - if os.environ.get('SAMBA_SELFTEST') == '1': - return DC_MODE_RETURN_NONE - - searches = self.get_negative_match_all_searches() - res = self.ldb_user.search(self.test_dn, expression=searches[0], - scope=SCOPE_SUBTREE) - - # we default to DC_MODE_RETURN_NONE (samba).Update this if it - # looks like we're talking to a Windows DC - if len(res) == self.total_objects: - return DC_MODE_RETURN_ALL - - # otherwise assume samba DC behaviour - return DC_MODE_RETURN_NONE - def get_user_dn(self, name): return "CN={0},{1}".format(name, self.ou) @@ -241,9 +209,9 @@ for attr in attr_filters: res = samdb.search(self.test_dn, expression=expr, scope=SCOPE_SUBTREE, attrs=attr) - self.assertTrue(len(res) == expected_num, - "%u results, not %u for search %s, attr %s" % - (len(res), expected_num, expr, str(attr))) + self.assertEqual(len(res), expected_num, + "%u results, not %u for search %s, attr %s" % + (len(res), expected_num, expr, str(attr))) # return a selection of searches that match exactly against the test object def get_exact_match_searches(self): @@ -359,7 +327,7 @@ return expected_results # Returns the expected negative (i.e. '!') search behaviour when talking to - # a DC with DC_MODE_RETURN_ALL behaviour, i.e. we assert that users + # a DC, i.e. we assert that users # without rights always see ALL objects in '!' searches def negative_searches_return_all(self, has_rights_to=0, total_objects=None): @@ -385,56 +353,29 @@ return expected_results - # Returns the expected negative (i.e. '!') search behaviour when talking to - # a DC with DC_MODE_RETURN_NONE behaviour, i.e. we assert that users - # without rights cannot see objects in '!' searches at all - def negative_searches_return_none(self, has_rights_to=0): - expected_results = {} - - # the 'match-all' searches should only return the objects we have - # access rights to (if any) - for search in self.get_negative_match_all_searches(): - expected_results[search] = has_rights_to - - # for inverse matches, we should NOT be told about any objects at all - inverse_searches = self.get_inverse_match_searches() - inverse_searches += ["(!({0}=*))".format(self.conf_attr)] - for search in inverse_searches: - expected_results[search] = 0 - - return expected_results - # Returns the expected negative (i.e. '!') search behaviour. This varies # depending on what type of DC we're talking to (i.e. Windows or Samba) # and what access rights the user has. # Note we only handle has_rights_to="all", 1 (the test object), or 0 (i.e. # we don't have rights to any objects) - def negative_search_expected_results(self, has_rights_to, dc_mode, - total_objects=None): + def negative_search_expected_results(self, has_rights_to, total_objects=None): if has_rights_to == "all": expect_results = self.negative_searches_all_rights(total_objects) - # if it's a Samba DC, we only expect the 'match-all' searches to return - # the objects that we have access rights to (all others are hidden). - # Whereas Windows 'hides' the objects by always returning all of them - elif dc_mode == DC_MODE_RETURN_NONE: - expect_results = self.negative_searches_return_none(has_rights_to) else: expect_results = self.negative_searches_return_all(has_rights_to, total_objects) return expect_results - def assert_negative_searches(self, has_rights_to=0, - dc_mode=DC_MODE_RETURN_NONE, samdb=None): + def assert_negative_searches(self, has_rights_to=0, samdb=None): """Asserts user without rights cannot see objects in '!' searches""" if samdb is None: samdb = self.ldb_user # build a dictionary of key=search-expr, value=expected_num assertions - expected_results = self.negative_search_expected_results(has_rights_to, - dc_mode) + expected_results = self.negative_search_expected_results(has_rights_to) for search, expected_num in expected_results.items(): self.assert_search_result(expected_num, search, samdb) @@ -444,7 +385,7 @@ # checks whether the confidential attribute is present res = samdb.search(self.conf_dn, expression="(objectClass=*)", scope=SCOPE_SUBTREE, attrs=attrs) - self.assertTrue(len(res) == 1) + self.assertEqual(1, len(res)) attr_returned = False for msg in res: @@ -490,8 +431,7 @@ self.make_attr_confidential() self.assert_conf_attr_searches(has_rights_to=0) - dc_mode = DC_MODE_RETURN_ALL - self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + self.assert_negative_searches(has_rights_to=0) self.assert_attr_visible(expect_attr=False) # sanity-check we haven't hidden the attribute from the admin as well @@ -503,8 +443,7 @@ self.make_attr_confidential() self.assert_conf_attr_searches(has_rights_to=0) - dc_mode = DC_MODE_RETURN_ALL - self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + self.assert_negative_searches(has_rights_to=0) self.assert_attr_visible(expect_attr=False) # apply the allow ACE to the object under test @@ -513,7 +452,7 @@ # the user should now be able to see the attribute for the one object # we gave it rights to self.assert_conf_attr_searches(has_rights_to=1) - self.assert_negative_searches(has_rights_to=1, dc_mode=dc_mode) + self.assert_negative_searches(has_rights_to=1) self.assert_attr_visible(expect_attr=True) # sanity-check the admin can still see the attribute @@ -566,8 +505,7 @@ self.make_attr_confidential() self.assert_conf_attr_searches(has_rights_to=0) - dc_mode = DC_MODE_RETURN_ALL - self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + self.assert_negative_searches(has_rights_to=0) self.assert_attr_visible(expect_attr=False) # apply the ACE to the object under test @@ -575,7 +513,7 @@ # this should make no difference to the user's ability to see the attr self.assert_conf_attr_searches(has_rights_to=0) - self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + self.assert_negative_searches(has_rights_to=0) self.assert_attr_visible(expect_attr=False) # sanity-check the admin can still see the attribute @@ -630,9 +568,9 @@ for attr in attr_filters: res = samdb.search(self.test_dn, expression=expr, scope=SCOPE_SUBTREE, attrs=attr) - self.assertTrue(len(res) == expected_num, - "%u results, not %u for search %s, attr %s" % - (len(res), expected_num, expr, str(attr))) + self.assertEqual(len(res), expected_num, + "%u results, not %u for search %s, attr %s" % + (len(res), expected_num, expr, str(attr))) # assert we haven't revealed the hidden test-object if excl_testobj: @@ -648,7 +586,7 @@ # make sure the test object is not returned if we've been denied rights # to it via an ACE - excl_testobj = True if has_rights_to == "deny-one" else False + excl_testobj = has_rights_to == "deny-one" # these first few searches we just expect to match against the one # object under test that we're trying to guess the value of @@ -669,24 +607,6 @@ self.assert_search_result(expected_num, search, samdb, excl_testobj) - # override method specifically for deny ACL test cases. Instead of being - # granted access to either no objects or only one, we are being denied - # access to only one object (but can still access the rest). - def negative_searches_return_none(self, has_rights_to=0): - expected_results = {} - - # on Samba we will see the objects we have rights to, but the one we - # are denied access to will be hidden - searches = self.get_negative_match_all_searches() - searches += self.get_inverse_match_searches() - for search in searches: - expected_results[search] = self.total_objects - 1 - - # The wildcard returns the objects without this attribute as normal. - search = "(!({0}=*))".format(self.conf_attr) - expected_results[search] = self.total_objects - self.objects_with_attr - return expected_results - # override method specifically for deny ACL test cases def negative_searches_return_all(self, has_rights_to=0, total_objects=None): @@ -707,8 +627,7 @@ return expected_results # override method specifically for deny ACL test cases - def assert_negative_searches(self, has_rights_to=0, - dc_mode=DC_MODE_RETURN_NONE, samdb=None): + def assert_negative_searches(self, has_rights_to=0, samdb=None): """Asserts user without rights cannot see objects in '!' searches""" if samdb is None: @@ -719,12 +638,9 @@ # assert this if the '!'/negative search behaviour is to suppress any # objects we don't have access rights to) excl_testobj = False - if has_rights_to != "all" and dc_mode == DC_MODE_RETURN_NONE: - excl_testobj = True # build a dictionary of key=search-expr, value=expected_num assertions - expected_results = self.negative_search_expected_results(has_rights_to, - dc_mode) + expected_results = self.negative_search_expected_results(has_rights_to) for search, expected_num in expected_results.items(): self.assert_search_result(expected_num, search, samdb, @@ -741,9 +657,7 @@ # the user shouldn't be able to see the attribute anymore self.assert_conf_attr_searches(has_rights_to="deny-one") - dc_mode = DC_MODE_RETURN_ALL - self.assert_negative_searches(has_rights_to="deny-one", - dc_mode=dc_mode) + self.assert_negative_searches(has_rights_to="deny-one") self.assert_attr_visible(expect_attr=False) # sanity-check we haven't hidden the attribute from the admin as well @@ -810,7 +724,7 @@ self.attr_filters = [None, ["*"], ["name"]] - # Note dirsync behaviour is slighty different for the attribute under + # Note dirsync behaviour is slightly different for the attribute under # test - when you have full access rights, it only returns the objects # that actually have this attribute (i.e. it doesn't return an empty # message with just the DN). So we add the 'name' attribute into the @@ -830,10 +744,16 @@ # want to weed out results from any previous test runs search = "(&{0}{1})".format(expr, self.extra_filter) - for attr in self.attr_filters: + # If we expect to return multiple results, only check the first + if expected_num > 0: + attr_filters = [self.attr_filters[0]] + else: + attr_filters = self.attr_filters + + for attr in attr_filters: res = samdb.search(base_dn, expression=search, scope=SCOPE_SUBTREE, attrs=attr, controls=self.dirsync) - self.assertTrue(len(res) == expected_num, + self.assertEqual(len(res), expected_num, "%u results, not %u for search %s, attr %s" % (len(res), expected_num, search, str(attr))) @@ -847,7 +767,8 @@ expr = self.single_obj_filter res = samdb.search(self.base_dn, expression=expr, scope=SCOPE_SUBTREE, attrs=attrs, controls=self.dirsync) - self.assertTrue(len(res) == 1 or no_result_ok) + if not no_result_ok: + self.assertEqual(1, len(res)) attr_returned = False for msg in res: @@ -887,8 +808,7 @@ attrs=['name']) # override method specifically for dirsync (total object count differs) - def assert_negative_searches(self, has_rights_to=0, - dc_mode=DC_MODE_RETURN_NONE, samdb=None): + def assert_negative_searches(self, has_rights_to=0, samdb=None): """Asserts user without rights cannot see objects in '!' searches""" if samdb is None: @@ -898,7 +818,6 @@ # here only includes the user objects (not the parent OU) total_objects = len(self.all_users) expected_results = self.negative_search_expected_results(has_rights_to, - dc_mode, total_objects) for search, expected_num in expected_results.items(): @@ -917,8 +836,7 @@ self.assert_conf_attr_searches(has_rights_to=0) self.assert_attr_visible(expect_attr=False) - dc_mode = DC_MODE_RETURN_ALL - self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + self.assert_negative_searches(has_rights_to=0) # as a final sanity-check, make sure the admin can still see the attr self.assert_conf_attr_searches(has_rights_to="all", @@ -941,7 +859,7 @@ search_flags = int(self.get_attr_search_flags(self.attr_dn)) # check we've already set the confidential flag - self.assertTrue(search_flags & SEARCH_FLAG_CONFIDENTIAL != 0) + self.assertNotEqual(0, search_flags & SEARCH_FLAG_CONFIDENTIAL) search_flags |= SEARCH_FLAG_PRESERVEONDELETE self.set_attr_search_flags(self.attr_dn, str(search_flags)) @@ -1012,18 +930,17 @@ # check we can't see the objects now, even with using dirsync controls self.assert_conf_attr_searches(has_rights_to=0) self.assert_attr_visible(expect_attr=False) - dc_mode = DC_MODE_RETURN_ALL - self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + self.assert_negative_searches(has_rights_to=0) # now delete the users (except for the user whose LDB connection # we're currently using) for user in self.all_users: - if user != self.user: + if user is not self.user: self.ldb_admin.delete(self.get_user_dn(user)) # check we still can't see the objects self.assert_conf_attr_searches(has_rights_to=0) - self.assert_negative_searches(has_rights_to=0, dc_mode=dc_mode) + self.assert_negative_searches(has_rights_to=0) def test_timing_attack(self): # Create the machine account. @@ -1183,5 +1100,38 @@ user_matching, user_non_matching = time_searches(self.ldb_user) assertRangesOverlap(user_matching, user_non_matching) +# Check that using the dirsync controls doesn't reveal confidential +# "RODC filtered attribute" values to users with only +# GUID_DRS_GET_CHANGES. The tests is so similar to the Confidential +# attribute test we base it on that. +class RodcFilteredAttrDirsync(ConfidentialAttrTestDirsync): + + def setUp(self): + super().setUp() + self.dirsync = ["dirsync:1:0:1000"] + + user_sid = self.sd_utils.get_object_sid(self.get_user_dn(self.user)) + mod = "(OA;;CR;%s;;%s)" % (security.GUID_DRS_GET_CHANGES, + str(user_sid)) + self.sd_utils.dacl_add_ace(self.base_dn, mod) + + self.ldb_user = self.get_ldb_connection(self.user, self.user_pass) + + self.addCleanup(self.sd_utils.dacl_delete_aces, self.base_dn, mod) + + def make_attr_confidential(self): + """Marks the attribute under test as being confidential AND RODC + filtered (which should mean it is not visible with only + GUID_DRS_GET_CHANGES) + """ + + # work out the original 'searchFlags' value before we overwrite it + old_value = self.get_attr_search_flags(self.attr_dn) + + self.set_attr_search_flags(self.attr_dn, str(SEARCH_FLAG_RODC_ATTRIBUTE|SEARCH_FLAG_CONFIDENTIAL)) + + # reset the value after the test completes + self.addCleanup(self.set_attr_search_flags, self.attr_dn, old_value) + TestProgram(module=__name__, opts=subunitopts) diff -Nru samba-4.17.11+dfsg/source4/dsdb/tests/python/dirsync.py samba-4.17.12+dfsg/source4/dsdb/tests/python/dirsync.py --- samba-4.17.11+dfsg/source4/dsdb/tests/python/dirsync.py 2022-08-08 14:15:39.568193400 +0000 +++ samba-4.17.12+dfsg/source4/dsdb/tests/python/dirsync.py 2023-10-09 20:16:08.411591500 +0000 @@ -3,6 +3,7 @@ # Unit tests for dirsync control # Copyright (C) Matthieu Patou 2011 # Copyright (C) Jelmer Vernooij 2014 +# Copyright (C) Catalyst.Net Ltd # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -30,7 +31,8 @@ import ldb from ldb import LdbError, SCOPE_BASE from ldb import Message, MessageElement, Dn -from ldb import FLAG_MOD_ADD, FLAG_MOD_DELETE +from ldb import FLAG_MOD_ADD, FLAG_MOD_DELETE, FLAG_MOD_REPLACE +from samba.dsdb import SEARCH_FLAG_CONFIDENTIAL, SEARCH_FLAG_RODC_ATTRIBUTE from samba.dcerpc import security, misc, drsblobs from samba.ndr import ndr_unpack, ndr_pack @@ -60,7 +62,6 @@ host = args.pop() if "://" not in host: ldaphost = "ldap://%s" % host - ldapshost = "ldaps://%s" % host else: ldaphost = host start = host.rindex("://") @@ -77,8 +78,8 @@ class DirsyncBaseTests(samba.tests.TestCase): def setUp(self): - super(DirsyncBaseTests, self).setUp() - self.ldb_admin = SamDB(ldapshost, credentials=creds, session_info=system_session(lp), lp=lp) + super().setUp() + self.ldb_admin = SamDB(ldaphost, credentials=creds, session_info=system_session(lp), lp=lp) self.base_dn = self.ldb_admin.domain_dn() self.domain_sid = security.dom_sid(self.ldb_admin.get_domain_sid()) self.user_pass = samba.generate_random_password(12, 16) @@ -87,8 +88,37 @@ # used for anonymous login print("baseDN: %s" % self.base_dn) + userou = "OU=dirsync-test" + self.ou = f"{userou},{self.base_dn}" + samba.tests.delete_force(self.ldb_admin, self.ou, controls=['tree_delete:1']) + self.ldb_admin.create_ou(self.ou) + self.addCleanup(samba.tests.delete_force, self.ldb_admin, self.ou, controls=['tree_delete:1']) + + # Regular user + self.dirsync_user = "test_dirsync_user" + self.simple_user = "test_simple_user" + self.admin_user = "test_admin_user" + self.dirsync_pass = self.user_pass + self.simple_pass = self.user_pass + self.admin_pass = self.user_pass + + self.ldb_admin.newuser(self.dirsync_user, self.dirsync_pass, userou=userou) + self.ldb_admin.newuser(self.simple_user, self.simple_pass, userou=userou) + self.ldb_admin.newuser(self.admin_user, self.admin_pass, userou=userou) + self.desc_sddl = self.sd_utils.get_sd_as_sddl(self.base_dn) + + user_sid = self.sd_utils.get_object_sid(self.get_user_dn(self.dirsync_user)) + mod = "(OA;;CR;%s;;%s)" % (security.GUID_DRS_GET_CHANGES, + str(user_sid)) + self.sd_utils.dacl_add_ace(self.base_dn, mod) + self.addCleanup(self.sd_utils.dacl_delete_aces, self.base_dn, mod) + + # add admins to the Domain Admins group + self.ldb_admin.add_remove_group_members("Domain Admins", [self.admin_user], + add_members_operation=True) + def get_user_dn(self, name): - return "CN=%s,CN=Users,%s" % (name, self.base_dn) + return ldb.Dn(self.ldb_admin, "CN={0},{1}".format(name, self.ou)) def get_ldb_connection(self, target_username, target_password): creds_tmp = Credentials() @@ -103,51 +133,15 @@ ldb_target = SamDB(url=ldaphost, credentials=creds_tmp, lp=lp) return ldb_target - # tests on ldap add operations class SimpleDirsyncTests(DirsyncBaseTests): - def setUp(self): - super(SimpleDirsyncTests, self).setUp() - # Regular user - self.dirsync_user = "test_dirsync_user" - self.simple_user = "test_simple_user" - self.admin_user = "test_admin_user" - self.ouname = None - - self.ldb_admin.newuser(self.dirsync_user, self.user_pass) - self.ldb_admin.newuser(self.simple_user, self.user_pass) - self.ldb_admin.newuser(self.admin_user, self.user_pass) - self.desc_sddl = self.sd_utils.get_sd_as_sddl(self.base_dn) - - user_sid = self.sd_utils.get_object_sid(self.get_user_dn(self.dirsync_user)) - mod = "(OA;;CR;%s;;%s)" % (security.GUID_DRS_GET_CHANGES, - str(user_sid)) - self.sd_utils.dacl_add_ace(self.base_dn, mod) - - # add admins to the Domain Admins group - self.ldb_admin.add_remove_group_members("Domain Admins", [self.admin_user], - add_members_operation=True) - - def tearDown(self): - super(SimpleDirsyncTests, self).tearDown() - delete_force(self.ldb_admin, self.get_user_dn(self.dirsync_user)) - delete_force(self.ldb_admin, self.get_user_dn(self.simple_user)) - delete_force(self.ldb_admin, self.get_user_dn(self.admin_user)) - if self.ouname: - delete_force(self.ldb_admin, self.ouname) - self.sd_utils.modify_sd_on_dn(self.base_dn, self.desc_sddl) - try: - self.ldb_admin.deletegroup("testgroup") - except Exception: - pass - # def test_dirsync_errors(self): def test_dirsync_supported(self): """Test the basic of the dirsync is supported""" self.ldb_dirsync = self.get_ldb_connection(self.dirsync_user, self.user_pass) - self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass) + self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass) res = self.ldb_admin.search(self.base_dn, expression="samaccountname=*", controls=["dirsync:1:0:1"]) res = self.ldb_dirsync.search(self.base_dn, expression="samaccountname=*", controls=["dirsync:1:0:1"]) try: @@ -173,8 +167,8 @@ def test_dirsync_errors(self): """Test if dirsync returns the correct LDAP errors in case of pb""" - self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass) - self.ldb_dirsync = self.get_ldb_connection(self.dirsync_user, self.user_pass) + self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass) + self.ldb_dirsync = self.get_ldb_connection(self.dirsync_user, self.dirsync_pass) try: self.ldb_simple.search(self.base_dn, expression="samaccountname=*", @@ -296,11 +290,11 @@ attrs=["parentGUID"], controls=["dirsync:1:0:1"]) self.assertEqual(len(res.msgs), 0) - ouname = "OU=testou,%s" % self.base_dn + ouname = "OU=testou,%s" % self.ou self.ouname = ouname self.ldb_admin.create_ou(ouname) delta = Message() - delta.dn = Dn(self.ldb_admin, str(ouname)) + delta.dn = Dn(self.ldb_admin, ouname) delta["cn"] = MessageElement("test ou", FLAG_MOD_ADD, "cn") @@ -338,7 +332,7 @@ self.assertEqual(len(res.msgs[0]), 3) def test_dirsync_othernc(self): - """Check that dirsync return information for entries that are normaly referrals (ie. other NCs)""" + """Check that dirsync return information for entries that are normally referrals (ie. other NCs)""" res = self.ldb_admin.search(self.base_dn, expression="(objectclass=configuration)", attrs=["name"], @@ -458,10 +452,10 @@ self.assertTrue(res[0].get("name") is not None) delete_force(self.ldb_admin, ouname) - def test_dirsync_linkedattributes(self): - """Check that dirsync returnd deleted objects too""" + def test_dirsync_linkedattributes_OBJECT_SECURITY(self): + """Check that dirsync returned deleted objects too""" # Let's search for members - self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass) + self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass) res = self.ldb_simple.search(self.base_dn, expression="(name=Administrators)", controls=["dirsync:1:1:1"]) @@ -499,6 +493,7 @@ self.assertEqual(len(res[0].get("member")), size) self.ldb_admin.newgroup("testgroup") + self.addCleanup(self.ldb_admin.deletegroup, "testgroup") self.ldb_admin.add_remove_group_members("testgroup", [self.simple_user], add_members_operation=True) @@ -537,11 +532,10 @@ attrs=["member"], controls=[control1]) - self.ldb_admin.deletegroup("testgroup") self.assertEqual(len(res[0].get("member")), 0) def test_dirsync_deleted_items(self): - """Check that dirsync returnd deleted objects too""" + """Check that dirsync returned deleted objects too""" # Let's create an OU ouname = "OU=testou3,%s" % self.base_dn self.ouname = ouname @@ -585,11 +579,8 @@ expression="(&(objectClass=organizationalUnit)(!(isDeleted=*)))", controls=controls) - -class ExtendedDirsyncTests(SimpleDirsyncTests): - def test_dirsync_linkedattributes_range(self): - self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass) + self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass) res = self.ldb_admin.search(self.base_dn, attrs=["member;range=1-1"], expression="(name=Administrators)", @@ -601,7 +592,7 @@ self.assertTrue(len(res[0].get("member")) > 0) def test_dirsync_linkedattributes_range_user(self): - self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass) + self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass) try: res = self.ldb_simple.search(self.base_dn, attrs=["member;range=1-1"], @@ -615,7 +606,7 @@ def test_dirsync_linkedattributes(self): flag_incr_linked = 2147483648 - self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass) + self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass) res = self.ldb_admin.search(self.base_dn, attrs=["member"], expression="(name=Administrators)", @@ -683,7 +674,7 @@ def test_dirsync_extended_dn(self): """Check that dirsync works together with the extended_dn control""" # Let's search for members - self.ldb_simple = self.get_ldb_connection(self.simple_user, self.user_pass) + self.ldb_simple = self.get_ldb_connection(self.simple_user, self.simple_pass) res = self.ldb_simple.search(self.base_dn, expression="(name=Administrators)", controls=["dirsync:1:1:1"]) @@ -711,10 +702,10 @@ self.assertIn(b";owner_sid); + + sd = security_descriptor_dacl_create(tctx, + 0, NULL, NULL, + owner_sid, + SEC_ACE_TYPE_ACCESS_ALLOWED, + SEC_FILE_READ_DATA, + 0, + NULL); + + ZERO_STRUCT(set); + set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC; + set.set_secdesc.in.file.handle = handle; + set.set_secdesc.in.secinfo_flags = SECINFO_DACL; + set.set_secdesc.in.sd = sd; + + status = smb2_setinfo_file(tree, &set); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_setinfo_file failed\n"); + + smb2_util_close(tree, handle); + ZERO_STRUCT(handle); + + for (i = 0; i < ARRAY_SIZE(tcases); i++) { + torture_comment(tctx, "Verify open with %s dispostion\n", + tcases[i].disposition_string); + + c = (struct smb2_create) { + .in.create_disposition = tcases[i].disposition, + .in.desired_access = SEC_FILE_READ_DATA, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS, + .in.fname = fname, + }; + + status = smb2_create(tree, tctx, &c); + smb2_util_close(tree, c.out.file.handle); + torture_assert_ntstatus_equal_goto( + tctx, status, tcases[i].expected_status, ret, done, + "smb2_create failed\n"); + }; + + torture_comment(tctx, "put back original sd\n"); + + c = (struct smb2_create) { + .in.desired_access = SEC_STD_WRITE_DAC, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.create_disposition = NTCREATEX_DISP_OPEN_IF, + .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS, + .in.fname = fname, + }; + + status = smb2_create(tree, tctx, &c); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + handle = c.out.file.handle; + + ZERO_STRUCT(set); + set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC; + set.set_secdesc.in.file.handle = handle; + set.set_secdesc.in.secinfo_flags = SECINFO_DACL; + set.set_secdesc.in.sd = sd_orig; + + status = smb2_setinfo_file(tree, &set); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_setinfo_file failed\n"); + + smb2_util_close(tree, handle); + ZERO_STRUCT(handle); + +done: + smb2_util_close(tree, handle); + smb2_util_unlink(tree, fname); + smb2_deltree(tree, BASEDIR); + return ret; +} + /* basic testing of SMB2 ACLs */ @@ -3017,6 +3159,7 @@ test_deny1); torture_suite_add_1smb2_test(suite, "MXAC-NOT-GRANTED", test_mxac_not_granted); + torture_suite_add_1smb2_test(suite, "OVERWRITE_READ_ONLY_FILE", test_overwrite_read_only_file); suite->description = talloc_strdup(suite, "SMB2-ACLS tests");