Skip to content

Commit

Permalink
CVE-2018-14628: python:descriptor: let samba-tool dbcheck fix the nTS…
Browse files Browse the repository at this point in the history 
…ecurityDescriptor on CN=Deleted Objects containers

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13595

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
  • Loading branch information
@metze-samba
metze-samba committed Oct 16, 2023
1 parent 7058606 commit 97e4aab
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 9 deletions.
10 changes: 8 additions & 2 deletions python/samba/dbchecker.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2469,7 +2469,7 @@ def check_object(self, dn, requested_attrs=None):
error_count += 1
continue

if self.reset_well_known_acls:
if dn == deleted_objects_dn or self.reset_well_known_acls:
try:
well_known_sd = self.get_wellknown_sd(dn)
except KeyError:
Expand All @@ -2478,7 +2478,13 @@ def check_object(self, dn, requested_attrs=None):
current_sd = ndr_unpack(security.descriptor,
obj[attrname][0])

diff = get_diff_sds(well_known_sd, current_sd, security.dom_sid(self.samdb.get_domain_sid()))
ignoreAdditionalACEs = False
if not self.reset_well_known_acls:
ignoreAdditionalACEs = True

diff = get_diff_sds(well_known_sd, current_sd,
security.dom_sid(self.samdb.get_domain_sid()),
ignoreAdditionalACEs=ignoreAdditionalACEs)
if diff != "":
self.err_wrong_default_sd(dn, well_known_sd, diff)
error_count += 1
Expand Down
15 changes: 14 additions & 1 deletion python/samba/descriptor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ def get_wellknown_sds(samdb):
# Then subcontainers
subcontainers = [
(ldb.Dn(samdb, "%s" % str(samdb.domain_dn())), get_domain_descriptor),
(ldb.Dn(samdb, "CN=Deleted Objects,%s" % str(samdb.domain_dn())), get_deletedobjects_descriptor),
(ldb.Dn(samdb, "CN=LostAndFound,%s" % str(samdb.domain_dn())), get_domain_delete_protected2_descriptor),
(ldb.Dn(samdb, "CN=System,%s" % str(samdb.domain_dn())), get_domain_delete_protected1_descriptor),
(ldb.Dn(samdb, "CN=Infrastructure,%s" % str(samdb.domain_dn())), get_domain_infrastructure_descriptor),
Expand All @@ -505,6 +506,7 @@ def get_wellknown_sds(samdb):
(ldb.Dn(samdb, "CN=MicrosoftDNS,CN=System,%s" % str(samdb.domain_dn())), get_dns_domain_microsoft_dns_descriptor),

(ldb.Dn(samdb, "%s" % str(samdb.get_config_basedn())), get_config_descriptor),
(ldb.Dn(samdb, "CN=Deleted Objects,%s" % str(samdb.get_config_basedn())), get_deletedobjects_descriptor),
(ldb.Dn(samdb, "CN=NTDS Quotas,%s" % str(samdb.get_config_basedn())), get_config_ntds_quotas_descriptor),
(ldb.Dn(samdb, "CN=LostAndFoundConfig,%s" % str(samdb.get_config_basedn())), get_config_delete_protected1wd_descriptor),
(ldb.Dn(samdb, "CN=Services,%s" % str(samdb.get_config_basedn())), get_config_delete_protected1_descriptor),
Expand All @@ -529,6 +531,9 @@ def get_wellknown_sds(samdb):
if ldb.Dn(samdb, nc.decode('utf8')) == dnsforestdn:
c = (ldb.Dn(samdb, "%s" % str(dnsforestdn)), get_dns_partition_descriptor)
subcontainers.append(c)
c = (ldb.Dn(samdb, "CN=Deleted Objects,%s" % str(dnsforestdn)),
get_deletedobjects_descriptor)
subcontainers.append(c)
c = (ldb.Dn(samdb, "CN=Infrastructure,%s" % str(dnsforestdn)),
get_domain_delete_protected1_descriptor)
subcontainers.append(c)
Expand All @@ -544,6 +549,9 @@ def get_wellknown_sds(samdb):
if ldb.Dn(samdb, nc.decode('utf8')) == dnsdomaindn:
c = (ldb.Dn(samdb, "%s" % str(dnsdomaindn)), get_dns_partition_descriptor)
subcontainers.append(c)
c = (ldb.Dn(samdb, "CN=Deleted Objects,%s" % str(dnsdomaindn)),
get_deletedobjects_descriptor)
subcontainers.append(c)
c = (ldb.Dn(samdb, "CN=Infrastructure,%s" % str(dnsdomaindn)),
get_domain_delete_protected1_descriptor)
subcontainers.append(c)
Expand Down Expand Up @@ -636,7 +644,8 @@ def get_clean_sd(sd):
return sd_clean


def get_diff_sds(refsd, cursd, domainsid, checkSacl=True):
def get_diff_sds(refsd, cursd, domainsid, checkSacl=True,
ignoreAdditionalACEs=False):
"""Get the difference between 2 sd
This function split the textual representation of ACL into smaller
Expand Down Expand Up @@ -691,6 +700,10 @@ def get_diff_sds(refsd, cursd, domainsid, checkSacl=True):
h_ref.remove(k)

if len(h_cur) + len(h_ref) > 0:
if txt == "" and len(h_ref) == 0:
if ignoreAdditionalACEs:
return ""

txt = "%s\tPart %s is different between reference" \
" and current here is the detail:\n" % (txt, part)

Expand Down
8 changes: 4 additions & 4 deletions ...rovisions/release-4-5-0-pre1/expected-dbcheck-link-output-missing-link-sid-corruption.txt
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
Change DN to <GUID=0da8f25e-d110-11e8-80b7-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=1>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3769>;<RMD_ORIGINATING_USN=3769>;<RMD_VERSION=2>;<SID=S-1-5-21-4177067393-1453636373-93818738-771>;CN=missingsidu1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES]
Change DN to <GUID=66eb8f52-d110-11e8-ab9b-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3768>;<RMD_ORIGINATING_USN=3768>;<RMD_VERSION=1>;<SID=S-1-5-21-4177067393-1453636373-93818738-772>;CN=missingsidu2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES]
Change DN to <GUID=0da8f25e-d110-11e8-80b7-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=1>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3773>;<RMD_ORIGINATING_USN=3773>;<RMD_VERSION=2>;<SID=S-1-5-21-4177067393-1453636373-93818738-771>;CN=missingsidu1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES]
Change DN to <GUID=66eb8f52-d110-11e8-ab9b-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3772>;<RMD_ORIGINATING_USN=3772>;<RMD_VERSION=1>;<SID=S-1-5-21-4177067393-1453636373-93818738-772>;CN=missingsidu2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp? [YES]
Checked 231 objects (2 errors)
Checking 231 objects
ERROR: missing DN SID component for member in object CN=missingsidg3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp - <GUID=0da8f25e-d110-11e8-80b7-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=1>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3769>;<RMD_ORIGINATING_USN=3769>;<RMD_VERSION=2>;CN=missingsidu1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
ERROR: missing DN SID component for member in object CN=missingsidg3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp - <GUID=66eb8f52-d110-11e8-ab9b-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3768>;<RMD_ORIGINATING_USN=3768>;<RMD_VERSION=1>;CN=missingsidu2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
ERROR: missing DN SID component for member in object CN=missingsidg3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp - <GUID=0da8f25e-d110-11e8-80b7-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=1>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3773>;<RMD_ORIGINATING_USN=3773>;<RMD_VERSION=2>;CN=missingsidu1,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
ERROR: missing DN SID component for member in object CN=missingsidg3,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp - <GUID=66eb8f52-d110-11e8-ab9b-3c970ec68461>;<RMD_ADDTIME=123456789000000000>;<RMD_CHANGETIME=123456789000000000>;<RMD_FLAGS=0>;<RMD_INVOCID=4e4496a3-7fb8-4f97-8a33-d238db8b5e2d>;<RMD_LOCAL_USN=3772>;<RMD_ORIGINATING_USN=3772>;<RMD_VERSION=1>;CN=missingsidu2,CN=Users,DC=release-4-5-0-pre1,DC=samba,DC=corp
Fixed missing DN SID on attribute member
Fixed missing DN SID on attribute member
2 changes: 1 addition & 1 deletion source4/selftest/provisions/release-4-5-0-pre1/expected-links-after-dbcheck.ldif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1381,7 +1381,7 @@ uSNChanged: 3597
dn: CN=Deleted Objects,DC=release-4-5-0-pre1,DC=samba,DC=corp
objectCategory: CN=Container,CN=Schema,CN=Configuration,DC=release-4-5-0-pre1,
DC=samba,DC=corp
uSNChanged: 3377
uSNChanged: 3750

# record 215
dn: CN=ForeignSecurityPrincipals,DC=release-4-5-0-pre1,DC=samba,DC=corp
Expand Down
2 changes: 1 addition & 1 deletion source4/selftest/provisions/release-4-5-0-pre1/rootdse-version.final.txt
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
highestCommittedUSN: 3746
highestCommittedUSN: 3750
12 changes: 12 additions & 0 deletions testprogs/blackbox/dbcheck-links.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@ dbcheck()
fi
}

dbcheck_acl_reset()
{
$PYTHON $BINDIR/samba-tool dbcheck -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --cross-ncs --fix --yes --attrs=nTSecurityDescriptor
}

dbcheck_acl_clean()
{
$PYTHON $BINDIR/samba-tool dbcheck -H tdb://$PREFIX_ABS/${RELEASE}/private/sam.ldb --cross-ncs --attrs=nTSecurityDescriptor
}

dbcheck_dangling()
{
dbcheck "" "1" "--selftest-check-expired-tombstones"
Expand Down Expand Up @@ -925,6 +935,8 @@ EOF
remove_directory $PREFIX_ABS/${RELEASE}

testit $RELEASE undump || failed=$(expr $failed + 1)
testit_expect_failure "dbcheck_acl_reset" dbcheck_acl_reset || failed=$(expr $failed + 1)
testit "dbcheck_acl_clean" dbcheck_acl_clean || failed=$(expr $failed + 1)
testit "add_two_more_users" add_two_more_users || failed=$(expr $failed + 1)
testit "add_four_more_links" add_four_more_links || failed=$(expr $failed + 1)
testit "remove_one_link" remove_one_link || failed=$(expr $failed + 1)
Expand Down

0 comments on commit 97e4aab

Please sign in to comment.