----- Original Message -----
> On Fri, May 23, 2014 at 12:29:37PM -0400, Yassir Elley wrote:
> >
> >
> > ----- Original Message -----
> > > On Tue, May 20, 2014 at 10:30:15PM -0400, Yassir Elley wrote:
> > > > Hi,
> > > >
> > > > The attached patch adds unit tests for some of the ad_gpo
> > > > functionality. It
> > > > also fixes some failure mode processing code in ad_gpo.c that was
> > > > uncovered by the unit tests. This patch depends on a previously
> > > > submitted
> > > > patch ("Remove GPO dependency on libsamba-security"), which has not yet
> > > > been pushed to master.
> > > >
> > > > Regards,
> > > > Yassir.
> > >
> > > Can you send a rebased version on top of the latest version of your
> > > 'AD-GPO: Remove dependency on libsamba-security' patch?
> > >
> > > bye,
> > > Sumit
> > > _______________________________________________
> > > sssd-devel mailing list
> > > [email protected]
> > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >
> >
> > Rebased version of patch attached.
>
> I'm sorry, but the test does not compile on top of you other patches. I
> think it is due to adding the idmap context:
>
> CC src/tests/cmocka/ad_gpo_tests-test_ad_gpo.o
> ../src/tests/cmocka/test_ad_gpo.c: In function
> 'test_ad_gpo_ace_includes_client_sid':
> ../src/tests/cmocka/test_ad_gpo.c:269:42: warning: passing argument 5 of
> 'ad_gpo_ace_includes_client_sid' from incompatible pointer type [enabled by
> default]
> ace_dom_sid, &includes_client_sid);
> ^
> In file included from ../src/tests/cmocka/test_ad_gpo.c:33:0:
> ../src/providers/ad/ad_gpo.c:253:1: note: expected 'struct sss_idmap_ctx *'
> but argument is of type '_Bool *'
> ad_gpo_ace_includes_client_sid(const char *user_sid,
> ^
> ../src/tests/cmocka/test_ad_gpo.c:269:42: error: too few arguments to
> function 'ad_gpo_ace_includes_client_sid'
> ace_dom_sid, &includes_client_sid);
> ^
> In file included from ../src/tests/cmocka/test_ad_gpo.c:33:0:
> ../src/providers/ad/ad_gpo.c:253:1: note: declared here
> ad_gpo_ace_includes_client_sid(const char *user_sid,
> ^
> make[3]: *** [src/tests/cmocka/ad_gpo_tests-test_ad_gpo.o] Error 1
>
>
> bye,
> Sumit
>
That was very sloppy of me. Sorry about that. I've attached a revised patch.
Regards,
Yassir.
From faac5e4aba3e92a5e6f4cf01842b5754534d9b3d Mon Sep 17 00:00:00 2001
From: Yassir Elley <[email protected]>
Date: Thu, 3 Apr 2014 11:21:43 -0400
Subject: [PATCH] AD-GPO: Add ad_gpo unit test; Fix some failure modes in
ad_gpo.c
---
Makefile.am | 29 ++++
src/providers/ad/ad_gpo.c | 26 ++-
src/tests/cmocka/test_ad_gpo.c | 378 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 424 insertions(+), 9 deletions(-)
create mode 100644 src/tests/cmocka/test_ad_gpo.c
diff --git a/Makefile.am b/Makefile.am
index 510cfc78ba59f08fc73ed0f971e77ce6f2288461..134163fee47a923808b866daed92453b147fd79d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -177,6 +177,7 @@ if HAVE_CMOCKA
test_sss_idmap \
test_ipa_idmap \
test_utils \
+ ad_gpo_tests \
ad_common_tests \
dp_opt_tests \
responder-get-domains-tests \
@@ -1723,6 +1724,34 @@ ad_access_filter_tests_LDADD = \
libsss_ad_common.la \
libsss_test_common.la
+ad_gpo_tests_SOURCES = \
+ $(sssd_be_SOURCES) \
+ src/util/sss_ldap.c \
+ src/util/sss_krb5.c \
+ src/util/find_uid.c \
+ src/util/user_info_msg.c \
+ src/providers/ad/ad_common.c \
+ src/tests/cmocka/test_ad_gpo.c
+ad_gpo_tests_CFLAGS = \
+ $(AM_CFLAGS) \
+ $(SYSTEMD_LOGIN_CFLAGS) \
+ $(NDR_NBT_CFLAGS) \
+ -DUNIT_TESTING
+ad_gpo_tests_LDADD = \
+ $(PAM_LIBS) \
+ $(CMOCKA_LIBS) \
+ $(SSSD_LIBS) \
+ $(CARES_LIBS) \
+ $(KRB5_LIBS) \
+ $(SSSD_INTERNAL_LTLIBS) \
+ $(SYSTEMD_LOGIN_LIBS) \
+ $(NDR_NBT_LIBS) \
+ libsss_ldap_common.la \
+ libsss_idmap.la \
+ libsss_krb5_common.la \
+ libsss_ad_common.la \
+ libsss_test_common.la
+
ad_common_tests_SOURCES = \
$(sssd_be_SOURCES) \
src/tests/cmocka/test_ad_common.c
diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
index 168ed2d7e8657799a7c773ec8b24897cdd303ec4..85b024bd005ae6034c97522f392bf7ff99f48750 100644
--- a/src/providers/ad/ad_gpo.c
+++ b/src/providers/ad/ad_gpo.c
@@ -246,26 +246,23 @@ ad_gpo_get_sids(TALLOC_CTX *mem_ctx,
}
/*
- * This function determines whether the input ACE includes any of the
+ * This function determines whether the input ace_dom_sid matches any of the
* client's SIDs. The boolean result is assigned to the _included output param.
*/
static errno_t
ad_gpo_ace_includes_client_sid(const char *user_sid,
const char **group_sids,
int group_size,
- struct security_ace *ace,
+ struct dom_sid ace_dom_sid,
struct sss_idmap_ctx *idmap_ctx,
bool *_included)
{
int i = 0;
- struct dom_sid ace_dom_sid;
struct dom_sid *user_dom_sid;
struct dom_sid *group_dom_sid;
enum idmap_error_code err;
bool included = false;
- ace_dom_sid = ace->trustee;
-
err = sss_idmap_sid_to_smb_sid(idmap_ctx, user_sid, &user_dom_sid);
if (err != IDMAP_SUCCESS) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to initialize idmap context.\n");
@@ -341,8 +338,8 @@ static enum ace_eval_status ad_gpo_evaluate_ace(struct security_ace *ace,
return AD_GPO_ACE_NEUTRAL;
}
- ret = ad_gpo_ace_includes_client_sid(user_sid, group_sids, group_size, ace,
- idmap_ctx, &included);
+ ret = ad_gpo_ace_includes_client_sid(user_sid, group_sids, group_size,
+ ace->trustee, idmap_ctx, &included);
if (ret != EOK) {
return AD_GPO_ACE_DENIED;
}
@@ -987,7 +984,16 @@ ad_gpo_populate_som_list(TALLOC_CTX *mem_ctx,
}
ldb_target_dn = ldb_dn_new(tmp_ctx, ldb_ctx, target_dn);
+ if (ldb_target_dn == NULL) {
+ ret = EINVAL;
+ goto done;
+ }
+
rdn_count = ldb_dn_get_comp_num(ldb_target_dn);
+ if (rdn_count == -1) {
+ ret = EINVAL;
+ goto done;
+ }
if (rdn_count == 0) {
*_som_list = NULL;
@@ -1120,7 +1126,8 @@ ad_gpo_populate_gplink_list(TALLOC_CTX *mem_ctx,
first = ptr + 1;
last = strchr(first, delim);
if (last == NULL) {
- break;
+ ret = EINVAL;
+ goto done;
}
*last = '\0';
last++;
@@ -1130,7 +1137,8 @@ ad_gpo_populate_gplink_list(TALLOC_CTX *mem_ctx,
}
gplink_options = strchr(first, ';');
if (gplink_options == NULL) {
- continue;
+ ret = EINVAL;
+ goto done;
}
*gplink_options = '\0';
gplink_options++;
diff --git a/src/tests/cmocka/test_ad_gpo.c b/src/tests/cmocka/test_ad_gpo.c
new file mode 100644
index 0000000000000000000000000000000000000000..acd3d22e672cd0f4994f700d39f88a5a5308495f
--- /dev/null
+++ b/src/tests/cmocka/test_ad_gpo.c
@@ -0,0 +1,378 @@
+/*
+ Authors:
+ Yassir Elley <[email protected]>
+
+ Copyright (C) 2014 Red Hat
+
+ SSSD tests: GPO unit tests
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <talloc.h>
+#include <tevent.h>
+#include <errno.h>
+#include <popt.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <ifaddrs.h>
+#include <arpa/inet.h>
+
+/* In order to access opaque types */
+#include "providers/ad/ad_gpo.c"
+
+#include "tests/cmocka/common_mock.h"
+
+struct ad_gpo_test_ctx {
+ struct ldb_context *ldb_ctx;
+};
+
+static struct ad_gpo_test_ctx *test_ctx;
+
+void ad_gpo_test_setup(void **state)
+{
+ assert_true(leak_check_setup());
+ test_ctx = talloc_zero(global_talloc_context,
+ struct ad_gpo_test_ctx);
+ assert_non_null(test_ctx);
+
+ test_ctx->ldb_ctx = ldb_init(test_ctx, NULL);
+ assert_non_null(test_ctx->ldb_ctx);
+}
+
+void ad_gpo_test_teardown(void **state)
+{
+ talloc_free(test_ctx);
+ assert_true(leak_check_teardown());
+}
+
+struct som_list_result {
+ const int result;
+ const int num_soms;
+ const char **som_dns;
+};
+
+/*
+ * Test parsing target dn into som components
+ */
+static void test_populate_som_list(const char *target_dn,
+ struct som_list_result *expected)
+{
+ errno_t ret;
+ int i;
+ int num_soms;
+ struct gp_som **som_list = NULL;
+ TALLOC_CTX *tmp_ctx;
+
+ tmp_ctx = talloc_new(global_talloc_context);
+ assert_non_null(tmp_ctx);
+ check_leaks_push(tmp_ctx);
+
+ ret = ad_gpo_populate_som_list(tmp_ctx,
+ test_ctx->ldb_ctx,
+ target_dn,
+ &num_soms,
+ &som_list);
+
+ assert_int_equal(ret, expected->result);
+ assert_int_equal(num_soms, expected->num_soms);
+
+ for (i=0; i<expected->num_soms; i++){
+ bool equal = true;
+ if (strncmp(som_list[i]->som_dn,
+ expected->som_dns[i],
+ strlen(expected->som_dns[i])) != 0) {
+ equal = false;
+ }
+
+ assert_int_equal(equal, true);
+ }
+
+ if (som_list) {
+ talloc_free(som_list);
+ }
+
+ assert_true(check_leaks_pop(tmp_ctx) == true);
+ talloc_free(tmp_ctx);
+}
+
+void test_populate_som_list_plain(void **state)
+{
+ const char *som_dns[] = {"OU=West OU,OU=Sales OU,DC=foo,DC=com",
+ "OU=Sales OU,DC=foo,DC=com",
+ "DC=foo,DC=com"};
+
+ struct som_list_result expected = {
+ .result = EOK,
+ .num_soms = 3,
+ .som_dns = som_dns
+ };
+
+ test_populate_som_list("CN=F21-Client,OU=West OU,OU=Sales OU,DC=foo,DC=com",
+ &expected);
+}
+
+void test_populate_som_list_malformed(void **state)
+{
+ struct som_list_result expected = {
+ .result = EINVAL,
+ };
+
+ test_populate_som_list("malformed target dn", &expected);
+}
+
+struct gplink_list_result {
+ const int result;
+ const int num_gplinks;
+ const char **gpo_dns;
+ bool *enforced;
+};
+
+/*
+ * Test parsing raw_gplink_value into gplink components
+ */
+static void test_populate_gplink_list(const char *input_gplink_value,
+ bool allow_enforced_only,
+ struct gplink_list_result *expected)
+{
+ errno_t ret;
+ int i;
+ struct gp_gplink **gplink_list = NULL;
+ TALLOC_CTX *tmp_ctx;
+
+ tmp_ctx = talloc_new(global_talloc_context);
+ assert_non_null(tmp_ctx);
+ check_leaks_push(tmp_ctx);
+
+ char *raw_gplink_value = talloc_strdup(tmp_ctx, input_gplink_value);
+
+ ret = ad_gpo_populate_gplink_list(tmp_ctx,
+ NULL,
+ raw_gplink_value,
+ &gplink_list,
+ allow_enforced_only);
+
+ assert_int_equal(ret, expected->result);
+
+ for (i=0; i<expected->num_gplinks; i++){
+ bool equal = true;
+ if (strncmp(gplink_list[i]->gpo_dn,
+ expected->gpo_dns[i],
+ strlen(expected->gpo_dns[i])) != 0) {
+ equal = false;
+ }
+
+ if (gplink_list[i]->enforced != expected->enforced[i])
+ equal = false;
+
+ assert_int_equal(equal, true);
+ }
+
+ if (gplink_list) {
+ talloc_free(gplink_list);
+ }
+
+ talloc_free(raw_gplink_value);
+
+ assert_true(check_leaks_pop(tmp_ctx) == true);
+ talloc_free(tmp_ctx);
+}
+
+void test_populate_gplink_list_plain(void **state)
+{
+ const char *gpo_dns[] = {"OU=Sales,DC=FOO,DC=COM", "DC=FOO,DC=COM"};
+ bool enforced[] = {false, true};
+
+ struct gplink_list_result expected = {
+ .result = EOK,
+ .num_gplinks = 2,
+ .gpo_dns = gpo_dns,
+ .enforced = enforced
+ };
+
+ test_populate_gplink_list("[OU=Sales,DC=FOO,DC=COM;0][DC=FOO,DC=COM;2]",
+ false,
+ &expected);
+}
+
+void test_populate_gplink_list_with_ignored(void **state)
+{
+ const char *gpo_dns[] = {"OU=Sales,DC=FOO,DC=COM"};
+ bool enforced[] = {false};
+
+ struct gplink_list_result expected = {
+ .result = EOK,
+ .num_gplinks = 1,
+ .gpo_dns = gpo_dns,
+ .enforced = enforced
+ };
+
+ test_populate_gplink_list("[OU=Sales,DC=FOO,DC=COM;0][DC=ignored;1]",
+ false,
+ &expected);
+}
+
+void test_populate_gplink_list_with_allow_enforced(void **state)
+{
+ const char *gpo_dns[] = {"DC=FOO,DC=COM"};
+ bool enforced[] = {true};
+
+ struct gplink_list_result expected = {
+ .result = EOK,
+ .num_gplinks = 1,
+ .gpo_dns = gpo_dns,
+ .enforced = enforced
+ };
+
+ test_populate_gplink_list("[OU=Sales,DC=FOO,DC=COM;0][DC=FOO,DC=COM;2]",
+ true,
+ &expected);
+}
+
+void test_populate_gplink_list_malformed(void **state)
+{
+ struct gplink_list_result expected = {
+ .result = EINVAL,
+ };
+
+ test_populate_gplink_list(NULL, false, &expected);
+ test_populate_gplink_list("[malformed", false, &expected);
+ test_populate_gplink_list("[malformed]", false, &expected);
+ // the GPLinkOptions value (after semicolon) must be between 0 and 3
+ test_populate_gplink_list("[gpo_dn; 4]", false, &expected);
+}
+
+/*
+ * Test sid-matching logic
+ */
+static void test_ad_gpo_ace_includes_client_sid(const char *user_sid,
+ const char **group_sids,
+ int group_size,
+ struct dom_sid ace_dom_sid,
+ bool expected)
+{
+ errno_t ret;
+ enum idmap_error_code err;
+ struct sss_idmap_ctx *idmap_ctx;
+ bool includes_client_sid;
+ TALLOC_CTX *tmp_ctx;
+
+ tmp_ctx = talloc_new(global_talloc_context);
+ assert_non_null(tmp_ctx);
+ check_leaks_push(tmp_ctx);
+
+ err = sss_idmap_init(sss_idmap_talloc, tmp_ctx, sss_idmap_talloc_free,
+ &idmap_ctx);
+ assert_int_equal(err, IDMAP_SUCCESS);
+
+ ret = ad_gpo_ace_includes_client_sid(user_sid, group_sids, group_size,
+ ace_dom_sid, idmap_ctx,
+ &includes_client_sid);
+ talloc_free(idmap_ctx);
+
+ assert_int_equal(ret, EOK);
+ assert_int_equal(includes_client_sid, expected);
+
+ assert_true(check_leaks_pop(tmp_ctx) == true);
+ talloc_free(tmp_ctx);
+}
+
+void test_ad_gpo_ace_includes_client_sid_true(void **state)
+{
+ // ace_dom_sid represents "S-1-5-21-2-3-4"
+ struct dom_sid ace_dom_sid = {1, 4, {0, 0, 0, 0, 0, 5}, {21, 2, 3, 4}};
+
+ const char *user_sid = "S-1-5-21-1175337206-4250576914-2321192831-1103";
+
+ int group_size = 2;
+ const char *group_sids[] = {"S-1-5-21-2-3-4",
+ "S-1-5-21-2-3-5"};
+
+ test_ad_gpo_ace_includes_client_sid(user_sid, group_sids, group_size,
+ ace_dom_sid, true);
+}
+
+void test_ad_gpo_ace_includes_client_sid_false(void **state)
+{
+ // ace_dom_sid represents "S-1-5-21-2-3-4"
+ struct dom_sid ace_dom_sid = {1, 4, {0, 0, 0, 0, 0, 5}, {21, 2, 3, 4}};
+
+ const char *user_sid = "S-1-5-21-1175337206-4250576914-2321192831-1103";
+
+ int group_size = 2;
+ const char *group_sids[] = {"S-1-5-21-2-3-5",
+ "S-1-5-21-2-3-6"};
+
+ test_ad_gpo_ace_includes_client_sid(user_sid, group_sids, group_size,
+ ace_dom_sid, false);
+}
+
+int main(int argc, const char *argv[])
+{
+ poptContext pc;
+ int opt;
+ struct poptOption long_options[] = {
+ POPT_AUTOHELP
+ SSSD_DEBUG_OPTS
+ POPT_TABLEEND
+ };
+
+ const UnitTest tests[] = {
+ unit_test_setup_teardown(test_populate_som_list_plain,
+ ad_gpo_test_setup,
+ ad_gpo_test_teardown),
+ unit_test_setup_teardown(test_populate_som_list_malformed,
+ ad_gpo_test_setup,
+ ad_gpo_test_teardown),
+ unit_test_setup_teardown(test_populate_gplink_list_plain,
+ ad_gpo_test_setup,
+ ad_gpo_test_teardown),
+ unit_test_setup_teardown(test_populate_gplink_list_with_ignored,
+ ad_gpo_test_setup,
+ ad_gpo_test_teardown),
+ unit_test_setup_teardown(test_populate_gplink_list_with_allow_enforced,
+ ad_gpo_test_setup,
+ ad_gpo_test_teardown),
+ unit_test_setup_teardown(test_populate_gplink_list_malformed,
+ ad_gpo_test_setup,
+ ad_gpo_test_teardown),
+ unit_test_setup_teardown(test_ad_gpo_ace_includes_client_sid_true,
+ ad_gpo_test_setup,
+ ad_gpo_test_teardown),
+ unit_test_setup_teardown(test_ad_gpo_ace_includes_client_sid_false,
+ ad_gpo_test_setup,
+ ad_gpo_test_teardown),
+ };
+
+ /* Set debug level to invalid value so we can deside if -d 0 was used. */
+ debug_level = SSSDBG_INVALID;
+
+ pc = poptGetContext(argv[0], argc, argv, long_options, 0);
+ while((opt = poptGetNextOpt(pc)) != -1) {
+ switch(opt) {
+ default:
+ fprintf(stderr, "\nInvalid option %s: %s\n\n",
+ poptBadOption(pc, 0), poptStrerror(opt));
+ poptPrintUsage(pc, stderr, 0);
+ return 1;
+ }
+ }
+ poptFreeContext(pc);
+
+ DEBUG_INIT(debug_level);
+
+ tests_set_cwd();
+
+ return run_tests(tests);
+}
--
1.8.5.3
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
