From e3fdb2d00152d86558a2ba29b92fd36440055461 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 2 Dec 2022 10:49:20 +1300 Subject: [PATCH] s4:kdc: Add resource SID compression The domain-local groups that are added to the PAC of a service ticket are now, if the service doesn't disclaim support for SID compression, placed into the resource groups structure in PAC_LOGON_INFO. In a TGS exchange directed to a KDC, rather than to a service, the resource groups structure is simply copied into the updated PAC without any processing being done. Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- auth/auth_sam_reply.c | 299 +++++++++++++++--- auth/auth_sam_reply.h | 7 +- librpc/idl/auth.idl | 16 +- librpc/idl/netlogon.idl | 1 + selftest/knownfail_heimdal_kdc | 16 - source3/auth/auth_samba4.c | 3 +- source4/auth/kerberos/kerberos.h | 1 + source4/auth/kerberos/kerberos_pac.c | 32 +- source4/kdc/db-glue.c | 8 + source4/kdc/mit_samba.c | 17 +- source4/kdc/pac-glue.c | 82 ++++- source4/kdc/wdc-samba4.c | 17 +- source4/rpc_server/netlogon/dcerpc_netlogon.c | 4 +- source4/torture/winbind/winbind.c | 18 +- 14 files changed, 437 insertions(+), 84 deletions(-) diff --git a/auth/auth_sam_reply.c b/auth/auth_sam_reply.c index b9d06161cb1..850ccae980b 100644 --- a/auth/auth_sam_reply.c +++ b/auth/auth_sam_reply.c @@ -25,6 +25,165 @@ #include "libcli/security/security.h" #include "auth/auth_sam_reply.h" +/* Returns true if this SID belongs in SamBaseInfo, otherwise false. */ +static bool is_base_sid(const struct auth_SidAttr *sid, + const struct dom_sid *domain_sid) +{ + if (sid->attrs & SE_GROUP_RESOURCE) { + /* + * Resource groups don't belong in the base + * RIDs, they're handled elsewhere. + */ + return false; + } + + /* + * This SID belongs in the base structure only if it's in the account's + * domain. + */ + return dom_sid_in_domain(domain_sid, &sid->sid); +} + +/* Stores a SID in a previously allocated array. */ +static NTSTATUS store_extra_sid(struct netr_SidAttr *sids, + uint32_t *sidcount, + const uint32_t allocated_sids, + const struct auth_SidAttr *sid) +{ + /* Check we aren't about to overflow our allocation. */ + if (*sidcount >= allocated_sids) { + return NT_STATUS_INVALID_PARAMETER; + } + + sids[*sidcount].sid = dom_sid_dup(sids, &sid->sid); + if (sids[*sidcount].sid == NULL) { + return NT_STATUS_NO_MEMORY; + } + sids[*sidcount].attributes = sid->attrs; + *sidcount += 1; + + return NT_STATUS_OK; +} + +/* + * Stores a resource SID in a previously allocated array, either Extra SIDs or + * Resource SIDs. Any SID within the domain of the first SID so added is stored + * there, while remaining SIDs are stored in Extra SIDs. + */ +static NTSTATUS store_resource_sid(struct netr_SidAttr *sids, + uint32_t *sidcount, + const uint32_t allocated_sids, + const struct auth_SidAttr *sid, + struct PAC_DOMAIN_GROUP_MEMBERSHIP *resource_groups, + const uint32_t allocated_resource_groups) +{ + NTSTATUS status; + + struct dom_sid *resource_domain = NULL; + uint32_t rid; + + if (resource_groups == NULL) { + return NT_STATUS_INVALID_PARAMETER; + } + + /* Split the SID into domain and RID. */ + status = dom_sid_split_rid(resource_groups, &sid->sid, &resource_domain, &rid); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + + if (resource_groups->domain_sid == NULL) { + /* + * There is no domain SID set. Set it to the domain of this SID. + */ + resource_groups->domain_sid = resource_domain; + } else { + /* + * A domain SID has already been set. Check whether this SID's + * domain matches. + * + * Assuming that resource SIDs have been obtained with + * dsdb_expand_nested_groups(), they should all be within the + * same domain (ours), so unless something has gone horribly + * wrong, we should always find that they match. + */ + bool match = dom_sid_equal(resource_groups->domain_sid, resource_domain); + talloc_free(resource_domain); + if (!match) { + /* + * It doesn't match, so we can't store this SID here. It + * will have to go in Extra SIDs. + */ + return store_extra_sid(sids, sidcount, allocated_sids, sid); + } + } + + /* Store the SID in Resource SIDs. */ + + /* Check we aren't about to overflow our allocation. */ + if (resource_groups->groups.count >= allocated_resource_groups) { + return NT_STATUS_INVALID_PARAMETER; + } + + resource_groups->groups.rids[resource_groups->groups.count].rid = rid; + resource_groups->groups.rids[resource_groups->groups.count].attributes = sid->attrs; + resource_groups->groups.count++; + + return NT_STATUS_OK; +} + +/* + * Stores a SID in a previously allocated array, or excludes it if we are not + * storing resource groups. It will be placed in either Extra SIDs or Resource + * SIDs, depending on which is appropriate. + */ +static NTSTATUS store_sid(struct netr_SidAttr *sids, + uint32_t *sidcount, + const uint32_t allocated_sids, + const struct auth_SidAttr *sid, + struct PAC_DOMAIN_GROUP_MEMBERSHIP *resource_groups, + const uint32_t allocated_resource_groups, + const enum auth_group_inclusion group_inclusion) +{ + /* See if it's a resource SID. */ + if (sid->attrs & SE_GROUP_RESOURCE) { + /* + * If this is the SID of a resource group, determine whether it + * should be included or filtered out. + */ + switch (group_inclusion) { + case AUTH_INCLUDE_RESOURCE_GROUPS: + /* Include this SID in Extra SIDs. */ + break; + case AUTH_INCLUDE_RESOURCE_GROUPS_COMPRESSED: + /* + * Try to include this SID in Resource Groups. If this + * can't be arranged, we shall fall back to Extra + * SIDs. + */ + return store_resource_sid(sids, + sidcount, + allocated_sids, + sid, + resource_groups, + allocated_resource_groups); + case AUTH_EXCLUDE_RESOURCE_GROUPS: + /* Ignore this SID. */ + return NT_STATUS_OK; + default: + /* This means we have a bug. */ + DBG_ERR("invalid group inclusion parameter: %u\n", group_inclusion); + return NT_STATUS_INVALID_PARAMETER; + } + } + + /* Just store the SID in Extra SIDs. */ + return store_extra_sid(sids, + sidcount, + allocated_sids, + sid); +} + static NTSTATUS auth_convert_user_info_dc_sambaseinfo(TALLOC_CTX *mem_ctx, const struct auth_user_info_dc *user_info_dc, struct netr_SamBaseInfo *sam) @@ -99,14 +258,9 @@ static NTSTATUS auth_convert_user_info_dc_sambaseinfo(TALLOC_CTX *mem_ctx, for (i=PRIMARY_GROUP_SID_INDEX; inum_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)) { + + bool belongs_in_base = is_base_sid(group_sid, sam->domain_sid); + if (!belongs_in_base) { /* We handle this elsewhere */ continue; } @@ -140,62 +294,103 @@ static NTSTATUS auth_convert_user_info_dc_sambaseinfo(TALLOC_CTX *mem_ctx, return NT_STATUS_OK; } -/* Note that the validity of the _sam6 structure is only as long as - * the user_info_dc it was generated from */ +/* Note that the validity of the _sam6 and resource_groups structures is only as + * long as 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) + struct netr_SamInfo6 **_sam6, + struct PAC_DOMAIN_GROUP_MEMBERSHIP **_resource_groups) { NTSTATUS status; struct netr_SamInfo6 *sam6 = NULL; + struct PAC_DOMAIN_GROUP_MEMBERSHIP *resource_groups = NULL; size_t i; + const uint32_t allocated_sids = user_info_dc->num_sids; + uint32_t allocated_resource_groups = 0; + sam6 = talloc_zero(mem_ctx, struct netr_SamInfo6); if (sam6 == NULL) { return NT_STATUS_NO_MEMORY; } + if (_resource_groups == NULL) { + if (group_inclusion == AUTH_INCLUDE_RESOURCE_GROUPS_COMPRESSED) { + DBG_ERR("_resource_groups parameter not provided to receive resource groups!\n"); + TALLOC_FREE(sam6); + return NT_STATUS_INVALID_PARAMETER; + } + } else if (group_inclusion == AUTH_INCLUDE_RESOURCE_GROUPS_COMPRESSED) { + *_resource_groups = NULL; + + /* Allocate resource groups structure. */ + resource_groups = talloc_zero(mem_ctx, struct PAC_DOMAIN_GROUP_MEMBERSHIP); + if (resource_groups == NULL) { + TALLOC_FREE(sam6); + return NT_STATUS_NO_MEMORY; + } + + /* + * Allocate enough space to store user_info_dc->num_sids + * RIDs in the worst case. + */ + allocated_resource_groups = user_info_dc->num_sids; + resource_groups->groups.rids = talloc_zero_array(resource_groups, + struct samr_RidWithAttribute, + allocated_resource_groups); + if (resource_groups->groups.rids == NULL) { + TALLOC_FREE(sam6); + TALLOC_FREE(resource_groups); + return NT_STATUS_NO_MEMORY; + } + } else { + /* No resource groups will be provided. */ + *_resource_groups = NULL; + } + status = auth_convert_user_info_dc_sambaseinfo(sam6, user_info_dc, &sam6->base); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(sam6); + TALLOC_FREE(resource_groups); return status; } - sam6->sids = talloc_array(sam6, struct netr_SidAttr, - user_info_dc->num_sids); + /* + * Allocate enough space to store user_info_dc->num_sids SIDs in the + * worst case. + */ + sam6->sids = talloc_zero_array(sam6, struct netr_SidAttr, + allocated_sids); if (sam6->sids == NULL) { TALLOC_FREE(sam6); + TALLOC_FREE(resource_groups); return NT_STATUS_NO_MEMORY; } /* We don't put the user and group SIDs in there */ for (i=2; inum_sids; i++) { - 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)) { + struct auth_SidAttr *group_sid = &user_info_dc->sids[i]; + bool belongs_in_base = is_base_sid(group_sid, sam6->base.domain_sid); + if (belongs_in_base) { + /* We already handled this in the base. */ continue; } - sam6->sids[sam6->sidcount].sid = dom_sid_dup(sam6->sids, &user_info_dc->sids[i].sid); - if (sam6->sids[sam6->sidcount].sid == NULL) { + + status = store_sid(sam6->sids, + &sam6->sidcount, + allocated_sids, + group_sid, + resource_groups, + allocated_resource_groups, + group_inclusion); + if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(sam6); - return NT_STATUS_NO_MEMORY; + TALLOC_FREE(resource_groups); + return status; } - sam6->sids[sam6->sidcount].attributes = user_info_dc->sids[i].attrs; - sam6->sidcount += 1; } if (sam6->sidcount) { sam6->base.user_flags |= NETLOGON_EXTRA_SIDS; @@ -208,6 +403,7 @@ NTSTATUS auth_convert_user_info_dc_saminfo6(TALLOC_CTX *mem_ctx, user_info_dc->info->dns_domain_name); if (sam6->dns_domainname.string == NULL) { TALLOC_FREE(sam6); + TALLOC_FREE(resource_groups); return NT_STATUS_NO_MEMORY; } } @@ -217,11 +413,19 @@ NTSTATUS auth_convert_user_info_dc_saminfo6(TALLOC_CTX *mem_ctx, user_info_dc->info->user_principal_name); if (sam6->principal_name.string == NULL) { TALLOC_FREE(sam6); + TALLOC_FREE(resource_groups); return NT_STATUS_NO_MEMORY; } } *_sam6 = sam6; + if (resource_groups != NULL) { + if (resource_groups->groups.count > 0) { + *_resource_groups = resource_groups; + } else { + TALLOC_FREE(resource_groups); + } + } return NT_STATUS_OK; } @@ -242,7 +446,8 @@ NTSTATUS auth_convert_user_info_dc_saminfo2(TALLOC_CTX *mem_ctx, } status = auth_convert_user_info_dc_saminfo6(sam2, user_info_dc, - group_inclusion, &sam6); + group_inclusion, &sam6, + NULL); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(sam2); return status; @@ -258,7 +463,8 @@ NTSTATUS auth_convert_user_info_dc_saminfo2(TALLOC_CTX *mem_ctx, 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) + struct netr_SamInfo3 **_sam3, + struct PAC_DOMAIN_GROUP_MEMBERSHIP **_resource_groups) { NTSTATUS status; struct netr_SamInfo6 *sam6 = NULL; @@ -270,7 +476,8 @@ NTSTATUS auth_convert_user_info_dc_saminfo3(TALLOC_CTX *mem_ctx, } status = auth_convert_user_info_dc_saminfo6(sam3, user_info_dc, - group_inclusion, &sam6); + group_inclusion, &sam6, + _resource_groups); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(sam3); return status; @@ -579,11 +786,15 @@ NTSTATUS make_user_info_dc_netlogon_validation(TALLOC_CTX *mem_ctx, } /** - * Make a user_info_dc struct from the PAC_LOGON_INFO supplied in the krb5 logon + * Make a user_info_dc struct from the PAC_LOGON_INFO supplied in the krb5 + * logon. For group_inclusion, pass AUTH_INCLUDE_RESOURCE_GROUPS if SIDs from + * the resource groups are to be included in the resulting structure, and pass + * AUTH_EXCLUDE_RESOURCE_GROUPS otherwise. */ NTSTATUS make_user_info_dc_pac(TALLOC_CTX *mem_ctx, const struct PAC_LOGON_INFO *pac_logon_info, const struct PAC_UPN_DNS_INFO *pac_upn_dns_info, + const enum auth_group_inclusion group_inclusion, struct auth_user_info_dc **_user_info_dc) { uint32_t i; @@ -603,7 +814,21 @@ NTSTATUS make_user_info_dc_pac(TALLOC_CTX *mem_ctx, } if (pac_logon_info->info3.base.user_flags & NETLOGON_RESOURCE_GROUPS) { - rg = &pac_logon_info->resource_groups; + switch (group_inclusion) { + case AUTH_INCLUDE_RESOURCE_GROUPS: + /* Take resource groups from the PAC. */ + rg = &pac_logon_info->resource_groups; + break; + case AUTH_EXCLUDE_RESOURCE_GROUPS: + /* + * The PAC is from a TGT, or we don't want to process + * its resource groups. + */ + break; + default: + DBG_ERR("invalid group inclusion parameter: %u\n", group_inclusion); + return NT_STATUS_INVALID_PARAMETER; + } } if (rg != NULL && rg->groups.count > 0) { diff --git a/auth/auth_sam_reply.h b/auth/auth_sam_reply.h index 4eebf0b06e3..57a98249b08 100644 --- a/auth/auth_sam_reply.h +++ b/auth/auth_sam_reply.h @@ -48,7 +48,8 @@ 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); + struct netr_SamInfo6 **_sam6, + struct PAC_DOMAIN_GROUP_MEMBERSHIP **_resource_groups); 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, @@ -56,7 +57,8 @@ NTSTATUS auth_convert_user_info_dc_saminfo2(TALLOC_CTX *mem_ctx, 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); + struct netr_SamInfo3 **_sam3, + struct PAC_DOMAIN_GROUP_MEMBERSHIP **_resource_groups); /** * Make a user_info_dc struct from the info3 returned by a domain logon @@ -74,6 +76,7 @@ NTSTATUS make_user_info_dc_netlogon_validation(TALLOC_CTX *mem_ctx, NTSTATUS make_user_info_dc_pac(TALLOC_CTX *mem_ctx, const struct PAC_LOGON_INFO *pac_logon_info, const struct PAC_UPN_DNS_INFO *pac_upn_dns_info, + enum auth_group_inclusion group_inclusion, struct auth_user_info_dc **_user_info_dc); /* The following definitions come from auth/wbc_auth_util.c */ diff --git a/librpc/idl/auth.idl b/librpc/idl/auth.idl index a6b4a118be2..351a2fcfb18 100644 --- a/librpc/idl/auth.idl +++ b/librpc/idl/auth.idl @@ -100,12 +100,20 @@ interface auth } ticket_type; /* - * Used to indicate whether or not to include resource groups in the - * formation of SamInfo or a PAC. + * Used to indicate whether or not to include or disregard resource + * groups when forming a SamInfo structure, user_info_dc structure, or + * PAC, and whether or not to compress them when forming a PAC. + * + * When producing a TGT, existing resource groups are always copied + * unmodified into the PAC. When producing a service ticket, existing + * resource groups and resource groups in other domains are always + * discarded. */ typedef enum { - AUTH_INCLUDE_RESOURCE_GROUPS = 0, - AUTH_EXCLUDE_RESOURCE_GROUPS = 1 + AUTH_GROUP_INCLUSION_INVALID = 0, /* require invalid values to be handled. */ + AUTH_INCLUDE_RESOURCE_GROUPS = 2, + AUTH_INCLUDE_RESOURCE_GROUPS_COMPRESSED = 3, + AUTH_EXCLUDE_RESOURCE_GROUPS = 4 } auth_group_inclusion; typedef [public] struct { diff --git a/librpc/idl/netlogon.idl b/librpc/idl/netlogon.idl index e563e114900..c6231c41aee 100644 --- a/librpc/idl/netlogon.idl +++ b/librpc/idl/netlogon.idl @@ -20,6 +20,7 @@ cpp_quote("#define ENC_HMAC_SHA1_96_AES256_SK KERB_ENCTYPE_AES256_CTS_HMAC_SHA1_ cpp_quote("#define ENC_FAST_SUPPORTED KERB_ENCTYPE_FAST_SUPPORTED") cpp_quote("#define ENC_COMPOUND_IDENTITY_SUPPORTED KERB_ENCTYPE_COMPOUND_IDENTITY_SUPPORTED") cpp_quote("#define ENC_CLAIMS_SUPPORTED KERB_ENCTYPE_CLAIMS_SUPPORTED") +cpp_quote("#define ENC_RESOURCE_SID_COMPRESSION_DISABLED KERB_ENCTYPE_RESOURCE_SID_COMPRESSION_DISABLED") cpp_quote("#define NETLOGON_SERVER_PIPE_STATE_MAGIC 0x4f555358") [ diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index ecca1230ad7..99f687e3212 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -142,19 +142,3 @@ ^samba.tests.krb5.claims_tests.samba.tests.krb5.claims_tests.ClaimsTests.test_tgs_claims_remove_claims.ad_dc ^samba.tests.krb5.claims_tests.samba.tests.krb5.claims_tests.ClaimsTests.test_tgs_claims_remove_claims_to_krbtgt.ad_dc ^samba.tests.krb5.claims_tests.samba.tests.krb5.claims_tests.ClaimsTests.test_tgs_claims_to_krbtgt.ad_dc -# -# Group tests -# -^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_domain_local_Samba_4_17_compression_tgs_req_to_service.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_from_trust_compression_to_service.ad_dc -^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_from_trust_no_compression_to_service.ad_dc -^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_from_trust_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_group_removal_compression_tgs_req_to_service.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_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_service.ad_dc -^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_resource_sids_given_tgs_req_to_krbtgt.ad_dc -^samba.tests.krb5.group_tests.samba.tests.krb5.group_tests.GroupTests.test_group_resource_sids_wrongly_given_tgs_req_to_krbtgt.ad_dc diff --git a/source3/auth/auth_samba4.c b/source3/auth/auth_samba4.c index be6fd9a92a2..5039e56e30d 100644 --- a/source3/auth/auth_samba4.c +++ b/source3/auth/auth_samba4.c @@ -153,7 +153,8 @@ static NTSTATUS check_samba4_security( nt_status = auth_convert_user_info_dc_saminfo3(mem_ctx, user_info_dc, AUTH_INCLUDE_RESOURCE_GROUPS, - &info3); + &info3, + NULL); 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 */ talloc_steal(info3, user_info_dc); diff --git a/source4/auth/kerberos/kerberos.h b/source4/auth/kerberos/kerberos.h index 33ee4f301ed..cc61dccd820 100644 --- a/source4/auth/kerberos/kerberos.h +++ b/source4/auth/kerberos/kerberos.h @@ -24,6 +24,7 @@ #if defined(HAVE_KRB5) #include "system/kerberos.h" +#include "auth/auth.h" #include "auth/kerberos/krb5_init_context.h" #include "librpc/gen_ndr/krb5pac.h" #include "lib/krb5_wrap/krb5_samba.h" diff --git a/source4/auth/kerberos/kerberos_pac.c b/source4/auth/kerberos/kerberos_pac.c index bc389fd237e..0c0cf9090cf 100644 --- a/source4/auth/kerberos/kerberos_pac.c +++ b/source4/auth/kerberos/kerberos_pac.c @@ -249,7 +249,7 @@ } nt_status = auth_convert_user_info_dc_saminfo3(LOGON_INFO, user_info_dc, AUTH_INCLUDE_RESOURCE_GROUPS, - &sam3); + &sam3, NULL); if (!NT_STATUS_IS_OK(nt_status)) { DEBUG(1, ("Getting Samba info failed: %s\n", nt_errstr(nt_status))); talloc_free(pac_data); @@ -310,8 +310,10 @@ krb5_error_code kerberos_pac_to_user_info_dc(TALLOC_CTX *mem_ctx, krb5_const_pac pac, krb5_context context, struct auth_user_info_dc **user_info_dc, + const enum auth_group_inclusion group_inclusion, struct PAC_SIGNATURE_DATA *pac_srv_sig, - struct PAC_SIGNATURE_DATA *pac_kdc_sig) + struct PAC_SIGNATURE_DATA *pac_kdc_sig, + struct PAC_DOMAIN_GROUP_MEMBERSHIP **resource_groups) { NTSTATUS nt_status; enum ndr_err_code ndr_err; @@ -341,7 +343,11 @@ krb5_error_code kerberos_pac_to_user_info_dc(TALLOC_CTX *mem_ctx, pac_logon_info_in = data_blob_const(k5pac_logon_info_in.data, k5pac_logon_info_in.length); - ndr_err = ndr_pull_union_blob(&pac_logon_info_in, tmp_ctx, &info, + /* + * Allocate this structure on mem_ctx so we can return its resource + * groups to the caller. + */ + ndr_err = ndr_pull_union_blob(&pac_logon_info_in, mem_ctx, &info, PAC_TYPE_LOGON_INFO, (ndr_pull_flags_fn_t)ndr_pull_PAC_INFO); smb_krb5_free_data_contents(context, &k5pac_logon_info_in); @@ -391,6 +397,7 @@ krb5_error_code kerberos_pac_to_user_info_dc(TALLOC_CTX *mem_ctx, nt_status = make_user_info_dc_pac(mem_ctx, info.logon_info.info, upn_dns_info, + group_inclusion, &user_info_dc_out); if (!NT_STATUS_IS_OK(nt_status)) { DBG_ERR("make_user_info_dc_pac() failed -%s\n", @@ -445,6 +452,16 @@ krb5_error_code kerberos_pac_to_user_info_dc(TALLOC_CTX *mem_ctx, } } + /* + * If we have resource groups and the caller wants them returned, we + * oblige. + */ + if (resource_groups != NULL && + info.logon_info.info->resource_groups.groups.count != 0) + { + *resource_groups = &info.logon_info.info->resource_groups; + } + /* * Based on the presence of a REQUESTER_SID PAC buffer, ascertain * whether the ticket is a TGT. This helps the KDC and kpasswd service @@ -489,7 +506,14 @@ NTSTATUS kerberos_pac_blob_to_user_info_dc(TALLOC_CTX *mem_ctx, } - ret = kerberos_pac_to_user_info_dc(mem_ctx, pac, context, user_info_dc, pac_srv_sig, pac_kdc_sig); + ret = kerberos_pac_to_user_info_dc(mem_ctx, + pac, + context, + user_info_dc, + AUTH_INCLUDE_RESOURCE_GROUPS, + pac_srv_sig, + pac_kdc_sig, + NULL); krb5_pac_free(context, pac); if (ret) { return map_nt_error_from_unix_common(ret); diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index 457d7994bc4..7a048a6a418 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -1448,6 +1448,12 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, if (enable_fast) { supported_enctypes |= ENC_FAST_SUPPORTED; } + + /* + * Resource SID compression is enabled implicitly, unless + * disabled in msDS-SupportedEncryptionTypes. + */ + } else if (userAccountControl & (UF_PARTIAL_SECRETS_ACCOUNT|UF_SERVER_TRUST_ACCOUNT)) { /* * DCs and RODCs computer accounts take @@ -3347,6 +3353,8 @@ krb5_error_code samba_kdc_check_s4u2proxy_rbcd( header_pac, context, &user_info_dc, + AUTH_INCLUDE_RESOURCE_GROUPS, + NULL, NULL, NULL); if (code != 0) { diff --git a/source4/kdc/mit_samba.c b/source4/kdc/mit_samba.c index af9f6eecc3d..a5929f42bad 100644 --- a/source4/kdc/mit_samba.c +++ b/source4/kdc/mit_samba.c @@ -475,11 +475,10 @@ int mit_samba_get_pac(struct mit_samba_context *smb_ctx, NTSTATUS nt_status; krb5_error_code code; struct samba_kdc_entry *skdc_entry; + struct samba_kdc_entry *server_entry = NULL; 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 auth_group_inclusion group_inclusion; enum samba_asserted_identity asserted_identity = (flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION) ? SAMBA_ASSERTED_IDENTITY_SERVICE : @@ -488,6 +487,18 @@ int mit_samba_get_pac(struct mit_samba_context *smb_ctx, skdc_entry = talloc_get_type_abort(client->e_data, struct samba_kdc_entry); + server_entry = talloc_get_type_abort(server->e_data, + struct samba_kdc_entry); + + /* Only include resource groups in a service ticket. */ + if (is_krbtgt) { + group_inclusion = AUTH_EXCLUDE_RESOURCE_GROUPS; + } else if (server_entry->supported_enctypes & KERB_ENCTYPE_RESOURCE_SID_COMPRESSION_DISABLED) { + group_inclusion = AUTH_INCLUDE_RESOURCE_GROUPS; + } else { + group_inclusion = AUTH_INCLUDE_RESOURCE_GROUPS_COMPRESSED; + } + tmp_ctx = talloc_named(smb_ctx, 0, "mit_samba_get_pac context"); diff --git a/source4/kdc/pac-glue.c b/source4/kdc/pac-glue.c index bc5408fefb3..35e4bf4c248 100644 --- a/source4/kdc/pac-glue.c +++ b/source4/kdc/pac-glue.c @@ -49,11 +49,14 @@ 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, + const struct PAC_DOMAIN_GROUP_MEMBERSHIP *override_resource_groups, + const enum auth_group_inclusion group_inclusion, DATA_BLOB *pac_data, DATA_BLOB *requester_sid_blob) { - struct netr_SamInfo3 *info3; + struct netr_SamInfo3 *info3 = NULL; + struct PAC_DOMAIN_GROUP_MEMBERSHIP *_resource_groups = NULL; + struct PAC_DOMAIN_GROUP_MEMBERSHIP **resource_groups = NULL; union PAC_INFO pac_info; enum ndr_err_code ndr_err; NTSTATUS nt_status; @@ -65,9 +68,22 @@ NTSTATUS samba_get_logon_info_pac_blob(TALLOC_CTX *mem_ctx, *requester_sid_blob = data_blob_null; } + if (override_resource_groups == NULL) { + resource_groups = &_resource_groups; + } else if (group_inclusion != AUTH_EXCLUDE_RESOURCE_GROUPS) { + /* + * It doesn't make sense to override resource groups if we claim + * to want resource groups from user_info_dc. + */ + DBG_ERR("supplied resource groups with invalid group inclusion parameter: %u\n", + group_inclusion); + return NT_STATUS_INVALID_PARAMETER; + } + nt_status = auth_convert_user_info_dc_saminfo3(mem_ctx, info, group_inclusion, - &info3); + &info3, + resource_groups); if (!NT_STATUS_IS_OK(nt_status)) { DEBUG(1, ("Getting Samba info failed: %s\n", nt_errstr(nt_status))); @@ -80,6 +96,26 @@ NTSTATUS samba_get_logon_info_pac_blob(TALLOC_CTX *mem_ctx, } pac_info.logon_info.info->info3 = *info3; + if (_resource_groups != NULL) { + pac_info.logon_info.info->resource_groups = *_resource_groups; + } + + if (override_resource_groups != NULL) { + pac_info.logon_info.info->resource_groups = *override_resource_groups; + } + + if (group_inclusion != AUTH_EXCLUDE_RESOURCE_GROUPS) { + /* + * Set the resource groups flag based on whether any groups are + * present. Otherwise, the flag is propagated from the + * originating PAC. + */ + if (pac_info.logon_info.info->resource_groups.groups.count > 0) { + pac_info.logon_info.info->info3.base.user_flags |= NETLOGON_RESOURCE_GROUPS; + } else { + pac_info.logon_info.info->info3.base.user_flags &= ~NETLOGON_RESOURCE_GROUPS; + } + } ndr_err = ndr_push_union_blob(pac_data, mem_ctx, &pac_info, PAC_TYPE_LOGON_INFO, @@ -848,7 +884,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, + const enum auth_group_inclusion group_inclusion, DATA_BLOB **_logon_info_blob, DATA_BLOB **_cred_ndr_blob, DATA_BLOB **_upn_info_blob, @@ -944,6 +980,7 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx, nt_status = samba_get_logon_info_pac_blob(logon_blob, user_info_dc, + NULL, group_inclusion, logon_blob, requester_sid_blob); @@ -1005,7 +1042,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 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) @@ -1013,9 +1050,27 @@ NTSTATUS samba_kdc_update_pac_blob(TALLOC_CTX *mem_ctx, struct auth_user_info_dc *user_info_dc; krb5_error_code ret; NTSTATUS nt_status; + struct PAC_DOMAIN_GROUP_MEMBERSHIP *_resource_groups = NULL; + struct PAC_DOMAIN_GROUP_MEMBERSHIP **resource_groups = NULL; - ret = kerberos_pac_to_user_info_dc(mem_ctx, pac, - context, &user_info_dc, pac_srv_sig, pac_kdc_sig); + if (group_inclusion == AUTH_EXCLUDE_RESOURCE_GROUPS) { + /* + * Since we are creating a TGT, resource groups from our domain + * are not to be put into the PAC. Instead, we take the resource + * groups directly from the original PAC and copy them + * unmodified into the new one. + */ + resource_groups = &_resource_groups; + } + + ret = kerberos_pac_to_user_info_dc(mem_ctx, + pac, + context, + &user_info_dc, + AUTH_EXCLUDE_RESOURCE_GROUPS, + pac_srv_sig, + pac_kdc_sig, + resource_groups); if (ret) { return NT_STATUS_UNSUCCESSFUL; } @@ -1033,6 +1088,7 @@ NTSTATUS samba_kdc_update_pac_blob(TALLOC_CTX *mem_ctx, nt_status = samba_get_logon_info_pac_blob(mem_ctx, user_info_dc, + _resource_groups, group_inclusion, pac_blob, NULL); return nt_status; @@ -1257,6 +1313,8 @@ krb5_error_code samba_kdc_validate_pac_blob( pac, context, &pac_user_info, + AUTH_EXCLUDE_RESOURCE_GROUPS, + NULL, NULL, NULL); if (code != 0) { @@ -1463,9 +1521,13 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx, } /* Only include resource groups in a service ticket. */ - group_inclusion = (is_tgs) - ? AUTH_EXCLUDE_RESOURCE_GROUPS - : AUTH_INCLUDE_RESOURCE_GROUPS; + if (is_tgs) { + group_inclusion = AUTH_EXCLUDE_RESOURCE_GROUPS; + } else if (server->supported_enctypes & KERB_ENCTYPE_RESOURCE_SID_COMPRESSION_DISABLED) { + group_inclusion = AUTH_INCLUDE_RESOURCE_GROUPS; + } else { + group_inclusion = AUTH_INCLUDE_RESOURCE_GROUPS_COMPRESSED; + } if (client != NULL) { /* diff --git a/source4/kdc/wdc-samba4.c b/source4/kdc/wdc-samba4.c index abc86d6c8b4..6e3cbe7b312 100644 --- a/source4/kdc/wdc-samba4.c +++ b/source4/kdc/wdc-samba4.c @@ -143,12 +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); + const struct samba_kdc_entry *server_entry = + talloc_get_type_abort(server->context, + struct samba_kdc_entry); 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; + enum auth_group_inclusion group_inclusion; bool is_s4u2self = samba_wdc_is_s4u2self_req(r); enum samba_asserted_identity asserted_identity = (is_s4u2self) ? @@ -156,6 +156,15 @@ static krb5_error_code samba_wdc_get_pac(void *priv, SAMBA_ASSERTED_IDENTITY_AUTHENTICATION_AUTHORITY; PAC_OPTIONS_FLAGS pac_options = {}; + /* Only include resource groups in a service ticket. */ + if (is_krbtgt) { + group_inclusion = AUTH_EXCLUDE_RESOURCE_GROUPS; + } else if (server_entry->supported_enctypes & KERB_ENCTYPE_RESOURCE_SID_COMPRESSION_DISABLED) { + group_inclusion = AUTH_INCLUDE_RESOURCE_GROUPS; + } else { + group_inclusion = AUTH_INCLUDE_RESOURCE_GROUPS_COMPRESSED; + } + ret = samba_wdc_pac_options(r, &pac_options); if (ret != 0) { return ret; diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c index 7456422af74..b06b542791d 100644 --- a/source4/rpc_server/netlogon/dcerpc_netlogon.c +++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c @@ -1484,7 +1484,7 @@ static void dcesrv_netr_LogonSamLogon_base_auth_done(struct tevent_req *subreq) nt_status = auth_convert_user_info_dc_saminfo3(mem_ctx, user_info_dc, AUTH_INCLUDE_RESOURCE_GROUPS, - &sam3); + &sam3, NULL); if (!NT_STATUS_IS_OK(nt_status)) { r->out.result = nt_status; dcesrv_netr_LogonSamLogon_base_reply(state); @@ -1498,7 +1498,7 @@ static void dcesrv_netr_LogonSamLogon_base_auth_done(struct tevent_req *subreq) nt_status = auth_convert_user_info_dc_saminfo6(mem_ctx, user_info_dc, AUTH_INCLUDE_RESOURCE_GROUPS, - &sam6); + &sam6, NULL); if (!NT_STATUS_IS_OK(nt_status)) { r->out.result = nt_status; dcesrv_netr_LogonSamLogon_base_reply(state); diff --git a/source4/torture/winbind/winbind.c b/source4/torture/winbind/winbind.c index b201534fc2c..4acfd384112 100644 --- a/source4/torture/winbind/winbind.c +++ b/source4/torture/winbind/winbind.c @@ -105,6 +105,7 @@ static bool torture_decode_compare_pac(struct torture_context *tctx, struct PAC_LOGON_INFO *logon_info; struct netr_SamInfo3 *info3; struct netr_SamBaseInfo *base; + struct PAC_DOMAIN_GROUP_MEMBERSHIP *resource_groups = NULL; wbcErr wbc_err; NTSTATUS status; int sid_idx, i; @@ -125,6 +126,7 @@ static bool torture_decode_compare_pac(struct torture_context *tctx, torture_assert(tctx, NT_STATUS_IS_OK(status), "pac_logon_info"); info3 = &logon_info->info3; base = &info3->base; + resource_groups = &logon_info->resource_groups; /* Compare the decoded data from winbind and from internal call */ torture_assert(tctx, info->user_flags == base->user_flags, "user_flags"); @@ -140,7 +142,7 @@ static bool torture_decode_compare_pac(struct torture_context *tctx, torture_assert(tctx, info->pass_last_set_time == nt_time_to_unix(base->last_password_change), "last_password_change"); torture_assert(tctx, info->pass_can_change_time == nt_time_to_unix(base->allow_password_change), "allow_password_change"); torture_assert(tctx, info->pass_must_change_time == nt_time_to_unix(base->force_password_change), "force_password_change"); - torture_assert(tctx, info->num_sids == 2 + base->groups.count + info3->sidcount, "num_sids"); + torture_assert(tctx, info->num_sids == 2 + base->groups.count + info3->sidcount + resource_groups->groups.count, "num_sids"); sid_idx = 0; wbcSidToStringBuf(&info->sids[sid_idx].sid, sid_str, sizeof(sid_str)); @@ -177,6 +179,20 @@ static bool torture_decode_compare_pac(struct torture_context *tctx, "extra SID mismatch"); } + for (i = 0; i < resource_groups->groups.count; i++) { + sid_idx++; + wbcSidToStringBuf(&info->sids[sid_idx].sid, + sid_str, sizeof(sid_str)); + torture_assert_sid_equal(tctx, + dom_sid_parse_talloc(tctx, sid_str), + dom_sid_add_rid(tctx, resource_groups->domain_sid, + resource_groups->groups.rids[i].rid), + "resource SID mismatch"); + } + + sid_idx++; + torture_assert_int_equal(tctx, sid_idx, info->num_sids, "some SIDs still unaccounted for"); + return true; }