[SSSD] [PATCH] IFP: Add a utility function to reply with an object path
Jakub Hrozek
jhrozek at redhat.com
Thu May 15 19:04:34 UTC 2014
On Thu, May 15, 2014 at 08:54:53PM +0200, Lukas Slebodnik wrote:
> You forgot to fix coverity issue
> https://lists.fedorahosted.org/pipermail/sssd-devel/2014-May/019503.html
>
> LS
I did, thanks.
-------------- next part --------------
>From 4da1f3d1adb1806229ebf04c7bc367d783a57a88 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sun, 11 May 2014 16:15:24 +0200
Subject: [PATCH 1/3] IFP: Add utility functions to escape and unescape object
paths
D-Bus spec says:
Each element must only contain the ASCII characters "[A-Z][a-z][0-9]_"
This patch adds two utility functions to escape raw input into format
suitable for DBus and conversely transform escaped paths back into raw
paths.
---
src/responder/ifp/ifp_private.h | 5 +-
src/responder/ifp/ifpsrv_util.c | 130 ++++++++++++++++++++++++++++++++++++++++
src/tests/cmocka/test_ifp.c | 106 ++++++++++++++++++++++++++++++++
3 files changed, 240 insertions(+), 1 deletion(-)
diff --git a/src/responder/ifp/ifp_private.h b/src/responder/ifp/ifp_private.h
index e7fcaa4c32063a2a387e9b51ca2872431c5246f0..75be038e40334757f7a1889b017f41e6e5c6b989 100644
--- a/src/responder/ifp/ifp_private.h
+++ b/src/responder/ifp/ifp_private.h
@@ -69,9 +69,12 @@ errno_t ifp_req_create(struct sbus_request *dbus_req,
int ifp_req_create_handle_failure(struct sbus_request *dbus_req, errno_t err);
const char *ifp_path_strip_prefix(const char *path, const char *prefix);
+
+char *ifp_bus_path_unescape(TALLOC_CTX *mem_ctx, const char *path);
+char *ifp_bus_path_escape(TALLOC_CTX *mem_ctx, const char *path);
+
errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict,
struct ldb_message_element *el);
-
const char **ifp_parse_attr_list(TALLOC_CTX *mem_ctx, const char *conf_str);
bool ifp_attr_allowed(const char *whitelist[], const char *attr);
#endif /* _IFPSRV_PRIVATE_H_ */
diff --git a/src/responder/ifp/ifpsrv_util.c b/src/responder/ifp/ifpsrv_util.c
index 4ef0be6afe5fc1b638eae69166a40dcd400d7c37..6fcd2247901137d328f8187c120ea1bb57db15f2 100644
--- a/src/responder/ifp/ifpsrv_util.c
+++ b/src/responder/ifp/ifpsrv_util.c
@@ -106,6 +106,136 @@ const char *ifp_path_strip_prefix(const char *path, const char *prefix)
return NULL;
}
+/* The following path related functions are based on similar code in
+ * storaged, just tailored to use talloc instead of glib
+ */
+char *ifp_bus_path_escape(TALLOC_CTX *mem_ctx, const char *path)
+{
+ size_t n;
+ char *safe_path = NULL;
+ TALLOC_CTX *tmp_ctx = NULL;
+
+ /* The path must be valid */
+ if (path == NULL) {
+ return NULL;
+ }
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ return NULL;
+ }
+
+ safe_path = talloc_strdup(tmp_ctx, "");
+ if (safe_path == NULL) {
+ goto done;
+ }
+
+ /* Special case for an empty string */
+ if (strcmp(path, "") == 0) {
+ /* the for loop would just fall through */
+ safe_path = talloc_asprintf_append_buffer(safe_path, "_");
+ }
+
+ for (n = 0; path[n]; n++) {
+ int c = path[n];
+ /* D-Bus spec says:
+ * *
+ * * Each element must only contain the ASCII characters
+ * "[A-Z][a-z][0-9]_"
+ * */
+ if ((c >= 'A' && c <= 'Z')
+ || (c >= 'a' && c <= 'z')
+ || (c >= '0' && c <= '9')) {
+ safe_path = talloc_asprintf_append_buffer(safe_path, "%c", c);
+ if (safe_path == NULL) {
+ goto done;
+ }
+ } else {
+ safe_path = talloc_asprintf_append_buffer(safe_path, "_%02x", c);
+ if (safe_path == NULL) {
+ goto done;
+ }
+ }
+ }
+
+ safe_path = talloc_steal(mem_ctx, safe_path);
+done:
+ talloc_free(tmp_ctx);
+ return safe_path;
+}
+
+static inline int unhexchar(char c)
+{
+ if (c >= '0' && c <= '9') {
+ return c - '0';
+ }
+
+ if (c >= 'a' && c <= 'f') {
+ return c - 'a' + 10;
+ }
+
+ if (c >= 'A' && c <= 'F') {
+ return c - 'A' + 10;
+ }
+
+ return -1;
+}
+
+char *ifp_bus_path_unescape(TALLOC_CTX *mem_ctx, const char *path)
+{
+ char *safe_path;
+ const char *p;
+ int a, b, c;
+ TALLOC_CTX *tmp_ctx = NULL;
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ return NULL;
+ }
+
+ safe_path = talloc_strdup(tmp_ctx, "");
+ if (safe_path == NULL) {
+ goto done;
+ }
+
+ /* Special case for the empty string */
+ if (strcmp(path, "_") == 0) {
+ safe_path = talloc_steal(mem_ctx, safe_path);
+ goto done;
+ }
+
+ for (p = path; *p; p++) {
+ if (*p == '_') {
+ /* There must be at least two more chars after underscore */
+ if (p[1] == '\0' || p[2] == '\0') {
+ safe_path = NULL;
+ goto done;
+ }
+
+ if ((a = unhexchar(p[1])) < 0
+ || (b = unhexchar(p[2])) < 0) {
+ /* Invalid escape code, let's take it literal then */
+ c = '_';
+ } else {
+ c = ((a << 4) | b);
+ p += 2;
+ }
+ } else {
+ c = *p;
+ }
+
+ safe_path = talloc_asprintf_append_buffer(safe_path, "%c", c);
+ if (safe_path == NULL) {
+ goto done;
+ }
+ }
+
+ safe_path = talloc_steal(mem_ctx, safe_path);
+done:
+ talloc_free(tmp_ctx);
+ return safe_path;
+}
+
errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict,
struct ldb_message_element *el)
{
diff --git a/src/tests/cmocka/test_ifp.c b/src/tests/cmocka/test_ifp.c
index 70ebf942dcb1e366516bed92620dece6dedd0a0d..8d9b8563563d1cb274eb44f044ebc05bb210596f 100644
--- a/src/tests/cmocka/test_ifp.c
+++ b/src/tests/cmocka/test_ifp.c
@@ -28,6 +28,11 @@
#include "responder/ifp/ifp_private.h"
#include "sbus/sssd_dbus_private.h"
+/* dbus library checks for valid object paths when unit testing, we don't
+ * want that */
+#undef DBUS_TYPE_OBJECT_PATH
+#define DBUS_TYPE_OBJECT_PATH ((int) 's')
+
static struct ifp_ctx *
mock_ifp_ctx(TALLOC_CTX *mem_ctx)
{
@@ -305,6 +310,106 @@ void test_attr_allowed(void **state)
assert_false(ifp_attr_allowed(NULL, "name"));
}
+void test_path_escape_unescape(void **state)
+{
+ char *escaped;
+ char *raw;
+ TALLOC_CTX *mem_ctx;
+
+ assert_true(leak_check_setup());
+ mem_ctx = talloc_new(global_talloc_context);
+
+ escaped = ifp_bus_path_escape(mem_ctx, "noescape");
+ assert_non_null(escaped);
+ assert_string_equal(escaped, "noescape");
+ raw = ifp_bus_path_unescape(mem_ctx, escaped);
+ talloc_free(escaped);
+ assert_non_null(raw);
+ assert_string_equal(raw, "noescape");
+ talloc_free(raw);
+
+ escaped = ifp_bus_path_escape(mem_ctx, "redhat.com");
+ assert_non_null(escaped);
+ assert_string_equal(escaped, "redhat_2ecom"); /* dot is 0x2E in ASCII */
+ raw = ifp_bus_path_unescape(mem_ctx, escaped);
+ talloc_free(escaped);
+ assert_non_null(raw);
+ assert_string_equal(raw, "redhat.com");
+ talloc_free(raw);
+
+ escaped = ifp_bus_path_escape(mem_ctx, "path_with_underscore");
+ assert_non_null(escaped);
+ /* underscore is 0x5F in ascii */
+ assert_string_equal(escaped, "path_5fwith_5funderscore");
+ raw = ifp_bus_path_unescape(mem_ctx, escaped);
+ talloc_free(escaped);
+ assert_non_null(raw);
+ assert_string_equal(raw, "path_with_underscore");
+ talloc_free(raw);
+
+ /* empty string */
+ escaped = ifp_bus_path_escape(mem_ctx, "");
+ assert_non_null(escaped);
+ assert_string_equal(escaped, "_");
+ raw = ifp_bus_path_unescape(mem_ctx, escaped);
+ talloc_free(escaped);
+ assert_non_null(raw);
+ assert_string_equal(raw, "");
+ talloc_free(raw);
+
+ /* negative tests */
+ escaped = ifp_bus_path_escape(mem_ctx, NULL);
+ assert_null(escaped);
+ raw = ifp_bus_path_unescape(mem_ctx, "wrongpath_");
+ assert_null(raw);
+
+ assert_true(leak_check_teardown());
+}
+
+#define PATH_BASE "/some/path"
+
+struct ifp_test_req_ctx {
+ struct ifp_req *ireq;
+ struct sbus_request *sr;
+ struct ifp_ctx *ifp_ctx;
+};
+
+void ifp_test_req_setup(void **state)
+{
+ struct ifp_test_req_ctx *test_ctx;
+ errno_t ret;
+
+ assert_true(leak_check_setup());
+
+ test_ctx = talloc_zero(global_talloc_context, struct ifp_test_req_ctx);
+ assert_non_null(test_ctx);
+ test_ctx->ifp_ctx = mock_ifp_ctx(test_ctx);
+ assert_non_null(test_ctx->ifp_ctx);
+
+ test_ctx->sr = mock_sbus_request(test_ctx, geteuid());
+ assert_non_null(test_ctx->sr);
+
+ ret = ifp_req_create(test_ctx->sr, test_ctx->ifp_ctx, &test_ctx->ireq);
+ assert_int_equal(ret, EOK);
+ assert_non_null(test_ctx->ireq);
+
+ check_leaks_push(test_ctx);
+ *state = test_ctx;
+}
+
+void ifp_test_req_teardown(void **state)
+{
+ struct ifp_test_req_ctx *test_ctx = talloc_get_type_abort(*state,
+ struct ifp_test_req_ctx);
+
+ assert_true(check_leaks_pop(test_ctx) == true);
+
+ dbus_message_unref(test_ctx->sr->message);
+ talloc_free(test_ctx);
+
+ assert_true(leak_check_teardown());
+}
+
int main(int argc, const char *argv[])
{
poptContext pc;
@@ -322,6 +427,7 @@ int main(int argc, const char *argv[])
unit_test(test_el_to_dict),
unit_test(test_attr_acl),
unit_test(test_attr_allowed),
+ unit_test(test_path_escape_unescape),
};
/* Set debug level to invalid value so we can deside if -d 0 was used. */
--
1.9.0
-------------- next part --------------
>From f1520f61ad84464bf679c08cfd1d1fff3408896d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
Date: Sun, 11 May 2014 16:18:06 +0200
Subject: [PATCH 2/3] IFP: Add a utility function to reply with an object path
---
src/responder/ifp/ifp_private.h | 6 ++++++
src/responder/ifp/ifpsrv_util.c | 42 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/src/responder/ifp/ifp_private.h b/src/responder/ifp/ifp_private.h
index 75be038e40334757f7a1889b017f41e6e5c6b989..bf2015a9722ff4b4f831cb63cd528881f0f259a8 100644
--- a/src/responder/ifp/ifp_private.h
+++ b/src/responder/ifp/ifp_private.h
@@ -73,6 +73,12 @@ const char *ifp_path_strip_prefix(const char *path, const char *prefix);
char *ifp_bus_path_unescape(TALLOC_CTX *mem_ctx, const char *path);
char *ifp_bus_path_escape(TALLOC_CTX *mem_ctx, const char *path);
+char *_ifp_reply_objpath(TALLOC_CTX *mem_ctx, const char *base,
+ const char *part, ...);
+
+#define ifp_reply_objpath(mem_ctx, base, ...) \
+ _ifp_reply_objpath(mem_ctx, base, ##__VA_ARGS__, NULL)
+
errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict,
struct ldb_message_element *el);
const char **ifp_parse_attr_list(TALLOC_CTX *mem_ctx, const char *conf_str);
diff --git a/src/responder/ifp/ifpsrv_util.c b/src/responder/ifp/ifpsrv_util.c
index 6fcd2247901137d328f8187c120ea1bb57db15f2..c0ab686e4527a2508f67b789151365dfdbb89f25 100644
--- a/src/responder/ifp/ifpsrv_util.c
+++ b/src/responder/ifp/ifpsrv_util.c
@@ -236,6 +236,48 @@ done:
return safe_path;
}
+char *
+_ifp_reply_objpath(TALLOC_CTX *mem_ctx, const char *base,
+ const char *part, ...)
+{
+ char *safe_part;
+ char *path = NULL;
+ va_list va;
+
+ if (base == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "Wrong object path base!\n");
+ return NULL;
+ }
+
+ path = talloc_strdup(mem_ctx, base);
+ if (path == NULL) return NULL;
+
+ va_start(va, part);
+ while (part != NULL) {
+ safe_part = ifp_bus_path_escape(mem_ctx, part);
+ if (safe_part == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "Could not add [%s] to objpath\n", part);
+ goto fail;
+ }
+
+ path = talloc_asprintf_append(path, "/%s", safe_part);
+ talloc_free(safe_part);
+ if (path == NULL) {
+ goto fail;
+ }
+
+ part = va_arg(va, const char *);
+ }
+ va_end(va);
+
+ return path;
+
+fail:
+ va_end(va);
+ talloc_free(path);
+ return NULL;
+}
+
errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict,
struct ldb_message_element *el)
{
--
1.9.0
-------------- next part --------------
>From 52e1cf07eadadc2b69b8f8cb9ee7b611621590b1 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sun, 11 May 2014 16:18:28 +0200
Subject: [PATCH 3/3] IFP: Add a unit test for ifp_reply_objpath
---
src/tests/cmocka/test_ifp.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/src/tests/cmocka/test_ifp.c b/src/tests/cmocka/test_ifp.c
index 8d9b8563563d1cb274eb44f044ebc05bb210596f..650a9496978854c4c1aa6b72f58e831d60ca7c10 100644
--- a/src/tests/cmocka/test_ifp.c
+++ b/src/tests/cmocka/test_ifp.c
@@ -410,6 +410,32 @@ void ifp_test_req_teardown(void **state)
assert_true(leak_check_teardown());
}
+void test_reply_path(void **state)
+{
+ struct ifp_test_req_ctx *test_ctx = talloc_get_type_abort(*state,
+ struct ifp_test_req_ctx);
+ char *path;
+
+ /* Doesn't need escaping */
+ path = ifp_reply_objpath(test_ctx->ireq, PATH_BASE, "domname", NULL);
+ assert_non_null(path);
+ assert_string_equal(path, PATH_BASE"/domname");
+ talloc_free(path);
+}
+
+void test_reply_path_escape(void **state)
+{
+ struct ifp_test_req_ctx *test_ctx = talloc_get_type_abort(*state,
+ struct ifp_test_req_ctx);
+ char *path;
+
+ /* A dot needs escaping */
+ path = ifp_reply_objpath(test_ctx->ireq, PATH_BASE, "redhat.com", NULL);
+ assert_non_null(path);
+ assert_string_equal(path, PATH_BASE"/redhat_2ecom");
+ talloc_free(path);
+}
+
int main(int argc, const char *argv[])
{
poptContext pc;
@@ -428,6 +454,10 @@ int main(int argc, const char *argv[])
unit_test(test_attr_acl),
unit_test(test_attr_allowed),
unit_test(test_path_escape_unescape),
+ unit_test_setup_teardown(test_reply_path,
+ ifp_test_req_setup, ifp_test_req_teardown),
+ unit_test_setup_teardown(test_reply_path_escape,
+ ifp_test_req_setup, ifp_test_req_teardown),
};
/* Set debug level to invalid value so we can deside if -d 0 was used. */
--
1.9.0
More information about the sssd-devel
mailing list