The Samba-Bugzilla – Attachment 15286 Details for
Bug 11362
GPO security filtering based on the groups in Kerberos PAC (but primary group is missing)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patches for v4-10-test
tmp410.diff.txt (text/plain), 15.97 KB, created by
Stefan Metzmacher
on 2019-07-04 07:08:24 UTC
(
hide
)
Description:
Patches for v4-10-test
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2019-07-04 07:08:24 UTC
Size:
15.97 KB
patch
obsolete
>From e781fac9314db9ace4a014f601535f3f0383576b Mon Sep 17 00:00:00 2001 >From: Isaac Boukris <iboukris@gmail.com> >Date: Fri, 31 May 2019 20:02:30 +0300 >Subject: [PATCH 1/3] selftest: remote_pac: s/s2u4self/s4u2self/g > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=11362 > >Signed-off-by: Isaac Boukris <iboukris@gmail.com> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >(cherry picked from commit 60afe949c3e664f81c9b0db9c54f701aa2874a5e) >--- > source4/torture/rpc/remote_pac.c | 65 ++++++++++++++++---------------- > 1 file changed, 33 insertions(+), 32 deletions(-) > >diff --git a/source4/torture/rpc/remote_pac.c b/source4/torture/rpc/remote_pac.c >index ab10013356bc..35d4eab6f530 100644 >--- a/source4/torture/rpc/remote_pac.c >+++ b/source4/torture/rpc/remote_pac.c >@@ -39,8 +39,8 @@ > #define TEST_MACHINE_NAME_BDC "torturepacbdc" > #define TEST_MACHINE_NAME_WKSTA "torturepacwksta" > #define TEST_MACHINE_NAME_WKSTA_DES "torturepacwkdes" >-#define TEST_MACHINE_NAME_S2U4SELF_BDC "tests2u4selfbdc" >-#define TEST_MACHINE_NAME_S2U4SELF_WKSTA "tests2u4selfwk" >+#define TEST_MACHINE_NAME_S4U2SELF_BDC "tests4u2selfbdc" >+#define TEST_MACHINE_NAME_S4U2SELF_WKSTA "tests4u2selfwk" > > struct pac_data { > DATA_BLOB pac_blob; >@@ -616,9 +616,10 @@ static bool test_PACVerify_workstation_des(struct torture_context *tctx, > } > > >-/* Check various ways to get the PAC, in particular check the group membership and other details between the PAC from a normal kinit, S2U4Self and a SamLogon */ >+/* Check various ways to get the PAC, in particular check the group membership and >+ * other details between the PAC from a normal kinit, S4U2Self and a SamLogon */ > #ifdef SAMBA4_USES_HEIMDAL >-static bool test_S2U4Self(struct torture_context *tctx, >+static bool test_S4U2Self(struct torture_context *tctx, > struct dcerpc_pipe *p1, > struct cli_credentials *credentials, > enum netr_SchannelType secure_channel_type, >@@ -647,7 +648,7 @@ static bool test_S2U4Self(struct torture_context *tctx, > > struct auth4_context *auth_context; > struct auth_session_info *kinit_session_info; >- struct auth_session_info *s2u4self_session_info; >+ struct auth_session_info *s4u2self_session_info; > struct auth_user_info_dc *netlogon_user_info_dc; > > struct netr_NetworkInfo ninfo; >@@ -745,7 +746,7 @@ static bool test_S2U4Self(struct torture_context *tctx, > torture_assert_ntstatus_ok(tctx, status, "gensec_session_info failed"); > > >- /* Now do the dance with S2U4Self */ >+ /* Now do the dance with S4U2Self */ > > /* Wipe out any existing ccache */ > cli_credentials_invalidate_ccache(client_creds, CRED_SPECIFIED); >@@ -804,7 +805,7 @@ static bool test_S2U4Self(struct torture_context *tctx, > > /* Extract the PAC using Samba's code */ > >- status = gensec_session_info(gensec_server_context, gensec_server_context, &s2u4self_session_info); >+ status = gensec_session_info(gensec_server_context, gensec_server_context, &s4u2self_session_info); > torture_assert_ntstatus_ok(tctx, status, "gensec_session_info failed"); > > cli_credentials_get_ntlm_username_domain(client_creds, tctx, >@@ -877,18 +878,18 @@ static bool test_S2U4Self(struct torture_context *tctx, > torture_assert_str_equal(tctx, netlogon_user_info_dc->info->account_name == NULL ? "" : netlogon_user_info_dc->info->account_name, > kinit_session_info->info->account_name, "Account name differs for kinit-based PAC"); > torture_assert_str_equal(tctx,netlogon_user_info_dc->info->account_name == NULL ? "" : netlogon_user_info_dc->info->account_name, >- s2u4self_session_info->info->account_name, "Account name differs for S2U4Self"); >+ s4u2self_session_info->info->account_name, "Account name differs for S4U2Self"); > torture_assert_str_equal(tctx, netlogon_user_info_dc->info->full_name == NULL ? "" : netlogon_user_info_dc->info->full_name, kinit_session_info->info->full_name, "Full name differs for kinit-based PAC"); >- torture_assert_str_equal(tctx, netlogon_user_info_dc->info->full_name == NULL ? "" : netlogon_user_info_dc->info->full_name, s2u4self_session_info->info->full_name, "Full name differs for S2U4Self"); >+ torture_assert_str_equal(tctx, netlogon_user_info_dc->info->full_name == NULL ? "" : netlogon_user_info_dc->info->full_name, s4u2self_session_info->info->full_name, "Full name differs for S4U2Self"); > torture_assert_int_equal(tctx, netlogon_user_info_dc->num_sids, kinit_session_info->torture->num_dc_sids, "Different numbers of domain groups for kinit-based PAC"); >- torture_assert_int_equal(tctx, netlogon_user_info_dc->num_sids, s2u4self_session_info->torture->num_dc_sids, "Different numbers of domain groups for S2U4Self"); >+ torture_assert_int_equal(tctx, netlogon_user_info_dc->num_sids, s4u2self_session_info->torture->num_dc_sids, "Different numbers of domain groups for S4U2Self"); > > builtin_domain = dom_sid_parse_talloc(tmp_ctx, SID_BUILTIN); > > for (i = 0; i < kinit_session_info->torture->num_dc_sids; i++) { > torture_assert(tctx, dom_sid_equal(&netlogon_user_info_dc->sids[i], &kinit_session_info->torture->dc_sids[i]), "Different domain groups for kinit-based PAC"); >- torture_assert(tctx, dom_sid_equal(&netlogon_user_info_dc->sids[i], &s2u4self_session_info->torture->dc_sids[i]), "Different domain groups for S2U4Self"); >- torture_assert(tctx, !dom_sid_in_domain(builtin_domain, &s2u4self_session_info->torture->dc_sids[i]), "Returned BUILTIN domain in groups for S2U4Self"); >+ torture_assert(tctx, dom_sid_equal(&netlogon_user_info_dc->sids[i], &s4u2self_session_info->torture->dc_sids[i]), "Different domain groups for S4U2Self"); >+ torture_assert(tctx, !dom_sid_in_domain(builtin_domain, &s4u2self_session_info->torture->dc_sids[i]), "Returned BUILTIN domain in groups for S4U2Self"); > torture_assert(tctx, !dom_sid_in_domain(builtin_domain, &kinit_session_info->torture->dc_sids[i]), "Returned BUILTIN domain in groups kinit-based PAC"); > torture_assert(tctx, !dom_sid_in_domain(builtin_domain, &netlogon_user_info_dc->sids[i]), "Returned BUILTIN domian in groups from NETLOGON SamLogon reply"); > } >@@ -896,39 +897,39 @@ static bool test_S2U4Self(struct torture_context *tctx, > return true; > } > >-static bool test_S2U4Self_bdc_arcfour(struct torture_context *tctx, >+static bool test_S4U2Self_bdc_arcfour(struct torture_context *tctx, > struct dcerpc_pipe *p, > struct cli_credentials *credentials) > { >- return test_S2U4Self(tctx, p, credentials, SEC_CHAN_BDC, >- TEST_MACHINE_NAME_S2U4SELF_BDC, >+ return test_S4U2Self(tctx, p, credentials, SEC_CHAN_BDC, >+ TEST_MACHINE_NAME_S4U2SELF_BDC, > NETLOGON_NEG_AUTH2_ADS_FLAGS); > } > >-static bool test_S2U4Self_bdc_aes(struct torture_context *tctx, >+static bool test_S4U2Self_bdc_aes(struct torture_context *tctx, > struct dcerpc_pipe *p, > struct cli_credentials *credentials) > { >- return test_S2U4Self(tctx, p, credentials, SEC_CHAN_BDC, >- TEST_MACHINE_NAME_S2U4SELF_BDC, >+ return test_S4U2Self(tctx, p, credentials, SEC_CHAN_BDC, >+ TEST_MACHINE_NAME_S4U2SELF_BDC, > NETLOGON_NEG_AUTH2_ADS_FLAGS | NETLOGON_NEG_SUPPORTS_AES); > } > >-static bool test_S2U4Self_workstation_arcfour(struct torture_context *tctx, >+static bool test_S4U2Self_workstation_arcfour(struct torture_context *tctx, > struct dcerpc_pipe *p, > struct cli_credentials *credentials) > { >- return test_S2U4Self(tctx, p, credentials, SEC_CHAN_WKSTA, >- TEST_MACHINE_NAME_S2U4SELF_WKSTA, >+ return test_S4U2Self(tctx, p, credentials, SEC_CHAN_WKSTA, >+ TEST_MACHINE_NAME_S4U2SELF_WKSTA, > NETLOGON_NEG_AUTH2_ADS_FLAGS); > } > >-static bool test_S2U4Self_workstation_aes(struct torture_context *tctx, >+static bool test_S4U2Self_workstation_aes(struct torture_context *tctx, > struct dcerpc_pipe *p, > struct cli_credentials *credentials) > { >- return test_S2U4Self(tctx, p, credentials, SEC_CHAN_WKSTA, >- TEST_MACHINE_NAME_S2U4SELF_WKSTA, >+ return test_S4U2Self(tctx, p, credentials, SEC_CHAN_WKSTA, >+ TEST_MACHINE_NAME_S4U2SELF_WKSTA, > NETLOGON_NEG_AUTH2_ADS_FLAGS | NETLOGON_NEG_SUPPORTS_AES); > } > #endif >@@ -959,20 +960,20 @@ struct torture_suite *torture_rpc_remote_pac(TALLOC_CTX *mem_ctx) > torture_rpc_tcase_add_test_join(tcase, "verify-sig", test_PACVerify_workstation_des); > #ifdef SAMBA4_USES_HEIMDAL > tcase = torture_suite_add_machine_bdc_rpc_iface_tcase(suite, "netr-bdc-arcfour", >- &ndr_table_netlogon, TEST_MACHINE_NAME_S2U4SELF_BDC); >- torture_rpc_tcase_add_test_creds(tcase, "s2u4self-arcfour", test_S2U4Self_bdc_arcfour); >+ &ndr_table_netlogon, TEST_MACHINE_NAME_S4U2SELF_BDC); >+ torture_rpc_tcase_add_test_creds(tcase, "s4u2self-arcfour", test_S4U2Self_bdc_arcfour); > > tcase = torture_suite_add_machine_bdc_rpc_iface_tcase(suite, "netr-bcd-aes", >- &ndr_table_netlogon, TEST_MACHINE_NAME_S2U4SELF_BDC); >- torture_rpc_tcase_add_test_creds(tcase, "s2u4self-aes", test_S2U4Self_bdc_aes); >+ &ndr_table_netlogon, TEST_MACHINE_NAME_S4U2SELF_BDC); >+ torture_rpc_tcase_add_test_creds(tcase, "s4u2self-aes", test_S4U2Self_bdc_aes); > > tcase = torture_suite_add_machine_workstation_rpc_iface_tcase(suite, "netr-mem-arcfour", >- &ndr_table_netlogon, TEST_MACHINE_NAME_S2U4SELF_WKSTA); >- torture_rpc_tcase_add_test_creds(tcase, "s2u4self-arcfour", test_S2U4Self_workstation_arcfour); >+ &ndr_table_netlogon, TEST_MACHINE_NAME_S4U2SELF_WKSTA); >+ torture_rpc_tcase_add_test_creds(tcase, "s4u2self-arcfour", test_S4U2Self_workstation_arcfour); > > tcase = torture_suite_add_machine_workstation_rpc_iface_tcase(suite, "netr-mem-aes", >- &ndr_table_netlogon, TEST_MACHINE_NAME_S2U4SELF_WKSTA); >- torture_rpc_tcase_add_test_creds(tcase, "s2u4self-aes", test_S2U4Self_workstation_aes); >+ &ndr_table_netlogon, TEST_MACHINE_NAME_S4U2SELF_WKSTA); >+ torture_rpc_tcase_add_test_creds(tcase, "s4u2self-aes", test_S4U2Self_workstation_aes); > #endif > return suite; > } >-- >2.17.1 > > >From d51992bb673ea89cc60566000a219d393257c441 Mon Sep 17 00:00:00 2001 >From: Isaac Boukris <iboukris@gmail.com> >Date: Fri, 31 May 2019 17:22:50 +0300 >Subject: [PATCH 2/3] selftest: check for PrimaryGroupId in DC returned group > array > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=11362 > >Signed-off-by: Isaac Boukris <iboukris@gmail.com> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >(cherry picked from commit 3700998419738caa1ca8672fbf5dbaccaaa498fa) >--- > selftest/knownfail.d/pac_primary_group | 1 + > source4/torture/rpc/remote_pac.c | 49 +++++++++++++++++++++++++- > 2 files changed, 49 insertions(+), 1 deletion(-) > create mode 100644 selftest/knownfail.d/pac_primary_group > >diff --git a/selftest/knownfail.d/pac_primary_group b/selftest/knownfail.d/pac_primary_group >new file mode 100644 >index 000000000000..b0efd7d63857 >--- /dev/null >+++ b/selftest/knownfail.d/pac_primary_group >@@ -0,0 +1 @@ >+^samba4.rpc.pac.*s4u2self >diff --git a/source4/torture/rpc/remote_pac.c b/source4/torture/rpc/remote_pac.c >index 35d4eab6f530..3ada0704612c 100644 >--- a/source4/torture/rpc/remote_pac.c >+++ b/source4/torture/rpc/remote_pac.c >@@ -615,10 +615,46 @@ static bool test_PACVerify_workstation_des(struct torture_context *tctx, > NETLOGON_NEG_AUTH2_ADS_FLAGS); > } > >+#ifdef SAMBA4_USES_HEIMDAL >+static NTSTATUS check_primary_group_in_validation(TALLOC_CTX *mem_ctx, >+ uint16_t validation_level, >+ const union netr_Validation *validation) >+{ >+ const struct netr_SamBaseInfo *base = NULL; >+ int i; >+ switch (validation_level) { >+ case 2: >+ if (!validation || !validation->sam2) { >+ return NT_STATUS_INVALID_PARAMETER; >+ } >+ base = &validation->sam2->base; >+ break; >+ case 3: >+ if (!validation || !validation->sam3) { >+ return NT_STATUS_INVALID_PARAMETER; >+ } >+ base = &validation->sam3->base; >+ break; >+ case 6: >+ if (!validation || !validation->sam6) { >+ return NT_STATUS_INVALID_PARAMETER; >+ } >+ base = &validation->sam6->base; >+ break; >+ default: >+ return NT_STATUS_INVALID_LEVEL; >+ } >+ >+ for (i = 0; i < base->groups.count; i++) { >+ if (base->groups.rids[i].rid == base->primary_gid) { >+ return NT_STATUS_OK; >+ } >+ } >+ return NT_STATUS_INVALID_PARAMETER; >+} > > /* Check various ways to get the PAC, in particular check the group membership and > * other details between the PAC from a normal kinit, S4U2Self and a SamLogon */ >-#ifdef SAMBA4_USES_HEIMDAL > static bool test_S4U2Self(struct torture_context *tctx, > struct dcerpc_pipe *p1, > struct cli_credentials *credentials, >@@ -875,6 +911,17 @@ static bool test_S4U2Self(struct torture_context *tctx, > > torture_assert_ntstatus_ok(tctx, status, "make_user_info_dc_netlogon_validation failed"); > >+ /* Check that the primary group is present in validation's RID array */ >+ status = check_primary_group_in_validation(tmp_ctx, r.in.validation_level, r.out.validation); >+ torture_assert_ntstatus_ok(tctx, status, "check_primary_group_in_validation failed"); >+ >+ /* Check that the primary group is not duplicated in user_info_dc SID array */ >+ for (i = 2; i < netlogon_user_info_dc->num_sids; i++) { >+ torture_assert(tctx, !dom_sid_equal(&netlogon_user_info_dc->sids[1], >+ &netlogon_user_info_dc->sids[i]), >+ "Duplicate PrimaryGroupId in return SID array"); >+ } >+ > torture_assert_str_equal(tctx, netlogon_user_info_dc->info->account_name == NULL ? "" : netlogon_user_info_dc->info->account_name, > kinit_session_info->info->account_name, "Account name differs for kinit-based PAC"); > torture_assert_str_equal(tctx,netlogon_user_info_dc->info->account_name == NULL ? "" : netlogon_user_info_dc->info->account_name, >-- >2.17.1 > > >From ff4b1195cc524798c71cd58e45cf40291f7f2c39 Mon Sep 17 00:00:00 2001 >From: Isaac Boukris <iboukris@gmail.com> >Date: Wed, 3 Apr 2019 19:45:02 +0300 >Subject: [PATCH 3/3] Add PrimaryGroupId to group array in DC response > >This is a simplified version of the original patch by: >Felix Botner <botner@univention.de> > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=11362 > >Pair-Programmed-With: Stefan Metzmacher <metze@samba.org> > >Signed-off-by: Isaac Boukris <iboukris@gmail.com> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> > >Autobuild-User(master): Stefan Metzmacher <metze@samba.org> >Autobuild-Date(master): Wed Jul 3 13:52:55 UTC 2019 on sn-devel-184 > >(cherry picked from commit 2ae75184fcb5dc90602aeef113d4c13540073324) >--- > auth/auth_sam_reply.c | 8 ++++++-- > selftest/knownfail.d/pac_primary_group | 1 - > 2 files changed, 6 insertions(+), 3 deletions(-) > delete mode 100644 selftest/knownfail.d/pac_primary_group > >diff --git a/auth/auth_sam_reply.c b/auth/auth_sam_reply.c >index bd695151dc0d..b5b6362dc93b 100644 >--- a/auth/auth_sam_reply.c >+++ b/auth/auth_sam_reply.c >@@ -89,7 +89,7 @@ static NTSTATUS auth_convert_user_info_dc_sambaseinfo(TALLOC_CTX *mem_ctx, > sam->groups.count = 0; > sam->groups.rids = NULL; > >- if (user_info_dc->num_sids > 2) { >+ if (user_info_dc->num_sids > PRIMARY_GROUP_SID_INDEX) { > size_t i; > sam->groups.rids = talloc_array(mem_ctx, struct samr_RidWithAttribute, > user_info_dc->num_sids); >@@ -97,7 +97,7 @@ static NTSTATUS auth_convert_user_info_dc_sambaseinfo(TALLOC_CTX *mem_ctx, > if (sam->groups.rids == NULL) > return NT_STATUS_NO_MEMORY; > >- for (i=2; i<user_info_dc->num_sids; i++) { >+ for (i=PRIMARY_GROUP_SID_INDEX; i<user_info_dc->num_sids; i++) { > struct dom_sid *group_sid = &user_info_dc->sids[i]; > if (!dom_sid_in_domain(sam->domain_sid, group_sid)) { > /* We handle this elsewhere */ >@@ -451,6 +451,10 @@ NTSTATUS make_user_info_dc_netlogon_validation(TALLOC_CTX *mem_ctx, > } > > for (i = 0; i < base->groups.count; i++) { >+ /* Skip primary group, already added above */ >+ if (base->groups.rids[i].rid == base->primary_gid) { >+ continue; >+ } > user_info_dc->sids[user_info_dc->num_sids] = *base->domain_sid; > if (!sid_append_rid(&user_info_dc->sids[user_info_dc->num_sids], base->groups.rids[i].rid)) { > return NT_STATUS_INVALID_PARAMETER; >diff --git a/selftest/knownfail.d/pac_primary_group b/selftest/knownfail.d/pac_primary_group >deleted file mode 100644 >index b0efd7d63857..000000000000 >--- a/selftest/knownfail.d/pac_primary_group >+++ /dev/null >@@ -1 +0,0 @@ >-^samba4.rpc.pac.*s4u2self >-- >2.17.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=15286">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
abartlet
:
review+
asn
:
review+
Actions:
View
Attachments on
bug 11362
:
11198
|
11199
|
11200
|
15285
| 15286