From: Joseph Sutton Date: Tue, 27 Sep 2022 01:51:54 +0000 (+1300) Subject: auth: Exclude resource groups from a TGT X-Git-Tag: talloc-2.4.1~1677 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=94cda2dfd58a4f3d3e0011b67fa0be7d11570cb6;p=thirdparty%2Fsamba.git auth: Exclude resource groups from a TGT 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 Reviewed-by: Andrew Bartlett --- diff --git a/auth/auth_sam_reply.c b/auth/auth_sam_reply.c index 552834a1bb0..8a68f045547 100644 --- a/auth/auth_sam_reply.c +++ b/auth/auth_sam_reply.c @@ -99,6 +99,13 @@ 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)) { /* 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; inum_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; diff --git a/auth/auth_sam_reply.h b/auth/auth_sam_reply.h index d8a30c6b36f..4eebf0b06e3 100644 --- a/auth/auth_sam_reply.h +++ b/auth/auth_sam_reply.h @@ -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); /** diff --git a/librpc/idl/auth.idl b/librpc/idl/auth.idl index 5985d554606..582587e062f 100644 --- a/librpc/idl/auth.idl +++ b/librpc/idl/auth.idl @@ -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; diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index 4a55b0709ea..f64e4c30e19 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -145,22 +145,11 @@ # # 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 diff --git a/source3/auth/auth_samba4.c b/source3/auth/auth_samba4.c index dec67a488d7..be6fd9a92a2 100644 --- a/source3/auth/auth_samba4.c +++ b/source3/auth/auth_samba4.c @@ -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 */ diff --git a/source4/auth/kerberos/kerberos_pac.c b/source4/auth/kerberos/kerberos_pac.c index 880211bdea2..bc389fd237e 100644 --- a/source4/auth/kerberos/kerberos_pac.c +++ b/source4/auth/kerberos/kerberos_pac.c @@ -247,7 +247,9 @@ 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); diff --git a/source4/kdc/mit_samba.c b/source4/kdc/mit_samba.c index 8df3ad36228..af9f6eecc3d 100644 --- a/source4/kdc/mit_samba.c +++ b/source4/kdc/mit_samba.c @@ -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, diff --git a/source4/kdc/pac-glue.c b/source4/kdc/pac-glue.c index e1a44bf8e1f..395bd7c0d56 100644 --- a/source4/kdc/pac-glue.c +++ b/source4/kdc/pac-glue.c @@ -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 diff --git a/source4/kdc/pac-glue.h b/source4/kdc/pac-glue.h index 046264cca12..6a1de6dda17 100644 --- a/source4/kdc/pac-glue.h +++ b/source4/kdc/pac-glue.h @@ -21,6 +21,8 @@ along with this program. If not, see . */ +#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); diff --git a/source4/kdc/wdc-samba4.c b/source4/kdc/wdc-samba4.c index f3ca04550b0..abc86d6c8b4 100644 --- a/source4/kdc/wdc-samba4.c +++ b/source4/kdc/wdc-samba4.c @@ -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, diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c index 314b469a718..7456422af74 100644 --- a/source4/rpc_server/netlogon/dcerpc_netlogon.c +++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c @@ -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;