On Wed, Jun 06, 2012 at 08:03:28AM +0200, Jakub Hrozek wrote: > On Tue, Jun 05, 2012 at 06:34:20PM +0200, Sumit Bose wrote: > > On Mon, Jun 04, 2012 at 02:49:48PM +0200, Sumit Bose wrote: > > > On Mon, Jun 04, 2012 at 02:15:37PM +0200, Jakub Hrozek wrote: > > > > On Fri, Jun 01, 2012 at 01:06:11PM +0200, Sumit Bose wrote: > > > > > On Thu, May 31, 2012 at 09:09:53AM +0200, Jakub Hrozek wrote: > > > > > > On Thu, May 24, 2012 at 03:04:36PM +0200, Sumit Bose wrote: > > > > > > > Hi, > > > > > > > > > > > > > > this patch allows us besides other conversions to convert the > > > > > > > dom_sid > > > > > > > structure used by samba to strings and back. This structure is > > > > > > > used by > > > > > > > various samba libraries, but there are no public inferfaces for > > > > > > > the > > > > > > > conversion. I've seen Simo adding code to the IPA kdb plugin > > > > > > > doing these > > > > > > > conversions and I need them for the PAC responder as well. So I > > > > > > > thought > > > > > > > it might be useful to put it in a library. > > > > > > > > > > > > > > bye, > > > > > > > Sumit > > > > > > > > > > > > Hi, the patch no longer applies to configure.ac, can you rebase? > > > > > > > > > > sure, new version attached. > > > > > > > > > > bye, > > > > > Sumit > > > > > > > > The code looks good to me and the unit tests pass. > > > > > > > > But the patch adds a new build-time dependency that should be added into > > > > contrib/sssd.spec.in. I also wonder if there should be a configure-time > > > > > > ah, I have this in the PAC responder patch. I will extract it there and > > > add it here. > > > > > > > switch to compile this feature in or out, mainly because nothing is > > > > using the feature at the moment? > > > > > > When starting the patch I was thinking the same, but then I realized > > > that this would change the API/ABI of the library. So the samba header > > > files are now required at compile time, but not at run time. > > > > > > > After some discussion on irc it became clear that we what to avoid any > > kind of dependencies here, because the needed header files are often not > > packaged. The new version of the patch copies the definition of the > > struct to reach this. > > > > bye, > > Sumit > > I'm not sure the new functions should use talloc_free() directly, I > think they should use ctx->free_func, there's no guarantee that > alloc_func will be talloc.
Thank you for catching this, you are completely right, new version attached. bye, Sumit
From 39c9adc87abf0e2a89688c50aac7878eaad83c62 Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> Date: Thu, 24 May 2012 12:39:56 +0200 Subject: [PATCH] sss_idmap: add support for samba struct dom_sid The samba ndr libraries use struct dom_sid to handle SIDs. Since there is no public samba library which offers conversion from other representations, e.g. as string, this is added to libsss_idmap. To avoid compile-time or run-time dependency to any samba library or header file the definition of the struct is copied here. --- src/lib/idmap/sss_idmap.h | 107 ++++++++++++++++++++++ src/lib/idmap/sss_idmap_conv.c | 179 +++++++++++++++++++++++++++++++++++++ src/lib/idmap/sss_idmap_private.h | 19 ++++ src/tests/sss_idmap-tests.c | 102 +++++++++++++++++++++- 4 files changed, 405 insertions(+), 2 deletions(-) diff --git a/src/lib/idmap/sss_idmap.h b/src/lib/idmap/sss_idmap.h index a3ec919..6b7cbe5 100644 --- a/src/lib/idmap/sss_idmap.h +++ b/src/lib/idmap/sss_idmap.h @@ -98,6 +98,13 @@ struct sss_dom_sid; struct sss_idmap_ctx; /** + * Placeholder for Samba's struct dom_sid. Consumers of libsss_idmap should + * include an appropriate Samba header file to define struct dom_sid. We use + * it here to avoid a hard dependency on Samba devel packages. + */ +struct dom_sid; + +/** * @brief Initialize idmap context * * @param[in] alloc_func Function to allocate memory for the context, if @@ -375,6 +382,106 @@ enum idmap_error_code sss_idmap_dom_sid_to_sid(struct sss_idmap_ctx *ctx, enum idmap_error_code sss_idmap_sid_to_dom_sid(struct sss_idmap_ctx *ctx, const char *sid, struct sss_dom_sid **dom_sid); + +/** + * @brief Convert SID string to Samba dom_sid structure + * + * @param[in] ctx Idmap context + * @param[in] sid Zero-terminated string representation of the SID + * @param[out] smb_sid Samba dom_sid structure, + * must be freed if not needed anymore + * + * @return + * - #IDMAP_SID_INVALID: Given SID is invalid + * - #IDMAP_OUT_OF_MEMORY: Failed to allocate memory for the result + */ +enum idmap_error_code sss_idmap_sid_to_smb_sid(struct sss_idmap_ctx *ctx, + const char *sid, + struct dom_sid **smb_sid); + +/** + * @brief Convert Samba dom_sid structure to SID string + * + * @param[in] ctx Idmap context + * @param[in] smb_sid Samba dom_sid structure + * @param[out] sid Zero-terminated string representation of the SID, + * must be freed if not needed anymore + * + * @return + * - #IDMAP_SID_INVALID: Given SID is invalid + * - #IDMAP_OUT_OF_MEMORY: Failed to allocate memory for the result + */ +enum idmap_error_code sss_idmap_smb_sid_to_sid(struct sss_idmap_ctx *ctx, + struct dom_sid *smb_sid, + char **sid); + +/** + * @brief Convert SID stucture to Samba dom_sid structure + * + * @param[in] ctx Idmap context + * @param[in] dom_sid SID structure + * @param[out] smb_sid Samba dom_sid structure, + * must be freed if not needed anymore + * + * @return + * - #IDMAP_SID_INVALID: Given SID is invalid + * - #IDMAP_OUT_OF_MEMORY: Failed to allocate memory for the result + */ +enum idmap_error_code sss_idmap_dom_sid_to_smb_sid(struct sss_idmap_ctx *ctx, + struct sss_dom_sid *dom_sid, + struct dom_sid **smb_sid); + +/** + * @brief Convert Samba dom_sid structure to SID structure + * + * @param[in] ctx Idmap context + * @param[in] smb_sid Samba dom_sid structure + * @param[out] dom_sid SID structure, + * must be freed if not needed anymore + * + * @return + * - #IDMAP_SID_INVALID: Given SID is invalid + * - #IDMAP_OUT_OF_MEMORY: Failed to allocate memory for the result + */ +enum idmap_error_code sss_idmap_smb_sid_to_dom_sid(struct sss_idmap_ctx *ctx, + struct dom_sid *smb_sid, + struct sss_dom_sid **dom_sid); + +/** + * @brief Convert binary SID to Samba dom_sid structure + * + * @param[in] ctx Idmap context + * @param[in] bin_sid Array with the binary SID + * @param[in] length Size of the array containing the binary SID + * @param[out] smb_sid Samba dom_sid structure, + * must be freed if not needed anymore + * + * @return + * - #IDMAP_SID_INVALID: Given SID is invalid + * - #IDMAP_OUT_OF_MEMORY: Failed to allocate memory for the result + */ +enum idmap_error_code sss_idmap_bin_sid_to_smb_sid(struct sss_idmap_ctx *ctx, + const uint8_t *bin_sid, + size_t length, + struct dom_sid **smb_sid); + +/** + * @brief Convert Samba dom_sid structure to binary SID + * + * @param[in] ctx Idmap context + * @param[in] smb_sid Samba dom_sid structure + * @param[out] bin_sid Array with the binary SID, + * must be freed if not needed anymore + * @param[out] length Size of the array containing the binary SID + * + * @return + * - #IDMAP_SID_INVALID: Given SID is invalid + * - #IDMAP_OUT_OF_MEMORY: Failed to allocate memory for the result + */ +enum idmap_error_code sss_idmap_smb_sid_to_bin_sid(struct sss_idmap_ctx *ctx, + struct dom_sid *smb_sid, + uint8_t **bin_sid, + size_t *length); /** * @} */ diff --git a/src/lib/idmap/sss_idmap_conv.c b/src/lib/idmap/sss_idmap_conv.c index df96fcc..1d3ffb3 100644 --- a/src/lib/idmap/sss_idmap_conv.c +++ b/src/lib/idmap/sss_idmap_conv.c @@ -59,6 +59,7 @@ enum idmap_error_code sss_idmap_bin_sid_to_dom_sid(struct sss_idmap_ctx *ctx, if (dom_sid == NULL) { return IDMAP_OUT_OF_MEMORY; } + memset(dom_sid, 0, sizeof(struct sss_dom_sid)); /* Safely copy in the SID revision number */ dom_sid->sid_rev_num = (uint8_t) *(bin_sid + p); @@ -387,3 +388,181 @@ done: return err; } + +enum idmap_error_code sss_idmap_sid_to_smb_sid(struct sss_idmap_ctx *ctx, + const char *sid, + struct dom_sid **_smb_sid) +{ + enum idmap_error_code err; + struct sss_dom_sid *dom_sid = NULL; + struct dom_sid *smb_sid = NULL; + + err = sss_idmap_sid_to_dom_sid(ctx, sid, &dom_sid); + if (err != IDMAP_SUCCESS) { + goto done; + } + + err = sss_idmap_dom_sid_to_smb_sid(ctx, dom_sid, &smb_sid); + if (err != IDMAP_SUCCESS) { + goto done; + } + + *_smb_sid = smb_sid; + err = IDMAP_SUCCESS; + +done: + ctx->free_func(dom_sid, ctx->alloc_pvt); + if (err != IDMAP_SUCCESS) { + ctx->free_func(smb_sid, ctx->alloc_pvt); + } + + return err; +} + +enum idmap_error_code sss_idmap_smb_sid_to_sid(struct sss_idmap_ctx *ctx, + struct dom_sid *smb_sid, + char **_sid) +{ + enum idmap_error_code err; + struct sss_dom_sid *dom_sid = NULL; + char *sid = NULL; + + err = sss_idmap_smb_sid_to_dom_sid(ctx, smb_sid, &dom_sid); + if (err != IDMAP_SUCCESS) { + goto done; + } + + err = sss_idmap_dom_sid_to_sid(ctx, dom_sid, &sid); + if (err != IDMAP_SUCCESS) { + goto done; + } + + *_sid = sid; + err = IDMAP_SUCCESS; + +done: + ctx->free_func(dom_sid, ctx->alloc_pvt); + if (err != IDMAP_SUCCESS) { + ctx->free_func(sid, ctx->alloc_pvt); + } + + return err; +} + +enum idmap_error_code sss_idmap_dom_sid_to_smb_sid(struct sss_idmap_ctx *ctx, + struct sss_dom_sid *dom_sid, + struct dom_sid **_smb_sid) +{ + struct dom_sid *smb_sid; + size_t c; + + smb_sid = ctx->alloc_func(sizeof(struct dom_sid), ctx->alloc_pvt); + if (smb_sid == NULL) { + return IDMAP_OUT_OF_MEMORY; + } + memset(smb_sid, 0, sizeof(struct dom_sid)); + + smb_sid->sid_rev_num = dom_sid->sid_rev_num; + smb_sid->num_auths = dom_sid->num_auths; + for (c = 0; c < SID_ID_AUTHS; c++) { + smb_sid->id_auth[c] = dom_sid->id_auth[c]; + } + for (c = 0; c < SID_SUB_AUTHS; c++) { + smb_sid->sub_auths[c] = dom_sid->sub_auths[c]; + } + + *_smb_sid = smb_sid; + + return IDMAP_SUCCESS; +} + +enum idmap_error_code sss_idmap_smb_sid_to_dom_sid(struct sss_idmap_ctx *ctx, + struct dom_sid *smb_sid, + struct sss_dom_sid **_dom_sid) +{ + struct sss_dom_sid *dom_sid; + size_t c; + + dom_sid = ctx->alloc_func(sizeof(struct sss_dom_sid), ctx->alloc_pvt); + if (dom_sid == NULL) { + return IDMAP_OUT_OF_MEMORY; + } + memset(dom_sid, 0, sizeof(struct sss_dom_sid)); + + dom_sid->sid_rev_num = smb_sid->sid_rev_num; + dom_sid->num_auths = smb_sid->num_auths; + for (c = 0; c < SID_ID_AUTHS; c++) { + dom_sid->id_auth[c] = smb_sid->id_auth[c]; + } + for (c = 0; c < SID_SUB_AUTHS; c++) { + dom_sid->sub_auths[c] = smb_sid->sub_auths[c]; + } + + *_dom_sid = dom_sid; + + return IDMAP_SUCCESS; +} + +enum idmap_error_code sss_idmap_bin_sid_to_smb_sid(struct sss_idmap_ctx *ctx, + const uint8_t *bin_sid, + size_t length, + struct dom_sid **_smb_sid) +{ + enum idmap_error_code err; + struct sss_dom_sid *dom_sid = NULL; + struct dom_sid *smb_sid = NULL; + + err = sss_idmap_bin_sid_to_dom_sid(ctx, bin_sid, length, &dom_sid); + if (err != IDMAP_SUCCESS) { + goto done; + } + + err = sss_idmap_dom_sid_to_smb_sid(ctx, dom_sid, &smb_sid); + if (err != IDMAP_SUCCESS) { + goto done; + } + + *_smb_sid = smb_sid; + err = IDMAP_SUCCESS; + +done: + ctx->free_func(dom_sid, ctx->alloc_pvt); + if (err != IDMAP_SUCCESS) { + ctx->free_func(smb_sid, ctx->alloc_pvt); + } + + return err; +} + +enum idmap_error_code sss_idmap_smb_sid_to_bin_sid(struct sss_idmap_ctx *ctx, + struct dom_sid *smb_sid, + uint8_t **_bin_sid, + size_t *_length) +{ + enum idmap_error_code err; + struct sss_dom_sid *dom_sid = NULL; + uint8_t *bin_sid = NULL; + size_t length; + + err = sss_idmap_smb_sid_to_dom_sid(ctx, smb_sid, &dom_sid); + if (err != IDMAP_SUCCESS) { + goto done; + } + + err = sss_idmap_dom_sid_to_bin_sid(ctx, dom_sid, &bin_sid, &length); + if (err != IDMAP_SUCCESS) { + goto done; + } + + *_bin_sid = bin_sid; + *_length = length; + err = IDMAP_SUCCESS; + +done: + ctx->free_func(dom_sid, ctx->alloc_pvt); + if (err != IDMAP_SUCCESS) { + ctx->free_func(bin_sid, ctx->alloc_pvt); + } + + return err; +} diff --git a/src/lib/idmap/sss_idmap_private.h b/src/lib/idmap/sss_idmap_private.h index 776c56a..bdb5289 100644 --- a/src/lib/idmap/sss_idmap_private.h +++ b/src/lib/idmap/sss_idmap_private.h @@ -38,4 +38,23 @@ struct sss_idmap_ctx { struct idmap_domain_info *idmap_domain_info; }; +/* This is a copy of the definition in the samba gen_ndr/security.h header + * file. We use it here to be able to offer conversions form struct dom_sid to + * string or binary representation since those are not made available by + * public samba libraries. + * + * If the definition ever changes on the samba side we have to adopt the + * change. But chances are very low that this will ever happen since e.g. this + * struct is also defined in public documentation from Microsoft. See e.g. + * section 2.4.2.3 of "[MS-DTYP]: Windows Data Types" + * http://msdn.microsoft.com/en-us/library/cc230364(v=prot.10) + */ + +struct dom_sid { + uint8_t sid_rev_num; + int8_t num_auths; + uint8_t id_auth[6]; + uint32_t sub_auths[15]; +}; + #endif /* SSS_IDMAP_PRIVATE_H_ */ diff --git a/src/tests/sss_idmap-tests.c b/src/tests/sss_idmap-tests.c index b821dfc..73c2d21 100644 --- a/src/tests/sss_idmap-tests.c +++ b/src/tests/sss_idmap-tests.c @@ -23,6 +23,7 @@ #include <check.h> #include "lib/idmap/sss_idmap.h" +#include "lib/idmap/sss_idmap_private.h" #include "tests/common.h" #define IDMAP_RANGE_MIN 1234 @@ -35,6 +36,8 @@ uint8_t test_bin_sid[] = {0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x15, 0x00}; size_t test_bin_sid_length = sizeof(test_bin_sid); +struct dom_sid test_smb_sid = {1, 5, {0, 0, 0, 0, 0, 5}, {21, 2127521184, 1604012920, 1887927527, 72713, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + struct sss_idmap_ctx *idmap_ctx; static void *idmap_talloc(size_t size, void *pvt) @@ -267,7 +270,7 @@ START_TEST(idmap_test_uid2bin_sid) } END_TEST -START_TEST(idmap_test_sid_bin2dom_sid) +START_TEST(idmap_test_bin_sid2dom_sid) { struct sss_dom_sid *dom_sid = NULL; enum idmap_error_code err; @@ -357,6 +360,96 @@ START_TEST(idmap_test_bin_sid2sid) } END_TEST +START_TEST(idmap_test_smb_sid2dom_sid) +{ + struct sss_dom_sid *dom_sid = NULL; + enum idmap_error_code err; + struct dom_sid *new_smb_sid = NULL; + + err = sss_idmap_smb_sid_to_dom_sid(idmap_ctx, &test_smb_sid, &dom_sid); + fail_unless(err == IDMAP_SUCCESS, + "Failed to convert samba dom_sid to struct sss_dom_sid."); + + err = sss_idmap_dom_sid_to_smb_sid(idmap_ctx, dom_sid, &new_smb_sid); + fail_unless(err == IDMAP_SUCCESS, + "Failed to convert struct sss_dom_sid to samba dom_sid."); + + fail_unless(memcmp(&test_smb_sid, new_smb_sid, sizeof(struct dom_sid)) == 0, + "Samba dom_sid-s do not match."); + + talloc_free(dom_sid); + talloc_free(new_smb_sid); +} +END_TEST + +START_TEST(idmap_test_smb_sid2bin_sid) +{ + enum idmap_error_code err; + size_t length; + uint8_t *bin_sid = NULL; + + err = sss_idmap_smb_sid_to_bin_sid(idmap_ctx, &test_smb_sid, + &bin_sid, &length); + fail_unless(err == IDMAP_SUCCESS, + "Failed to convert samba dom_sid to binary sid."); + fail_unless(length == test_bin_sid_length, + "Size of binary SIDs do not match, got [%d], expected [%d]", + length, test_bin_sid_length); + fail_unless(memcmp(bin_sid, test_bin_sid, test_bin_sid_length) == 0, + "Binary SIDs do not match."); + + talloc_free(bin_sid); +} +END_TEST + +START_TEST(idmap_test_bin_sid2smb_sid) +{ + enum idmap_error_code err; + struct dom_sid *smb_sid = NULL; + + err = sss_idmap_bin_sid_to_smb_sid(idmap_ctx, test_bin_sid, + test_bin_sid_length, &smb_sid); + fail_unless(err == IDMAP_SUCCESS, + "Failed to convert binary sid to samba dom_sid."); + fail_unless(memcmp(&test_smb_sid, smb_sid, sizeof(struct dom_sid)) == 0, + "Samba dom_sid structs do not match."); + + talloc_free(smb_sid); +} +END_TEST + +START_TEST(idmap_test_smb_sid2sid) +{ + enum idmap_error_code err; + char *sid = NULL; + + err = sss_idmap_smb_sid_to_sid(idmap_ctx, &test_smb_sid, &sid); + fail_unless(err == IDMAP_SUCCESS, + "Failed to convert samba dom_sid to sid string."); + fail_unless(strcmp(sid, test_sid) == 0, "SID strings do not match, " + "expected [%s], get [%s]", + test_sid, sid); + + talloc_free(sid); +} +END_TEST + +START_TEST(idmap_test_sid2smb_sid) +{ + enum idmap_error_code err; + struct dom_sid *smb_sid = NULL; + + err = sss_idmap_sid_to_smb_sid(idmap_ctx, test_sid, &smb_sid); + fail_unless(err == IDMAP_SUCCESS, + "Failed to convert binary sid to samba dom_sid."); + fail_unless(memcmp(&test_smb_sid, smb_sid, sizeof(struct dom_sid)) == 0, + "Samba dom_sid structs do not match."); + + talloc_free(smb_sid); +} +END_TEST + + Suite *idmap_test_suite (void) { Suite *s = suite_create ("IDMAP"); @@ -392,10 +485,15 @@ Suite *idmap_test_suite (void) idmap_ctx_setup, idmap_ctx_teardown); - tcase_add_test(tc_conv, idmap_test_sid_bin2dom_sid); + tcase_add_test(tc_conv, idmap_test_bin_sid2dom_sid); tcase_add_test(tc_conv, idmap_test_sid2dom_sid); tcase_add_test(tc_conv, idmap_test_sid2bin_sid); tcase_add_test(tc_conv, idmap_test_bin_sid2sid); + tcase_add_test(tc_conv, idmap_test_smb_sid2dom_sid); + tcase_add_test(tc_conv, idmap_test_smb_sid2bin_sid); + tcase_add_test(tc_conv, idmap_test_bin_sid2smb_sid); + tcase_add_test(tc_conv, idmap_test_smb_sid2sid); + tcase_add_test(tc_conv, idmap_test_sid2smb_sid); suite_add_tcase(s, tc_conv); -- 1.7.7.6
_______________________________________________ sssd-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/sssd-devel
