From 69cf2562cac1d6e2d4ff5bc4c412391d2f5b3f6f Mon Sep 17 00:00:00 2001 From: Isaac Boukris 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 Reviewed-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (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 89cc5ed3a8be9266c39503c6c1f3346096290d10 Mon Sep 17 00:00:00 2001 From: Isaac Boukris 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 Reviewed-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (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 906e1f43f8530fed9fd2c9c981adebbd3e630fac Mon Sep 17 00:00:00 2001 From: Isaac Boukris 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 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11362 Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Isaac Boukris Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett Autobuild-User(master): Stefan Metzmacher 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; inum_sids; i++) { + for (i=PRIMARY_GROUP_SID_INDEX; inum_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