I'm afraid I don't understand and given the tight time schedule I would like to
ask you if you could be so kind and prepare the proper patch for this. It would be really
helpful.
@see git log for other files
./src/sss_client/idmap/sss_nss_idmap.exports
./src/providers/ipa/ipa_hbac.exports
You also forgot to bump version-info in Makefile.am for this library.
> diff --git a/src/lib/idmap/sss_idmap.h b/src/lib/idmap/sss_idmap.h
> index
0797083293f7e010962828ddcd72709b290859b9..c19f6ad7bd4c4628d08bf5d90888d89bf1c79d5d 100644
> --- a/src/lib/idmap/sss_idmap.h
> +++ b/src/lib/idmap/sss_idmap.h
> @@ -175,6 +175,17 @@ enum idmap_error_code
> sss_idmap_ctx_set_rangesize(struct sss_idmap_ctx *ctx, id_t rangesize);
>
> /**
> + * @brief Set the number of secondary slices available for domain
> + *
> + * @param[in] ctx idmap context
> + * @param[in] extra_slice_init number of secondary slices to be generated
> + * at startup
> + */
> +enum idmap_error_code
> +sss_idmap_ctx_set_extra_slice_init(struct sss_idmap_ctx *ctx,
> + int extra_slice_init);
> +
> +/**
> * @brief Check if autorid compatibility mode is set
> *
> * @param[in] ctx idmap context
> @@ -211,6 +222,16 @@ enum idmap_error_code
> sss_idmap_ctx_get_rangesize(struct sss_idmap_ctx *ctx, id_t *rangesize);
>
> /**
> + * @brief Get the maximal number of secondary slices available for domain
> + *
> + * @param[in] ctx idmap context
> + * @param[out] _extra_slice_init maximal number of secondary slices
> + */
> +enum idmap_error_code
> +sss_idmap_ctx_get_extra_slide_max(struct sss_idmap_ctx *ctx,
> + int *_extra_slide_max);
> +
> +/**
> * @brief Calculate new range of available POSIX IDs
> *
> * @param[in] ctx Idmap context
> @@ -291,6 +312,44 @@ enum idmap_error_code sss_idmap_add_domain_ex(struct
sss_idmap_ctx *ctx,
> bool external_mapping);
>
> /**
> + * @brief Add a domain with the first mappable RID to the idmap context and
> + * generate automatically secondary slices
> + *
> + * @param[in] ctx Idmap context
> + * @param[in] domain_name Zero-terminated string with the domain name
> + * @param[in] domain_sid Zero-terminated string representation of the domain
> + * SID (S-1-15-.....)
> + * @param[in] range TBD Some information about the id ranges of this
> + * domain
> + * @param[in] range_id optional unique identifier of a range, it is needed
> + * to allow updates at runtime
> + * @param[in] rid The RID that should be mapped to the first ID of the
> + * given range.
> + * @param[in] external_mapping If set to true the ID will not be mapped
> + * algorithmically, but the *_to_unix and *_unix_to_*
> + * calls will return IDMAP_EXTERNAL to instruct the
> + * caller to check external sources. For a single
> + * domain all ranges must be of the same type. It is
> + * not possible to mix algorithmic and external
> + * mapping.
> + *
> + * @return
> + * - #IDMAP_OUT_OF_MEMORY: Insufficient memory to store the data in the idmap
> + * context
> + * - #IDMAP_SID_INVALID: Invalid SID provided
> + * - #IDMAP_NO_DOMAIN: No domain domain name given
> + * - #IDMAP_COLLISION: New domain collides with existing one
> + */
> +enum idmap_error_code
> +sss_idmap_add_auto_domain_ex(struct sss_idmap_ctx *ctx,
> + const char *domain_name,
> + const char *domain_sid,
> + struct sss_idmap_range *range,
> + const char *range_id,
> + uint32_t rid,
> + bool external_mapping);
> +
> +/**
> * @brief Check if a new range would collide with any existing one
> *
> * @param[in] ctx Idmap context
> diff --git a/src/lib/idmap/sss_idmap_private.h b/src/lib/idmap/sss_idmap_private.h
> index
1d3a36901781ae51ab79015d0b789559325c8de5..15300d11fc50a47c6d37149fdb79477069d931f4 100644
> --- a/src/lib/idmap/sss_idmap_private.h
> +++ b/src/lib/idmap/sss_idmap_private.h
> @@ -29,6 +29,7 @@
> #define SSS_IDMAP_DEFAULT_UPPER 2000200000
> #define SSS_IDMAP_DEFAULT_RANGESIZE 200000
> #define SSS_IDMAP_DEFAULT_AUTORID false
> +#define SSS_IDMAP_DEFAULT_EXTRA_SLICE_INIT 10
>
> #define CHECK_IDMAP_CTX(ctx, ret) do { \
> if (ctx == NULL || ctx->alloc_func == NULL || ctx->free_func == NULL) { \
> @@ -48,6 +49,9 @@ struct sss_idmap_opts {
>
> /* number of available UIDs (for single domain) */
> id_t rangesize;
> +
> + /* maximal number of secondary slices */
> + int extra_slice_init;
> };
>
> struct sss_idmap_ctx {
> diff --git a/src/man/include/ldap_id_mapping.xml
b/src/man/include/ldap_id_mapping.xml
> index
17ef803289d14fa52b725c90062ee4ba0379acd0..301a7cc9ef62e85f5553d90c0323cc614ec6938c 100644
> --- a/src/man/include/ldap_id_mapping.xml
> +++ b/src/man/include/ldap_id_mapping.xml
> @@ -243,6 +243,26 @@ ldap_schema = ad
> </para>
> </listitem>
> </varlistentry>
> + <varlistentry>
> + <term>ldap_idmap_extra_slice_init (integer)</term>
Previously used max was supposed to imply that there will be maximal such number of
slices, later init was used as the number could grove above this number during run
time....name fill change in next iteration, might to some think
ldap_idmap_cut_off_limit - as it will describe how many 'additional/secundary'
slices will be tried during unix_to_sid() conversion.
^^^^^
Could you explain the suffix?
I cannot see the string "init" anywhere
in
man page.
BTW the design document has the name ''ldap_idmap_extra_slice_max''
> + <listitem>
> + <para>
> + Number of secondary slices to use for ID mapping
> + that are generated at the startup.
> + </para>
> + <para>
> + Note: Additional secondary slices might be generated
> + when SID is being mapped to UNIX uid and RID part of
> + SID is out of range for secondary slices generated so
> + far. If value of ldap_idmap_extra_slice_init is equal
> + to 0 then no additional secondary slices are
> + generated.
> + </para>
> + <para>
> + Default: 10
I'm not sure about the defualt value.
Simo wrote few comments on IRC few days ago that this feature is not
deterministic among more machines.
e.g.
there are few users
jakub -- S-1-5-21-100-200-300-12809
simo -- S-1-5-21-100-200-300-212809
sumit -- S-1-5-21-300-200-100-12809
Let say that S-1-5-21-100-200-300 will be mapped to the slice 10
and S-1-5-21-300-200-100 to the slice 11
machine A:
* getent passwd jakub -> slice 10 is created
* getent passwd simo -> slice 11 is created based on this feature
machine B:
* getent passwd jakub -> slice 10 is created
* getent passwd sumit -> slice 11 is created
And user 'simo' will have the same UID on 'machine A' as user
'sumit' on 'machine B'. It can be a problem on nfs.
So I'm not sure we should enable this feature by default.
But on the other hand this feature might fix many users complications.
> + </para>
> + </listitem>
> + </varlistentry>
> </variablelist>
> </refsect3>
> </refsect2>
> diff --git a/src/providers/ad/ad_opts.c b/src/providers/ad/ad_opts.c
> index
4ea96637ca7264c76109ed8c2f7b5e8a94f73bfe..18a21e205cebd0ac14672ebf97b0ede54c63d8f8 100644
> --- a/src/providers/ad/ad_opts.c
> +++ b/src/providers/ad/ad_opts.c
> @@ -135,6 +135,7 @@ struct dp_option ad_def_ldap_opts[] = {
> { "ldap_idmap_autorid_compat", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
> { "ldap_idmap_default_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING
},
> { "ldap_idmap_default_domain_sid", DP_OPT_STRING, NULL_STRING,
NULL_STRING },
> + { "ldap_idmap_extra_slice_init", DP_OPT_NUMBER, { .number = 10 },
NULL_NUMBER },
> { "ldap_groups_use_matching_rule_in_chain", DP_OPT_BOOL, BOOL_FALSE,
BOOL_FALSE },
> { "ldap_initgroups_use_matching_rule_in_chain", DP_OPT_BOOL,
BOOL_FALSE, BOOL_FALSE },
> { "ldap_use_tokengroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE},
> diff --git a/src/providers/ipa/ipa_opts.c b/src/providers/ipa/ipa_opts.c
> index
bc983ec32d63c37b6fdf06d6009df9084f82d4bf..7ff6589f8cd5342d3f0ef5a8f4210013bbe06bc7 100644
> --- a/src/providers/ipa/ipa_opts.c
> +++ b/src/providers/ipa/ipa_opts.c
> @@ -147,6 +147,7 @@ struct dp_option ipa_def_ldap_opts[] = {
> { "ldap_idmap_autorid_compat", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
> { "ldap_idmap_default_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING
},
> { "ldap_idmap_default_domain_sid", DP_OPT_STRING, NULL_STRING,
NULL_STRING },
> + { "ldap_idmap_extra_slice_init", DP_OPT_NUMBER, { .number = 10 },
NULL_NUMBER },
> { "ldap_groups_use_matching_rule_in_chain", DP_OPT_BOOL, BOOL_FALSE,
BOOL_FALSE },
> { "ldap_initgroups_use_matching_rule_in_chain", DP_OPT_BOOL,
BOOL_FALSE, BOOL_FALSE },
> { "ldap_use_tokengroups", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE},
> diff --git a/src/providers/ldap/ldap_opts.c b/src/providers/ldap/ldap_opts.c
> index
54926c6c31b1b6edfc1f07b22e79225c5fdddf5b..85bc42fdf8cd198e29fcadaa161f92d76092d5bd 100644
> --- a/src/providers/ldap/ldap_opts.c
> +++ b/src/providers/ldap/ldap_opts.c
> @@ -111,6 +111,7 @@ struct dp_option default_basic_opts[] = {
> { "ldap_idmap_autorid_compat", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
> { "ldap_idmap_default_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING
},
> { "ldap_idmap_default_domain_sid", DP_OPT_STRING, NULL_STRING,
NULL_STRING },
> + { "ldap_idmap_extra_slice_init", DP_OPT_NUMBER, { .number = 10 },
NULL_NUMBER },
> { "ldap_groups_use_matching_rule_in_chain", DP_OPT_BOOL, BOOL_FALSE,
BOOL_FALSE },
> { "ldap_initgroups_use_matching_rule_in_chain", DP_OPT_BOOL,
BOOL_FALSE, BOOL_FALSE },
> { "ldap_use_tokengroups", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE},
> diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
> index
edfbf229b4c4396592020de931eba5f83a8f06ed..b82c5475bcb12188fb400c62be6f682e1e2ca541 100644
> --- a/src/providers/ldap/sdap.h
> +++ b/src/providers/ldap/sdap.h
> @@ -223,6 +223,7 @@ enum sdap_basic_opt {
> SDAP_IDMAP_AUTORID_COMPAT,
> SDAP_IDMAP_DEFAULT_DOMAIN,
> SDAP_IDMAP_DEFAULT_DOMAIN_SID,
> + SDAP_IDMAP_EXTRA_SLICE_INIT,
> SDAP_AD_MATCHING_RULE_GROUPS,
> SDAP_AD_MATCHING_RULE_INITGROUPS,
> SDAP_AD_USE_TOKENGROUPS,
> diff --git a/src/providers/ldap/sdap_idmap.c b/src/providers/ldap/sdap_idmap.c
> index
4d9de8a447d46dedc8df49dc5663037248eb6996..2bdb4c290d6d2c2a70ec72e2d5e8bd72f44d0130 100644
> --- a/src/providers/ldap/sdap_idmap.c
> +++ b/src/providers/ldap/sdap_idmap.c
> @@ -94,9 +94,10 @@ sdap_idmap_add_configured_external_range(struct sdap_idmap_ctx
*idmap_ctx)
>
> id_ctx = idmap_ctx->id_ctx;
>
> - err = sss_idmap_add_domain_ex(idmap_ctx->map,
id_ctx->be->domain->name,
> - id_ctx->be->domain->domain_id,
&range,
> - NULL, 0, true);
> + err = sss_idmap_add_auto_domain_ex(idmap_ctx->map,
> + id_ctx->be->domain->name,
> + id_ctx->be->domain->domain_id,
&range,
> + NULL, 0, true);
> if (err != IDMAP_SUCCESS) {
> DEBUG(SSSDBG_CRIT_FAILURE,
> "Could not add domain [%s] to the map: [%d]\n",
> @@ -142,6 +143,7 @@ sdap_idmap_init(TALLOC_CTX *mem_ctx,
> id_t idmap_upper;
> id_t rangesize;
> bool autorid_mode;
> + int extra_slice_init;
> struct sdap_idmap_ctx *idmap_ctx = NULL;
>
> tmp_ctx = talloc_new(NULL);
> @@ -163,6 +165,8 @@ sdap_idmap_init(TALLOC_CTX *mem_ctx,
> SDAP_IDMAP_RANGESIZE);
> autorid_mode = dp_opt_get_bool(idmap_ctx->id_ctx->opts->basic,
> SDAP_IDMAP_AUTORID_COMPAT);
> + extra_slice_init = dp_opt_get_int(idmap_ctx->id_ctx->opts->basic,
> + SDAP_IDMAP_EXTRA_SLICE_INIT);
>
> /* Validate that the values make sense */
> if (rangesize <= 0
> @@ -203,6 +207,7 @@ sdap_idmap_init(TALLOC_CTX *mem_ctx,
> err |= sss_idmap_ctx_set_lower(idmap_ctx->map, idmap_lower);
> err |= sss_idmap_ctx_set_upper(idmap_ctx->map, idmap_upper);
> err |= sss_idmap_ctx_set_rangesize(idmap_ctx->map, rangesize);
> + err |= sss_idmap_ctx_set_extra_slice_init(idmap_ctx->map, extra_slice_init);
> if (err != IDMAP_SUCCESS) {
> /* This should never happen */
> DEBUG(SSSDBG_CRIT_FAILURE, "sss_idmap_ctx corrupted\n");
> @@ -377,8 +382,8 @@ sdap_idmap_add_domain(struct sdap_idmap_ctx *idmap_ctx,
> }
>
> /* Add this domain to the map */
> - err = sss_idmap_add_domain_ex(idmap_ctx->map, dom_name, dom_sid, &range,
> - NULL, 0, external_mapping);
> + err = sss_idmap_add_auto_domain_ex(idmap_ctx->map, dom_name, dom_sid,
> + &range, NULL, 0, external_mapping);
> if (err != IDMAP_SUCCESS) {
> DEBUG(SSSDBG_CRIT_FAILURE,
> "Could not add domain [%s] to the map: [%d]\n",
> diff --git a/src/tests/cmocka/test_sss_idmap.c b/src/tests/cmocka/test_sss_idmap.c
> index
1e52c8507461ab3caa04eb2c0e63410c363ba723..91e38546436bb3c248e63dcba0bbc3ba425c3483 100644
> --- a/src/tests/cmocka/test_sss_idmap.c
> +++ b/src/tests/cmocka/test_sss_idmap.c
> @@ -82,7 +82,7 @@ static int test_sss_idmap_setup(void **state)
> }
>
> static int setup_ranges(struct test_ctx *test_ctx, bool external_mapping,
> - bool second_domain)
> + bool second_domain, bool sec_slices)
> {
> struct sss_idmap_range range;
> enum idmap_error_code err;
> @@ -103,15 +103,26 @@ static int setup_ranges(struct test_ctx *test_ctx, bool
external_mapping,
> sid = TEST_DOM_SID;
> }
>
> - err = sss_idmap_add_domain_ex(test_ctx->idmap_ctx, name, sid, &range,
NULL,
> - 0, external_mapping);
> + if (sec_slices) {
> + err = sss_idmap_add_auto_domain_ex(test_ctx->idmap_ctx, name, sid,
> + &range, NULL, 0, external_mapping);
> + } else {
> + err = sss_idmap_add_domain_ex(test_ctx->idmap_ctx, name, sid,
&range,
> + NULL, 0, external_mapping);
> + }
> assert_int_equal(err, IDMAP_SUCCESS);
>
> range.min += TEST_OFFSET;
> range.max += TEST_OFFSET;
>
> - err = sss_idmap_add_domain_ex(test_ctx->idmap_ctx, name, sid, &range,
NULL,
> - TEST_OFFSET, external_mapping);
> + if (sec_slices) {
> + err = sss_idmap_add_auto_domain_ex(test_ctx->idmap_ctx, name, sid,
> + &range, NULL, TEST_OFFSET,
> + external_mapping);
> + } else {
> + err = sss_idmap_add_domain_ex(test_ctx->idmap_ctx, name, sid,
&range,
> + NULL, TEST_OFFSET, external_mapping);
> + }
> assert_int_equal(err, IDMAP_SUCCESS);
> return 0;
> }
> @@ -124,7 +135,19 @@ static int test_sss_idmap_setup_with_domains(void **state) {
> test_ctx = talloc_get_type(*state, struct test_ctx);
> assert_non_null(test_ctx);
>
> - setup_ranges(test_ctx, false, false);
> + setup_ranges(test_ctx, false, false, false);
> + return 0;
> +}
> +
> +static int test_sss_idmap_setup_with_domains_sec_slices(void **state) {
> + struct test_ctx *test_ctx;
> +
> + test_sss_idmap_setup(state);
> +
> + test_ctx = talloc_get_type(*state, struct test_ctx);
> + assert_non_null(test_ctx);
> +
> + setup_ranges(test_ctx, false, false, true);
> return 0;
> }
>
> @@ -136,7 +159,7 @@ static int test_sss_idmap_setup_with_external_mappings(void
**state) {
> test_ctx = talloc_get_type(*state, struct test_ctx);
> assert_non_null(test_ctx);
>
> - setup_ranges(test_ctx, true, false);
> + setup_ranges(test_ctx, true, false, false);
> return 0;
> }
>
> @@ -148,8 +171,8 @@ static int test_sss_idmap_setup_with_both(void **state) {
> test_ctx = talloc_get_type(*state, struct test_ctx);
> assert_non_null(test_ctx);
>
> - setup_ranges(test_ctx, false, false);
> - setup_ranges(test_ctx, true, true);
> + setup_ranges(test_ctx, false, false, false);
> + setup_ranges(test_ctx, true, true, false);
> return 0;
> }
>
> @@ -274,6 +297,48 @@ void test_map_id(void **state)
> sss_idmap_free_sid(test_ctx->idmap_ctx, sid);
> }
>
> +void test_map_id_sec_slices(void **state)
> +{
> + struct test_ctx *test_ctx;
> + enum idmap_error_code err;
> + uint32_t id;
> + char *sid = NULL;
> +
> + test_ctx = talloc_get_type(*state, struct test_ctx);
> +
> + assert_non_null(test_ctx);
> +
> + err = sss_idmap_sid_to_unix(test_ctx->idmap_ctx, TEST_DOM_SID"1-1",
&id);
> + assert_int_equal(err, IDMAP_NO_DOMAIN);
> +
> + err = sss_idmap_sid_to_unix(test_ctx->idmap_ctx,
TEST_DOM_SID"-4000000",
> + &id);
> + assert_int_equal(err, IDMAP_SUCCESS);
> + assert_int_equal(id, 351800000);
> +
> + err = sss_idmap_unix_to_sid(test_ctx->idmap_ctx, TEST_OFFSET - 1, &sid);
> + assert_int_equal(err, IDMAP_NO_DOMAIN);
> +
> + err = sss_idmap_sid_to_unix(test_ctx->idmap_ctx, TEST_DOM_SID"-0",
&id);
> + assert_int_equal(err, IDMAP_SUCCESS);
> + assert_int_equal(id, TEST_RANGE_MIN);
> +
> + err = sss_idmap_unix_to_sid(test_ctx->idmap_ctx, id, &sid);
> + assert_int_equal(err, IDMAP_SUCCESS);
> + assert_string_equal(sid, TEST_DOM_SID"-0");
> + sss_idmap_free_sid(test_ctx->idmap_ctx, sid);
> +
> + err = sss_idmap_sid_to_unix(test_ctx->idmap_ctx,
> + TEST_DOM_SID"-"TEST_OFFSET_STR, &id);
> + assert_int_equal(err, IDMAP_SUCCESS);
> + assert_int_equal(id, TEST_RANGE_MIN+TEST_OFFSET);
> +
> + err = sss_idmap_unix_to_sid(test_ctx->idmap_ctx, id, &sid);
> + assert_int_equal(err, IDMAP_SUCCESS);
> + assert_string_equal(sid, TEST_DOM_SID"-"TEST_OFFSET_STR);
> + sss_idmap_free_sid(test_ctx->idmap_ctx, sid);
> +}
> +
> void test_map_id_external(void **state)
> {
> struct test_ctx *test_ctx;
> @@ -523,6 +588,9 @@ int main(int argc, const char *argv[])
> cmocka_unit_test_setup_teardown(test_map_id,
> test_sss_idmap_setup_with_domains,
> test_sss_idmap_teardown),
> + cmocka_unit_test_setup_teardown(test_map_id_sec_slices,
> +
test_sss_idmap_setup_with_domains_sec_slices,
> + test_sss_idmap_teardown),
> cmocka_unit_test_setup_teardown(test_map_id_external,
>
test_sss_idmap_setup_with_external_mappings,
> test_sss_idmap_teardown),
> diff --git a/src/tests/sss_idmap-tests.c b/src/tests/sss_idmap-tests.c
> index
f5ec68383679bfc685467bd625c86b8d6f474d48..a1d1659f7152960f002d84eb16d244fa83618079 100644
> --- a/src/tests/sss_idmap-tests.c
> +++ b/src/tests/sss_idmap-tests.c
> @@ -68,6 +68,20 @@ void idmap_ctx_setup(void)
> fail_unless(idmap_ctx != NULL, "sss_idmap_init returned NULL.");
> }
>
> +void idmap_ctx_setup_additional_seconary_slices(void)
> +{
> + enum idmap_error_code err;
> +
> + err = sss_idmap_init(idmap_talloc, global_talloc_context, idmap_talloc_free,
> + &idmap_ctx);
> +
> + fail_unless(err == IDMAP_SUCCESS, "sss_idmap_init failed.");
> + fail_unless(idmap_ctx != NULL, "sss_idmap_init returned NULL.");
> +
> + idmap_ctx->idmap_opts.rangesize = 10;
> + idmap_ctx->idmap_opts.extra_slice_init = 5;
> +}
> +
> void idmap_ctx_teardown(void)
> {
> enum idmap_error_code err;
> @@ -85,6 +99,20 @@ void idmap_add_domain_setup(void)
> fail_unless(err == IDMAP_SUCCESS, "sss_idmap_add_domain failed.");
> }
>
> +void idmap_add_domain_with_sec_slices_setup(void)
> +{
> + enum idmap_error_code err;
> + struct sss_idmap_range range = {
> + IDMAP_RANGE_MIN,
> + IDMAP_RANGE_MIN + idmap_ctx->idmap_opts.rangesize - 1,
> + };
> +
> + err = sss_idmap_add_auto_domain_ex(idmap_ctx, "test.dom",
"S-1-5-21-1-2-3",
> + &range, NULL, 0, false);
> +
> + fail_unless(err == IDMAP_SUCCESS, "sss_idmap_add_auto_domain_ex
failed.");
> +}
> +
> START_TEST(idmap_test_is_domain_sid)
> {
> size_t c;
> @@ -225,6 +253,100 @@ START_TEST(idmap_test_sid2uid)
> }
> END_TEST
>
> +START_TEST(idmap_test_sid2uid_ss)
> +{
> + enum idmap_error_code err;
> + uint32_t id;
> +
> + err = sss_idmap_sid_to_unix(idmap_ctx, "S-1-5-21-1-2-3333-1000",
&id);
> + fail_unless(err == IDMAP_NO_DOMAIN, "sss_idmap_sid_to_unix did not detect
"
> + "unknown domain");
> +
> + /* RID out of primary and secondary range */
> + err = sss_idmap_sid_to_unix(idmap_ctx, "S-1-5-21-1-2-3-4000000",
&id);
> + fail_unless(err == IDMAP_SUCCESS, "sss_idmap_sid_to_unix failed.");
> + fail_unless(id == 351800000,
> + "sss_idmap_sid_to_unix returned wrong id, "
> + "got [%d], expected [%d].", id, 351800000);
> +
> + err = sss_idmap_sid_to_unix(idmap_ctx, "S-1-5-21-1-2-3-1000",
&id);
> + fail_unless(err == IDMAP_SUCCESS, "sss_idmap_sid_to_unix failed.");
> + fail_unless(id == (1000 + IDMAP_RANGE_MIN),
> + "sss_idmap_sid_to_unix returned wrong id, "
> + "got [%d], expected [%d].", id, 1000 + IDMAP_RANGE_MIN);
> +
> + err = sss_idmap_sid_to_unix(idmap_ctx, "S-1-5-21-1-2-3-210000",
&id);
> + fail_unless(err == IDMAP_SUCCESS, "sss_idmap_sid_to_unix failed.");
> + fail_unless(id == 1638210000,
> + "sss_idmap_sid_to_unix returned wrong id, "
> + "got [%d], expected [%d].", id, 1638210000);
> +}
> +END_TEST
> +
> +START_TEST(idmap_test_sid2uid_ext_sec_slices)
> +{
> + enum idmap_error_code err;
> + uint32_t id;
> + char *sid;
> +
> + err = sss_idmap_unix_to_sid(idmap_ctx, 351800000, &sid);
> + fail_unless(err == IDMAP_NO_DOMAIN, "sss_idmap_unix_to_sid did not detect
"
> + "id out of range");
> +
> + /* RID out of primary and secondary range */
> + err = sss_idmap_sid_to_unix(idmap_ctx, "S-1-5-21-1-2-3-4000000",
&id);
> + fail_unless(err == IDMAP_SUCCESS, "sss_idmap_sid_to_unix failed.");
> + fail_unless(id == 351800000,
> + "sss_idmap_sid_to_unix returned wrong id, "
> + "got [%d], expected [%d].", id, 351800000);
> +
> + /* Secondary ranges were expanded by sid_to_unix call */
> + err = sss_idmap_unix_to_sid(idmap_ctx, 351800000, &sid);
> + fail_unless(err == IDMAP_SUCCESS, "sss_idmap_unix_to_sid failed.");
> + fail_unless(strcmp(sid, "S-1-5-21-1-2-3-4000000") == 0,
> + "sss_idmap_unix_to_sid returned wrong SID, "
> + "expected [%s], got [%s].",
"S-1-5-21-1-2-3-4000000", sid);
> + sss_idmap_free_sid(idmap_ctx, sid);
> +}
> +END_TEST
> +
> +START_TEST(idmap_test_sid2uid_additional_secondary_slices)
> +{
> + enum idmap_error_code err;
> + struct TALLOC_CTX *tmp_ctx;
> + const char *dom_prefix = "S-1-5-21-1-2-3";
> + const int max_rid = 80;
> + const char *sids[max_rid];
> + unsigned int ids[max_rid];
> +
> + tmp_ctx = talloc_new(NULL);
> + fail_unless(tmp_ctx != NULL, "Out of memory.");
> +
> + for (unsigned int i = 0; i < max_rid + 1; i++) {
> + sids[i] = talloc_asprintf(tmp_ctx, "%s-%u", dom_prefix, i);
> +
> + fail_unless(sids[i] != NULL, "Out of memory");
> +
> + err = sss_idmap_sid_to_unix(idmap_ctx, sids[i], &ids[i]);
> + fail_unless(err == IDMAP_SUCCESS, "sss_idmap_sid_to_unix
failed.");
> + }
> +
> + for (unsigned int i = 0; i < max_rid + 1; i++) {
> + char *sid;
> +
> + err = sss_idmap_unix_to_sid(idmap_ctx, ids[i], &sid);
> + fail_unless(err == IDMAP_SUCCESS, "sss_idmap_sid_to_unix
failed.");
> +
> + fail_unless(strcmp(sid, sids[i]) == 0,
> + "sss_idmap_unix_to_sid returned wrong sid, "
> + "got [%s], expected [%s].", sid, sids[i]);
> + talloc_free(sid);
> + }
> +
> + talloc_free(tmp_ctx);
> +}
> +END_TEST
> +
> START_TEST(idmap_test_bin_sid2uid)
> {
> enum idmap_error_code err;
> @@ -284,6 +406,38 @@ START_TEST(idmap_test_uid2sid)
> }
> END_TEST
>
> +START_TEST(idmap_test_uid2sid_ss)
> +{
> + enum idmap_error_code err;
> + char *sid;
> +
> + err = sss_idmap_unix_to_sid(idmap_ctx,
> + IDMAP_RANGE_MIN + idmap_ctx->idmap_opts.rangesize
+ 1,
> + &sid);
> + fail_unless(err == IDMAP_NO_DOMAIN, "sss_idmap_unix_to_sid did not detect
"
> + "id out of range");
> +
> + err = sss_idmap_unix_to_sid(idmap_ctx, 2234, &sid);
> + fail_unless(err == IDMAP_SUCCESS, "sss_idmap_unix_to_sid failed.");
> + fail_unless(strcmp(sid, "S-1-5-21-1-2-3-1000") == 0,
> + "sss_idmap_unix_to_sid returned wrong SID, "
> + "expected [%s], got [%s].",
"S-1-5-21-1-2-3-1000", sid);
> +
> + sss_idmap_free_sid(idmap_ctx, sid);
> +
> + /* Secondary ranges */
> + err = sss_idmap_unix_to_sid(idmap_ctx,
> + 111400000,
> + &sid);
> + fail_unless(err == IDMAP_SUCCESS, "sss_idmap_unix_to_sid failed.");
> + fail_unless(strcmp(sid, "S-1-5-21-1-2-3-400000") == 0,
> + "sss_idmap_unix_to_sid returned wrong SID, "
> + "expected [%s], got [%s].",
"S-1-5-21-1-2-3-400000", sid);
> +
> + sss_idmap_free_sid(idmap_ctx, sid);
> +}
> +END_TEST
> +
> START_TEST(idmap_test_uid2dom_sid)
> {
> enum idmap_error_code err;
> @@ -618,6 +772,43 @@ Suite *idmap_test_suite (void)
>
> suite_add_tcase(s, tc_map);
>
> + /* Test secondary slices */
> + TCase *tc_map_ss = tcase_create("IDMAP mapping tests");
> + tcase_add_checked_fixture(tc_map_ss,
> + ck_leak_check_setup,
> + ck_leak_check_teardown);
> + tcase_add_checked_fixture(tc_map_ss,
> + idmap_ctx_setup,
> + idmap_ctx_teardown);
> + tcase_add_checked_fixture(tc_map_ss,
> + idmap_add_domain_with_sec_slices_setup,
> + NULL);
> +
> + tcase_add_test(tc_map_ss, idmap_test_sid2uid_ss);
> + tcase_add_test(tc_map_ss, idmap_test_uid2sid_ss);
> + tcase_add_test(tc_map_ss, idmap_test_sid2uid_ext_sec_slices);
> +
> + suite_add_tcase(s, tc_map_ss);
> +
> + /* Test additional secondary slices */
> + TCase *tc_map_additional_secondary_slices = \
> + tcase_create("IDMAP additional secondary slices");
> +
> + tcase_add_checked_fixture(tc_map_additional_secondary_slices,
> + ck_leak_check_setup,
> + ck_leak_check_teardown);
> + tcase_add_checked_fixture(tc_map_additional_secondary_slices,
> + idmap_ctx_setup_additional_seconary_slices,
> + idmap_ctx_teardown);
> + tcase_add_checked_fixture(tc_map_additional_secondary_slices,
> + idmap_add_domain_with_sec_slices_setup,
> + NULL);
> +
> + tcase_add_test(tc_map_additional_secondary_slices,
> + idmap_test_sid2uid_additional_secondary_slices);
> +
> + suite_add_tcase(s, tc_map_additional_secondary_slices);
> +
> return s;
> }
> int main(int argc, const char *argv[])
> --
> 2.4.3
>
>From 08c6caf4dd7cb0acfb2057aa3e96ea7b6fc5b898 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl(a)redhat.com>
> Date: Wed, 13 Jan 2016 09:07:39 -0500
> Subject: [PATCH 3/4] IDMAP: Fix computing max id for slice range
>
> Max value of id mapping range was 1 unit too high.
>
> Resolves:
>
https://fedorahosted.org/sssd/ticket/2922
> ---
> src/lib/idmap/sss_idmap.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
> index
fd4be746ba338891ae5686e1b9f49d7664d91694..a2272e6882355912afa122e9a208c4d530e6600a 100644
> --- a/src/lib/idmap/sss_idmap.c
> +++ b/src/lib/idmap/sss_idmap.c
> @@ -377,7 +377,7 @@ enum idmap_error_code sss_idmap_calculate_range(struct
sss_idmap_ctx *ctx,
> }
>
> min = (rangesize * new_slice) + idmap_lower;
> - max = min + rangesize;
> + max = min + rangesize - 1;
> /* Verify that this slice is not already in use */
> do {
> for (dom = ctx->idmap_domain_info; dom != NULL; dom = dom->next)
{
> @@ -394,7 +394,7 @@ enum idmap_error_code sss_idmap_calculate_range(struct
sss_idmap_ctx *ctx,
> }
>
> min = (rangesize * new_slice) + idmap_lower;
> - max = min + rangesize;
> + max = min + rangesize - 1;
> break;
> }
> }
> @@ -412,7 +412,7 @@ enum idmap_error_code sss_idmap_calculate_range(struct
sss_idmap_ctx *ctx,
> }
>
> _range->min = (rangesize * new_slice) + idmap_lower;
> - _range->max = _range->min + rangesize;
> + _range->max = _range->min + rangesize - 1;
>
> if (slice_num) {
> *slice_num = new_slice;
Thank you very much for unit test in previous patches.
But it would be good to write unit test also for this change/fix.
especially for off by one error.
>From dc8870ea03084189367545f6b66424662453173f Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl(a)redhat.com>
> Date: Tue, 19 Jan 2016 06:28:37 -0500
> Subject: [PATCH 4/4] IDMAP: change seed for hash of secondary slices
>
> Resolves:
>
https://fedorahosted.org/sssd/ticket/2188
> ---
> src/lib/idmap/sss_idmap.c | 24 +++++++++++-------------
> src/tests/cmocka/test_sss_idmap.c | 2 +-
> src/tests/sss_idmap-tests.c | 21 ++++++++++++---------
> 3 files changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
> index
a2272e6882355912afa122e9a208c4d530e6600a..63caab4eca2a5ff45d9027f4f219c66efef079f1 100644
> --- a/src/lib/idmap/sss_idmap.c
> +++ b/src/lib/idmap/sss_idmap.c
> @@ -540,13 +540,13 @@ idmap_error_code dom_check_collision(struct idmap_domain_info
*dom_list,
>
> static char*
> generate_sec_slice_name(struct sss_idmap_ctx *ctx,
> - const char *domain_name, int slice_index)
> + const char *domain_name, uint32_t rid)
> {
> - const char *SEC_SLICE_NAME_FMT = "%s#%d";
> + const char *SEC_SLICE_NAME_FMT = "%s-%d";
^^
uint32_t has different format string
@see man inttypes.h
> char *slice_name;
> int len, len2;
>
> - len = snprintf(NULL, 0, SEC_SLICE_NAME_FMT, domain_name, slice_index + 1);
> + len = snprintf(NULL, 0, SEC_SLICE_NAME_FMT, domain_name, rid);
> if (len <= 0) {
> return NULL;
> }
> @@ -557,7 +557,7 @@ generate_sec_slice_name(struct sss_idmap_ctx *ctx,
> }
>
> len2 = snprintf(slice_name, len + 1, SEC_SLICE_NAME_FMT, domain_name,
> - slice_index + 1);
> + rid);
> if (len != len2) {
> ctx->free_func(slice_name, ctx->alloc_pvt);
> return NULL;
> @@ -598,7 +598,7 @@ generate_slice(struct sss_idmap_ctx *ctx, char *slice_name,
uint32_t first_rid,
>
> static enum idmap_error_code
> get_sec_slices(struct sss_idmap_ctx *ctx,
> - const char *domain_name,
> + const char *domain_sid,
> uint32_t first_rid,
> struct idmap_range_params **_sec_slices)
> {
> @@ -609,7 +609,7 @@ get_sec_slices(struct sss_idmap_ctx *ctx,
> char *secondary_name;
>
> for (int i = 0; i < ctx->idmap_opts.extra_slice_init; i++) {
> - secondary_name = generate_sec_slice_name(ctx, domain_name, i + 1);
> + secondary_name = generate_sec_slice_name(ctx, domain_sid, first_rid);
> if (secondary_name == NULL) {
> return IDMAP_OUT_OF_MEMORY;
> }
> @@ -756,7 +756,7 @@ sss_idmap_add_auto_domain_ex(struct sss_idmap_ctx *ctx,
>
> /* Add size of primary slice for first_rid of secondary slices. */
> rid += ctx->idmap_opts.rangesize;
> - err = get_sec_slices(ctx, domain_name, rid,
> + err = get_sec_slices(ctx, domain_sid, rid,
> &ctx->idmap_domain_info->sec_ranges);
> if (err == IDMAP_SUCCESS) {
> ctx->idmap_domain_info->use_sec_ranges = true;
> @@ -844,20 +844,18 @@ static bool comp_id(struct idmap_range_params *range_params,
long long rid,
>
> static enum idmap_error_code
> get_new_sec_slide(struct sss_idmap_ctx *ctx,
> - const char *dom_name,
> + const char *dom_sid,
> long long rid,
> struct idmap_range_params **_slice)
> {
> char *secondary_name;
> enum idmap_error_code err;
> - int index;
> int first_rid;
> struct idmap_range_params *slice;
>
> - index = rid / ctx->idmap_opts.rangesize;
> - first_rid = index * ctx->idmap_opts.rangesize;
> + first_rid = (rid / ctx->idmap_opts.rangesize) *
ctx->idmap_opts.rangesize;
>
> - secondary_name = generate_sec_slice_name(ctx, dom_name, index);
> + secondary_name = generate_sec_slice_name(ctx, dom_sid, first_rid);
> if (secondary_name == NULL) {
> return IDMAP_OUT_OF_MEMORY;
> }
> @@ -899,7 +897,7 @@ try_secondary_slices(struct sss_idmap_ctx *ctx,
> }
>
> /* If no secondary slide matched then new slide is added. */
> - err = get_new_sec_slide(ctx, matched_dom->name, rid, &new_slice);
> + err = get_new_sec_slide(ctx, matched_dom->sid, rid, &new_slice);
> if (err != IDMAP_SUCCESS) {
> return err;
> }
> diff --git a/src/tests/cmocka/test_sss_idmap.c b/src/tests/cmocka/test_sss_idmap.c
> index
91e38546436bb3c248e63dcba0bbc3ba425c3483..807a6ea173b84031c5ba6034ae59d7bed97bc6f5 100644
> --- a/src/tests/cmocka/test_sss_idmap.c
> +++ b/src/tests/cmocka/test_sss_idmap.c
> @@ -314,7 +314,7 @@ void test_map_id_sec_slices(void **state)
> err = sss_idmap_sid_to_unix(test_ctx->idmap_ctx,
TEST_DOM_SID"-4000000",
> &id);
> assert_int_equal(err, IDMAP_SUCCESS);
> - assert_int_equal(id, 351800000);
> + assert_int_equal(id, 575600000);
>
> err = sss_idmap_unix_to_sid(test_ctx->idmap_ctx, TEST_OFFSET - 1, &sid);
> assert_int_equal(err, IDMAP_NO_DOMAIN);
> diff --git a/src/tests/sss_idmap-tests.c b/src/tests/sss_idmap-tests.c
> index
a1d1659f7152960f002d84eb16d244fa83618079..d3aa691fa79ca1060bd1f49b14bcbecb47448335 100644
> --- a/src/tests/sss_idmap-tests.c
> +++ b/src/tests/sss_idmap-tests.c
> @@ -257,6 +257,8 @@ START_TEST(idmap_test_sid2uid_ss)
> {
> enum idmap_error_code err;
> uint32_t id;
> + const uint32_t exp_id = 351800000;
> + const uint32_t exp_id2 = 832610000;
Previous value was a little big bigger
1638210000. Could you exaplain why did you
change that?
The algorithm for computaion of secondary range changed as seed for hash function changed
- values are different now.
>>
>> err = sss_idmap_sid_to_unix(idmap_ctx, "S-1-5-21-1-2-3333-1000",
&id);
>> fail_unless(err == IDMAP_NO_DOMAIN, "sss_idmap_sid_to_unix did not
detect "
>> @@ -265,9 +267,9 @@ START_TEST(idmap_test_sid2uid_ss)
>> /* RID out of primary and secondary range */
>> err = sss_idmap_sid_to_unix(idmap_ctx, "S-1-5-21-1-2-3-4000000",
&id);
>> fail_unless(err == IDMAP_SUCCESS, "sss_idmap_sid_to_unix
failed.");
>> - fail_unless(id == 351800000,
>> + fail_unless(id == exp_id,
>> "sss_idmap_sid_to_unix returned wrong id, "
>> - "got [%d], expected [%d].", id, 351800000);
>> + "got [%d], expected [%d].", id, exp_id);
>>
>> err = sss_idmap_sid_to_unix(idmap_ctx, "S-1-5-21-1-2-3-1000",
&id);
>> fail_unless(err == IDMAP_SUCCESS, "sss_idmap_sid_to_unix
failed.");
>> @@ -277,9 +279,9 @@ START_TEST(idmap_test_sid2uid_ss)
>>
>> err = sss_idmap_sid_to_unix(idmap_ctx, "S-1-5-21-1-2-3-210000",
&id);
>> fail_unless(err == IDMAP_SUCCESS, "sss_idmap_sid_to_unix
failed.");
>> - fail_unless(id == 1638210000,
>> + fail_unless(id == exp_id2,
>> "sss_idmap_sid_to_unix returned wrong id, "
>> - "got [%d], expected [%d].", id, 1638210000);
>> + "got [%d], expected [%d].", id, exp_id2);
>> }
>> END_TEST
>>
>> @@ -288,20 +290,21 @@ START_TEST(idmap_test_sid2uid_ext_sec_slices)
>> enum idmap_error_code err;
>> uint32_t id;
>> char *sid;
>> + const uint32_t exp_id = 351800000;
>>
>> - err = sss_idmap_unix_to_sid(idmap_ctx, 351800000, &sid);
>> + err = sss_idmap_unix_to_sid(idmap_ctx, exp_id, &sid);
>> fail_unless(err == IDMAP_NO_DOMAIN, "sss_idmap_unix_to_sid did not
detect "
>> "id out of range");
>>
>> /* RID out of primary and secondary range */
>> err = sss_idmap_sid_to_unix(idmap_ctx, "S-1-5-21-1-2-3-4000000",
&id);
>> fail_unless(err == IDMAP_SUCCESS, "sss_idmap_sid_to_unix
failed.");
>> - fail_unless(id == 351800000,
>> + fail_unless(id == exp_id,
>> "sss_idmap_sid_to_unix returned wrong id, "
>> - "got [%d], expected [%d].", id, 351800000);
>> + "got [%d], expected [%d].", id, exp_id);
>>
>> /* Secondary ranges were expanded by sid_to_unix call */
>> - err = sss_idmap_unix_to_sid(idmap_ctx, 351800000, &sid);
>> + err = sss_idmap_unix_to_sid(idmap_ctx, exp_id, &sid);
>> fail_unless(err == IDMAP_SUCCESS, "sss_idmap_unix_to_sid
failed.");
>> fail_unless(strcmp(sid, "S-1-5-21-1-2-3-4000000") == 0,
>> "sss_idmap_unix_to_sid returned wrong SID, "
>> @@ -427,7 +430,7 @@ START_TEST(idmap_test_uid2sid_ss)
>>
>> /* Secondary ranges */
>> err = sss_idmap_unix_to_sid(idmap_ctx,
>> - 111400000,
>> + 313800000,
>> &sid);
>> fail_unless(err == IDMAP_SUCCESS, "sss_idmap_unix_to_sid
failed.");
>> fail_unless(strcmp(sid, "S-1-5-21-1-2-3-400000") == 0,
>> --
>> 2.4.3
>>
>
>> _______________________________________________
>> sssd-devel mailing list
>> sssd-devel(a)lists.fedorahosted.org
>>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>