Hi, I run some tests on PPC and found an issue in the SID conversion functions of libidmap with respect to the byte-order. With the attached patch make test passed on big- and little-endian platforms.
bye, Sumit
From c445ba99ee01053edaf02ca8d3b034cdb3e6488c Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> Date: Thu, 3 May 2012 12:51:55 +0200 Subject: [PATCH] Fix endian issue in SID conversion Since the byte-order is only important when dealing with the binary SID the sub-auth values are stored in host order and are only converted while reading or writing the binary SID. --- src/lib/idmap/sss_idmap_conv.c | 22 +++++++++++++--------- src/tests/sss_idmap-tests.c | 4 +++- src/util/util.h | 2 ++ 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/lib/idmap/sss_idmap_conv.c b/src/lib/idmap/sss_idmap_conv.c index 83041ef..e2064f6 100644 --- a/src/lib/idmap/sss_idmap_conv.c +++ b/src/lib/idmap/sss_idmap_conv.c @@ -35,9 +35,9 @@ #define SID_SUB_AUTHS 15 struct dom_sid { uint8_t sid_rev_num; - int8_t num_auths;/* [range(0,15)] */ - uint8_t id_auth[SID_ID_AUTHS]; - uint32_t sub_auths[SID_SUB_AUTHS]; + int8_t num_auths; /* [range(0,15)] */ + uint8_t id_auth[SID_ID_AUTHS]; /* highest order byte has index 0 */ + uint32_t sub_auths[SID_SUB_AUTHS]; /* host byte-order */ }; enum idmap_error_code sss_idmap_bin_sid_to_dom_sid(struct sss_idmap_ctx *ctx, @@ -49,6 +49,7 @@ enum idmap_error_code sss_idmap_bin_sid_to_dom_sid(struct sss_idmap_ctx *ctx, struct dom_sid *dom_sid; size_t i = 0; size_t p = 0; + uint32_t val; CHECK_IDMAP_CTX(ctx, IDMAP_CONTEXT_INVALID); @@ -83,7 +84,10 @@ enum idmap_error_code sss_idmap_bin_sid_to_dom_sid(struct sss_idmap_ctx *ctx, /* Safely copy in the sub_auths values */ for (i = 0; i < dom_sid->num_auths; i++) { - SAFEALIGN_COPY_UINT32(&dom_sid->sub_auths[i], bin_sid + p, &p); + /* SID sub auth values in Active Directory are stored little-endian, + * we store them in host order */ + SAFEALIGN_COPY_UINT32(&val, bin_sid + p, &p); + dom_sid->sub_auths[i] = le32toh(val); } *_dom_sid = dom_sid; @@ -106,6 +110,7 @@ enum idmap_error_code sss_idmap_dom_sid_to_bin_sid(struct sss_idmap_ctx *ctx, size_t length; size_t i = 0; size_t p = 0; + uint32_t val; CHECK_IDMAP_CTX(ctx, IDMAP_CONTEXT_INVALID); @@ -136,7 +141,8 @@ enum idmap_error_code sss_idmap_dom_sid_to_bin_sid(struct sss_idmap_ctx *ctx, err = IDMAP_SID_INVALID; goto done; } - SAFEALIGN_COPY_UINT32(bin_sid + p, &dom_sid->sub_auths[i], &p); + val = htole32(dom_sid->sub_auths[i]); + SAFEALIGN_COPY_UINT32(bin_sid + p, &val, &p); } *_bin_sid = bin_sid; @@ -161,7 +167,6 @@ enum idmap_error_code sss_idmap_dom_sid_to_sid(struct sss_idmap_ctx *ctx, int nc; int8_t i; uint32_t id_auth_val = 0; - uint32_t sub_auth_val; if (dom_sid->num_auths > SID_SUB_AUTHS) { return IDMAP_SID_INVALID; @@ -195,10 +200,9 @@ enum idmap_error_code sss_idmap_dom_sid_to_sid(struct sss_idmap_ctx *ctx, for (i = 0; i < dom_sid->num_auths ; i++) { p += nc; sid_buf_len -= nc; - /* SID values in Active Directory are stored little-endian */ - sub_auth_val = le32toh(dom_sid->sub_auths[i]); - nc = snprintf(p, sid_buf_len, "-%lu", (unsigned long) sub_auth_val); + nc = snprintf(p, sid_buf_len, "-%lu", + (unsigned long) dom_sid->sub_auths[i]); if (nc < 0 || nc >= sid_buf_len) { err = IDMAP_SID_INVALID; goto done; diff --git a/src/tests/sss_idmap-tests.c b/src/tests/sss_idmap-tests.c index 5be7a5f..d81922f 100644 --- a/src/tests/sss_idmap-tests.c +++ b/src/tests/sss_idmap-tests.c @@ -349,7 +349,9 @@ START_TEST(idmap_test_bin_sid2sid) &sid); fail_unless(err == IDMAP_SUCCESS, "Failed to convert binary SID to SID string."); - fail_unless(strcmp(sid, test_sid) == 0, "SID strings do not match"); + fail_unless(strcmp(sid, test_sid) == 0, "SID strings do not match, " + "expected [%s], get [%s]", + test_sid, sid); talloc_free(sid); } diff --git a/src/util/util.h b/src/util/util.h index bfa0546..2912f89 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -546,8 +546,10 @@ struct sss_domain_info *copy_subdomain(TALLOC_CTX *mem_ctx, /* Conversion interfaces. */ # if __BYTE_ORDER == __LITTLE_ENDIAN # define le32toh(x) (x) +# define htole32(x) (x) # else # define le32toh(x) __bswap_32 (x) +# define htole32(x) __bswap_32 (x) # endif #endif /* __USE_BSD */ -- 1.7.7.6
_______________________________________________ sssd-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/sssd-devel
