On Fri, Oct 25, 2013 at 11:21:28AM +0200, Jakub Hrozek wrote:
> On Thu, Oct 24, 2013 at 02:14:59PM +0200, Sumit Bose wrote:
> > Hi,
> >
> > this patch set tries to fix https://fedorahosted.org/sssd/ticket/2101 .
> > Currently we rely on the fact that the external mapping is the default
> > in the case of an error. But this leads to a loot of useless CPU cycles
> > and maybe irritating error messages. With this patch set each trusted AD
> > member domain gets a proper range assigned if the forest root is
> > configured to manage the POSIX attributes.
> >
> > bye,
> > Sumit
>
> Hi,
>
> I only have two minor comments, see below:
>
> > From e1a86771137f67e0e8d24c4bbb014bfc21d85538 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sb...@redhat.com>
> > Date: Wed, 23 Oct 2013 14:39:55 +0200
> > Subject: [PATCH 1/4] find_subdomain_by_sid: skip domains with missing
> > domain_id
>
> ACK. Thanks for the unit test.
>
> > From dc0115802904c954c79d90cd846da9f5bd965019 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sb...@redhat.com>
> > Date: Thu, 24 Oct 2013 11:44:11 +0200
> > Subject: [PATCH 2/4] idmap: add
> > sss_idmap_domain_by_name_has_algorithmic_mapping()
>
> The code itself is good, but we need to bump the version number. See the
> attached patch, can you squash it in if you agree?
There was no patch but I changed version-info to 2:0:2. Is this what you
would suggest?
>
> > From dffdd7cc99025cb8790f34f79bdf294762f3468d Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sb...@redhat.com>
> > Date: Thu, 24 Oct 2013 11:45:57 +0200
> > Subject: [PATCH 3/4] sdap_idmap_domain_has_algorithmic_mapping: add domain
> > name argument
>
> ACK
>
> > From 3e7283a39994ac24a9ff9dfede0d735cc89b4bb0 Mon Sep 17 00:00:00 2001
> > From: Sumit Bose <sb...@redhat.com>
> > Date: Wed, 23 Oct 2013 13:30:57 +0200
> > Subject: [PATCH 4/4] IPA: add trusted domains with missing idrange
> >
> > If the forest root of a trusted forest is managing POSIX IDs for its
> > users and groups the same is assumed for all member domains in the
> > forest which do not have explicitly have an idrange set.
> >
> > To reflect this SSSD will create the matching ranges automatically.
>
> One nitpick:
>
> > + tmp_ctx = talloc_new(NULL);
> > + if (tmp_ctx == NULL) {
> > + DEBUG(SSSDBG_OP_FAILURE, ("talloc_new failed.\n"));
> > + return ENOMEM;
> > + }
> > +
> > + for (c = 0; c < range_count; c++) {
> > + r = range_list[c];
> > + if (r->trusted_dom_sid != NULL
> > + && strcmp(r->trusted_dom_sid, forest_root->domain_id) ==
> > 0) {
> > +
> > + if (r->range_type == NULL
> > + || strcmp(r->range_type, IPA_RANGE_AD_TRUST_POSIX) !=
> > 0) {
> > + DEBUG(SSSDBG_MINOR_FAILURE,
> > + ("Forest root does not have range type [%s].\n",
> > + IPA_RANGE_AD_TRUST_POSIX));
>
> we would leak tmp_ctx here, we should goto done instead.
good catch, fixed.
>
> > + return EINVAL;
> > + }
> > +
> > + range.min = r->base_id;
> > + range.max = r->base_id + r->id_range_size -1;
> > + range_id = talloc_asprintf(tmp_ctx, "%s-%s", dom_sid_str,
> > r->name);
> > + if (range_id == NULL) {
> > + DEBUG(SSSDBG_OP_FAILURE, ("talloc_asprintf failed.\n"));
> > + ret = ENOMEM;
> > + goto done;
> > + }
> > +
> > + err = sss_idmap_add_domain_ex(idmap_ctx->map, dom_name,
> > dom_sid_str,
> > + &range, range_id, 0, true);
> > + if (err != IDMAP_SUCCESS && err != IDMAP_COLLISION) {
> > + DEBUG(SSSDBG_CRIT_FAILURE, ("Could not add range [%s] to
> > ID map\n",
> > + range_id));
>
> If you're changing the patch, can you also reflow this line? It's past
> 80 characters and my editor highlights the line..
fixed.
Thank you for the review, new versions attached.
bye,
Sumit
>
> > + ret = EIO;
> > + goto done;
> > + }
> > +
> > + found = true;
> > + }
> > + }
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
From e1a86771137f67e0e8d24c4bbb014bfc21d85538 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Wed, 23 Oct 2013 14:39:55 +0200
Subject: [PATCH 1/4] find_subdomain_by_sid: skip domains with missing
domain_id
---
Makefile.am | 12 ++-
src/tests/cmocka/test_utils.c | 221 ++++++++++++++++++++++++++++++++++++++++++
src/util/domain_info_utils.c | 30 +++---
3 files changed, 251 insertions(+), 12 deletions(-)
create mode 100644 src/tests/cmocka/test_utils.c
diff --git a/Makefile.am b/Makefile.am
index cb24df8..d2a7384 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -153,7 +153,8 @@ if HAVE_CMOCKA
sss_nss_idmap-tests \
dyndns-tests \
fqnames-tests \
- test_sss_idmap
+ test_sss_idmap \
+ test_utils
endif
check_PROGRAMS = \
@@ -1397,6 +1398,15 @@ test_sss_idmap_LDADD = \
$(SSSD_INTERNAL_LTLIBS) \
libsss_test_common.la
+test_utils_SOURCES = \
+ $(TEST_MOCK_OBJ) \
+ src/tests/cmocka/test_utils.c
+test_utils_CFLAGS = \
+ $(AM_CFLAGS)
+test_utils_LDADD = \
+ $(CMOCKA_LIBS) \
+ $(SSSD_INTERNAL_LTLIBS) \
+ libsss_test_common.la
endif
noinst_PROGRAMS = pam_test_client
diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c
new file mode 100644
index 0000000..9152dcf
--- /dev/null
+++ b/src/tests/cmocka/test_utils.c
@@ -0,0 +1,221 @@
+/*
+ Authors:
+ Sumit Bose <sb...@redhat.com>
+
+ Copyright (C) 2013 Red Hat
+
+ SSSD tests: Tests for utility functions
+
+ 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 <popt.h>
+
+#include "tests/cmocka/common_mock.h"
+
+#define DOM_COUNT 10
+#define DOMNAME_TMPL "name_%u.dom"
+#define FLATNAME_TMPL "name_%u"
+#define SID_TMPL "S-1-5-21-1-2-%u"
+
+struct dom_list_test_ctx {
+ size_t dom_count;
+ struct sss_domain_info *dom_list;
+};
+
+void setup_dom_list(void **state)
+{
+ struct dom_list_test_ctx *test_ctx;
+ struct sss_domain_info *dom = NULL;
+ size_t c;
+
+ assert_true(leak_check_setup());
+
+ test_ctx = talloc_zero(global_talloc_context, struct dom_list_test_ctx);
+ assert_non_null(test_ctx);
+
+ test_ctx->dom_count = DOM_COUNT;
+
+ for (c = 0; c < test_ctx->dom_count; c++) {
+ dom = talloc_zero(test_ctx, struct sss_domain_info);
+ assert_non_null(dom);
+
+ dom->name = talloc_asprintf(dom, DOMNAME_TMPL, c);
+ assert_non_null(dom->name);
+
+ dom->flat_name = talloc_asprintf(dom, FLATNAME_TMPL, c);
+ assert_non_null(dom->flat_name);
+
+ dom->domain_id = talloc_asprintf(dom, SID_TMPL, c);
+ assert_non_null(dom->domain_id);
+
+ DLIST_ADD(test_ctx->dom_list, dom);
+ }
+
+ check_leaks_push(test_ctx);
+ *state = test_ctx;
+}
+
+void teardown_dom_list(void **state)
+{
+ struct dom_list_test_ctx *test_ctx = talloc_get_type(*state,
+ struct
dom_list_test_ctx);
+ if (test_ctx == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("Type mismatch\n"));
+ return;
+ }
+
+ assert_true(check_leaks_pop(test_ctx) == true);
+ talloc_free(test_ctx);
+ assert_true(leak_check_teardown());
+}
+
+void test_find_subdomain_by_sid_null(void **state)
+{
+ struct dom_list_test_ctx *test_ctx = talloc_get_type(*state,
+ struct
dom_list_test_ctx);
+ struct sss_domain_info *dom;
+
+ dom = find_subdomain_by_sid(NULL, NULL);
+ assert_null(dom);
+
+ dom = find_subdomain_by_sid(test_ctx->dom_list, NULL);
+ assert_null(dom);
+
+ dom = find_subdomain_by_sid(NULL, "S-1-5-21-1-2-3");
+ assert_null(dom);
+}
+
+void test_find_subdomain_by_sid(void **state)
+{
+ struct dom_list_test_ctx *test_ctx = talloc_get_type(*state,
+ struct
dom_list_test_ctx);
+ struct sss_domain_info *dom;
+ size_t c;
+ char *name;
+ char *flat_name;
+ char *sid;
+
+ for (c = 0; c < test_ctx->dom_count; c++) {
+ name = talloc_asprintf(global_talloc_context, DOMNAME_TMPL, c);
+ assert_non_null(name);
+
+ flat_name = talloc_asprintf(global_talloc_context, FLATNAME_TMPL, c);
+ assert_non_null(flat_name);
+
+ sid = talloc_asprintf(global_talloc_context, SID_TMPL, c);
+ assert_non_null(sid);
+
+ dom = find_subdomain_by_sid(test_ctx->dom_list, sid);
+ assert_non_null(dom);
+ assert_string_equal(name, dom->name);
+ assert_string_equal(flat_name, dom->flat_name);
+ assert_string_equal(sid, dom->domain_id);
+
+ talloc_free(name);
+ talloc_free(flat_name);
+ talloc_free(sid);
+ }
+}
+
+void test_find_subdomain_by_sid_missing_sid(void **state)
+{
+ struct dom_list_test_ctx *test_ctx = talloc_get_type(*state,
+ struct
dom_list_test_ctx);
+ struct sss_domain_info *dom;
+ size_t c;
+ char *name;
+ char *flat_name;
+ char *sid;
+ size_t mis;
+
+ mis = test_ctx->dom_count/2;
+ assert_true((mis >= 1 && mis < test_ctx->dom_count));
+
+ dom = test_ctx->dom_list;
+ for (c = 0; c < mis; c++) {
+ assert_non_null(dom);
+ dom = dom->next;
+ }
+ assert_non_null(dom);
+ dom->domain_id = NULL;
+
+ for (c = 0; c < test_ctx->dom_count; c++) {
+ name = talloc_asprintf(global_talloc_context, DOMNAME_TMPL, c);
+ assert_non_null(name);
+
+ flat_name = talloc_asprintf(global_talloc_context, FLATNAME_TMPL, c);
+ assert_non_null(flat_name);
+
+ sid = talloc_asprintf(global_talloc_context, SID_TMPL, c);
+ assert_non_null(sid);
+
+ dom = find_subdomain_by_sid(test_ctx->dom_list, sid);
+ if (c == mis - 1) {
+ assert_null(dom);
+ } else {
+ assert_non_null(dom);
+ assert_string_equal(name, dom->name);
+ assert_string_equal(flat_name, dom->flat_name);
+ assert_string_equal(sid, dom->domain_id);
+ }
+
+ talloc_free(name);
+ talloc_free(flat_name);
+ talloc_free(sid);
+ }
+}
+
+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_find_subdomain_by_sid_null,
+ setup_dom_list, teardown_dom_list),
+ unit_test_setup_teardown(test_find_subdomain_by_sid,
+ setup_dom_list, teardown_dom_list),
+ unit_test_setup_teardown(test_find_subdomain_by_sid_missing_sid,
+ setup_dom_list, teardown_dom_list),
+ };
+
+ /* 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);
+
+ /* Even though normally the tests should clean up after themselves
+ * they might not after a failed run. Remove the old db to be sure */
+ tests_set_cwd();
+
+ return run_tests(tests);
+}
diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c
index 9d7bb5f..9f2f154 100644
--- a/src/util/domain_info_utils.c
+++ b/src/util/domain_info_utils.c
@@ -112,26 +112,34 @@ struct sss_domain_info *find_subdomain_by_sid(struct
sss_domain_info *domain,
const char *sid)
{
struct sss_domain_info *dom = domain;
- size_t sid_len = strlen(sid);
+ size_t sid_len;
size_t dom_sid_len;
+ if (sid == NULL) {
+ return NULL;
+ }
+
+ sid_len = strlen(sid);
+
while (dom && dom->disabled) {
dom = get_next_domain(dom, true);
}
while (dom) {
- dom_sid_len = strlen(dom->domain_id);
+ if (dom->domain_id != NULL) {
+ dom_sid_len = strlen(dom->domain_id);
- if (strncasecmp(dom->domain_id, sid, dom_sid_len) == 0) {
- if (dom_sid_len == sid_len) {
- /* sid is domain sid */
- return dom;
- }
+ if (strncasecmp(dom->domain_id, sid, dom_sid_len) == 0) {
+ if (dom_sid_len == sid_len) {
+ /* sid is domain sid */
+ return dom;
+ }
- /* sid is object sid, check if domain sid is align with
- * sid first subauthority component */
- if (sid[dom_sid_len] == '-') {
- return dom;
+ /* sid is object sid, check if domain sid is align with
+ * sid first subauthority component */
+ if (sid[dom_sid_len] == '-') {
+ return dom;
+ }
}
}
--
1.8.3.1
From 2e4da942e069ceabfad517446213931aa0ce1474 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Thu, 24 Oct 2013 11:44:11 +0200
Subject: [PATCH 2/4] idmap: add
sss_idmap_domain_by_name_has_algorithmic_mapping()
---
Makefile.am | 2 +-
src/lib/idmap/sss_idmap.c | 33 +++++++++++++++++++++++++++++
src/lib/idmap/sss_idmap.h | 44 +++++++++++++++++++++++++++++++++++----
src/tests/cmocka/test_sss_idmap.c | 41 ++++++++++++++++++++++++++++++++++++
4 files changed, 115 insertions(+), 5 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index d2a7384..f3a5e76 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -624,7 +624,7 @@ libsss_idmap_la_SOURCES = \
src/lib/idmap/sss_idmap_conv.c \
src/util/murmurhash3.c
libsss_idmap_la_LDFLAGS = \
- -version-info 1:0:1
+ -version-info 2:0:2
dist_pkgconfig_DATA += src/sss_client/idmap/sss_nss_idmap.pc
libsss_nss_idmap_la_SOURCES = \
diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
index 17bd577..9278e10 100644
--- a/src/lib/idmap/sss_idmap.c
+++ b/src/lib/idmap/sss_idmap.c
@@ -953,3 +953,36 @@ sss_idmap_domain_has_algorithmic_mapping(struct
sss_idmap_ctx *ctx,
return IDMAP_SID_UNKNOWN;
}
+
+enum idmap_error_code
+sss_idmap_domain_by_name_has_algorithmic_mapping(struct sss_idmap_ctx *ctx,
+ const char *dom_name,
+ bool *has_algorithmic_mapping)
+{
+ struct idmap_domain_info *idmap_domain_info;
+
+ if (dom_name == NULL) {
+ return IDMAP_ERROR;
+ }
+
+ CHECK_IDMAP_CTX(ctx, IDMAP_CONTEXT_INVALID);
+
+ if (ctx->idmap_domain_info == NULL) {
+ return IDMAP_NO_DOMAIN;
+ }
+
+ idmap_domain_info = ctx->idmap_domain_info;
+
+ while (idmap_domain_info != NULL) {
+ if (idmap_domain_info->name != NULL
+ && strcmp(dom_name, idmap_domain_info->name) == 0) {
+
+ *has_algorithmic_mapping = !idmap_domain_info->external_mapping;
+ return IDMAP_SUCCESS;
+ }
+
+ idmap_domain_info = idmap_domain_info->next;
+ }
+
+ return IDMAP_NAME_UNKNOWN;
+}
diff --git a/src/lib/idmap/sss_idmap.h b/src/lib/idmap/sss_idmap.h
index 91e5d0b..4101fb9 100644
--- a/src/lib/idmap/sss_idmap.h
+++ b/src/lib/idmap/sss_idmap.h
@@ -81,7 +81,10 @@ enum idmap_error_code {
IDMAP_COLLISION,
/** External source should be consulted for idmapping */
- IDMAP_EXTERNAL
+ IDMAP_EXTERNAL,
+
+ /** The provided name was not found */
+ IDMAP_NAME_UNKNOWN
};
/**
@@ -524,11 +527,21 @@ bool is_domain_sid(const char *str);
/**
* @brief Check if a domain is configured with algorithmic mapping
*
- * @param[in] ctx Idmap context
- * @param[in] dom_sid SID string, can be either a domain SID or an object SID
+ * @param[in] ctx Idmap context
+ * @param[in] dom_sid SID string, can be either a domain SID
+ * or an object SID
+ * @param[out] has_algorithmic_mapping Boolean value indicating if the given
+ * domain is configured for algorithmic
+ * mapping or not.
*
* @return
- * TODO ....
+ * - #IDMAP_SUCCESS: Domain for the given SID was found and
+ * has_algorithmic_mapping is set accordingly
+ * - #IDMAP_SID_INVALID: Provided SID is invalid
+ * - #IDMAP_CONTEXT_INVALID: Provided idmap context is invalid
+ * - #IDMAP_NO_DOMAIN: No domains are available in the idmap context
+ * - #IDMAP_SID_UNKNOWN: No domain with the given SID was found in the
+ * idmap context
*/
enum idmap_error_code
sss_idmap_domain_has_algorithmic_mapping(struct sss_idmap_ctx *ctx,
@@ -536,6 +549,29 @@ sss_idmap_domain_has_algorithmic_mapping(struct
sss_idmap_ctx *ctx,
bool *has_algorithmic_mapping);
/**
+ * @brief Check if a domain is configured with algorithmic mapping
+ *
+ * @param[in] ctx Idmap context
+ * @param[in] dom_name Name of the domain
+ * @param[out] has_algorithmic_mapping Boolean value indicating if the given
+ * domain is configured for algorithmic
+ * mapping or not.
+ *
+ * @return
+ * - #IDMAP_SUCCESS: Domain for the given name was found and
+ * has_algorithmic_mapping is set accordingly
+ * - #IDMAP_ERROR: Provided name is invalid
+ * - #IDMAP_CONTEXT_INVALID: Provided idmap context is invalid
+ * - #IDMAP_NO_DOMAIN: No domains are available in the idmap context
+ * - #IDMAP_NAME_UNKNOWN: No domain with the given name was found in the
+ * idmap context
+ */
+enum idmap_error_code
+sss_idmap_domain_by_name_has_algorithmic_mapping(struct sss_idmap_ctx *ctx,
+ const char *dom_name,
+ bool
*has_algorithmic_mapping);
+
+/**
* @brief Convert binary SID to SID structure
*
* @param[in] ctx Idmap context
diff --git a/src/tests/cmocka/test_sss_idmap.c
b/src/tests/cmocka/test_sss_idmap.c
index 2f6a67d..53ed35a 100644
--- a/src/tests/cmocka/test_sss_idmap.c
+++ b/src/tests/cmocka/test_sss_idmap.c
@@ -367,6 +367,44 @@ void test_has_algorithmic(void **state)
assert_false(use_id_mapping);
}
+void test_has_algorithmic_by_name(void **state)
+{
+ struct test_ctx *test_ctx;
+ bool use_id_mapping;
+ enum idmap_error_code err;
+
+ test_ctx = talloc_get_type(*state, struct test_ctx);
+
+ assert_non_null(test_ctx);
+
+ err = sss_idmap_domain_by_name_has_algorithmic_mapping(NULL, NULL,
&use_id_mapping);
+ assert_int_equal(err, IDMAP_ERROR);
+
+ err = sss_idmap_domain_by_name_has_algorithmic_mapping(NULL, TEST_DOM_SID,
+ &use_id_mapping);
+ assert_int_equal(err, IDMAP_CONTEXT_INVALID);
+
+ err =
sss_idmap_domain_by_name_has_algorithmic_mapping(test_ctx->idmap_ctx, NULL,
+ &use_id_mapping);
+ assert_int_equal(err, IDMAP_ERROR);
+
+ err = sss_idmap_domain_by_name_has_algorithmic_mapping(test_ctx->idmap_ctx,
+ TEST_DOM_NAME"1",
+ &use_id_mapping);
+ assert_int_equal(err, IDMAP_NAME_UNKNOWN);
+
+ err = sss_idmap_domain_by_name_has_algorithmic_mapping(test_ctx->idmap_ctx,
+ TEST_DOM_NAME,
+ &use_id_mapping);
+ assert_int_equal(err, IDMAP_SUCCESS);
+ assert_true(use_id_mapping);
+
+ err = sss_idmap_domain_by_name_has_algorithmic_mapping(test_ctx->idmap_ctx,
+ TEST_2_DOM_NAME,
+ &use_id_mapping);
+ assert_int_equal(err, IDMAP_SUCCESS);
+ assert_false(use_id_mapping);
+}
int main(int argc, const char *argv[])
{
@@ -396,6 +434,9 @@ int main(int argc, const char *argv[])
unit_test_setup_teardown(test_has_algorithmic,
test_sss_idmap_setup_with_both,
test_sss_idmap_teardown),
+ unit_test_setup_teardown(test_has_algorithmic_by_name,
+ test_sss_idmap_setup_with_both,
+ test_sss_idmap_teardown),
};
/* Set debug level to invalid value so we can deside if -d 0 was used. */
--
1.8.3.1
From c8cd2f2d743804b171c0a75e08b6bfa7655946c6 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Thu, 24 Oct 2013 11:45:57 +0200
Subject: [PATCH 3/4] sdap_idmap_domain_has_algorithmic_mapping: add domain
name argument
When libss_idmap was only used to algorithmically map a SID to a POSIX
ID a domain SID was strictly necessary and the only information needed
to find a domain.
With the introduction of external mappings there are cases where a
domain SID is not available. Currently we relied on the fact that
external mapping was always used as a default if not specific
information about the domain was found. The lead to extra CPU cycles and
potentially confusing debug messages. Adding the domain name as a search
parameter will avoid this.
---
src/providers/ad/ad_subdomains.c | 1 +
src/providers/ipa/ipa_subdomains.c | 2 +-
src/providers/ldap/ldap_id.c | 2 ++
src/providers/ldap/sdap_async_enum.c | 2 ++
src/providers/ldap/sdap_async_groups.c | 1 +
src/providers/ldap/sdap_async_initgroups.c | 4 ++++
src/providers/ldap/sdap_async_users.c | 1 +
src/providers/ldap/sdap_idmap.c | 12 +++++++++++-
src/providers/ldap/sdap_idmap.h | 1 +
9 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index d8e9b26..30c510c 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -162,6 +162,7 @@ ad_subdom_store(struct ad_subdomains_ctx *ctx,
mpg = sdap_idmap_domain_has_algorithmic_mapping(
ctx->sdap_id_ctx->opts->idmap_ctx,
+ domain->name,
domain->domain_id);
/* AD subdomains are currently all mpg and do not enumerate */
diff --git a/src/providers/ipa/ipa_subdomains.c
b/src/providers/ipa/ipa_subdomains.c
index d6cb0c6..93bbdae 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -577,7 +577,7 @@ static errno_t ipa_subdom_store(struct sss_domain_info
*parent,
goto done;
}
- mpg = sdap_idmap_domain_has_algorithmic_mapping(sdap_idmap_ctx, id);
+ mpg = sdap_idmap_domain_has_algorithmic_mapping(sdap_idmap_ctx, name, id);
ret = ipa_subdom_get_forest(tmp_ctx, sysdb_ctx_get_ldb(parent->sysdb),
attrs, &forest);
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index 59dfd0a..9fd95ce 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -105,6 +105,7 @@ struct tevent_req *users_get_send(TALLOC_CTX *memctx,
use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(
ctx->opts->idmap_ctx,
+ sdom->dom->name,
sdom->dom->domain_id);
switch (filter_type) {
case BE_FILTER_NAME:
@@ -471,6 +472,7 @@ struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(
ctx->opts->idmap_ctx,
+ sdom->dom->name,
sdom->dom->domain_id);
switch(filter_type) {
diff --git a/src/providers/ldap/sdap_async_enum.c
b/src/providers/ldap/sdap_async_enum.c
index b03c19a..a1bc097 100644
--- a/src/providers/ldap/sdap_async_enum.c
+++ b/src/providers/ldap/sdap_async_enum.c
@@ -366,6 +366,7 @@ static struct tevent_req *enum_users_send(TALLOC_CTX
*memctx,
use_mapping = sdap_idmap_domain_has_algorithmic_mapping(
ctx->opts->idmap_ctx,
+ sdom->dom->name,
sdom->dom->domain_id);
/* We always want to filter on objectclass and an available name */
@@ -540,6 +541,7 @@ static struct tevent_req *enum_groups_send(TALLOC_CTX
*memctx,
use_mapping = sdap_idmap_domain_has_algorithmic_mapping(
ctx->opts->idmap_ctx,
+ sdom->dom->name,
sdom->dom->domain_id);
/* We always want to filter on objectclass and an available name */
diff --git a/src/providers/ldap/sdap_async_groups.c
b/src/providers/ldap/sdap_async_groups.c
index b111895..00ac3e9 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -478,6 +478,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
}
use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(opts->idmap_ctx,
+ dom->name,
sid_str);
if (use_id_mapping) {
posix_group = true;
diff --git a/src/providers/ldap/sdap_async_initgroups.c
b/src/providers/ldap/sdap_async_initgroups.c
index e8de8d5..c16d484 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -100,6 +100,7 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx
*sysdb,
}
use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(opts->idmap_ctx,
+ domain->name,
domain->domain_id);
ret = sysdb_transaction_start(sysdb);
@@ -1542,6 +1543,7 @@ static struct tevent_req *sdap_initgr_rfc2307bis_send(
use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(
opts->idmap_ctx,
+ sdom->dom->name,
sdom->dom->domain_id);
state->base_filter =
@@ -2637,6 +2639,7 @@ struct tevent_req *sdap_get_initgr_send(TALLOC_CTX
*memctx,
use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(
id_ctx->opts->idmap_ctx,
+ sdom->dom->name,
sdom->dom->domain_id);
ret = sss_filter_sanitize(state, name, &clean_name);
@@ -2684,6 +2687,7 @@ struct tevent_req *sdap_get_initgr_send(TALLOC_CTX
*memctx,
state->use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(
state->opts->idmap_ctx,
+ state->dom->name,
state->dom->domain_id);
ret = sdap_get_initgr_next_base(req);
diff --git a/src/providers/ldap/sdap_async_users.c
b/src/providers/ldap/sdap_async_users.c
index 860e8fe..2807b07 100644
--- a/src/providers/ldap/sdap_async_users.c
+++ b/src/providers/ldap/sdap_async_users.c
@@ -215,6 +215,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(opts->idmap_ctx,
+ dom->name,
sid_str);
/* Retrieve or map the UID as appropriate */
diff --git a/src/providers/ldap/sdap_idmap.c b/src/providers/ldap/sdap_idmap.c
index af69ee1..249201d 100644
--- a/src/providers/ldap/sdap_idmap.c
+++ b/src/providers/ldap/sdap_idmap.c
@@ -513,6 +513,7 @@ done:
}
bool sdap_idmap_domain_has_algorithmic_mapping(struct sdap_idmap_ctx *ctx,
+ const char *dom_name,
const char *dom_sid)
{
enum idmap_error_code err;
@@ -529,6 +530,15 @@ bool sdap_idmap_domain_has_algorithmic_mapping(struct
sdap_idmap_ctx *ctx,
return false;
}
+ err = sss_idmap_domain_by_name_has_algorithmic_mapping(ctx->map,
+ dom_name,
+ &has_algorithmic_mapping);
+ if (err == IDMAP_SUCCESS) {
+ return has_algorithmic_mapping;
+ } else if (err != IDMAP_NAME_UNKNOWN && err != IDMAP_NO_DOMAIN) {
+ return false;
+ }
+
/* This is the first time we've seen this domain
* Create a new domain for it. We'll use the dom-sid
* as the domain name for now, since we don't have
@@ -554,7 +564,7 @@ bool sdap_idmap_domain_has_algorithmic_mapping(struct
sdap_idmap_ctx *ctx,
}
}
- ret = ctx->find_new_domain(ctx, new_dom_sid, new_dom_sid);
+ ret = ctx->find_new_domain(ctx, dom_name, new_dom_sid);
talloc_free(tmp_ctx);
if (ret != EOK) {
DEBUG(SSSDBG_MINOR_FAILURE,
diff --git a/src/providers/ldap/sdap_idmap.h b/src/providers/ldap/sdap_idmap.h
index c8bc4e2..07499dc 100644
--- a/src/providers/ldap/sdap_idmap.h
+++ b/src/providers/ldap/sdap_idmap.h
@@ -57,6 +57,7 @@ sdap_idmap_sid_to_unix(struct sdap_idmap_ctx *idmap_ctx,
id_t *id);
bool sdap_idmap_domain_has_algorithmic_mapping(struct sdap_idmap_ctx *ctx,
+ const char *name,
const char *dom_sid);
#endif /* SDAP_IDMAP_H_ */
--
1.8.3.1
From 02740ef18b5a1abdd0c27afca0971b50ff8e6103 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Wed, 23 Oct 2013 13:30:57 +0200
Subject: [PATCH 4/4] IPA: add trusted domains with missing idrange
If the forest root of a trusted forest is managing POSIX IDs for its
users and groups the same is assumed for all member domains in the
forest which do not have explicitly have an idrange set.
To reflect this SSSD will create the matching ranges automatically.
Fixes https://fedorahosted.org/sssd/ticket/2101
---
src/providers/ipa/ipa_idmap.c | 137 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 137 insertions(+)
diff --git a/src/providers/ipa/ipa_idmap.c b/src/providers/ipa/ipa_idmap.c
index 2f141f8..eaca0ed 100644
--- a/src/providers/ipa/ipa_idmap.c
+++ b/src/providers/ipa/ipa_idmap.c
@@ -26,6 +26,136 @@
#include "providers/ipa/ipa_common.h"
#include "util/util_sss_idmap.h"
+static errno_t ipa_idmap_check_posix_child(struct sdap_idmap_ctx *idmap_ctx,
+ const char *dom_name,
+ const char *dom_sid_str,
+ size_t range_count,
+ struct range_info **range_list)
+{
+ bool has_algorithmic_mapping;
+ enum idmap_error_code err;
+ struct sss_domain_info *dom;
+ struct sss_domain_info *forest_root;
+ size_t c;
+ struct sss_idmap_range range;
+ struct range_info *r;
+ char *range_id;
+ TALLOC_CTX *tmp_ctx;
+ bool found = false;
+ int ret;
+
+ err = sss_idmap_domain_has_algorithmic_mapping(idmap_ctx->map, dom_sid_str,
+ &has_algorithmic_mapping);
+ if (err == IDMAP_SUCCESS) {
+ DEBUG(SSSDBG_TRACE_ALL,
+ ("Idmap of domain [%s] already known, nothing to do.\n",
+ dom_sid_str));
+ return EOK;
+ } else {
+ err = sss_idmap_domain_by_name_has_algorithmic_mapping(idmap_ctx->map,
+ dom_name,
+
&has_algorithmic_mapping);
+ if (err == IDMAP_SUCCESS) {
+ DEBUG(SSSDBG_TRACE_ALL,
+ ("Idmap of domain [%s] already known, nothing to do.\n",
+ dom_sid_str));
+ return EOK;
+ }
+ }
+ DEBUG(SSSDBG_TRACE_ALL, ("Trying to add idmap for domain [%s].\n",
+ dom_sid_str));
+
+ if (err != IDMAP_SID_UNKNOWN && err != IDMAP_NAME_UNKNOWN) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("sss_idmap_domain_has_algorithmic_mapping failed.\n"));
+ return EINVAL;
+ }
+
+ dom = find_subdomain_by_sid(idmap_ctx->id_ctx->be->domain, dom_sid_str);
+ if (dom == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("find_subdomain_by_sid failed with SID [%s].\n", dom_sid_str));
+ return EINVAL;
+ }
+
+ if (dom->forest == NULL) {
+ DEBUG(SSSDBG_MINOR_FAILURE, ("No forest available for domain [%s].\n",
+ dom_sid_str));
+ return EINVAL;
+ }
+
+ forest_root = find_subdomain_by_name(idmap_ctx->id_ctx->be->domain,
+ dom->forest, true);
+ if (forest_root == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ ("find_subdomain_by_name failed to find forest root [%s].\n",
+ dom->forest));
+ return ENOENT;
+ }
+
+ if (forest_root->domain_id == NULL) {
+ DEBUG(SSSDBG_MINOR_FAILURE, ("Forest root [%s] does not have a SID.\n",
+ dom->forest));
+ return EINVAL;
+ }
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, ("talloc_new failed.\n"));
+ return ENOMEM;
+ }
+
+ for (c = 0; c < range_count; c++) {
+ r = range_list[c];
+ if (r->trusted_dom_sid != NULL
+ && strcmp(r->trusted_dom_sid, forest_root->domain_id) == 0) {
+
+ if (r->range_type == NULL
+ || strcmp(r->range_type, IPA_RANGE_AD_TRUST_POSIX) != 0) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("Forest root does not have range type [%s].\n",
+ IPA_RANGE_AD_TRUST_POSIX));
+ ret = EINVAL;
+ goto done;
+ }
+
+ range.min = r->base_id;
+ range.max = r->base_id + r->id_range_size -1;
+ range_id = talloc_asprintf(tmp_ctx, "%s-%s", dom_sid_str, r->name);
+ if (range_id == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, ("talloc_asprintf failed.\n"));
+ ret = ENOMEM;
+ goto done;
+ }
+
+ err = sss_idmap_add_domain_ex(idmap_ctx->map, dom_name,
dom_sid_str,
+ &range, range_id, 0, true);
+ if (err != IDMAP_SUCCESS && err != IDMAP_COLLISION) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Could not add range [%s] to ID map\n", range_id));
+ ret = EIO;
+ goto done;
+ }
+
+ found = true;
+ }
+ }
+
+ if (!found) {
+ DEBUG(SSSDBG_MINOR_FAILURE, ("No idrange found for forest root
[%s].\n",
+ forest_root->domain_id));
+ ret = ENOENT;
+ goto done;
+ }
+
+ ret = EOK;
+
+done:
+ talloc_free(tmp_ctx);
+
+ return ret;
+}
+
errno_t ipa_idmap_find_new_domain(struct sdap_idmap_ctx *idmap_ctx,
const char *dom_name,
const char *dom_sid_str)
@@ -118,6 +248,13 @@ errno_t ipa_idmap_find_new_domain(struct sdap_idmap_ctx
*idmap_ctx,
}
}
+ ret = ipa_idmap_check_posix_child(idmap_ctx, dom_name, dom_sid_str,
+ range_count, range_list);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_OP_FAILURE, ("ipa_idmap_check_posix_child failed.\n"));
+ goto done;
+ }
+
ret = EOK;
done:
--
1.8.3.1
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
