auth: Exclude resource groups from a TGT
authorJoseph Sutton <josephsutton@catalyst.net.nz>
Tue, 27 Sep 2022 01:51:54 +0000 (14:51 +1300)
committerAndrew Bartlett <abartlet@samba.org>
Wed, 8 Feb 2023 00:03:39 +0000 (00:03 +0000)
Resource group SIDs should only be placed into a service ticket, but we
were including them in all tickets. Now that we have access to the group
attributes, we'll filter out any groups with SE_GROUP_RESOURCE set if
we're creating a TGT.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
auth/auth_sam_reply.c
auth/auth_sam_reply.h
librpc/idl/auth.idl
selftest/knownfail_heimdal_kdc
source3/auth/auth_samba4.c
source4/auth/kerberos/kerberos_pac.c
source4/kdc/mit_samba.c
source4/kdc/pac-glue.c
source4/kdc/pac-glue.h
source4/kdc/wdc-samba4.c
source4/rpc_server/netlogon/dcerpc_netlogon.c

index 552834a1bb0c4651be57a6c5c73be85f912f025d..8a68f045547f23d763b9f493cc1203bd41ce646e 100644 (file)
@@ -99,6 +99,13 @@ static NTSTATUS auth_convert_user_info_dc_sambaseinfo(TALLOC_CTX *mem_ctx,
 
                for (i=PRIMARY_GROUP_SID_INDEX; i<user_info_dc->num_sids; i++) {
                        struct auth_SidAttr *group_sid = &user_info_dc->sids[i];
+                       if (group_sid->attrs & SE_GROUP_RESOURCE) {
+                               /*
+                                * Resource groups don't belong in the base
+                                * RIDs, they're handled elsewhere.
+                                */
+                               continue;
+                       }
                        if (!dom_sid_in_domain(sam->domain_sid, &group_sid->sid)) {
                                /* We handle this elsewhere */
                                continue;
@@ -140,6 +147,7 @@ static NTSTATUS auth_convert_user_info_dc_sambaseinfo(TALLOC_CTX *mem_ctx,
  * the user_info_dc it was generated from */
 NTSTATUS auth_convert_user_info_dc_saminfo6(TALLOC_CTX *mem_ctx,
                                            const struct auth_user_info_dc *user_info_dc,
+                                           enum auth_group_inclusion group_inclusion,
                                            struct netr_SamInfo6 **_sam6)
 {
        NTSTATUS status;
@@ -168,7 +176,20 @@ NTSTATUS auth_convert_user_info_dc_saminfo6(TALLOC_CTX *mem_ctx,
 
        /* We don't put the user and group SIDs in there */
        for (i=2; i<user_info_dc->num_sids; i++) {
-               if (dom_sid_in_domain(sam6->base.domain_sid, &user_info_dc->sids[i].sid)) {
+               if (user_info_dc->sids[i].attrs & SE_GROUP_RESOURCE) {
+                       /*
+                        * If it's a resource group, check whether it should be
+                        * included or filtered out.
+                        */
+                       switch (group_inclusion) {
+                       case AUTH_INCLUDE_RESOURCE_GROUPS:
+                               /* Include it. */
+                               break;
+                       case AUTH_EXCLUDE_RESOURCE_GROUPS:
+                               /* Ignore it. */
+                               continue;
+                       }
+               } else if (dom_sid_in_domain(sam6->base.domain_sid, &user_info_dc->sids[i].sid)) {
                        continue;
                }
                sam6->sids[sam6->sidcount].sid = dom_sid_dup(sam6->sids, &user_info_dc->sids[i].sid);
@@ -211,6 +232,7 @@ NTSTATUS auth_convert_user_info_dc_saminfo6(TALLOC_CTX *mem_ctx,
  * the user_info_dc it was generated from */
 NTSTATUS auth_convert_user_info_dc_saminfo2(TALLOC_CTX *mem_ctx,
                                           const struct auth_user_info_dc *user_info_dc,
+                                          enum auth_group_inclusion group_inclusion,
                                           struct netr_SamInfo2 **_sam2)
 {
        NTSTATUS status;
@@ -222,7 +244,8 @@ NTSTATUS auth_convert_user_info_dc_saminfo2(TALLOC_CTX *mem_ctx,
                return NT_STATUS_NO_MEMORY;
        }
 
-       status = auth_convert_user_info_dc_saminfo6(sam2, user_info_dc, &sam6);
+       status = auth_convert_user_info_dc_saminfo6(sam2, user_info_dc,
+                                                   group_inclusion, &sam6);
        if (!NT_STATUS_IS_OK(status)) {
                TALLOC_FREE(sam2);
                return status;
@@ -237,6 +260,7 @@ NTSTATUS auth_convert_user_info_dc_saminfo2(TALLOC_CTX *mem_ctx,
  * the user_info_dc it was generated from */
 NTSTATUS auth_convert_user_info_dc_saminfo3(TALLOC_CTX *mem_ctx,
                                           const struct auth_user_info_dc *user_info_dc,
+                                          enum auth_group_inclusion group_inclusion,
                                           struct netr_SamInfo3 **_sam3)
 {
        NTSTATUS status;
@@ -248,7 +272,8 @@ NTSTATUS auth_convert_user_info_dc_saminfo3(TALLOC_CTX *mem_ctx,
                return NT_STATUS_NO_MEMORY;
        }
 
-       status = auth_convert_user_info_dc_saminfo6(sam3, user_info_dc, &sam6);
+       status = auth_convert_user_info_dc_saminfo6(sam3, user_info_dc,
+                                                   group_inclusion, &sam6);
        if (!NT_STATUS_IS_OK(status)) {
                TALLOC_FREE(sam3);
                return status;
index d8a30c6b36f4b5a7df9f7e823c1a10d913219872..4eebf0b06e30f174e0bc73778ab1011c7abeb7a4 100644 (file)
@@ -47,12 +47,15 @@ struct auth_user_info *auth_user_info_copy(TALLOC_CTX *mem_ctx,
 
 NTSTATUS auth_convert_user_info_dc_saminfo6(TALLOC_CTX *mem_ctx,
                                           const struct auth_user_info_dc *user_info_dc,
+                                          enum auth_group_inclusion group_inclusion,
                                           struct netr_SamInfo6 **_sam6);
 NTSTATUS auth_convert_user_info_dc_saminfo2(TALLOC_CTX *mem_ctx,
                                           const struct auth_user_info_dc *user_info_dc,
+                                          enum auth_group_inclusion group_inclusion,
                                           struct netr_SamInfo2 **_sam2);
 NTSTATUS auth_convert_user_info_dc_saminfo3(TALLOC_CTX *mem_ctx,
                                           const struct auth_user_info_dc *user_info_dc,
+                                          enum auth_group_inclusion group_inclusion,
                                           struct netr_SamInfo3 **_sam3);
 
 /**
index 5985d554606e3ca66e134a2141d6bd23694713a3..582587e062ff54875f3af2145d1373f58de4e7b4 100644 (file)
@@ -95,6 +95,15 @@ interface auth
                TICKET_TYPE_NON_TGT = 2
        } ticket_type;
 
+       /*
+        * Used to indicate whether or not to include resource groups in the
+        * formation of SamInfo or a PAC.
+        */
+       typedef enum {
+               AUTH_INCLUDE_RESOURCE_GROUPS = 0,
+               AUTH_EXCLUDE_RESOURCE_GROUPS = 1
+       } auth_group_inclusion;
+
        typedef [public] struct {
                dom_sid sid;
                security_GroupAttrs attrs;
index 4a55b0709ea8f29612f7affb4bd1a8ee546013fb..f64e4c30e190e26881e1a3a941285b73d133fed7 100644 (file)
 #
 # Group tests
 #
-^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_domain_local_Samba_4_17_tgs_req_to_krbtgt.ad_dc
-^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_domain_local_Samba_4_17_tgs_req_to_service.ad_dc
-^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_domain_local_as_req_to_krbtgt.ad_dc
 ^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_domain_local_compression_as_req_to_service.ad_dc
 ^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_domain_local_compression_tgs_req_to_service.ad_dc
-^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_domain_local_no_compression_as_req_to_service.ad_dc
-^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_domain_local_no_compression_tgs_req_to_service.ad_dc
-^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_nested_domain_local_as_req_to_krbtgt.ad_dc
 ^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_nested_domain_local_compression_as_req_to_service.ad_dc
-^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_nested_domain_local_no_compression_as_req_to_service.ad_dc
 ^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_nested_group_removal_compression_tgs_req_to_service.ad_dc
-^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_nested_group_removal_no_compression_tgs_req_to_service.ad_dc
-^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_nested_group_removal_tgs_req_to_krbtgt.ad_dc
-^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_nested_universal_as_req_to_krbtgt.ad_dc
 ^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_nested_universal_compression_as_req_to_service.ad_dc
-^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_nested_universal_no_compression_as_req_to_service.ad_dc
 ^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_resource_sids_given_compression_tgs_req_to_krbtgt.ad_dc
 ^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_resource_sids_given_compression_tgs_req_to_service.ad_dc
 ^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_resource_sids_given_no_compression_tgs_req_to_krbtgt.ad_dc
index dec67a488d71083eaa81b11d99df161002516328..be6fd9a92a2ae4740fe1880d24e68500ac392cac 100644 (file)
@@ -152,6 +152,7 @@ static NTSTATUS check_samba4_security(
 
        nt_status = auth_convert_user_info_dc_saminfo3(mem_ctx,
                                                       user_info_dc,
+                                                      AUTH_INCLUDE_RESOURCE_GROUPS,
                                                       &info3);
        if (NT_STATUS_IS_OK(nt_status)) {
                /* We need the strings from the server_info to be valid as long as the info3 is around */
index 880211bdea2addc77c6d61580b86b1371582147e..bc389fd237e8b89fb3154ebdf6ab87db19306043 100644 (file)
                talloc_free(pac_data);
                return ENOMEM;
        }
-       nt_status = auth_convert_user_info_dc_saminfo3(LOGON_INFO, user_info_dc, &sam3);
+       nt_status = auth_convert_user_info_dc_saminfo3(LOGON_INFO, user_info_dc,
+                                                      AUTH_INCLUDE_RESOURCE_GROUPS,
+                                                      &sam3);
        if (!NT_STATUS_IS_OK(nt_status)) {
                DEBUG(1, ("Getting Samba info failed: %s\n", nt_errstr(nt_status)));
                talloc_free(pac_data);
index 8df3ad3622882e2cefac8a4d8345eb939e3ef44b..af9f6eecc3d9d3b43933ecf06f0de03ef03d5cd0 100644 (file)
@@ -475,7 +475,11 @@ int mit_samba_get_pac(struct mit_samba_context *smb_ctx,
        NTSTATUS nt_status;
        krb5_error_code code;
        struct samba_kdc_entry *skdc_entry;
-       bool is_krbtgt;
+       bool is_krbtgt = ks_is_tgs_principal(smb_ctx, server->princ);
+       /* Only include resource groups in a service ticket. */
+       enum auth_group_inclusion group_inclusion = (is_krbtgt)
+               ? AUTH_EXCLUDE_RESOURCE_GROUPS
+               : AUTH_INCLUDE_RESOURCE_GROUPS;
        enum samba_asserted_identity asserted_identity =
                (flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION) ?
                        SAMBA_ASSERTED_IDENTITY_SERVICE :
@@ -496,11 +500,10 @@ int mit_samba_get_pac(struct mit_samba_context *smb_ctx,
                cred_ndr_ptr = &cred_ndr;
        }
 
-       is_krbtgt = ks_is_tgs_principal(smb_ctx, server->princ);
-
        nt_status = samba_kdc_get_pac_blobs(tmp_ctx,
                                            skdc_entry,
                                            asserted_identity,
+                                           group_inclusion,
                                            &logon_info_blob,
                                            cred_ndr_ptr,
                                            &upn_dns_info_blob,
index e1a44bf8e1f9110585f0beca0b5db80aa9578acf..395bd7c0d568e9968d3c458c88cb2fde4f2a5f82 100644 (file)
@@ -49,6 +49,7 @@
 static
 NTSTATUS samba_get_logon_info_pac_blob(TALLOC_CTX *mem_ctx,
                                       const struct auth_user_info_dc *info,
+                                      enum auth_group_inclusion group_inclusion,
                                       DATA_BLOB *pac_data,
                                       DATA_BLOB *requester_sid_blob)
 {
@@ -64,7 +65,9 @@ NTSTATUS samba_get_logon_info_pac_blob(TALLOC_CTX *mem_ctx,
                *requester_sid_blob = data_blob_null;
        }
 
-       nt_status = auth_convert_user_info_dc_saminfo3(mem_ctx, info, &info3);
+       nt_status = auth_convert_user_info_dc_saminfo3(mem_ctx, info,
+                                                      group_inclusion,
+                                                      &info3);
        if (!NT_STATUS_IS_OK(nt_status)) {
                DEBUG(1, ("Getting Samba info failed: %s\n",
                          nt_errstr(nt_status)));
@@ -845,6 +848,7 @@ NTSTATUS samba_kdc_get_user_info_from_db(struct samba_kdc_entry *skdc_entry,
 NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
                                 struct samba_kdc_entry *p,
                                 enum samba_asserted_identity asserted_identity,
+                                enum auth_group_inclusion group_inclusion,
                                 DATA_BLOB **_logon_info_blob,
                                 DATA_BLOB **_cred_ndr_blob,
                                 DATA_BLOB **_upn_info_blob,
@@ -940,6 +944,7 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
 
        nt_status = samba_get_logon_info_pac_blob(logon_blob,
                                                  user_info_dc,
+                                                 group_inclusion,
                                                  logon_blob,
                                                  requester_sid_blob);
        if (!NT_STATUS_IS_OK(nt_status)) {
@@ -1000,6 +1005,7 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
 NTSTATUS samba_kdc_update_pac_blob(TALLOC_CTX *mem_ctx,
                                   krb5_context context,
                                   struct ldb_context *samdb,
+                                  enum auth_group_inclusion group_inclusion,
                                   const krb5_pac pac, DATA_BLOB *pac_blob,
                                   struct PAC_SIGNATURE_DATA *pac_srv_sig,
                                   struct PAC_SIGNATURE_DATA *pac_kdc_sig)
@@ -1026,8 +1032,9 @@ NTSTATUS samba_kdc_update_pac_blob(TALLOC_CTX *mem_ctx,
        }
 
        nt_status = samba_get_logon_info_pac_blob(mem_ctx,
-                                                 user_info_dc, pac_blob, NULL);
-
+                                                 user_info_dc,
+                                                 group_inclusion,
+                                                 pac_blob, NULL);
        return nt_status;
 }
 
@@ -1427,6 +1434,7 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
        DATA_BLOB *client_claims_blob = NULL;
        bool is_untrusted = flags & SAMBA_KDC_FLAG_KRBTGT_IS_UNTRUSTED;
        int is_tgs = false;
+       enum auth_group_inclusion group_inclusion;
        size_t num_types = 0;
        uint32_t *types = NULL;
        /*
@@ -1448,6 +1456,17 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
        ssize_t requester_sid_idx = -1;
        ssize_t full_checksum_idx = -1;
 
+       is_tgs = smb_krb5_principal_is_tgs(context, server_principal);
+       if (is_tgs == -1) {
+               code = ENOMEM;
+               goto done;
+       }
+
+       /* Only include resource groups in a service ticket. */
+       group_inclusion = (is_tgs)
+               ? AUTH_EXCLUDE_RESOURCE_GROUPS
+               : AUTH_INCLUDE_RESOURCE_GROUPS;
+
        if (client != NULL) {
                /*
                 * Check the objectSID of the client and pac data are the same.
@@ -1511,6 +1530,7 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
                nt_status = samba_kdc_get_pac_blobs(mem_ctx,
                                                    client,
                                                    asserted_identity,
+                                                   group_inclusion,
                                                    &pac_blob,
                                                    NULL,
                                                    &upn_blob,
@@ -1589,6 +1609,7 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
                nt_status = samba_kdc_update_pac_blob(mem_ctx,
                                                      context,
                                                      samdb,
+                                                     group_inclusion,
                                                      old_pac,
                                                      pac_blob,
                                                      NULL,
@@ -1778,12 +1799,6 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
                goto done;
        }
 
-       is_tgs = smb_krb5_principal_is_tgs(context, server_principal);
-       if (is_tgs == -1) {
-               code = ENOMEM;
-               goto done;
-       }
-
        if (!is_untrusted && !is_tgs) {
                /*
                 * The client may have requested no PAC when obtaining the
index 046264cca123455da71e6d3abb8272b4d5055c2c..6a1de6dda1792530e3f8b006e213891987dcb648 100644 (file)
@@ -21,6 +21,8 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+#include "librpc/gen_ndr/auth.h"
+
 enum samba_asserted_identity {
        SAMBA_ASSERTED_IDENTITY_IGNORE = 0,
        SAMBA_ASSERTED_IDENTITY_SERVICE,
@@ -71,6 +73,7 @@ NTSTATUS samba_kdc_get_user_info_from_db(struct samba_kdc_entry *skdc_entry,
 NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
                                 struct samba_kdc_entry *skdc_entry,
                                 enum samba_asserted_identity asserted_identity,
+                                enum auth_group_inclusion group_inclusion,
                                 DATA_BLOB **_logon_info_blob,
                                 DATA_BLOB **_cred_ndr_blob,
                                 DATA_BLOB **_upn_info_blob,
@@ -81,6 +84,7 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
 NTSTATUS samba_kdc_update_pac_blob(TALLOC_CTX *mem_ctx,
                                   krb5_context context,
                                   struct ldb_context *samdb,
+                                  enum auth_group_inclusion group_inclusion,
                                   const krb5_pac pac, DATA_BLOB *pac_blob,
                                   struct PAC_SIGNATURE_DATA *pac_srv_sig,
                                   struct PAC_SIGNATURE_DATA *pac_kdc_sig);
index f3ca04550b0aa05ab8d0285c87d47a5bec2fcffc..abc86d6c8b47c3e221425ee9d6311c5a1191fa2c 100644 (file)
@@ -143,7 +143,12 @@ static krb5_error_code samba_wdc_get_pac(void *priv,
        struct samba_kdc_entry *skdc_entry =
                talloc_get_type_abort(client->context,
                struct samba_kdc_entry);
-       bool is_krbtgt;
+       bool is_krbtgt = krb5_principal_is_krbtgt(context, server->principal);
+       /* Only include resource groups in a service ticket. */
+       enum auth_group_inclusion group_inclusion =
+               (is_krbtgt) ?
+                       AUTH_EXCLUDE_RESOURCE_GROUPS :
+                       AUTH_INCLUDE_RESOURCE_GROUPS;
        bool is_s4u2self = samba_wdc_is_s4u2self_req(r);
        enum samba_asserted_identity asserted_identity =
                (is_s4u2self) ?
@@ -165,10 +170,9 @@ static krb5_error_code samba_wdc_get_pac(void *priv,
                cred_ndr_ptr = &cred_ndr;
        }
 
-       is_krbtgt = krb5_principal_is_krbtgt(context, server->principal);
-
        nt_status = samba_kdc_get_pac_blobs(mem_ctx, skdc_entry,
                                            asserted_identity,
+                                           group_inclusion,
                                            &logon_blob,
                                            cred_ndr_ptr,
                                            &upn_blob,
index 314b469a718ae1c09fc53b825a49a1ed8f943bc9..7456422af74fa5b5169d48074821392dbc05dd7a 100644 (file)
@@ -1469,6 +1469,7 @@ static void dcesrv_netr_LogonSamLogon_base_auth_done(struct tevent_req *subreq)
        case 2:
                nt_status = auth_convert_user_info_dc_saminfo2(mem_ctx,
                                                               user_info_dc,
+                                                              AUTH_INCLUDE_RESOURCE_GROUPS,
                                                               &sam2);
                if (!NT_STATUS_IS_OK(nt_status)) {
                        r->out.result = nt_status;
@@ -1482,6 +1483,7 @@ static void dcesrv_netr_LogonSamLogon_base_auth_done(struct tevent_req *subreq)
        case 3:
                nt_status = auth_convert_user_info_dc_saminfo3(mem_ctx,
                                                               user_info_dc,
+                                                              AUTH_INCLUDE_RESOURCE_GROUPS,
                                                               &sam3);
                if (!NT_STATUS_IS_OK(nt_status)) {
                        r->out.result = nt_status;
@@ -1495,6 +1497,7 @@ static void dcesrv_netr_LogonSamLogon_base_auth_done(struct tevent_req *subreq)
        case 6:
                nt_status = auth_convert_user_info_dc_saminfo6(mem_ctx,
                                                               user_info_dc,
+                                                              AUTH_INCLUDE_RESOURCE_GROUPS,
                                                               &sam6);
                if (!NT_STATUS_IS_OK(nt_status)) {
                        r->out.result = nt_status;