Attachment 17333 Details for Bug 14833 – proposed patch for master v2

[patch] proposed patch for master v2

bug-14833v2.patch (text/plain), 83.14 KB, created by Jennifer Sutton on 2022-06-09 08:35:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jennifer Sutton

Created: 2022-06-09 08:35:05 UTC

Size: 83.14 KB

patch

obsolete

>From c4990af941de117fb38aba06e26bad7097d7632e Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Wed, 1 Jun 2022 16:07:17 +1200
>Subject: [PATCH 01/14] s4-acl: Add tests for validated dNSHostName write
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> selftest/knownfail.d/validated-dns-host-name |  15 +
> source4/dsdb/tests/python/acl.py             | 757 +++++++++++++++++++
> 2 files changed, 772 insertions(+)
> create mode 100644 selftest/knownfail.d/validated-dns-host-name
>
>diff --git a/selftest/knownfail.d/validated-dns-host-name b/selftest/knownfail.d/validated-dns-host-name
>new file mode 100644
>index 00000000000..ee51f440969
>--- /dev/null
>+++ b/selftest/knownfail.d/validated-dns-host-name
>@@ -0,0 +1,15 @@
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_account_no_dollar\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_allowed_suffixes\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_case\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_dollar\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_empty_string\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_invalid\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_no_suffix\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_no_value\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_spn\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_spn_matching_account_name_new\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_spn_matching_account_name_original\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_wrong_prefix\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_wrong_suffix\(
>+^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_spn_matching_dns_host_name_invalid\(
>diff --git a/source4/dsdb/tests/python/acl.py b/source4/dsdb/tests/python/acl.py
>index 70dca9b7678..9052c986a34 100755
>--- a/source4/dsdb/tests/python/acl.py
>+++ b/source4/dsdb/tests/python/acl.py
>@@ -300,6 +300,7 @@ class AclModifyTests(AclTests):
>         delete_force(self.ldb_admin, "CN=test_modify_group1,CN=Users," + self.base_dn)
>         delete_force(self.ldb_admin, "CN=test_modify_group2,CN=Users," + self.base_dn)
>         delete_force(self.ldb_admin, "CN=test_modify_group3,CN=Users," + self.base_dn)
>+        delete_force(self.ldb_admin, "CN=test_mod_hostname,OU=test_modify_ou1," + self.base_dn)
>         delete_force(self.ldb_admin, "OU=test_modify_ou1," + self.base_dn)
>         delete_force(self.ldb_admin, self.get_user_dn(self.user_with_wp))
>         delete_force(self.ldb_admin, self.get_user_dn(self.user_with_sm))
>@@ -651,6 +652,762 @@ Member: CN=test_modify_user2,CN=Users,""" + self.base_dn
>         else:
>             self.fail()
> 
>+    def test_modify_dns_host_name(self):
>+        '''Test modifying dNSHostName with validated write'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        host_name = f'{account_name}.{self.ldb_user.domain_dns_name()}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_no_validated_write(self):
>+        '''Test modifying dNSHostName without validated write'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        host_name = f'{account_name}.{self.ldb_user.domain_dns_name()}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError as err:
>+            num, estr = err.args
>+            self.assertEqual(ERR_INSUFFICIENT_ACCESS_RIGHTS, num)
>+        else:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_invalid(self):
>+        '''Test modifying dNSHostName to an invalid value'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        host_name = 'invalid'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError as err:
>+            num, estr = err.args
>+            self.assertEqual(ERR_CONSTRAINT_VIOLATION, num)
>+        else:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_invalid_wp(self):
>+        '''Test modifying dNSHostName to an invalid value when we have WP'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Write Property.
>+        mod = (f'(OA;CI;WP;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        host_name = 'invalid'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_invalid_non_computer(self):
>+        '''Test modifying dNSHostName to an invalid value on a non-computer'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'user',
>+            'sAMAccountName': f'{account_name}',
>+        })
>+
>+        host_name = 'invalid'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError as err:
>+            num, estr = err.args
>+            self.assertEqual(ERR_INSUFFICIENT_ACCESS_RIGHTS, num)
>+        else:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_no_value(self):
>+        '''Test modifying dNSHostName with validated write with no value'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement([],
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError as err:
>+            num, estr = err.args
>+            self.assertEqual(ERR_OPERATIONS_ERROR, num)
>+        else:
>+            # Windows accepts this.
>+            pass
>+
>+    def test_modify_dns_host_name_empty_string(self):
>+        '''Test modifying dNSHostName with validated write of an empty string'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement('\0',
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError as err:
>+            num, estr = err.args
>+            self.assertEqual(ERR_CONSTRAINT_VIOLATION, num)
>+        else:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_dollar(self):
>+        '''Test modifying dNSHostName with validated write of a value including a dollar'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        host_name = f'{account_name}$.{self.ldb_user.domain_dns_name()}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError as err:
>+            num, estr = err.args
>+            self.assertEqual(ERR_CONSTRAINT_VIOLATION, num)
>+        else:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_account_no_dollar(self):
>+        '''Test modifying dNSHostName with validated write with no dollar in sAMAccountName'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}',
>+        })
>+
>+        host_name = f'{account_name}.{self.ldb_user.domain_dns_name()}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_no_suffix(self):
>+        '''Test modifying dNSHostName with validated write of a value missing the suffix'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        host_name = f'{account_name}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError as err:
>+            num, estr = err.args
>+            self.assertEqual(ERR_CONSTRAINT_VIOLATION, num)
>+        else:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_wrong_prefix(self):
>+        '''Test modifying dNSHostName with validated write of a value with the wrong prefix'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        host_name = f'invalid.{self.ldb_user.domain_dns_name()}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError as err:
>+            num, estr = err.args
>+            self.assertEqual(ERR_CONSTRAINT_VIOLATION, num)
>+        else:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_wrong_suffix(self):
>+        '''Test modifying dNSHostName with validated write of a value with the wrong suffix'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        host_name = f'{account_name}.invalid.example.com'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError as err:
>+            num, estr = err.args
>+            self.assertEqual(ERR_CONSTRAINT_VIOLATION, num)
>+        else:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_case(self):
>+        '''Test modifying dNSHostName with validated write of a value with irregular case'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        host_name = f'{account_name}.{self.ldb_user.domain_dns_name()}'
>+        host_name = host_name.capitalize()
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_allowed_suffixes(self):
>+        '''Test modifying dNSHostName with validated write and an allowed suffix'''
>+
>+        allowed_suffix = 'suffix.that.is.allowed'
>+
>+        # Add the allowed suffix.
>+
>+        res = self.ldb_admin.search(self.base_dn,
>+                                    scope=SCOPE_BASE,
>+                                    attrs=['msDS-AllowedDNSSuffixes'])
>+        self.assertEqual(1, len(res))
>+        old_allowed_suffixes = res[0].get('msDS-AllowedDNSSuffixes')
>+
>+        def modify_allowed_suffixes(suffixes):
>+            if suffixes is None:
>+                suffixes = []
>+                flag = FLAG_MOD_DELETE
>+            else:
>+                flag = FLAG_MOD_REPLACE
>+
>+            m = Message(Dn(self.ldb_admin, self.base_dn))
>+            m['msDS-AllowedDNSSuffixes'] = MessageElement(
>+                suffixes,
>+                flag,
>+                'msDS-AllowedDNSSuffixes')
>+            self.ldb_admin.modify(m)
>+
>+        self.addCleanup(modify_allowed_suffixes, old_allowed_suffixes)
>+
>+        if old_allowed_suffixes is None:
>+            allowed_suffixes = []
>+        else:
>+            allowed_suffixes = list(old_allowed_suffixes)
>+
>+        if (allowed_suffix not in allowed_suffixes and
>+            allowed_suffix.encode('utf-8') not in allowed_suffixes):
>+                allowed_suffixes.append(allowed_suffix)
>+
>+        modify_allowed_suffixes(allowed_suffixes)
>+
>+        # Create the account and run the test.
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        host_name = f'{account_name}.{allowed_suffix}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['dNSHostName'] = MessageElement(host_name,
>+                                          FLAG_MOD_REPLACE,
>+                                          'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_spn(self):
>+        '''Test modifying dNSHostName and SPN with validated write'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_VALIDATE_SPN};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        host_name = f'{account_name}.{self.ldb_user.domain_dns_name()}'
>+        spn = f'host/{host_name}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['0'] = MessageElement(host_name,
>+                                FLAG_MOD_REPLACE,
>+                                'dNSHostName')
>+        m['1'] = MessageElement(spn,
>+                                FLAG_MOD_ADD,
>+                                'servicePrincipalName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError:
>+            self.fail()
>+
>+    def test_modify_spn_matching_dns_host_name_invalid(self):
>+        '''Test modifying SPN with validated write, matching a valid dNSHostName '''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Write Property.
>+        mod = (f'(OA;CI;WP;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_VALIDATE_SPN};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        invalid_host_name = 'invalid'
>+
>+        host_name = f'{account_name}.{self.ldb_user.domain_dns_name()}'
>+        spn = f'host/{host_name}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['0'] = MessageElement(invalid_host_name,
>+                                FLAG_MOD_REPLACE,
>+                                'dNSHostName')
>+        m['1'] = MessageElement(spn,
>+                                FLAG_MOD_ADD,
>+                                'servicePrincipalName')
>+        m['2'] = MessageElement(host_name,
>+                                FLAG_MOD_REPLACE,
>+                                'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError:
>+            self.fail()
>+
>+    def test_modify_spn_matching_dns_host_name_original(self):
>+        '''Test modifying SPN with validated write, matching the original dNSHostName '''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_VALIDATE_SPN};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        original_host_name = 'invalid_host_name'
>+        original_spn = 'host/{original_host_name}'
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+            'dNSHostName': original_host_name,
>+        })
>+
>+        host_name = f'{account_name}.{self.ldb_user.domain_dns_name()}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['0'] = MessageElement(original_spn,
>+                                FLAG_MOD_ADD,
>+                                'servicePrincipalName')
>+        m['1'] = MessageElement(host_name,
>+                                FLAG_MOD_REPLACE,
>+                                'dNSHostName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError as err:
>+            num, estr = err.args
>+            self.assertEqual(ERR_CONSTRAINT_VIOLATION, num)
>+        else:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_spn_matching_account_name_original(self):
>+        '''Test modifying dNSHostName and SPN with validated write, matching the original sAMAccountName'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        sam_account_name = '3e0abfd0-126a-11d0-a060-00aa006c33ed'
>+
>+        # Grant Write Property.
>+        mod = (f'(OA;CI;WP;{sam_account_name};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_VALIDATE_SPN};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        new_account_name = 'test_mod_hostname2'
>+        host_name = f'{account_name}.{self.ldb_user.domain_dns_name()}'
>+        spn = f'host/{host_name}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['0'] = MessageElement(host_name,
>+                                FLAG_MOD_REPLACE,
>+                                'dNSHostName')
>+        m['1'] = MessageElement(spn,
>+                                FLAG_MOD_ADD,
>+                                'servicePrincipalName')
>+        m['2'] = MessageElement(f'{new_account_name}$',
>+                                FLAG_MOD_REPLACE,
>+                                'sAMAccountName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError as err:
>+            num, estr = err.args
>+            self.assertEqual(ERR_CONSTRAINT_VIOLATION, num)
>+        else:
>+            self.fail()
>+
>+    def test_modify_dns_host_name_spn_matching_account_name_new(self):
>+        '''Test modifying dNSHostName and SPN with validated write, matching the new sAMAccountName'''
>+
>+        ou_dn = f'OU=test_modify_ou1,{self.base_dn}'
>+
>+        account_name = 'test_mod_hostname'
>+        dn = f'CN={account_name},{ou_dn}'
>+
>+        self.ldb_admin.create_ou(ou_dn)
>+
>+        sam_account_name = '3e0abfd0-126a-11d0-a060-00aa006c33ed'
>+
>+        # Grant Write Property.
>+        mod = (f'(OA;CI;WP;{sam_account_name};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+        # Grant Validated Write.
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+        mod = (f'(OA;CI;SW;{security.GUID_DRS_VALIDATE_SPN};;'
>+               f'{self.user_sid})')
>+        self.sd_utils.dacl_add_ace(ou_dn, mod)
>+
>+        # Create the account.
>+        self.ldb_admin.add({
>+            'dn': dn,
>+            'objectClass': 'computer',
>+            'sAMAccountName': f'{account_name}$',
>+        })
>+
>+        new_account_name = 'test_mod_hostname2'
>+        new_host_name = f'{new_account_name}.{self.ldb_user.domain_dns_name()}'
>+        new_spn = f'host/{new_host_name}'
>+
>+        m = Message(Dn(self.ldb_user, dn))
>+        m['0'] = MessageElement(new_spn,
>+                                FLAG_MOD_ADD,
>+                                'servicePrincipalName')
>+        m['1'] = MessageElement(new_host_name,
>+                                FLAG_MOD_REPLACE,
>+                                'dNSHostName')
>+        m['2'] = MessageElement(f'{new_account_name}$',
>+                                FLAG_MOD_REPLACE,
>+                                'sAMAccountName')
>+        try:
>+            self.ldb_user.modify(m)
>+        except LdbError:
>+            self.fail()
>+
> # enable these when we have search implemented
> 
> 
>-- 
>2.35.0
>
>
>From e30a3f75f9f6aeabe6b49546780282dff0887955 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 7 Jun 2022 17:35:35 +1200
>Subject: [PATCH 02/14] tests/py_credentials: Add tests for setting dNSHostName
> with LogonGetDomainInfo()
>
>Test that the value is properly validated, and that it can be set
>regardless of rights on the account.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> python/samba/tests/py_credentials.py        | 236 +++++++++++++++++++-
> selftest/knownfail.d/netlogon-dns-host-name |   2 +
> 2 files changed, 236 insertions(+), 2 deletions(-)
> create mode 100644 selftest/knownfail.d/netlogon-dns-host-name
>
>diff --git a/python/samba/tests/py_credentials.py b/python/samba/tests/py_credentials.py
>index ecb8271b595..f106eb70c55 100644
>--- a/python/samba/tests/py_credentials.py
>+++ b/python/samba/tests/py_credentials.py
>@@ -18,6 +18,8 @@
> from samba.tests import TestCase, delete_force
> import os
> 
>+import ldb
>+
> import samba
> from samba.auth import system_session
> from samba.credentials import (
>@@ -25,7 +27,7 @@ from samba.credentials import (
>     CLI_CRED_NTLMv2_AUTH,
>     CLI_CRED_NTLM_AUTH,
>     DONT_USE_KERBEROS)
>-from samba.dcerpc import netlogon, ntlmssp, srvsvc
>+from samba.dcerpc import lsa, netlogon, ntlmssp, security, srvsvc
> from samba.dcerpc.netlogon import (
>     netr_Authenticator,
>     netr_WorkstationInformation,
>@@ -36,10 +38,11 @@ from samba.dsdb import (
>     UF_WORKSTATION_TRUST_ACCOUNT,
>     UF_PASSWD_NOTREQD,
>     UF_NORMAL_ACCOUNT)
>-from samba.ndr import ndr_pack
>+from samba.ndr import ndr_pack, ndr_unpack
> from samba.samdb import SamDB
> from samba import NTSTATUSError, ntstatus
> from samba.common import get_string
>+from samba.sd_utils import SDUtils
> 
> import ctypes
> 
>@@ -105,6 +108,235 @@ class PyCredentialsTests(TestCase):
>         (authenticator, subsequent) = self.get_authenticator(c)
>         self.do_NetrLogonGetDomainInfo(c, authenticator, subsequent)
> 
>+    # Test using LogonGetDomainInfo to update dNSHostName to an allowed value.
>+    def test_set_dns_hostname_valid(self):
>+        c = self.get_netlogon_connection()
>+        authenticator, subsequent = self.get_authenticator(c)
>+
>+        domain_hostname = self.ldb.domain_dns_name()
>+
>+        new_dns_hostname = f'{self.machine_name}.{domain_hostname}'
>+        new_dns_hostname = new_dns_hostname.encode('utf-8')
>+
>+        query = netr_WorkstationInformation()
>+        query.os_name = lsa.String('some OS')
>+        query.dns_hostname = new_dns_hostname
>+
>+        c.netr_LogonGetDomainInfo(
>+            server_name=self.server,
>+            computer_name=self.user_creds.get_workstation(),
>+            credential=authenticator,
>+            return_authenticator=subsequent,
>+            level=1,
>+            query=query)
>+
>+        # Check the result.
>+
>+        res = self.ldb.search(self.machine_dn,
>+                              scope=ldb.SCOPE_BASE,
>+                              attrs=['dNSHostName'])
>+        self.assertEqual(1, len(res))
>+
>+        got_dns_hostname = res[0].get('dNSHostName', idx=0)
>+        self.assertEqual(new_dns_hostname, got_dns_hostname)
>+
>+    # Test using LogonGetDomainInfo to update dNSHostName to an allowed value,
>+    # when we are denied the right to do so.
>+    def test_set_dns_hostname_valid_denied(self):
>+        c = self.get_netlogon_connection()
>+        authenticator, subsequent = self.get_authenticator(c)
>+
>+        res = self.ldb.search(self.machine_dn,
>+                              scope=ldb.SCOPE_BASE,
>+                              attrs=['objectSid'])
>+        self.assertEqual(1, len(res))
>+
>+        machine_sid = ndr_unpack(security.dom_sid,
>+                                 res[0].get('objectSid', idx=0))
>+
>+        sd_utils = SDUtils(self.ldb)
>+
>+        # Deny Validated Write and Write Property.
>+        mod = (f'(OD;;SWWP;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{machine_sid})')
>+        sd_utils.dacl_add_ace(self.machine_dn, mod)
>+
>+        domain_hostname = self.ldb.domain_dns_name()
>+
>+        new_dns_hostname = f'{self.machine_name}.{domain_hostname}'
>+        new_dns_hostname = new_dns_hostname.encode('utf-8')
>+
>+        query = netr_WorkstationInformation()
>+        query.os_name = lsa.String('some OS')
>+        query.dns_hostname = new_dns_hostname
>+
>+        c.netr_LogonGetDomainInfo(
>+            server_name=self.server,
>+            computer_name=self.user_creds.get_workstation(),
>+            credential=authenticator,
>+            return_authenticator=subsequent,
>+            level=1,
>+            query=query)
>+
>+        # Check the result.
>+
>+        res = self.ldb.search(self.machine_dn,
>+                              scope=ldb.SCOPE_BASE,
>+                              attrs=['dNSHostName'])
>+        self.assertEqual(1, len(res))
>+
>+        got_dns_hostname = res[0].get('dNSHostName', idx=0)
>+        self.assertEqual(new_dns_hostname, got_dns_hostname)
>+
>+    # Ensure we can't use LogonGetDomainInfo to update dNSHostName to an
>+    # invalid value, even with rights.
>+    def test_set_dns_hostname_invalid(self):
>+        c = self.get_netlogon_connection()
>+        authenticator, subsequent = self.get_authenticator(c)
>+
>+        res = self.ldb.search(self.machine_dn,
>+                              scope=ldb.SCOPE_BASE,
>+                              attrs=['objectSid'])
>+        self.assertEqual(1, len(res))
>+
>+        machine_sid = ndr_unpack(security.dom_sid,
>+                                 res[0].get('objectSid', idx=0))
>+
>+        sd_utils = SDUtils(self.ldb)
>+
>+        # Grant Validated Write and Write Property.
>+        mod = (f'(OA;;SWWP;{security.GUID_DRS_DNS_HOST_NAME};;'
>+               f'{machine_sid})')
>+        sd_utils.dacl_add_ace(self.machine_dn, mod)
>+
>+        new_dns_hostname = b'invalid'
>+
>+        query = netr_WorkstationInformation()
>+        query.os_name = lsa.String('some OS')
>+        query.dns_hostname = new_dns_hostname
>+
>+        c.netr_LogonGetDomainInfo(
>+            server_name=self.server,
>+            computer_name=self.user_creds.get_workstation(),
>+            credential=authenticator,
>+            return_authenticator=subsequent,
>+            level=1,
>+            query=query)
>+
>+        # Check the result.
>+
>+        res = self.ldb.search(self.machine_dn,
>+                              scope=ldb.SCOPE_BASE,
>+                              attrs=['dNSHostName'])
>+        self.assertEqual(1, len(res))
>+
>+        got_dns_hostname = res[0].get('dNSHostName', idx=0)
>+        self.assertIsNone(got_dns_hostname)
>+
>+    # Show we can't use LogonGetDomainInfo to set the dNSHostName to just the
>+    # machine name.
>+    def test_set_dns_hostname_to_machine_name(self):
>+        c = self.get_netlogon_connection()
>+        authenticator, subsequent = self.get_authenticator(c)
>+
>+        new_dns_hostname = self.machine_name.encode('utf-8')
>+
>+        query = netr_WorkstationInformation()
>+        query.os_name = lsa.String('some OS')
>+        query.dns_hostname = new_dns_hostname
>+
>+        c.netr_LogonGetDomainInfo(
>+            server_name=self.server,
>+            computer_name=self.user_creds.get_workstation(),
>+            credential=authenticator,
>+            return_authenticator=subsequent,
>+            level=1,
>+            query=query)
>+
>+        # Check the result.
>+
>+        res = self.ldb.search(self.machine_dn,
>+                              scope=ldb.SCOPE_BASE,
>+                              attrs=['dNSHostName'])
>+        self.assertEqual(1, len(res))
>+
>+        got_dns_hostname = res[0].get('dNSHostName', idx=0)
>+        self.assertIsNone(got_dns_hostname)
>+
>+    # Show we can't use LogonGetDomainInfo to set dNSHostName with an invalid
>+    # suffix.
>+    def test_set_dns_hostname_invalid_suffix(self):
>+        c = self.get_netlogon_connection()
>+        authenticator, subsequent = self.get_authenticator(c)
>+
>+        domain_hostname = self.ldb.domain_dns_name()
>+
>+        new_dns_hostname = f'{self.machine_name}.foo.{domain_hostname}'
>+        new_dns_hostname = new_dns_hostname.encode('utf-8')
>+
>+        query = netr_WorkstationInformation()
>+        query.os_name = lsa.String('some OS')
>+        query.dns_hostname = new_dns_hostname
>+
>+        c.netr_LogonGetDomainInfo(
>+            server_name=self.server,
>+            computer_name=self.user_creds.get_workstation(),
>+            credential=authenticator,
>+            return_authenticator=subsequent,
>+            level=1,
>+            query=query)
>+
>+        # Check the result.
>+
>+        res = self.ldb.search(self.machine_dn,
>+                              scope=ldb.SCOPE_BASE,
>+                              attrs=['dNSHostName'])
>+        self.assertEqual(1, len(res))
>+
>+        got_dns_hostname = res[0].get('dNSHostName', idx=0)
>+        self.assertIsNone(got_dns_hostname)
>+
>+    # Test that setting the HANDLES_SPN_UPDATE flag inhibits the dNSHostName
>+    # update, but other attributes are still updated.
>+    def test_set_dns_hostname_with_flag(self):
>+        c = self.get_netlogon_connection()
>+        authenticator, subsequent = self.get_authenticator(c)
>+
>+        domain_hostname = self.ldb.domain_dns_name()
>+
>+        new_dns_hostname = f'{self.machine_name}.{domain_hostname}'
>+        new_dns_hostname = new_dns_hostname.encode('utf-8')
>+
>+        operating_system = 'some OS'
>+
>+        query = netr_WorkstationInformation()
>+        query.os_name = lsa.String(operating_system)
>+
>+        query.dns_hostname = new_dns_hostname
>+        query.workstation_flags = netlogon.NETR_WS_FLAG_HANDLES_SPN_UPDATE
>+
>+        c.netr_LogonGetDomainInfo(
>+            server_name=self.server,
>+            computer_name=self.user_creds.get_workstation(),
>+            credential=authenticator,
>+            return_authenticator=subsequent,
>+            level=1,
>+            query=query)
>+
>+        # Check the result.
>+
>+        res = self.ldb.search(self.machine_dn,
>+                              scope=ldb.SCOPE_BASE,
>+                              attrs=['dNSHostName',
>+                                     'operatingSystem'])
>+        self.assertEqual(1, len(res))
>+
>+        got_dns_hostname = res[0].get('dNSHostName', idx=0)
>+        self.assertIsNone(got_dns_hostname)
>+
>+        got_os = res[0].get('operatingSystem', idx=0)
>+        self.assertEqual(operating_system.encode('utf-8'), got_os)
>+
>     def test_SamLogonEx(self):
>         c = self.get_netlogon_connection()
> 
>diff --git a/selftest/knownfail.d/netlogon-dns-host-name b/selftest/knownfail.d/netlogon-dns-host-name
>new file mode 100644
>index 00000000000..2d0a0ec570a
>--- /dev/null
>+++ b/selftest/knownfail.d/netlogon-dns-host-name
>@@ -0,0 +1,2 @@
>+^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_invalid_suffix\(
>+^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_with_flag\(
>-- 
>2.35.0
>
>
>From e40b582f962c738b944a1289be9a030e746598d3 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Wed, 1 Jun 2022 16:08:42 +1200
>Subject: [PATCH 03/14] dsdb: Implement validated dNSHostName write
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> selftest/knownfail.d/validated-dns-host-name |  12 -
> source4/dsdb/samdb/ldb_modules/acl.c         | 235 +++++++++++++++++++
> 2 files changed, 235 insertions(+), 12 deletions(-)
>
>diff --git a/selftest/knownfail.d/validated-dns-host-name b/selftest/knownfail.d/validated-dns-host-name
>index ee51f440969..4b6165833e2 100644
>--- a/selftest/knownfail.d/validated-dns-host-name
>+++ b/selftest/knownfail.d/validated-dns-host-name
>@@ -1,15 +1,3 @@
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_account_no_dollar\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_allowed_suffixes\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_case\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_dollar\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_empty_string\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_invalid\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_no_suffix\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_no_value\(
> ^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_spn\(
> ^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_spn_matching_account_name_new\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_spn_matching_account_name_original\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_wrong_prefix\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_wrong_suffix\(
> ^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_spn_matching_dns_host_name_invalid\(
>diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
>index 289c885cdb1..d94dc10659d 100644
>--- a/source4/dsdb/samdb/ldb_modules/acl.c
>+++ b/source4/dsdb/samdb/ldb_modules/acl.c
>@@ -801,6 +801,229 @@ static int acl_check_spn(TALLOC_CTX *mem_ctx,
> 	return LDB_SUCCESS;
> }
> 
>+static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
>+				   struct ldb_module *module,
>+				   struct ldb_request *req,
>+				   const struct ldb_message_element *el,
>+				   struct security_descriptor *sd,
>+				   struct dom_sid *sid,
>+				   const struct dsdb_attribute *attr,
>+				   const struct dsdb_class *objectclass)
>+{
>+	int ret;
>+	unsigned i;
>+	TALLOC_CTX *tmp_ctx = NULL;
>+	struct ldb_context *ldb = ldb_module_get_ctx(module);
>+	const struct ldb_message_element *oc_el = NULL;
>+	const struct ldb_message_element *allowed_suffixes = NULL;
>+	struct ldb_result *nc_res = NULL;
>+	struct ldb_dn *nc_root = NULL;
>+	const char *nc_dns_name = NULL;
>+	const char *dns_host_name_value = NULL;
>+	const char *dns_host_name = NULL;
>+	const char *samAccountName = NULL;
>+	size_t account_name_len;
>+	const struct ldb_message *search_res = NULL;
>+
>+	static const char *nc_attrs[] = {
>+		"msDS-AllowedDNSSuffixes",
>+		NULL
>+	};
>+
>+	if (el->num_values == 0) {
>+		return LDB_SUCCESS;
>+	}
>+	dns_host_name_value = (const char *)el->values[0].data;
>+
>+	tmp_ctx = talloc_new(mem_ctx);
>+	if (tmp_ctx == NULL) {
>+		return ldb_oom(ldb);
>+	}
>+
>+	/* if we have wp, we can do whatever we like */
>+	ret = acl_check_access_on_attribute(module,
>+					    tmp_ctx,
>+					    sd,
>+					    sid,
>+					    SEC_ADS_WRITE_PROP,
>+					    attr, objectclass);
>+	if (ret == LDB_SUCCESS) {
>+		talloc_free(tmp_ctx);
>+		return LDB_SUCCESS;
>+	}
>+
>+	ret = acl_check_extended_right(tmp_ctx,
>+				       module,
>+				       req,
>+				       objectclass,
>+				       sd,
>+				       acl_user_token(module),
>+				       GUID_DRS_DNS_HOST_NAME,
>+				       SEC_ADS_SELF_WRITE,
>+				       sid);
>+
>+	if (ret != LDB_SUCCESS) {
>+		dsdb_acl_debug(sd, acl_user_token(module),
>+			       req->op.mod.message->dn,
>+			       true,
>+			       10);
>+		talloc_free(tmp_ctx);
>+		return ret;
>+	}
>+
>+	/*
>+	 * If we have "validated write dnshostname", allow delete of
>+	 * any existing value (this keeps constrained delete to the
>+	 * same rules as unconstrained)
>+	 */
>+	if (req->operation == LDB_MODIFY) {
>+		struct ldb_result *acl_res = NULL;
>+
>+		static const char *acl_attrs[] = {
>+			"sAMAccountName",
>+			"objectClass",
>+			NULL
>+		};
>+
>+		/*
>+		 * If not add or replace (eg delete),
>+		 * return success
>+		 */
>+		if ((el->flags
>+		     & (LDB_FLAG_MOD_ADD|LDB_FLAG_MOD_REPLACE)) == 0)
>+		{
>+			talloc_free(tmp_ctx);
>+			return LDB_SUCCESS;
>+		}
>+
>+		ret = dsdb_module_search_dn(module, tmp_ctx,
>+					    &acl_res, req->op.mod.message->dn,
>+					    acl_attrs,
>+					    DSDB_FLAG_NEXT_MODULE |
>+					    DSDB_FLAG_AS_SYSTEM |
>+					    DSDB_SEARCH_SHOW_RECYCLED,
>+					    req);
>+		if (ret != LDB_SUCCESS) {
>+			talloc_free(tmp_ctx);
>+			return ret;
>+		}
>+
>+		search_res = acl_res->msgs[0];
>+	} else if (req->operation == LDB_ADD) {
>+		search_res = req->op.add.message;
>+	} else {
>+		talloc_free(tmp_ctx);
>+		return LDB_ERR_OPERATIONS_ERROR;
>+	}
>+
>+        /* Check if the account has objectclass 'computer' or 'server'. */
>+	oc_el = samdb_find_attribute(ldb,
>+				     search_res,
>+				     "objectclass",
>+				     "computer");
>+	if (oc_el == NULL) {
>+		oc_el = samdb_find_attribute(ldb,
>+					     search_res,
>+					     "objectclass",
>+					     "server");
>+
>+		if (oc_el == NULL) {
>+			/* Not a computer or server, so no need to validate. */
>+			talloc_free(tmp_ctx);
>+			return LDB_SUCCESS;
>+		}
>+	}
>+
>+	samAccountName = ldb_msg_find_attr_as_string(search_res, "sAMAccountName", NULL);
>+	if (samAccountName == NULL) {
>+		talloc_free(tmp_ctx);
>+		return ldb_operr(ldb);
>+	}
>+	account_name_len = strlen(samAccountName);
>+	if (account_name_len && samAccountName[account_name_len - 1] == '$') {
>+		/* Account for the '$' character. */
>+		--account_name_len;
>+	}
>+
>+	dns_host_name = dns_host_name_value;
>+
>+	/* Check that sAMAccountName matches the new dNSHostName. */
>+	if (strncasecmp(dns_host_name, samAccountName,
>+			account_name_len) != 0)
>+	{
>+		goto fail;
>+	}
>+
>+	dns_host_name += account_name_len;
>+
>+	/* Check the '.' character */
>+	if (*dns_host_name++ != '.') {
>+		goto fail;
>+	}
>+
>+	/* Now we check the suffix. */
>+
>+	ret = dsdb_find_nc_root(ldb,
>+				tmp_ctx,
>+				search_res->dn,
>+				&nc_root);
>+	if (ret != LDB_SUCCESS) {
>+		talloc_free(tmp_ctx);
>+		return ret;
>+	}
>+
>+	nc_dns_name = samdb_dn_to_dns_domain(tmp_ctx, nc_root);
>+	if (nc_dns_name == NULL) {
>+		talloc_free(tmp_ctx);
>+		return ldb_operr(ldb);
>+	}
>+
>+	if (strcasecmp(dns_host_name, nc_dns_name) == 0) {
>+		/* It matches -- success. */
>+		talloc_free(tmp_ctx);
>+		return LDB_SUCCESS;
>+	}
>+
>+	/* We didn't get a match, so now try msDS-AllowedDNSSuffixes. */
>+
>+	ret = dsdb_module_search_dn(module, tmp_ctx,
>+				    &nc_res, nc_root,
>+				    nc_attrs,
>+				    DSDB_FLAG_NEXT_MODULE |
>+				    DSDB_FLAG_AS_SYSTEM |
>+				    DSDB_SEARCH_SHOW_RECYCLED,
>+				    req);
>+	if (ret != LDB_SUCCESS) {
>+		talloc_free(tmp_ctx);
>+		return ret;
>+	}
>+
>+	allowed_suffixes = ldb_msg_find_element(nc_res->msgs[0],
>+						"msDS-AllowedDNSSuffixes");
>+	if (allowed_suffixes == NULL) {
>+		goto fail;
>+	}
>+
>+	for (i = 0; i < allowed_suffixes->num_values; ++i) {
>+		const char *suffix = ldb_val_as_string(&allowed_suffixes->values[i],
>+						       NULL);
>+
>+		if (suffix != NULL && strcasecmp(dns_host_name, suffix) == 0) {
>+			/* It matches -- success. */
>+			talloc_free(tmp_ctx);
>+			return LDB_SUCCESS;
>+		}
>+	}
>+
>+fail:
>+	talloc_free(tmp_ctx);
>+	ldb_debug_set(ldb, LDB_DEBUG_WARNING,
>+		      "acl: hostname validation failed for "
>+		      "hostname[%s] account[%s]\n",
>+		      dns_host_name_value, samAccountName);
>+	return LDB_ERR_CONSTRAINT_VIOLATION;
>+}
>+
> static int acl_add(struct ldb_module *module, struct ldb_request *req)
> {
> 	int ret;
>@@ -1535,6 +1758,18 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
> 			if (ret != LDB_SUCCESS) {
> 				goto fail;
> 			}
>+		} else if (ldb_attr_cmp("dnsHostName", el->name) == 0) {
>+			ret = acl_check_dns_host_name(tmp_ctx,
>+						      module,
>+						      req,
>+						      el,
>+						      sd,
>+						      sid,
>+						      attr,
>+						      objectclass);
>+			if (ret != LDB_SUCCESS) {
>+				goto fail;
>+			}
> 		} else if (is_undelete != NULL && (ldb_attr_cmp("isDeleted", el->name) == 0)) {
> 			/*
> 			 * in case of undelete op permissions on
>-- 
>2.35.0
>
>
>From fa6762ea61f5e7c046a68f62c8fdecdc73130d2b Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 7 Jun 2022 17:36:43 +1200
>Subject: [PATCH 04/14] dsdb/common: Add VALIDATED_DNS_HOSTNAME_SPN_WRITE
> control
>
>Passing this control will grant the right to set validated values for
>dNSHostName and servicePrincipalName, and non-validated values for other
>attributes.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> source4/dsdb/common/util.c            | 7 +++++++
> source4/dsdb/samdb/ldb_modules/util.h | 1 +
> source4/dsdb/samdb/samdb.h            | 6 ++++++
> 3 files changed, 14 insertions(+)
>
>diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
>index bd59de5cb32..41b1f13a442 100644
>--- a/source4/dsdb/common/util.c
>+++ b/source4/dsdb/common/util.c
>@@ -4481,6 +4481,13 @@ int dsdb_request_add_controls(struct ldb_request *req, uint32_t dsdb_flags)
> 		}
> 	}
> 
>+	if (dsdb_flags & DSDB_FLAG_VALIDATED_DNS_HOSTNAME_SPN_WRITE) {
>+		ret = ldb_request_add_control(req, DSDB_CONTROL_VALIDATED_DNS_HOSTNAME_SPN_WRITE_OID, true, NULL);
>+		if (ret != LDB_SUCCESS) {
>+			return ret;
>+		}
>+	}
>+
> 	return LDB_SUCCESS;
> }
> 
>diff --git a/source4/dsdb/samdb/ldb_modules/util.h b/source4/dsdb/samdb/ldb_modules/util.h
>index 5ecf0eee0d2..f3ac3bf5f5a 100644
>--- a/source4/dsdb/samdb/ldb_modules/util.h
>+++ b/source4/dsdb/samdb/ldb_modules/util.h
>@@ -39,3 +39,4 @@ struct netlogon_samlogon_response;
> #define DSDB_FLAG_TOP_MODULE		      0x00800000
> #define DSDB_FLAG_TRUSTED		      0x01000000
> #define DSDB_FLAG_REPLICATED_UPDATE           0x02000000
>+#define DSDB_FLAG_VALIDATED_DNS_HOSTNAME_SPN_WRITE 0x04000000
>diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
>index 286c97f2ea5..6e3baa54495 100644
>--- a/source4/dsdb/samdb/samdb.h
>+++ b/source4/dsdb/samdb/samdb.h
>@@ -226,6 +226,12 @@ struct dsdb_control_transaction_identifier {
> 	struct GUID transaction_guid;
> };
> 
>+/*
>+ * passed when we want to allow validated writes to dNSHostName and
>+ * servicePrincipalName.
>+ */
>+#define DSDB_CONTROL_VALIDATED_DNS_HOSTNAME_SPN_WRITE_OID "1.3.6.1.4.1.7165.4.3.35"
>+
> #define DSDB_EXTENDED_REPLICATED_OBJECTS_OID "1.3.6.1.4.1.7165.4.4.1"
> struct dsdb_extended_replicated_object {
> 	struct ldb_message *msg;
>-- 
>2.35.0
>
>
>From 504620f3a26a6a84411aa2f343fadb643474c737 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 7 Jun 2022 17:39:07 +1200
>Subject: [PATCH 05/14] dsdb/modules/acl: Handle
> VALIDATED_DNS_HOSTNAME_SPN_WRITE control
>
>When this control is specified, we'll assume we have Validated Write on
>dNSHostName and servicePrincipalName, and Write Property on other
>attributes.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> source4/dsdb/samdb/ldb_modules/acl.c | 131 +++++++++++++++------------
> 1 file changed, 74 insertions(+), 57 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
>index d94dc10659d..a97e1057d65 100644
>--- a/source4/dsdb/samdb/ldb_modules/acl.c
>+++ b/source4/dsdb/samdb/ldb_modules/acl.c
>@@ -667,7 +667,8 @@ static int acl_check_spn(TALLOC_CTX *mem_ctx,
> 			 struct security_descriptor *sd,
> 			 struct dom_sid *sid,
> 			 const struct dsdb_attribute *attr,
>-			 const struct dsdb_class *objectclass)
>+			 const struct dsdb_class *objectclass,
>+			 bool have_validated_write)
> {
> 	int ret;
> 	unsigned int i;
>@@ -694,34 +695,36 @@ static int acl_check_spn(TALLOC_CTX *mem_ctx,
> 		NULL
> 	};
> 
>-	/* if we have wp, we can do whatever we like */
>-	if (acl_check_access_on_attribute(module,
>-					  tmp_ctx,
>-					  sd,
>-					  sid,
>-					  SEC_ADS_WRITE_PROP,
>-					  attr, objectclass) == LDB_SUCCESS) {
>-		talloc_free(tmp_ctx);
>-		return LDB_SUCCESS;
>-	}
>+	if (!have_validated_write) {
>+		/* if we have wp, we can do whatever we like */
>+		if (acl_check_access_on_attribute(module,
>+						  tmp_ctx,
>+						  sd,
>+						  sid,
>+						  SEC_ADS_WRITE_PROP,
>+						  attr, objectclass) == LDB_SUCCESS) {
>+			talloc_free(tmp_ctx);
>+			return LDB_SUCCESS;
>+		}
> 
>-	ret = acl_check_extended_right(tmp_ctx,
>-				       module,
>-				       req,
>-				       objectclass,
>-				       sd,
>-				       acl_user_token(module),
>-				       GUID_DRS_VALIDATE_SPN,
>-				       SEC_ADS_SELF_WRITE,
>-				       sid);
>+		ret = acl_check_extended_right(tmp_ctx,
>+					       module,
>+					       req,
>+					       objectclass,
>+					       sd,
>+					       acl_user_token(module),
>+					       GUID_DRS_VALIDATE_SPN,
>+					       SEC_ADS_SELF_WRITE,
>+					       sid);
> 
>-	if (ret != LDB_SUCCESS) {
>-		dsdb_acl_debug(sd, acl_user_token(module),
>-			       req->op.mod.message->dn,
>-			       true,
>-			       10);
>-		talloc_free(tmp_ctx);
>-		return ret;
>+		if (ret != LDB_SUCCESS) {
>+			dsdb_acl_debug(sd, acl_user_token(module),
>+				       req->op.mod.message->dn,
>+				       true,
>+				       10);
>+			talloc_free(tmp_ctx);
>+			return ret;
>+		}
> 	}
> 
> 	/*
>@@ -808,7 +811,8 @@ static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
> 				   struct security_descriptor *sd,
> 				   struct dom_sid *sid,
> 				   const struct dsdb_attribute *attr,
>-				   const struct dsdb_class *objectclass)
>+				   const struct dsdb_class *objectclass,
>+				   bool have_validated_write)
> {
> 	int ret;
> 	unsigned i;
>@@ -840,35 +844,37 @@ static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
> 		return ldb_oom(ldb);
> 	}
> 
>-	/* if we have wp, we can do whatever we like */
>-	ret = acl_check_access_on_attribute(module,
>-					    tmp_ctx,
>-					    sd,
>-					    sid,
>-					    SEC_ADS_WRITE_PROP,
>-					    attr, objectclass);
>-	if (ret == LDB_SUCCESS) {
>-		talloc_free(tmp_ctx);
>-		return LDB_SUCCESS;
>-	}
>+	if (!have_validated_write) {
>+		/* if we have wp, we can do whatever we like */
>+		ret = acl_check_access_on_attribute(module,
>+						    tmp_ctx,
>+						    sd,
>+						    sid,
>+						    SEC_ADS_WRITE_PROP,
>+						    attr, objectclass);
>+		if (ret == LDB_SUCCESS) {
>+			talloc_free(tmp_ctx);
>+			return LDB_SUCCESS;
>+		}
> 
>-	ret = acl_check_extended_right(tmp_ctx,
>-				       module,
>-				       req,
>-				       objectclass,
>-				       sd,
>-				       acl_user_token(module),
>-				       GUID_DRS_DNS_HOST_NAME,
>-				       SEC_ADS_SELF_WRITE,
>-				       sid);
>+		ret = acl_check_extended_right(tmp_ctx,
>+					       module,
>+					       req,
>+					       objectclass,
>+					       sd,
>+					       acl_user_token(module),
>+					       GUID_DRS_DNS_HOST_NAME,
>+					       SEC_ADS_SELF_WRITE,
>+					       sid);
> 
>-	if (ret != LDB_SUCCESS) {
>-		dsdb_acl_debug(sd, acl_user_token(module),
>-			       req->op.mod.message->dn,
>-			       true,
>-			       10);
>-		talloc_free(tmp_ctx);
>-		return ret;
>+		if (ret != LDB_SUCCESS) {
>+			dsdb_acl_debug(sd, acl_user_token(module),
>+				       req->op.mod.message->dn,
>+				       true,
>+				       10);
>+			talloc_free(tmp_ctx);
>+			return ret;
>+		}
> 	}
> 
> 	/*
>@@ -1572,6 +1578,7 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
> 	struct dom_sid *sid = NULL;
> 	struct ldb_control *as_system;
> 	struct ldb_control *is_undelete;
>+	struct ldb_control *validated_write = NULL;
> 	bool userPassword;
> 	bool password_rights_checked = false;
> 	TALLOC_CTX *tmp_ctx;
>@@ -1598,6 +1605,11 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
> 
> 	is_undelete = ldb_request_get_control(req, DSDB_CONTROL_RESTORE_TOMBSTONE_OID);
> 
>+	validated_write = ldb_request_get_control(req, DSDB_CONTROL_VALIDATED_DNS_HOSTNAME_SPN_WRITE_OID);
>+	if (validated_write != NULL) {
>+		validated_write->critical = 0;
>+	}
>+
> 	/* Don't print this debug statement if elements[0].name is going to be NULL */
> 	if (msg->num_elements > 0) {
> 		DEBUG(10, ("ldb:acl_modify: %s\n", msg->elements[0].name));
>@@ -1754,7 +1766,8 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
> 					    sd,
> 					    sid,
> 					    attr,
>-					    objectclass);
>+					    objectclass,
>+					    validated_write != NULL);
> 			if (ret != LDB_SUCCESS) {
> 				goto fail;
> 			}
>@@ -1766,7 +1779,8 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
> 						      sd,
> 						      sid,
> 						      attr,
>-						      objectclass);
>+						      objectclass,
>+						      validated_write != NULL);
> 			if (ret != LDB_SUCCESS) {
> 				goto fail;
> 			}
>@@ -1778,6 +1792,9 @@ static int acl_modify(struct ldb_module *module, struct ldb_request *req)
> 			 * tombstone_reanimate module
> 			 */
> 			continue;
>+		} else if (validated_write != NULL) {
>+			/* Allow the update. */
>+			continue;
> 		} else {
> 			ret = acl_check_access_on_attribute(module,
> 							    tmp_ctx,
>-- 
>2.35.0
>
>
>From 72fc12e55a2ff8c807edeaf95976d6fc46617882 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Thu, 2 Jun 2022 17:11:08 +1200
>Subject: [PATCH 06/14] s4:rpc_server/netlogon: Remove dNSHostName prefix check
>
>This check is not exhaustive (it does not check the suffix of the
>dNSHostName), and should be covered by a validated write check in
>acl_modify().
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> selftest/knownfail.d/netlogon-dns-host-name   |  1 +
> source4/rpc_server/netlogon/dcerpc_netlogon.c | 21 ++-----------------
> 2 files changed, 3 insertions(+), 19 deletions(-)
>
>diff --git a/selftest/knownfail.d/netlogon-dns-host-name b/selftest/knownfail.d/netlogon-dns-host-name
>index 2d0a0ec570a..50bb15f4b28 100644
>--- a/selftest/knownfail.d/netlogon-dns-host-name
>+++ b/selftest/knownfail.d/netlogon-dns-host-name
>@@ -1,2 +1,3 @@
>+^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_invalid\(
> ^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_invalid_suffix\(
> ^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_with_flag\(
>diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>index 062a92597ce..6db5d854cdb 100644
>--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
>+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>@@ -2413,7 +2413,7 @@ static NTSTATUS dcesrv_netr_LogonGetDomainInfo(struct dcesrv_call_state *dce_cal
> 	};
> 	const char * const attrs2[] = { "sAMAccountName", "dNSHostName",
> 		"msDS-SupportedEncryptionTypes", NULL };
>-	const char *sam_account_name, *old_dns_hostname, *prefix1, *prefix2;
>+	const char *sam_account_name, *old_dns_hostname;
> 	struct ldb_context *sam_ctx;
> 	const struct GUID *our_domain_guid = NULL;
> 	struct lsa_TrustDomainInfoInfoEx *our_tdo = NULL;
>@@ -2483,24 +2483,7 @@ static NTSTATUS dcesrv_netr_LogonGetDomainInfo(struct dcesrv_call_state *dce_cal
> 			return NT_STATUS_INTERNAL_DB_CORRUPTION;
> 		}
> 
>-		/*
>-		 * Checks that the sam account name without a possible "$"
>-		 * matches as prefix with the DNS hostname in the workstation
>-		 * info structure.
>-		 */
>-		prefix1 = talloc_strndup(mem_ctx, sam_account_name,
>-					 strcspn(sam_account_name, "$"));
>-		NT_STATUS_HAVE_NO_MEMORY(prefix1);
>-		if (r->in.query->workstation_info->dns_hostname != NULL) {
>-			prefix2 = talloc_strndup(mem_ctx,
>-						 r->in.query->workstation_info->dns_hostname,
>-						 strcspn(r->in.query->workstation_info->dns_hostname, "."));
>-			NT_STATUS_HAVE_NO_MEMORY(prefix2);
>-
>-			if (strcasecmp(prefix1, prefix2) != 0) {
>-				update_dns_hostname = false;
>-			}
>-		} else {
>+		if (r->in.query->workstation_info->dns_hostname == NULL) {
> 			update_dns_hostname = false;
> 		}
> 
>-- 
>2.35.0
>
>
>From f614631880f9a37c709d33ff9d765a1f6b740543 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 7 Jun 2022 17:25:28 +1200
>Subject: [PATCH 07/14] s4:rpc_server/netlogon: Always observe
> NETR_WS_FLAG_HANDLES_SPN_UPDATE flag
>
>Even when there is no old DNS hostname present.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> selftest/knownfail.d/netlogon-dns-host-name   | 1 -
> source4/rpc_server/netlogon/dcerpc_netlogon.c | 7 ++-----
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
>diff --git a/selftest/knownfail.d/netlogon-dns-host-name b/selftest/knownfail.d/netlogon-dns-host-name
>index 50bb15f4b28..360e45e0692 100644
>--- a/selftest/knownfail.d/netlogon-dns-host-name
>+++ b/selftest/knownfail.d/netlogon-dns-host-name
>@@ -1,3 +1,2 @@
> ^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_invalid\(
> ^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_invalid_suffix\(
>-^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_with_flag\(
>diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>index 6db5d854cdb..6c6aab6e0e6 100644
>--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
>+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>@@ -2495,13 +2495,10 @@ static NTSTATUS dcesrv_netr_LogonGetDomainInfo(struct dcesrv_call_state *dce_cal
> 		/*
> 		 * Updates the DNS hostname when the client wishes that the
> 		 * server should handle this for him
>-		 * ("NETR_WS_FLAG_HANDLES_SPN_UPDATE" not set). And this is
>-		 * obviously only checked when we do already have a
>-		 * "dNSHostName".
>+		 * ("NETR_WS_FLAG_HANDLES_SPN_UPDATE" not set).
> 		 * See MS-NRPC section 3.5.4.3.9
> 		 */
>-		if ((old_dns_hostname != NULL) &&
>-		    (r->in.query->workstation_info->workstation_flags
>+		if ((r->in.query->workstation_info->workstation_flags
> 		    & NETR_WS_FLAG_HANDLES_SPN_UPDATE) != 0) {
> 			update_dns_hostname = false;
> 		}
>-- 
>2.35.0
>
>
>From a09e2fe7a7613a6fae6666d1efc7ed65f03aef2f Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 7 Jun 2022 17:29:02 +1200
>Subject: [PATCH 08/14] s4:rpc_server/netlogon: Connect to samdb as a user,
> rather than as system
>
>This allows us to perform validation on a client-specified dNSHostName
>value, to ensure that it matches the sAMAccountName.
>
>We might not have any rights to modify the account, so pass the control
>VALIDATED_DNS_HOSTNAME_SPN_WRITE which allows us to perform a validated
>write to dNSHostName and servicePrincipalName (and unvalidated writes to
>other attributes, such as operatingSystem).
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> selftest/knownfail.d/netlogon-dns-host-name   | 4 ++--
> source4/rpc_server/netlogon/dcerpc_netlogon.c | 5 +++--
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
>diff --git a/selftest/knownfail.d/netlogon-dns-host-name b/selftest/knownfail.d/netlogon-dns-host-name
>index 360e45e0692..3eca0cd3f75 100644
>--- a/selftest/knownfail.d/netlogon-dns-host-name
>+++ b/selftest/knownfail.d/netlogon-dns-host-name
>@@ -1,2 +1,2 @@
>-^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_invalid\(
>-^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_invalid_suffix\(
>+^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_valid\(
>+^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_valid_denied\(
>diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>index 6c6aab6e0e6..189b6d94424 100644
>--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
>+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>@@ -2450,7 +2450,8 @@ static NTSTATUS dcesrv_netr_LogonGetDomainInfo(struct dcesrv_call_state *dce_cal
> 	}
> 	NT_STATUS_NOT_OK_RETURN(status);
> 
>-	sam_ctx = dcesrv_samdb_connect_as_system(mem_ctx, dce_call);
>+	/* We want to avoid connecting as system. */
>+	sam_ctx = dcesrv_samdb_connect_as_user(mem_ctx, dce_call);
> 	if (sam_ctx == NULL) {
> 		return NT_STATUS_INVALID_SYSTEM_SERVICE;
> 	}
>@@ -2607,7 +2608,7 @@ static NTSTATUS dcesrv_netr_LogonGetDomainInfo(struct dcesrv_call_state *dce_cal
> 			}
> 		}
> 
>-		if (dsdb_replace(sam_ctx, new_msg, 0) != LDB_SUCCESS) {
>+		if (dsdb_replace(sam_ctx, new_msg, DSDB_FLAG_VALIDATED_DNS_HOSTNAME_SPN_WRITE) != LDB_SUCCESS) {
> 			DEBUG(3,("Impossible to update samdb: %s\n",
> 				ldb_errstring(sam_ctx)));
> 		}
>-- 
>2.35.0
>
>
>From b2f9bc9afd25e016626d167520579d264b65b035 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 7 Jun 2022 17:36:18 +1200
>Subject: [PATCH 09/14] ldb: Add ldb_val_as_string()
>
>This converts an ldb_val to a string, using the same logic as
>ldb_msg_find_attr_as_string().
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> lib/ldb/common/ldb_msg.c | 15 +++++++++++----
> lib/ldb/include/ldb.h    |  3 +++
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
>diff --git a/lib/ldb/common/ldb_msg.c b/lib/ldb/common/ldb_msg.c
>index 57dfc5a04c2..b988e774079 100644
>--- a/lib/ldb/common/ldb_msg.c
>+++ b/lib/ldb/common/ldb_msg.c
>@@ -790,11 +790,9 @@ int ldb_msg_find_attr_as_bool(const struct ldb_message *msg,
> 	return default_value;
> }
> 
>-const char *ldb_msg_find_attr_as_string(const struct ldb_message *msg,
>-					const char *attr_name,
>-					const char *default_value)
>+const char *ldb_val_as_string(const struct ldb_val *v,
>+			      const char *default_value)
> {
>-	const struct ldb_val *v = ldb_msg_find_ldb_val(msg, attr_name);
> 	if (!v || !v->data) {
> 		return default_value;
> 	}
>@@ -804,6 +802,15 @@ const char *ldb_msg_find_attr_as_string(const struct ldb_message *msg,
> 	return (const char *)v->data;
> }
> 
>+const char *ldb_msg_find_attr_as_string(const struct ldb_message *msg,
>+					const char *attr_name,
>+					const char *default_value)
>+{
>+	const struct ldb_val *v = ldb_msg_find_ldb_val(msg, attr_name);
>+
>+	return ldb_val_as_string(v, default_value);
>+}
>+
> struct ldb_dn *ldb_msg_find_attr_as_dn(struct ldb_context *ldb,
> 				       TALLOC_CTX *mem_ctx,
> 				       const struct ldb_message *msg,
>diff --git a/lib/ldb/include/ldb.h b/lib/ldb/include/ldb.h
>index bc44157eaf4..a7822151546 100644
>--- a/lib/ldb/include/ldb.h
>+++ b/lib/ldb/include/ldb.h
>@@ -2046,6 +2046,9 @@ struct ldb_dn *ldb_msg_find_attr_as_dn(struct ldb_context *ldb,
> 				       const struct ldb_message *msg,
> 				       const char *attr_name);
> 
>+const char *ldb_val_as_string(const struct ldb_val *v,
>+			      const char *default_value);
>+
> void ldb_msg_sort_elements(struct ldb_message *msg);
> 
> struct ldb_message *ldb_msg_copy_shallow(TALLOC_CTX *mem_ctx,
>-- 
>2.35.0
>
>
>From 6f08a94acfbb76fb76b0c27b6d6453215cb34503 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 7 Jun 2022 17:36:56 +1200
>Subject: [PATCH 10/14] s4/dsdb/util: Add dsdb_msg_get_single_value()
>
>This function simulates an add or modify operation for an ldb message to
>determine the final value of a particular single-valued attribute. This
>is useful when validating attributes that should stay in sync with other
>attributes, such as servicePrincipalName and dNSHostName.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> source4/dsdb/samdb/ldb_modules/util.c | 91 +++++++++++++++++++++++++++
> 1 file changed, 91 insertions(+)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/util.c b/source4/dsdb/samdb/ldb_modules/util.c
>index 405febf0b3d..ec03b523204 100644
>--- a/source4/dsdb/samdb/ldb_modules/util.c
>+++ b/source4/dsdb/samdb/ldb_modules/util.c
>@@ -1564,6 +1564,97 @@ int dsdb_get_expected_new_values(TALLOC_CTX *mem_ctx,
> 	return LDB_SUCCESS;
> }
> 
>+/*
>+ * Get the value of a single-valued attribute after processing a
>+ * message. 'operation' is either LDB_ADD or LDB_MODIFY. 'val' will only live as
>+ * long as 'msg' and 'original_val' do, and must not be freed.
>+ */
>+int dsdb_msg_get_single_value(const struct ldb_message *msg,
>+			      const char *attr_name,
>+			      const struct ldb_val *original_val,
>+			      const struct ldb_val **val,
>+			      enum ldb_request_type operation)
>+{
>+	unsigned idx;
>+
>+	*val = NULL;
>+
>+	if (operation == LDB_ADD) {
>+		const struct ldb_message_element *el = NULL;
>+
>+		/*
>+		 * The ldb_msg_normalize() call in ldb_request() ensures that
>+		 * there is at most one message element for each
>+		 * attribute. Thus, we don't need a loop to deal with an
>+		 * LDB_ADD.
>+		 */
>+		el = ldb_msg_find_element(msg, attr_name);
>+		if (el == NULL) {
>+			*val = original_val;
>+			return LDB_SUCCESS;
>+		}
>+		if (el->num_values != 1) {
>+			return LDB_ERR_CONSTRAINT_VIOLATION;
>+		}
>+
>+		*val = &el->values[0];
>+		return LDB_SUCCESS;
>+	}
>+
>+	SMB_ASSERT(operation == LDB_MODIFY);
>+
>+	*val = original_val;
>+
>+	for (idx = 0; idx < msg->num_elements; ++idx) {
>+		const struct ldb_message_element *el = &msg->elements[idx];
>+
>+		if (ldb_attr_cmp(el->name, attr_name) != 0) {
>+			continue;
>+		}
>+
>+		switch (el->flags & LDB_FLAG_MOD_MASK) {
>+		case LDB_FLAG_MOD_ADD:
>+			if (el->num_values != 1) {
>+				return LDB_ERR_CONSTRAINT_VIOLATION;
>+			}
>+			if (*val != NULL) {
>+				return LDB_ERR_CONSTRAINT_VIOLATION;
>+			}
>+
>+			*val = &el->values[0];
>+
>+			break;
>+
>+		case LDB_FLAG_MOD_REPLACE:
>+			if (el->num_values > 1) {
>+				return LDB_ERR_CONSTRAINT_VIOLATION;
>+			}
>+
>+			*val = el->num_values ? &el->values[0] : NULL;
>+
>+			break;
>+
>+		case LDB_FLAG_MOD_DELETE:
>+			if (el->num_values > 1) {
>+				return LDB_ERR_CONSTRAINT_VIOLATION;
>+			}
>+
>+			/*
>+			 * If a value was specified for the delete, we don't
>+			 * bother checking it matches the value we currently
>+			 * have. Any mismatch will be caught later (e.g. in
>+			 * ldb_kv_modify_internal).
>+			 */
>+
>+			*val = NULL;
>+
>+			break;
>+		}
>+	}
>+
>+	return LDB_SUCCESS;
>+}
>+
> /*
>  * This function determines the (last) structural or 88 object class of a passed
>  * "objectClass" attribute - per MS-ADTS 3.1.1.1.4 this is the last value.
>-- 
>2.35.0
>
>
>From 130067b5c3c52ff9d148599d1c12f8df3909028c Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 7 Jun 2022 17:37:34 +1200
>Subject: [PATCH 11/14] dsdb/modules/acl: Account for sAMAccountName without $
>
>If we have an account without a trailing $, we should ensure the
>servicePrincipalName matches the entire sAMAccountName. We should not
>allow a match against the sAMAccountName prefix of length
>strlen(samAccountName) - 1, as that could conflict with a different
>account.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> source4/dsdb/samdb/ldb_modules/acl.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
>index a97e1057d65..cbf0687c73f 100644
>--- a/source4/dsdb/samdb/ldb_modules/acl.c
>+++ b/source4/dsdb/samdb/ldb_modules/acl.c
>@@ -543,6 +543,7 @@ static int acl_validate_spn_value(TALLOC_CTX *mem_ctx,
> 	char *instanceName;
> 	char *serviceType;
> 	char *serviceName;
>+	size_t account_name_len;
> 	const char *forest_name = samdb_forest_name(ldb, mem_ctx);
> 	const char *base_domain = samdb_default_domain_name(ldb, mem_ctx);
> 	struct loadparm_context *lp_ctx = talloc_get_type(ldb_get_opaque(ldb, "loadparm"),
>@@ -616,11 +617,18 @@ static int acl_validate_spn_value(TALLOC_CTX *mem_ctx,
> 			}
> 		}
> 	}
>+
>+	account_name_len = strlen(samAccountName);
>+	if (account_name_len && samAccountName[account_name_len - 1] == '$') {
>+		/* Account for the '$' character. */
>+		--account_name_len;
>+	}
>+
> 	/* instanceName can be samAccountName without $ or dnsHostName
> 	 * or "ntds_guid._msdcs.forest_domain for DC objects */
>-	if (strlen(instanceName) == (strlen(samAccountName) - 1)
>+	if (strlen(instanceName) == account_name_len
> 	    && strncasecmp(instanceName, samAccountName,
>-			   strlen(samAccountName) - 1) == 0) {
>+			   account_name_len) == 0) {
> 		goto success;
> 	}
> 	if ((dnsHostName != NULL) &&
>-- 
>2.35.0
>
>
>From fd4102214f5358950c398028235a003fe25358d6 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 7 Jun 2022 17:38:55 +1200
>Subject: [PATCH 12/14] dsdb/modules/acl: Allow simultaneous sAMAccountName,
> dNSHostName, and servicePrincipalName change
>
>If the message changes the sAMAccountName, we'll check dNSHostName and
>servicePrincipalName values against the new value of sAMAccountName,
>rather than the account's current value. Similarly, if the message
>changes the dNSHostName, we'll check servicePrincipalName values against
>the new dNSHostName. This allows setting more than one of these
>attributes simultaneously with validated write rights.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> selftest/knownfail.d/netlogon-dns-host-name  |  2 -
> selftest/knownfail.d/validated-dns-host-name |  3 -
> source4/dsdb/samdb/ldb_modules/acl.c         | 74 +++++++++++++++++---
> 3 files changed, 66 insertions(+), 13 deletions(-)
> delete mode 100644 selftest/knownfail.d/netlogon-dns-host-name
> delete mode 100644 selftest/knownfail.d/validated-dns-host-name
>
>diff --git a/selftest/knownfail.d/netlogon-dns-host-name b/selftest/knownfail.d/netlogon-dns-host-name
>deleted file mode 100644
>index 3eca0cd3f75..00000000000
>--- a/selftest/knownfail.d/netlogon-dns-host-name
>+++ /dev/null
>@@ -1,2 +0,0 @@
>-^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_valid\(
>-^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_set_dns_hostname_valid_denied\(
>diff --git a/selftest/knownfail.d/validated-dns-host-name b/selftest/knownfail.d/validated-dns-host-name
>deleted file mode 100644
>index 4b6165833e2..00000000000
>--- a/selftest/knownfail.d/validated-dns-host-name
>+++ /dev/null
>@@ -1,3 +0,0 @@
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_spn\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_dns_host_name_spn_matching_account_name_new\(
>-^samba4.ldap.acl.python.*__main__.AclModifyTests.test_modify_spn_matching_dns_host_name_invalid\(
>diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
>index cbf0687c73f..150a2165785 100644
>--- a/source4/dsdb/samdb/ldb_modules/acl.c
>+++ b/source4/dsdb/samdb/ldb_modules/acl.c
>@@ -686,9 +686,11 @@ static int acl_check_spn(TALLOC_CTX *mem_ctx,
> 	struct ldb_result *netbios_res;
> 	struct ldb_dn *partitions_dn = samdb_partitions_dn(ldb, tmp_ctx);
> 	uint32_t userAccountControl;
>-	const char *samAccountName;
>-	const char *dnsHostName;
>+	const char *samAccountName = NULL;
>+	const char *dnsHostName = NULL;
> 	const char *netbios_name;
>+	const struct ldb_val *dns_host_name_val = NULL;
>+	const struct ldb_val *sam_account_name_val = NULL;
> 	struct GUID ntds;
> 	char *ntds_guid = NULL;
> 
>@@ -764,9 +766,44 @@ static int acl_check_spn(TALLOC_CTX *mem_ctx,
> 		return ret;
> 	}
> 
>+	dns_host_name_val = ldb_msg_find_ldb_val(acl_res->msgs[0], "dNSHostName");
>+
>+	ret = dsdb_msg_get_single_value(req->op.mod.message,
>+					"dNSHostName",
>+					dns_host_name_val,
>+					&dns_host_name_val,
>+					req->operation);
>+	if (ret != LDB_SUCCESS) {
>+		talloc_free(tmp_ctx);
>+		return ret;
>+	}
>+
>+	if (dns_host_name_val != NULL) {
>+		dnsHostName = ldb_val_as_string(dns_host_name_val, NULL);
>+	}
>+
> 	userAccountControl = ldb_msg_find_attr_as_uint(acl_res->msgs[0], "userAccountControl", 0);
>-	dnsHostName = ldb_msg_find_attr_as_string(acl_res->msgs[0], "dnsHostName", NULL);
>-	samAccountName = ldb_msg_find_attr_as_string(acl_res->msgs[0], "samAccountName", NULL);
>+
>+	sam_account_name_val = ldb_msg_find_ldb_val(acl_res->msgs[0], "sAMAccountName");
>+
>+	ret = dsdb_msg_get_single_value(req->op.mod.message,
>+					"sAMAccountName",
>+					sam_account_name_val,
>+					&sam_account_name_val,
>+					req->operation);
>+	if (ret != LDB_SUCCESS) {
>+		talloc_free(tmp_ctx);
>+		return ret;
>+	}
>+
>+	if (sam_account_name_val != NULL) {
>+		samAccountName = ldb_val_as_string(sam_account_name_val, NULL);
>+	}
>+
>+	if (samAccountName == NULL) {
>+		talloc_free(tmp_ctx);
>+		return LDB_ERR_CONSTRAINT_VIOLATION;
>+	}
> 
> 	ret = dsdb_module_search(module, tmp_ctx,
> 				 &netbios_res, partitions_dn,
>@@ -835,7 +872,9 @@ static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
> 	const char *dns_host_name = NULL;
> 	const char *samAccountName = NULL;
> 	size_t account_name_len;
>+	const struct ldb_message *msg = NULL;
> 	const struct ldb_message *search_res = NULL;
>+	const struct ldb_val *sam_account_name_val = NULL;
> 
> 	static const char *nc_attrs[] = {
> 		"msDS-AllowedDNSSuffixes",
>@@ -899,6 +938,8 @@ static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
> 			NULL
> 		};
> 
>+		msg = req->op.mod.message;
>+
> 		/*
> 		 * If not add or replace (eg delete),
> 		 * return success
>@@ -911,7 +952,7 @@ static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
> 		}
> 
> 		ret = dsdb_module_search_dn(module, tmp_ctx,
>-					    &acl_res, req->op.mod.message->dn,
>+					    &acl_res, msg->dn,
> 					    acl_attrs,
> 					    DSDB_FLAG_NEXT_MODULE |
> 					    DSDB_FLAG_AS_SYSTEM |
>@@ -924,7 +965,8 @@ static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
> 
> 		search_res = acl_res->msgs[0];
> 	} else if (req->operation == LDB_ADD) {
>-		search_res = req->op.add.message;
>+		msg = req->op.add.message;
>+		search_res = msg;
> 	} else {
> 		talloc_free(tmp_ctx);
> 		return LDB_ERR_OPERATIONS_ERROR;
>@@ -948,11 +990,27 @@ static int acl_check_dns_host_name(TALLOC_CTX *mem_ctx,
> 		}
> 	}
> 
>-	samAccountName = ldb_msg_find_attr_as_string(search_res, "sAMAccountName", NULL);
>+	sam_account_name_val = ldb_msg_find_ldb_val(search_res, "sAMAccountName");
>+
>+	ret = dsdb_msg_get_single_value(msg,
>+					"sAMAccountName",
>+					sam_account_name_val,
>+					&sam_account_name_val,
>+					req->operation);
>+	if (ret != LDB_SUCCESS) {
>+		talloc_free(tmp_ctx);
>+		return ret;
>+	}
>+
>+	if (sam_account_name_val != NULL) {
>+		samAccountName = ldb_val_as_string(sam_account_name_val, NULL);
>+	}
>+
> 	if (samAccountName == NULL) {
> 		talloc_free(tmp_ctx);
>-		return ldb_operr(ldb);
>+		return LDB_ERR_CONSTRAINT_VIOLATION;
> 	}
>+
> 	account_name_len = strlen(samAccountName);
> 	if (account_name_len && samAccountName[account_name_len - 1] == '$') {
> 		/* Account for the '$' character. */
>-- 
>2.35.0
>
>
>From 45fb72bdd37b98c47b50efcf1f494792680dce0f Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Thu, 9 Jun 2022 19:32:30 +1200
>Subject: [PATCH 13/14] s4:rpc_server/common: Add
> dcesrv_samdb_connect_session_info()
>
>This function allows us to connect to samdb as a particular user by
>passing in that user's session info.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> source4/rpc_server/common/common.h      |  1 +
> source4/rpc_server/common/server_info.c | 30 ++++++++++++-------------
> 2 files changed, 15 insertions(+), 16 deletions(-)
>
>diff --git a/source4/rpc_server/common/common.h b/source4/rpc_server/common/common.h
>index 7d2f8c569e4..b57ddf20db5 100644
>--- a/source4/rpc_server/common/common.h
>+++ b/source4/rpc_server/common/common.h
>@@ -30,6 +30,7 @@ struct dcesrv_context;
> struct dcesrv_call_state;
> struct ndr_interface_table;
> struct ncacn_packet;
>+struct auth_session_info;
> 
> struct dcerpc_server_info { 
> 	const char *domain_name;
>diff --git a/source4/rpc_server/common/server_info.c b/source4/rpc_server/common/server_info.c
>index a2af37653ef..ceddeff8a5f 100644
>--- a/source4/rpc_server/common/server_info.c
>+++ b/source4/rpc_server/common/server_info.c
>@@ -190,13 +190,12 @@ bool dcesrv_common_validate_share_name(TALLOC_CTX *mem_ctx, const char *share_na
> 	return true;
> }
> 
>-static struct ldb_context *dcesrv_samdb_connect_common(
>+struct ldb_context *dcesrv_samdb_connect_session_info(
> 	TALLOC_CTX *mem_ctx,
> 	struct dcesrv_call_state *dce_call,
>-	bool as_system)
>+	struct auth_session_info *session_info)
> {
> 	struct ldb_context *samdb = NULL;
>-	struct auth_session_info *system_session_info = NULL;
> 	const struct auth_session_info *call_session_info =
> 		dcesrv_call_session_info(dce_call);
> 	struct auth_session_info *user_session_info = NULL;
>@@ -204,13 +203,6 @@ static struct ldb_context *dcesrv_samdb_connect_common(
> 	struct auth_session_info *audit_session_info = NULL;
> 	struct tsocket_address *remote_address = NULL;
> 
>-	if (as_system) {
>-		system_session_info = system_session(dce_call->conn->dce_ctx->lp_ctx);
>-		if (system_session_info == NULL) {
>-			return NULL;
>-		}
>-	}
>-
> 	user_session_info = copy_session_info(mem_ctx, call_session_info);
> 	if (user_session_info == NULL) {
> 		return NULL;
>@@ -224,8 +216,8 @@ static struct ldb_context *dcesrv_samdb_connect_common(
> 		}
> 	}
> 
>-	if (system_session_info != NULL) {
>-		ldb_session_info = system_session_info;
>+	if (session_info != NULL) {
>+		ldb_session_info = session_info;
> 		audit_session_info = user_session_info;
> 	} else {
> 		ldb_session_info = user_session_info;
>@@ -288,8 +280,15 @@ struct ldb_context *dcesrv_samdb_connect_as_system(
> 	TALLOC_CTX *mem_ctx,
> 	struct dcesrv_call_state *dce_call)
> {
>-	return dcesrv_samdb_connect_common(mem_ctx, dce_call,
>-					   true /* as_system */);
>+	struct auth_session_info *system_session_info = NULL;
>+
>+	system_session_info = system_session(dce_call->conn->dce_ctx->lp_ctx);
>+	if (system_session_info == NULL) {
>+		return NULL;
>+	}
>+
>+	return dcesrv_samdb_connect_session_info(mem_ctx, dce_call,
>+						 system_session_info);
> }
> 
> /*
>@@ -301,6 +300,5 @@ struct ldb_context *dcesrv_samdb_connect_as_user(
> 	TALLOC_CTX *mem_ctx,
> 	struct dcesrv_call_state *dce_call)
> {
>-	return dcesrv_samdb_connect_common(mem_ctx, dce_call,
>-					   false /* not as_system */);
>+	return dcesrv_samdb_connect_session_info(mem_ctx, dce_call, NULL);
> }
>-- 
>2.35.0
>
>
>From a12d2c9df76d36aa69b130c0c05aeca1b71eb974 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Thu, 9 Jun 2022 19:46:07 +1200
>Subject: [PATCH 14/14] s4:rpc_server/netlogon: Reconnect to samdb as
> workstation account
>
>This ensures that the database update can be attributed to the
>workstation account, rather than to the anonymous SID, in the audit
>logs.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14833
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>---
> source4/rpc_server/netlogon/dcerpc_netlogon.c | 27 +++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
>diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>index 189b6d94424..d6a3da6fecc 100644
>--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
>+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
>@@ -2422,6 +2422,7 @@ static NTSTATUS dcesrv_netr_LogonGetDomainInfo(struct dcesrv_call_state *dce_cal
> 	struct ldb_dn *workstation_dn;
> 	struct netr_DomainInformation *domain_info;
> 	struct netr_LsaPolicyInformation *lsa_policy_info;
>+	struct auth_session_info *workstation_session_info = NULL;
> 	uint32_t default_supported_enc_types = 0xFFFFFFFF;
> 	bool update_dns_hostname = true;
> 	int ret, i;
>@@ -2468,6 +2469,32 @@ static NTSTATUS dcesrv_netr_LogonGetDomainInfo(struct dcesrv_call_state *dce_cal
> 						dom_sid_string(mem_ctx, creds->sid));
> 		NT_STATUS_HAVE_NO_MEMORY(workstation_dn);
> 
>+		/* Get the workstation's session info from the database. */
>+		status = authsam_get_session_info_principal(mem_ctx,
>+							    dce_call->conn->dce_ctx->lp_ctx,
>+							    sam_ctx,
>+							    NULL, /* principal */
>+							    workstation_dn,
>+							    0, /* session_info_flags */
>+							    &workstation_session_info);
>+		if (!NT_STATUS_IS_OK(status)) {
>+			return status;
>+		}
>+
>+		/*
>+		 * Reconnect to samdb as the workstation, now that we have its
>+		 * session info. We do this so the database update can be
>+		 * attributed to the workstation account in the audit logs --
>+		 * otherwise it might be incorrectly attributed to
>+		 * SID_NT_ANONYMOUS.
>+		 */
>+		sam_ctx = dcesrv_samdb_connect_session_info(mem_ctx,
>+							    dce_call,
>+							    workstation_session_info);
>+		if (sam_ctx == NULL) {
>+			return NT_STATUS_INVALID_SYSTEM_SERVICE;
>+		}
>+
> 		/* Lookup for attributes in workstation object */
> 		ret = gendb_search_dn(sam_ctx, mem_ctx, workstation_dn, &res1,
> 				      attrs2);
>-- 
>2.35.0
>

Flags:

abartlet: review-

Actions: View

Attachments on bug 14833: 17315 | 17333