From e18610a197aab80a32cae8c1e09b96496679bbad Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 11 Mar 2019 16:55:57 +0100 Subject: [PATCH] lib: Make sid_parse return the parsed length Use a temporary struct as a return value to make the compiler catch all callers. If we just changed bool->ssize_t, this would just generate a warning. struct sid_parse_ret will go away in the next commit Signed-off-by: Volker Lendecke Reviewed-by: Andrew Bartlett --- libcli/security/dom_sid.h | 4 +++- libcli/security/util_sid.c | 7 ++++--- source3/lib/smbldap.c | 4 ++-- source3/lib/tldap_util.c | 4 +++- source3/libads/ldap.c | 14 +++++++++----- source3/modules/vfs_default.c | 4 +++- source3/torture/torture.c | 10 +++++++--- source4/dsdb/common/util.c | 13 +++++++------ source4/torture/unix/whoami.c | 26 ++++++++++++++++---------- 9 files changed, 54 insertions(+), 32 deletions(-) diff --git a/libcli/security/dom_sid.h b/libcli/security/dom_sid.h index abaf305f96a..bc66280291b 100644 --- a/libcli/security/dom_sid.h +++ b/libcli/security/dom_sid.h @@ -112,7 +112,9 @@ bool sid_split_rid(struct dom_sid *sid, uint32_t *rid); bool sid_peek_rid(const struct dom_sid *sid, uint32_t *rid); bool sid_peek_check_rid(const struct dom_sid *exp_dom_sid, const struct dom_sid *sid, uint32_t *rid); void sid_copy(struct dom_sid *dst, const struct dom_sid *src); -bool sid_parse(const uint8_t *inbuf, size_t len, struct dom_sid *sid); +struct sid_parse_ret { ssize_t len; }; +struct sid_parse_ret sid_parse( + const uint8_t *inbuf, size_t len, struct dom_sid *sid); int sid_compare_domain(const struct dom_sid *sid1, const struct dom_sid *sid2); NTSTATUS add_sid_to_array(TALLOC_CTX *mem_ctx, const struct dom_sid *sid, struct dom_sid **sids, uint32_t *num); diff --git a/libcli/security/util_sid.c b/libcli/security/util_sid.c index 531d3809565..4a186d69cda 100644 --- a/libcli/security/util_sid.c +++ b/libcli/security/util_sid.c @@ -300,7 +300,8 @@ void sid_copy(struct dom_sid *dst, const struct dom_sid *src) Parse a on-the-wire SID to a struct dom_sid. *****************************************************************/ -bool sid_parse(const uint8_t *inbuf, size_t len, struct dom_sid *sid) +struct sid_parse_ret sid_parse( + const uint8_t *inbuf, size_t len, struct dom_sid *sid) { DATA_BLOB in = data_blob_const(inbuf, len); enum ndr_err_code ndr_err; @@ -308,9 +309,9 @@ bool sid_parse(const uint8_t *inbuf, size_t len, struct dom_sid *sid) ndr_err = ndr_pull_struct_blob_all( &in, NULL, sid, (ndr_pull_flags_fn_t)ndr_pull_dom_sid); if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - return false; + return (struct sid_parse_ret) { .len = -1 }; } - return true; + return (struct sid_parse_ret) { .len = ndr_size_dom_sid(sid, 0) }; } /***************************************************************** diff --git a/source3/lib/smbldap.c b/source3/lib/smbldap.c index 5a67ab79058..cdd350fffa1 100644 --- a/source3/lib/smbldap.c +++ b/source3/lib/smbldap.c @@ -278,7 +278,7 @@ void smbldap_set_bind_callback(struct smbldap_state *state, struct dom_sid *sid) { DATA_BLOB blob; - bool ret; + struct sid_parse_ret ret; if (!smbldap_talloc_single_blob(talloc_tos(), ld, msg, attrib, &blob)) { @@ -286,7 +286,7 @@ void smbldap_set_bind_callback(struct smbldap_state *state, } ret = sid_parse(blob.data, blob.length, sid); TALLOC_FREE(blob.data); - return ret; + return (ret.len != -1); } static int ldapmsg_destructor(LDAPMessage **result) { diff --git a/source3/lib/tldap_util.c b/source3/lib/tldap_util.c index 3938fca901f..efc37e48e7c 100644 --- a/source3/lib/tldap_util.c +++ b/source3/lib/tldap_util.c @@ -88,11 +88,13 @@ bool tldap_pull_binsid(struct tldap_message *msg, const char *attribute, struct dom_sid *sid) { DATA_BLOB val; + struct sid_parse_ret ret; if (!tldap_get_single_valueblob(msg, attribute, &val)) { return false; } - return sid_parse(val.data, val.length, sid); + ret = sid_parse(val.data, val.length, sid); + return (ret.len != -1); } bool tldap_pull_guid(struct tldap_message *msg, const char *attribute, diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c index 728c821f32d..8e0ecb87569 100644 --- a/source3/libads/ldap.c +++ b/source3/libads/ldap.c @@ -2264,10 +2264,12 @@ static void dump_sid(ADS_STRUCT *ads, const char *field, struct berval **values) { int i; for (i=0; values[i]; i++) { + struct sid_parse_ret ret; struct dom_sid sid; struct dom_sid_buf tmp; - if (!sid_parse((const uint8_t *)values[i]->bv_val, - values[i]->bv_len, &sid)) { + ret = sid_parse((const uint8_t *)values[i]->bv_val, + values[i]->bv_len, &sid); + if (ret.len == -1) { return; } printf("%s: %s\n", field, dom_sid_str_buf(&sid, &tmp)); @@ -2773,7 +2775,6 @@ int ads_count_replies(ADS_STRUCT *ads, void *res) LDAPMessage *msg, const char *field, struct dom_sid **sids) { struct berval **values; - bool ret; int count, i; values = ldap_get_values_len(ads->ldap.ld, msg, field); @@ -2796,9 +2797,10 @@ int ads_count_replies(ADS_STRUCT *ads, void *res) count = 0; for (i=0; values[i]; i++) { + struct sid_parse_ret ret; ret = sid_parse((const uint8_t *)values[i]->bv_val, values[i]->bv_len, &(*sids)[count]); - if (ret) { + if (ret.len != -1) { struct dom_sid_buf buf; DBG_DEBUG("pulling SID: %s\n", dom_sid_str_buf(&(*sids)[count], &buf)); @@ -3356,6 +3358,7 @@ ADS_STATUS ads_get_sid_from_extended_dn(TALLOC_CTX *mem_ctx, } break; case ADS_EXTENDED_DN_HEX_STRING: { + struct sid_parse_ret ret; fstring buf; size_t buf_len; @@ -3364,7 +3367,8 @@ ADS_STATUS ads_get_sid_from_extended_dn(TALLOC_CTX *mem_ctx, return ADS_ERROR_NT(NT_STATUS_INVALID_PARAMETER); } - if (!sid_parse((const uint8_t *)buf, buf_len, sid)) { + ret = sid_parse((const uint8_t *)buf, buf_len, sid); + if (ret.len == -1) { DEBUG(10,("failed to parse sid\n")); return ADS_ERROR_NT(NT_STATUS_INVALID_PARAMETER); } diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 1ed2c810667..7ce3775e54f 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1355,6 +1355,7 @@ static NTSTATUS vfswrap_fsctl(struct vfs_handle_struct *handle, * * but I have to check that --metze */ + struct sid_parse_ret ret; struct dom_sid sid; struct dom_sid_buf buf; uid_t uid; @@ -1373,7 +1374,8 @@ static NTSTATUS vfswrap_fsctl(struct vfs_handle_struct *handle, /* unknown 4 bytes: this is not the length of the sid :-( */ /*unknown = IVAL(pdata,0);*/ - if (!sid_parse(_in_data + 4, sid_len, &sid)) { + ret = sid_parse(_in_data + 4, sid_len, &sid); + if (ret.len == -1) { return NT_STATUS_INVALID_PARAMETER; } DEBUGADD(10, ("for SID: %s\n", diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 3df5e409c57..7a209859b3f 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -10878,6 +10878,7 @@ static bool run_local_sid_to_string(int dummy) { } static bool run_local_binary_to_sid(int dummy) { + struct sid_parse_ret ret; struct dom_sid *sid = talloc(NULL, struct dom_sid); static const uint8_t good_binary_sid[] = { 0x1, /* revision number */ @@ -10962,13 +10963,16 @@ static bool run_local_binary_to_sid(int dummy) { 0x1, 0x1, 0x1, 0x1, /* auth[31] */ }; - if (!sid_parse(good_binary_sid, sizeof(good_binary_sid), sid)) { + ret = sid_parse(good_binary_sid, sizeof(good_binary_sid), sid); + if (ret.len == -1) { return false; } - if (sid_parse(long_binary_sid2, sizeof(long_binary_sid2), sid)) { + ret = sid_parse(long_binary_sid2, sizeof(long_binary_sid2), sid); + if (ret.len != -1) { return false; } - if (sid_parse(long_binary_sid, sizeof(long_binary_sid), sid)) { + ret = sid_parse(long_binary_sid, sizeof(long_binary_sid), sid); + if (ret.len != -1) { return false; } return true; diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 7cc9729bc3f..411d1dd22b0 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -342,7 +342,7 @@ uint32_t samdb_result_rid_from_sid(TALLOC_CTX *mem_ctx, const struct ldb_message struct dom_sid *samdb_result_dom_sid(TALLOC_CTX *mem_ctx, const struct ldb_message *msg, const char *attr) { - bool ok; + struct sid_parse_ret ret; const struct ldb_val *v; struct dom_sid *sid; v = ldb_msg_find_ldb_val(msg, attr); @@ -353,8 +353,8 @@ struct dom_sid *samdb_result_dom_sid(TALLOC_CTX *mem_ctx, const struct ldb_messa if (sid == NULL) { return NULL; } - ok = sid_parse(v->data, v->length, sid); - if (!ok) { + ret = sid_parse(v->data, v->length, sid); + if (ret.len == -1) { talloc_free(sid); return NULL; } @@ -5809,7 +5809,8 @@ static int dsdb_count_domain_callback( case LDB_REPLY_ENTRY: { struct dsdb_count_domain_context *context = NULL; - bool ok, in_domain; + struct sid_parse_ret ret; + bool in_domain; struct dom_sid sid; const struct ldb_val *v; @@ -5824,8 +5825,8 @@ static int dsdb_count_domain_callback( break; } - ok = sid_parse(v->data, v->length, &sid); - if (!ok) { + ret = sid_parse(v->data, v->length, &sid); + if (ret.len == -1) { break; } diff --git a/source4/torture/unix/whoami.c b/source4/torture/unix/whoami.c index efd9efaab57..f554c9e1711 100644 --- a/source4/torture/unix/whoami.c +++ b/source4/torture/unix/whoami.c @@ -303,11 +303,13 @@ static bool test_against_ldap(struct torture_context *torture, struct ldb_contex for (i = 0; i < el->num_values; i++) { struct dom_sid *sid = talloc(torture, struct dom_sid); + struct sid_parse_ret ret; torture_assert(torture, sid != NULL, "talloc failed"); - + + ret = sid_parse(el->values[i].data, + el->values[i].length, sid); torture_assert(torture, - sid_parse(el->values[i].data, - el->values[i].length, sid), + ret.len != -1, "sid parse failed"); torture_assert_str_equal(torture, dom_sid_string(sid, sid), dom_sid_string(sid, whoami->sid_list[i]), "SID from LDAP and SID from CIFS does not match!"); talloc_free(sid); @@ -318,20 +320,24 @@ static bool test_against_ldap(struct torture_context *torture, struct ldb_contex struct dom_sid *dom_sid = talloc(torture, struct dom_sid); struct dom_sid *dc_sids = talloc_array(torture, struct dom_sid, el->num_values); struct dom_sid *member_sids = talloc_array(torture, struct dom_sid, whoami->num_sids); + struct sid_parse_ret ret; torture_assert(torture, user_sid != NULL, "talloc failed"); - torture_assert(torture, sid_parse(el->values[0].data, - el->values[0].length, - user_sid), + ret = sid_parse(el->values[0].data, + el->values[0].length, + user_sid); + torture_assert(torture, + ret.len != -1, "sid parse failed"); torture_assert_ntstatus_equal(torture, dom_sid_split_rid(torture, user_sid, &dom_sid, NULL), NT_STATUS_OK, "failed to split domain SID from user SID"); for (i = 0; i < el->num_values; i++) { struct dom_sid *sid = talloc(dc_sids, struct dom_sid); torture_assert(torture, sid != NULL, "talloc failed"); - + + ret = sid_parse(el->values[i].data, + el->values[i].length, + sid); torture_assert(torture, - sid_parse(el->values[i].data, - el->values[i].length, - sid), + ret.len != -1, "sid parse failed"); if (dom_sid_in_domain(dom_sid, sid)) { dc_sids[num_domain_sids_dc] = *sid; -- 2.34.1