diff options
| -rw-r--r-- | src/providers/data_provider_be.c | 72 | ||||
| -rw-r--r-- | src/providers/proxy/proxy_init.c | 19 | ||||
| -rw-r--r-- | src/responder/nss/nsssrv.c | 23 | ||||
| -rw-r--r-- | src/sbus/sssd_dbus.h | 49 | ||||
| -rw-r--r-- | src/sbus/sssd_dbus_request.c | 106 | ||||
| -rw-r--r-- | src/tests/sbus_tests.c | 99 |
6 files changed, 287 insertions, 81 deletions
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index 6c0a82360..0957bedc0 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -564,7 +564,6 @@ static int be_get_subdomains(struct sbus_request *dbus_req, void *user_data) struct be_subdom_req *req; struct be_req *be_req = NULL; struct be_client *becli; - DBusError dbus_error; dbus_bool_t force; char *domain_hint; dbus_uint16_t err_maj; @@ -575,17 +574,11 @@ static int be_get_subdomains(struct sbus_request *dbus_req, void *user_data) becli = talloc_get_type(user_data, struct be_client); if (!becli) return EINVAL; - dbus_error_init(&dbus_error); - - ret = dbus_message_get_args(dbus_req->message, &dbus_error, - DBUS_TYPE_BOOLEAN, &force, - DBUS_TYPE_STRING, &domain_hint, - DBUS_TYPE_INVALID); - if (!ret) { - DEBUG(SSSDBG_CRIT_FAILURE,"Failed, to parse message!\n"); - if (dbus_error_is_set(&dbus_error)) dbus_error_free(&dbus_error); - return EIO; - } + if (!sbus_request_parse_or_finish(dbus_req, + DBUS_TYPE_BOOLEAN, &force, + DBUS_TYPE_STRING, &domain_hint, + DBUS_TYPE_INVALID)) + return EOK; /* handled */ /* return an error if corresponding backend target is not configured */ if (becli->bectx->bet_info[BET_SUBDOMAINS].bet_ops == NULL) { @@ -1042,7 +1035,6 @@ static int be_get_account_info(struct sbus_request *dbus_req, void *user_data) struct be_acct_req *req; struct be_req *be_req; struct be_client *becli; - DBusError dbus_error; uint32_t type; char *filter; char *domain; @@ -1057,19 +1049,13 @@ static int be_get_account_info(struct sbus_request *dbus_req, void *user_data) becli = talloc_get_type(user_data, struct be_client); if (!becli) return EINVAL; - dbus_error_init(&dbus_error); - - ret = dbus_message_get_args(dbus_req->message, &dbus_error, - DBUS_TYPE_UINT32, &type, - DBUS_TYPE_UINT32, &attr_type, - DBUS_TYPE_STRING, &filter, - DBUS_TYPE_STRING, &domain, - DBUS_TYPE_INVALID); - if (!ret) { - DEBUG(SSSDBG_CRIT_FAILURE,"Failed, to parse message!\n"); - if (dbus_error_is_set(&dbus_error)) dbus_error_free(&dbus_error); - return EIO; - } + if (!sbus_request_parse_or_finish(dbus_req, + DBUS_TYPE_UINT32, &type, + DBUS_TYPE_UINT32, &attr_type, + DBUS_TYPE_STRING, &filter, + DBUS_TYPE_STRING, &domain, + DBUS_TYPE_INVALID)) + return EOK; /* handled */ DEBUG(SSSDBG_CONF_SETTINGS, "Got request for [%u][%d][%s]\n", type, attr_type, filter); @@ -1613,7 +1599,6 @@ static void be_autofs_handler_callback(struct be_req *req, static int be_autofs_handler(struct sbus_request *dbus_req, void *user_data) { - DBusError dbus_error; struct be_client *be_cli = NULL; struct be_req *be_req = NULL; struct be_autofs_req *be_autofs_req = NULL; @@ -1633,17 +1618,11 @@ static int be_autofs_handler(struct sbus_request *dbus_req, void *user_data) return EINVAL; } - dbus_error_init(&dbus_error); - - ret = dbus_message_get_args(dbus_req->message, &dbus_error, - DBUS_TYPE_UINT32, &type, - DBUS_TYPE_STRING, &filter, - DBUS_TYPE_INVALID); - if (!ret) { - DEBUG(SSSDBG_CRIT_FAILURE, "Failed, to parse message!\n"); - if (dbus_error_is_set(&dbus_error)) dbus_error_free(&dbus_error); - return EIO; - } + if (!sbus_request_parse_or_finish(dbus_req, + DBUS_TYPE_UINT32, &type, + DBUS_TYPE_STRING, &filter, + DBUS_TYPE_INVALID)) + return EOK; /* handled */ /* If we are offline and fast reply was requested * return offline immediately @@ -1817,7 +1796,6 @@ static int be_host_handler(struct sbus_request *dbus_req, void *user_data) struct be_host_req *req; struct be_req *be_req; struct be_client *becli; - DBusError dbus_error; uint32_t flags; char *filter; int ret; @@ -1830,17 +1808,11 @@ static int be_host_handler(struct sbus_request *dbus_req, void *user_data) becli = talloc_get_type(user_data, struct be_client); if (!becli) return EINVAL; - dbus_error_init(&dbus_error); - - ret = dbus_message_get_args(dbus_req->message, &dbus_error, - DBUS_TYPE_UINT32, &flags, - DBUS_TYPE_STRING, &filter, - DBUS_TYPE_INVALID); - if (!ret) { - DEBUG(SSSDBG_CRIT_FAILURE, "Failed, to parse message!\n"); - if (dbus_error_is_set(&dbus_error)) dbus_error_free(&dbus_error); - return EIO; - } + if (!sbus_request_parse_or_finish(dbus_req, + DBUS_TYPE_UINT32, &flags, + DBUS_TYPE_STRING, &filter, + DBUS_TYPE_INVALID)) + return EOK; /* request finished */ DEBUG(SSSDBG_TRACE_LIBS, "Got request for [%u][%s]\n", flags, filter); diff --git a/src/providers/proxy/proxy_init.c b/src/providers/proxy/proxy_init.c index 0206ec15b..dd1b75826 100644 --- a/src/providers/proxy/proxy_init.c +++ b/src/providers/proxy/proxy_init.c @@ -397,10 +397,8 @@ static int client_registration(struct sbus_request *dbus_req, void *data) dbus_uint16_t version = DATA_PROVIDER_VERSION; struct sbus_connection *conn; struct proxy_client *proxy_cli; - DBusError dbus_error; dbus_uint16_t cli_ver; uint32_t cli_id; - dbus_bool_t dbret; int hret; hash_key_t key; hash_value_t value; @@ -421,19 +419,12 @@ static int client_registration(struct sbus_request *dbus_req, void *data) "Cancel proxy client ID timeout [%p]\n", proxy_cli->timeout); talloc_zfree(proxy_cli->timeout); - dbus_error_init(&dbus_error); - - dbret = dbus_message_get_args(dbus_req->message, &dbus_error, - DBUS_TYPE_UINT16, &cli_ver, - DBUS_TYPE_UINT32, &cli_id, - DBUS_TYPE_INVALID); - if (!dbret) { - DEBUG(SSSDBG_CRIT_FAILURE, - "Failed to parse message, killing connection\n"); - if (dbus_error_is_set(&dbus_error)) dbus_error_free(&dbus_error); + if (!sbus_request_parse_or_finish(dbus_req, + DBUS_TYPE_UINT16, &cli_ver, + DBUS_TYPE_UINT32, &cli_id, + DBUS_TYPE_INVALID)) { sbus_disconnect(conn); - /* FIXME: should we just talloc_zfree(conn) ? */ - return EIO; + return EOK; /* handled */ } DEBUG(SSSDBG_FUNC_DATA, "Proxy client [%"PRIu32"] connected\n", cli_id); diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c index ca4fbfd02..e4896a79d 100644 --- a/src/responder/nss/nsssrv.c +++ b/src/responder/nss/nsssrv.c @@ -311,28 +311,17 @@ static int nss_memcache_initgr_check(struct sbus_request *dbus_req, void *data) { struct resp_ctx *rctx = talloc_get_type(data, struct resp_ctx); struct nss_ctx *nctx = talloc_get_type(rctx->pvt_ctx, struct nss_ctx); - DBusError dbus_error; - dbus_bool_t dbret; char *user; char *domain; uint32_t *groups; int gnum; - dbus_error_init(&dbus_error); - - dbret = dbus_message_get_args(dbus_req->message, &dbus_error, - DBUS_TYPE_STRING, &user, - DBUS_TYPE_STRING, &domain, - DBUS_TYPE_ARRAY, DBUS_TYPE_UINT32, - &groups, &gnum, - DBUS_TYPE_INVALID); - - if (!dbret) { - DEBUG(SSSDBG_CRIT_FAILURE, "Failed, to parse message!\n"); - if (dbus_error_is_set(&dbus_error)) { - dbus_error_free(&dbus_error); - } - return EIO; + if (!sbus_request_parse_or_finish(dbus_req, + DBUS_TYPE_STRING, &user, + DBUS_TYPE_STRING, &domain, + DBUS_TYPE_ARRAY, DBUS_TYPE_UINT32, &groups, &gnum, + DBUS_TYPE_INVALID)) { + return EOK; /* handled */ } DEBUG(SSSDBG_TRACE_LIBS, diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h index ef728bd30..3bf70c8cc 100644 --- a/src/sbus/sssd_dbus.h +++ b/src/sbus/sssd_dbus.h @@ -252,4 +252,53 @@ int sbus_request_return_and_finish(struct sbus_request *dbus_req, int sbus_request_fail_and_finish(struct sbus_request *dbus_req, const DBusError *error); +/* + * Parse a DBus method call request. + * + * If parsing the method call message does not succeed, then an error is + * sent to the DBus caller and the request is finished. If this function + * returns false then @request is no longer valid. + * + * This also means if this method returns false within a handler, you should + * return EOK from the handler. The message has been handled, appropriate + * logs have been written, and everything should just move on. + * + * If the method call does not match the expected arguments, then a + * org.freedesktop.DBus.Error.InvalidArgs is returned to the caller as + * expected. + * + * The variable arguments are (unfortunately) formatted exactly the same + * as those of the dbus_message_get_args() function. Documented here: + * + * http://dbus.freedesktop.org/doc/api/html/group__DBusMessage.html + * + * Exception: You don't need to free string arrays returned by this + * function. They are automatically talloc parented to the request memory + * context and can be used until the request has been finished. + * + * Important: don't pass int or bool or such types as values to this + * function. That's not portable. Use actual dbus types. You must also pass + * pointers as the values: + * + * dbus_bool_t val1; + * dbus_int32_t val2; + * ret = sbus_request_parse_or_finish(request, + * DBUS_TYPE_BOOLEAN, &val1, + * DBUS_TYPE_INT32, &val2, + * DBUS_TYPE_INVALID); + * + * To pass arrays to this function, use the following syntax. Never + * pass actual C arrays with [] syntax to this function. The C standard is + * rather vague with C arrays and varargs, and it just plain doesn't work. + * + * int count; // yes, a plain int + * const char **array; + * ret = sbus_request_parse_or_finish(request, + * DBUS_TYPE_ARRAY, DBUS_TYPE_STRING, &array, &count, + * DBUS_TYPE_INVALID); + */ +bool sbus_request_parse_or_finish(struct sbus_request *request, + int first_arg_type, + ...); + #endif /* _SSSD_DBUS_H_*/ diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c index 973089c4b..c45f3a9fd 100644 --- a/src/sbus/sssd_dbus_request.c +++ b/src/sbus/sssd_dbus_request.c @@ -110,3 +110,109 @@ int sbus_request_fail_and_finish(struct sbus_request *dbus_req, dbus_message_unref(reply); return ret; } + +struct array_arg { + char **dbus_array; +}; + +static int array_arg_destructor(struct array_arg *arg) +{ + dbus_free_string_array(arg->dbus_array); + return 0; +} + +static bool +parent_dbus_string_arrays(struct sbus_request *request, int first_arg_type, + va_list va) +{ + struct array_arg *array_arg; + int arg_type; + void **arg_ptr; + + /* + * Here we iterate through the entire thing again and look for + * things we need to fix allocation for. Normally certain types + * returned from dbus_message_get_args() and friends require + * later freeing. We tie those to the talloc context here. + * + * The list of argument has already been validated by the previous + * dbus_message_get_args() call, so we can be cheap. + */ + + arg_type = first_arg_type; + while (arg_type != DBUS_TYPE_INVALID) { + + if (arg_type == DBUS_TYPE_ARRAY) { + arg_type = va_arg(va, int); /* the array element type */ + arg_ptr = va_arg(va, void **); /* the array elements */ + va_arg(va, int *); /* the array length */ + + /* Arrays of these things need to be freed */ + if (arg_type == DBUS_TYPE_STRING || + arg_type == DBUS_TYPE_OBJECT_PATH || + arg_type == DBUS_TYPE_SIGNATURE) { + + array_arg = talloc_zero(request, struct array_arg); + if(array_arg == NULL) { + /* no kidding ... */ + DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory while trying not to leak memory\n"); + return false; + } + + array_arg->dbus_array = *arg_ptr; + talloc_set_destructor(array_arg, array_arg_destructor); + } + + /* A non array argument */ + } else { + arg_ptr = va_arg(va, void**); + } + + /* The next type */ + arg_type = va_arg(va, int); + } + + return true; +} + +bool +sbus_request_parse_or_finish(struct sbus_request *request, + int first_arg_type, + ...) +{ + DBusError error = DBUS_ERROR_INIT; + bool ret = true; + va_list va2; + va_list va; + + va_start(va, first_arg_type); + va_copy(va2, va); + + if (dbus_message_get_args_valist(request->message, &error, + first_arg_type, va)) { + ret = parent_dbus_string_arrays (request, first_arg_type, va2); + + } else { + /* Trying to send the error back to the caller in this case is a joke */ + if (!dbus_error_is_set(&error) || dbus_error_has_name(&error, DBUS_ERROR_NO_MEMORY)) { + DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory parsing DBus message\n"); + sbus_request_finish(request, NULL); + + /* Log other errors and send them back, this include o.f.d.InvalidArgs */ + } else { + DEBUG(SSSDBG_OP_FAILURE, "Couldn't parse DBus message %s.%s: %s\n", + dbus_message_get_interface(request->message), + dbus_message_get_member(request->message), + error.message); + sbus_request_fail_and_finish(request, &error); + } + + dbus_error_free(&error); + ret = false; + } + + va_end(va2); + va_end(va); + + return ret; +} diff --git a/src/tests/sbus_tests.c b/src/tests/sbus_tests.c index 7290fe7db..b8cd4abb0 100644 --- a/src/tests/sbus_tests.c +++ b/src/tests/sbus_tests.c @@ -42,11 +42,13 @@ #define PILOT_IFACE "test.Pilot" #define PILOT_BLINK "Blink" +#define PILOT_EAT "Eat" /* our vtable */ struct pilot_vtable { struct sbus_vtable vtable; sbus_msg_handler_fn Blink; + sbus_msg_handler_fn Eat; }; const struct sbus_method_meta pilot_methods[] = { @@ -56,6 +58,12 @@ const struct sbus_method_meta pilot_methods[] = { NULL, /* out args: manually parsed */ offsetof(struct pilot_vtable, Blink), }, + { + PILOT_EAT, /* method name */ + NULL, /* in args: manually parsed */ + NULL, /* out args: manually parsed */ + offsetof(struct pilot_vtable, Eat), + }, { NULL, } }; @@ -104,9 +112,35 @@ static int blink_handler(struct sbus_request *req, void *data) DBUS_TYPE_INVALID); } +static int eat_handler(struct sbus_request *req, void *data) +{ + dbus_int32_t integer; + dbus_bool_t boolean; + const char **array; + int count; + + if (!sbus_request_parse_or_finish (req, + DBUS_TYPE_INT32, &integer, + DBUS_TYPE_ARRAY, DBUS_TYPE_STRING, &array, &count, + DBUS_TYPE_BOOLEAN, &boolean, + DBUS_TYPE_INVALID)) { + return EOK; /* handled */ + } + + ck_assert_int_eq(integer, 5); + ck_assert(boolean == TRUE); + ck_assert_int_eq(count, 3); + ck_assert_str_eq(array[0], "one"); + ck_assert_str_eq(array[1], "two"); + ck_assert_str_eq(array[2], "three"); + + return sbus_request_return_and_finish(req, DBUS_TYPE_INVALID); +} + struct pilot_vtable pilot_impl = { { &pilot_meta, 0 }, .Blink = blink_handler, + .Eat = eat_handler, }; static int pilot_test_server_init(struct sbus_connection *server, void *unused) @@ -179,11 +213,76 @@ START_TEST(test_raw_handler) } END_TEST +START_TEST(test_request_parse_ok) +{ + const char *args[] = { "one", "two", "three" }; + const char **array; + TALLOC_CTX *ctx; + DBusConnection *client; + DBusError error = DBUS_ERROR_INIT; + DBusMessage *reply; + dbus_bool_t boolean; + dbus_int32_t integer; + int count; + + ctx = talloc_new(NULL); + client = test_dbus_setup_mock(ctx, NULL, pilot_test_server_init, NULL); + + boolean = TRUE; + integer = 5; + count = 3; + array = args; + reply = test_dbus_call_sync(client, + "/test/leela", + PILOT_IFACE, + PILOT_EAT, + &error, + DBUS_TYPE_INT32, &integer, + DBUS_TYPE_ARRAY, DBUS_TYPE_STRING, &array, count, + DBUS_TYPE_BOOLEAN, &boolean, + DBUS_TYPE_INVALID); + ck_assert(reply != NULL); + ck_assert(!dbus_error_is_set(&error)); + ck_assert(dbus_message_get_args(reply, NULL, + DBUS_TYPE_INVALID)); + dbus_message_unref (reply); + + talloc_free(ctx); +} +END_TEST + +START_TEST(test_request_parse_bad_args) +{ + TALLOC_CTX *ctx; + DBusConnection *client; + DBusError error = DBUS_ERROR_INIT; + DBusMessage *reply; + + ctx = talloc_new(NULL); + client = test_dbus_setup_mock(ctx, NULL, pilot_test_server_init, NULL); + + reply = test_dbus_call_sync(client, + "/test/leela", + PILOT_IFACE, + PILOT_EAT, + &error, + DBUS_TYPE_INVALID); /* bad agruments */ + ck_assert(reply == NULL); + ck_assert(dbus_error_is_set(&error)); + ck_assert(dbus_error_has_name(&error, DBUS_ERROR_INVALID_ARGS)); + dbus_error_free(&error); + + talloc_free(ctx); +} +END_TEST + TCase *create_sbus_tests(void) { TCase *tc = tcase_create("tests"); tcase_add_test(tc, test_raw_handler); + tcase_add_test(tc, test_request_parse_ok); + tcase_add_test(tc, test_request_parse_bad_args); return tc; } |