[SCM] Samba Shared Repository - branch master updated

Stefan Metzmacher metze at samba.org
Wed Oct 20 09:23:01 UTC 2021


The branch, master has been updated
       via  83a654a4efd tests/krb5: Add tests for constrained delegation to NO_AUTH_DATA_REQUIRED service
       via  cc3d27596b9 tests/krb5: Ensure PAC is not present if expect_pac is false
       via  031a8287642 kdc: Correctly strip PAC, rather than error on UF_NO_AUTH_DATA_REQUIRED for servers
       via  92e8ce18a79 kdc: Remove UF_NO_AUTH_DATA_REQUIRED from client principals
      from  8a607e7577a netlogon_creds_cli: add netlogon_creds_cli_SendToSam_recv() and don't ignore result

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 83a654a4efd39a6e792a6d49e0ecf586e9bc53ef
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Mon Oct 18 16:07:11 2021 +1300

    tests/krb5: Add tests for constrained delegation to NO_AUTH_DATA_REQUIRED service
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14871
    
    Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    
    Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
    Autobuild-Date(master): Wed Oct 20 09:22:43 UTC 2021 on sn-devel-184

commit cc3d27596b9e8a8a46e8ba9c3c1a445477d458cf
Author: Joseph Sutton <josephsutton at catalyst.net.nz>
Date:   Mon Oct 18 16:05:19 2021 +1300

    tests/krb5: Ensure PAC is not present if expect_pac is false
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14871
    
    Signed-off-by: Joseph Sutton <josephsutton at catalyst.net.nz>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 031a8287642e3c4b9d0b7c6b51f3b1d79b227542
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Oct 18 16:00:45 2021 +1300

    kdc: Correctly strip PAC, rather than error on UF_NO_AUTH_DATA_REQUIRED for servers
    
    UF_NO_AUTH_DATA_REQUIRED on a server/service account should cause
    the PAC to be stripped not to given an error if the PAC was still
    present.
    
    Tested against Windows 2019
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14871
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 92e8ce18a79e88c9b961dc20e39436c4cf653013
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Oct 18 15:21:50 2021 +1300

    kdc: Remove UF_NO_AUTH_DATA_REQUIRED from client principals
    
    Tests against Windows 2019 show that UF_NO_AUTH_DATA_REQUIRED
    applies to services only, not to clients.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14871
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 python/samba/tests/krb5/raw_testcase.py |  14 ++---
 python/samba/tests/krb5/s4u_tests.py    | 107 +++++++++++++++++++++++++++++++-
 selftest/knownfail_heimdal_kdc          |   9 +--
 selftest/knownfail_mit_kdc              |   1 -
 source4/kdc/mit_samba.c                 |   7 ---
 source4/kdc/pac-glue.c                  |   5 --
 source4/kdc/wdc-samba4.c                |  38 ++++++++----
 7 files changed, 144 insertions(+), 37 deletions(-)


Changeset truncated at 500 lines:

diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py
index 0790ac13f99..0b9fe8e7a04 100644
--- a/python/samba/tests/krb5/raw_testcase.py
+++ b/python/samba/tests/krb5/raw_testcase.py
@@ -2385,13 +2385,6 @@ class RawKerberosTest(TestCaseInTempDir):
             self.assertElementPresent(ticket_private, 'authorization-data',
                                       expect_empty=not expect_pac)
 
-            if expect_pac:
-                authorization_data = self.getElementValue(ticket_private,
-                                                          'authorization-data')
-                pac_data = self.get_pac(authorization_data)
-
-                self.check_pac_buffers(pac_data, kdc_exchange_dict)
-
         encpart_session_key = None
         if encpart_private is not None:
             self.assertElementPresent(encpart_private, 'key')
@@ -2493,6 +2486,13 @@ class RawKerberosTest(TestCaseInTempDir):
             ticket_private=ticket_private,
             encpart_private=encpart_private)
 
+        if ticket_private is not None:
+            pac_data = self.get_ticket_pac(ticket_creds, expect_pac=expect_pac)
+            if expect_pac:
+                self.check_pac_buffers(pac_data, kdc_exchange_dict)
+            else:
+                self.assertIsNone(pac_data)
+
         expect_ticket_checksum = kdc_exchange_dict['expect_ticket_checksum']
         if expect_ticket_checksum:
             self.assertIsNotNone(ticket_decryption_key)
diff --git a/python/samba/tests/krb5/s4u_tests.py b/python/samba/tests/krb5/s4u_tests.py
index 9a25256081a..bbb7135b55b 100755
--- a/python/samba/tests/krb5/s4u_tests.py
+++ b/python/samba/tests/krb5/s4u_tests.py
@@ -538,6 +538,8 @@ class S4UKerberosTests(KDCBaseTest):
         transited_service = f'host/{service1_name}@{service1_realm}'
         expected_transited_services.append(transited_service)
 
+        expect_pac = kdc_dict.pop('expect_pac', True)
+
         kdc_exchange_dict = self.tgs_exchange_dict(
             expected_crealm=client_realm,
             expected_cname=client_cname,
@@ -557,7 +559,8 @@ class S4UKerberosTests(KDCBaseTest):
             pac_options=pac_options,
             expect_edata=expect_edata,
             expected_proxy_target=expected_proxy_target,
-            expected_transited_services=expected_transited_services)
+            expected_transited_services=expected_transited_services,
+            expect_pac=expect_pac)
 
         self._generic_kdc_exchange(kdc_exchange_dict,
                                    cname=None,
@@ -577,6 +580,18 @@ class S4UKerberosTests(KDCBaseTest):
                 'allow_delegation': True
             })
 
+    def test_constrained_delegation_no_auth_data_required(self):
+        # Test constrained delegation.
+        self._run_delegation_test(
+            {
+                'expected_error_mode': 0,
+                'allow_delegation': True,
+                'service2_opts': {
+                    'no_auth_data_required': True
+                },
+                'expect_pac': False
+            })
+
     def test_constrained_delegation_existing_delegation_info(self):
         # Test constrained delegation with an existing S4U_DELEGATION_INFO
         # structure in the PAC.
@@ -624,6 +639,35 @@ class S4UKerberosTests(KDCBaseTest):
                 'modify_service_tgt_fn': self.remove_ticket_pac
             })
 
+    def test_constrained_delegation_no_client_pac_no_auth_data_required(self):
+        # Test constrained delegation when the client service ticket does not
+        # contain a PAC.
+        self._run_delegation_test(
+            {
+                'expected_error_mode': (KDC_ERR_BADOPTION,
+                                        KDC_ERR_MODIFIED),
+                'allow_delegation': True,
+                'modify_client_tkt_fn': self.remove_ticket_pac,
+                'expect_edata': False,
+                'service2_opts': {
+                    'no_auth_data_required': True
+                }
+            })
+
+    def test_constrained_delegation_no_service_pac_no_auth_data_required(self):
+        # Test constrained delegation when the service TGT does not contain a
+        # PAC.
+        self._run_delegation_test(
+            {
+                'expected_error_mode': (KDC_ERR_BADOPTION,
+                                        KDC_ERR_MODIFIED),
+                'allow_delegation': True,
+                'modify_service_tgt_fn': self.remove_ticket_pac,
+                'service2_opts': {
+                    'no_auth_data_required': True
+                }
+            })
+
     def test_constrained_delegation_non_forwardable(self):
         # Test constrained delegation with a non-forwardable ticket.
         self._run_delegation_test(
@@ -645,6 +689,18 @@ class S4UKerberosTests(KDCBaseTest):
                 'allow_delegation': True
             })
 
+    def test_rbcd_no_auth_data_required(self):
+        self._run_delegation_test(
+            {
+                'expected_error_mode': 0,
+                'allow_rbcd': True,
+                'pac_options': '0001',  # supports RBCD
+                'service2_opts': {
+                    'no_auth_data_required': True
+                },
+                'expect_pac': False
+            })
+
     def test_rbcd_existing_delegation_info(self):
         # Test constrained delegation with an existing S4U_DELEGATION_INFO
         # structure in the PAC.
@@ -712,6 +768,55 @@ class S4UKerberosTests(KDCBaseTest):
                 'modify_service_tgt_fn': self.remove_ticket_pac
             })
 
+    def test_rbcd_no_client_pac_no_auth_data_required_a(self):
+        # Test constrained delegation when the client service ticket does not
+        # contain a PAC, and an empty msDS-AllowedToDelegateTo attribute.
+        self._run_delegation_test(
+            {
+                'expected_error_mode': KDC_ERR_MODIFIED,
+                'expected_status': ntstatus.NT_STATUS_NOT_SUPPORTED,
+                'allow_rbcd': True,
+                'pac_options': '0001',  # supports RBCD
+                'modify_client_tkt_fn': self.remove_ticket_pac,
+                'service2_opts': {
+                    'no_auth_data_required': True
+                }
+            })
+
+    def test_rbcd_no_client_pac_no_auth_data_required_b(self):
+        # Test constrained delegation when the client service ticket does not
+        # contain a PAC, and a non-empty msDS-AllowedToDelegateTo attribute.
+        self._run_delegation_test(
+            {
+                'expected_error_mode': KDC_ERR_MODIFIED,
+                'expected_status': ntstatus.NT_STATUS_NO_MATCH,
+                'allow_rbcd': True,
+                'pac_options': '0001',  # supports RBCD
+                'modify_client_tkt_fn': self.remove_ticket_pac,
+                'service1_opts': {
+                    'delegation_to_spn': ('host/test')
+                },
+                'service2_opts': {
+                    'no_auth_data_required': True
+                }
+            })
+
+    def test_rbcd_no_service_pac_no_auth_data_required(self):
+        # Test constrained delegation when the service TGT does not contain a
+        # PAC.
+        self._run_delegation_test(
+            {
+                'expected_error_mode': KDC_ERR_BADOPTION,
+                'expected_status':
+                    ntstatus.NT_STATUS_NOT_FOUND,
+                'allow_rbcd': True,
+                'pac_options': '0001',  # supports RBCD
+                'modify_service_tgt_fn': self.remove_ticket_pac,
+                'service2_opts': {
+                    'no_auth_data_required': True
+                }
+            })
+
     def test_rbcd_non_forwardable(self):
         # Test resource-based constrained delegation with a non-forwardable
         # ticket.
diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc
index 5008b998b78..5e0ee77ff6a 100644
--- a/selftest/knownfail_heimdal_kdc
+++ b/selftest/knownfail_heimdal_kdc
@@ -71,7 +71,7 @@
 # S4U tests
 #
 ^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_bronze_bit_rbcd_old_checksum
-^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_constrained_delegation_no_service_pac
+^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_constrained_delegation_no_service_pac\(.*\)$
 ^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_rbcd_existing_delegation_info
 ^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_rbcd_missing_client_checksum
 ^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_rbcd_no_client_pac_a
@@ -88,7 +88,8 @@
 #
 ^samba4.krb5.kdc with machine account.as-req-pac-request.fl2000dc:local
 #
-# TGS tests
 #
-^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_client_no_auth_data_required
-^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_service_no_auth_data_required
+^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_constrained_delegation_no_auth_data_required
+^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_rbcd_no_auth_data_required
+^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_rbcd_no_client_pac_no_auth_data_required_a
+^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_rbcd_no_client_pac_no_auth_data_required_b
diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc
index 5c04b677e38..7aa95cbb1c7 100644
--- a/selftest/knownfail_mit_kdc
+++ b/selftest/knownfail_mit_kdc
@@ -256,7 +256,6 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_ldap_service_ticket\(ad_dc\)
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_get_ticket_for_host_service_of_machine_account\(ad_dc\)
 #
-^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_client_no_auth_data_required\(ad_dc\)
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_remove_pac\(ad_dc\)
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_request_no_pac\(ad_dc\)
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_service_no_auth_data_required\(ad_dc\)
diff --git a/source4/kdc/mit_samba.c b/source4/kdc/mit_samba.c
index 60ba46cf2c3..22f9a54a05b 100644
--- a/source4/kdc/mit_samba.c
+++ b/source4/kdc/mit_samba.c
@@ -521,18 +521,11 @@ krb5_error_code mit_samba_reget_pac(struct mit_samba_context *ctx,
 	ssize_t srv_checksum_idx = -1;
 	ssize_t kdc_checksum_idx = -1;
 	krb5_pac new_pac = NULL;
-	bool ok;
 
 	if (client != NULL) {
 		client_skdc_entry =
 			talloc_get_type_abort(client->e_data,
 					      struct samba_kdc_entry);
-
-		/* The user account may be set not to want the PAC */
-		ok = samba_princ_needs_pac(client_skdc_entry);
-		if (!ok) {
-			return EINVAL;
-		}
 	}
 
 	if (server == NULL) {
diff --git a/source4/kdc/pac-glue.c b/source4/kdc/pac-glue.c
index 88bcb734fc5..688103d8477 100644
--- a/source4/kdc/pac-glue.c
+++ b/source4/kdc/pac-glue.c
@@ -651,11 +651,6 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
 	}
 	*_upn_info_blob = NULL;
 
-	/* The user account may be set not to want the PAC */
-	if ( ! samba_princ_needs_pac(p)) {
-		return NT_STATUS_OK;
-	}
-
 	logon_blob = talloc_zero(mem_ctx, DATA_BLOB);
 	if (logon_blob == NULL) {
 		return NT_STATUS_NO_MEMORY;
diff --git a/source4/kdc/wdc-samba4.c b/source4/kdc/wdc-samba4.c
index 589df8a651d..ac9d7d51733 100644
--- a/source4/kdc/wdc-samba4.c
+++ b/source4/kdc/wdc-samba4.c
@@ -105,13 +105,15 @@ static krb5_error_code samba_wdc_reget_pac2(krb5_context context,
 					    krb5_pac *pac,
 					    krb5_cksumtype ctype)
 {
-	struct samba_kdc_entry *p =
+	struct samba_kdc_entry *server_skdc_entry =
 		talloc_get_type_abort(server->ctx,
 		struct samba_kdc_entry);
 	struct samba_kdc_entry *krbtgt_skdc_entry =
 		talloc_get_type_abort(krbtgt->ctx,
 		struct samba_kdc_entry);
-	TALLOC_CTX *mem_ctx = talloc_named(p, 0, "samba_kdc_reget_pac2 context");
+	TALLOC_CTX *mem_ctx = talloc_named(server_skdc_entry,
+					   0,
+					   "samba_kdc_reget_pac2 context");
 	krb5_pac new_pac = NULL;
 	DATA_BLOB *pac_blob = NULL;
 	DATA_BLOB *upn_blob = NULL;
@@ -135,12 +137,6 @@ static krb5_error_code samba_wdc_reget_pac2(krb5_context context,
 		return ENOMEM;
 	}
 
-	/* The user account may be set not to want the PAC */
-	if (!samba_princ_needs_pac(p)) {
-		talloc_free(mem_ctx);
-		return EINVAL;
-	}
-
 	/* If the krbtgt was generated by an RODC, and we are not that
 	 * RODC, then we need to regenerate the PAC - we can't trust
 	 * it */
@@ -373,12 +369,28 @@ static krb5_error_code samba_wdc_reget_pac2(krb5_context context,
 		return EINVAL;
 	}
 
-	/* Build an updated PAC */
+	/*
+	 * The server account may be set not to want the PAC.
+	 *
+	 * While this is wasteful if the above cacluations were done
+	 * and now thrown away, this is cleaner as we do any ticket
+	 * signature checking etc always.
+	 *
+	 * UF_NO_AUTH_DATA_REQUIRED is the rare case and most of the
+	 * time (eg not accepting a ticket from the RODC) we do not
+	 * need to re-generate anything anyway.
+	 */
+	if (!samba_princ_needs_pac(server_skdc_entry)) {
+		ret = 0;
+		new_pac = NULL;
+		goto out;
+	}
+
+	/* Otherwise build an updated PAC */
 	ret = krb5_pac_init(context, &new_pac);
 	if (ret != 0) {
-		SAFE_FREE(types);
-		talloc_free(mem_ctx);
-		return ret;
+		new_pac = NULL;
+		goto out;
 	}
 
 	for (i = 0;;) {
@@ -496,6 +508,8 @@ static krb5_error_code samba_wdc_reget_pac2(krb5_context context,
 		}
 	}
 
+out:
+
 	SAFE_FREE(types);
 
 	/* We now replace the pac */


-- 
Samba Shared Repository



More information about the samba-cvs mailing list