Attachment 112846 Details for Bug 88810 – [PATCH 5/6] Add a test for uid-controlled permissions

[patch] [PATCH 5/6] Add a test for uid-controlled permissions

0005-Add-a-test-for-uid-controlled-permissions.patch (text/plain), 10.75 KB, created by Simon McVittie on 2015-01-26 20:37:58 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2015-01-26 20:37:58 UTC

Size: 10.75 KB

patch

obsolete

>From 24b7e1c8291cfe683cede75a87c653a45f6ecf4f Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Mon, 26 Jan 2015 19:10:11 +0000
>Subject: [PATCH 5/6] Add a test for uid-controlled permissions
>
>This is technical debt from mitigating CVE-2014-8148, which should
>really have had a regression test at the time.
>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88810
>---
> test/Makefile.am                                |  11 ++
> test/data/valid-config-files/multi-user.conf.in |  15 ++
> test/test-utils-glib.c                          |  82 +++++++++++
> test/test-utils-glib.h                          |   3 +
> test/uid-permissions.c                          | 186 ++++++++++++++++++++++++
> 5 files changed, 297 insertions(+)
> create mode 100644 test/data/valid-config-files/multi-user.conf.in
> create mode 100644 test/uid-permissions.c
>
>diff --git a/test/Makefile.am b/test/Makefile.am
>index 763fe20..a306e6c 100644
>--- a/test/Makefile.am
>+++ b/test/Makefile.am
>@@ -157,6 +157,7 @@ installable_tests += \
> 	test-relay \
> 	test-syntax \
> 	test-syslog \
>+	test-uid-permissions \
> 	$(NULL)
> installable_manual_tests += \
> 	manual-authz \
>@@ -235,6 +236,15 @@ test_syntax_LDADD = \
>     $(GLIB_LIBS) \
>     $(NULL)
> 
>+test_uid_permissions_SOURCES = \
>+    uid-permissions.c \
>+    $(NULL)
>+test_uid_permissions_CPPFLAGS = $(testutils_shared_if_possible_cppflags)
>+test_uid_permissions_LDADD = \
>+    $(testutils_shared_if_possible_libs) \
>+    $(GLIB_LIBS) \
>+    $(NULL)
>+
> if DBUS_ENABLE_MODULAR_TESTS
> TESTS += $(installable_tests)
> installcheck_tests += $(installable_tests)
>@@ -271,6 +281,7 @@ in_data = \
> 	data/valid-config-files/debug-allow-all.conf.in \
> 	data/valid-config-files/finite-timeout.conf.in \
> 	data/valid-config-files/incoming-limit.conf.in \
>+	data/valid-config-files/multi-user.conf.in \
> 	data/invalid-service-files-system/org.freedesktop.DBus.TestSuiteNoExec.service.in \
> 	data/invalid-service-files-system/org.freedesktop.DBus.TestSuiteNoService.service.in \
> 	data/invalid-service-files-system/org.freedesktop.DBus.TestSuiteNoUser.service.in \
>diff --git a/test/data/valid-config-files/multi-user.conf.in b/test/data/valid-config-files/multi-user.conf.in
>new file mode 100644
>index 0000000..37a7da6
>--- /dev/null
>+++ b/test/data/valid-config-files/multi-user.conf.in
>@@ -0,0 +1,15 @@
>+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
>+ "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
>+<busconfig>
>+  <listen>@TEST_LISTEN@</listen>
>+
>+  <policy context="default">
>+    <allow send_interface="*"/>
>+    <allow receive_interface="*"/>
>+    <allow own="*"/>
>+    <allow user="*"/>
>+  </policy>
>+
>+  
>+  <servicehelper>/bin/false</servicehelper>
>+</busconfig>
>diff --git a/test/test-utils-glib.c b/test/test-utils-glib.c
>index 465a81c..ab060e7 100644
>--- a/test/test-utils-glib.c
>+++ b/test/test-utils-glib.c
>@@ -270,6 +270,88 @@ test_connect_to_bus (TestMainContext *ctx,
>   return conn;
> }
> 
>+DBusConnection *
>+test_connect_to_bus_as_user (TestMainContext *ctx,
>+    const char *address,
>+    TestUser user)
>+{
>+  /* For now we only do tests like this on Linux, because I don't know how
>+   * safe this use of setresuid() is on other platforms */
>+#if defined(HAVE_GETRESUID) && defined(HAVE_SETRESUID) && defined(__linux__)
>+  uid_t ruid, euid, suid;
>+  struct passwd *pwd;
>+  DBusConnection *conn;
>+  const char *username;
>+
>+  switch (user)
>+    {
>+      case TEST_USER_ME:
>+        return test_connect_to_bus (ctx, address);
>+
>+      case TEST_USER_ROOT:
>+        username = "root";
>+        break;
>+
>+      case TEST_USER_MESSAGEBUS:
>+        username = DBUS_USER;
>+        break;
>+
>+      case TEST_USER_OTHER:
>+        username = DBUS_TEST_USER;
>+        break;
>+
>+      default:
>+        g_return_val_if_reached (NULL);
>+    }
>+
>+  if (getresuid (&ruid, &euid, &suid) != 0)
>+    g_error ("getresuid: %s", g_strerror (errno));
>+
>+  if (ruid != 0 || euid != 0 || suid != 0)
>+    {
>+      g_message ("SKIP: not uid 0 (ruid=%ld euid=%ld suid=%ld)",
>+          (unsigned long) ruid, (unsigned long) euid, (unsigned long) suid);
>+      return NULL;
>+    }
>+
>+  pwd = getpwnam (username);
>+
>+  if (pwd == NULL)
>+    {
>+      g_message ("SKIP: getpwnam(\"%s\"): %s", username, g_strerror (errno));
>+      return NULL;
>+    }
>+
>+  /* Impersonate the desired user while we connect to the bus.
>+   * This should work, because we're root. */
>+  if (setresuid (pwd->pw_uid, pwd->pw_uid, 0) != 0)
>+    g_error ("setresuid(%ld, (same), 0): %s",
>+        (unsigned long) pwd->pw_uid, g_strerror (errno));
>+
>+  conn = test_connect_to_bus (ctx, address);
>+
>+  /* go back to our saved uid */
>+  if (setresuid (0, 0, 0) != 0)
>+    g_error ("setresuid(0, 0, 0): %s", g_strerror (errno));
>+
>+  return conn;
>+
>+#else
>+
>+  switch (user)
>+    {
>+      case TEST_USER_ME:
>+        return test_connect_to_bus (ctx, address);
>+
>+      default:
>+        g_message ("SKIP: setresuid() not available, or unsure about "
>+            "credentials-passing semantics on this platform");
>+        return NULL;
>+    }
>+
>+#endif
>+}
>+
> void
> test_kill_pid (GPid pid)
> {
>diff --git a/test/test-utils-glib.h b/test/test-utils-glib.h
>index 20c31c7..ebc53b1 100644
>--- a/test/test-utils-glib.h
>+++ b/test/test-utils-glib.h
>@@ -25,6 +25,9 @@ gchar *test_get_dbus_daemon (const gchar *config_file,
> 
> DBusConnection *test_connect_to_bus (TestMainContext *ctx,
>     const gchar *address);
>+DBusConnection *test_connect_to_bus_as_user (TestMainContext *ctx,
>+    const char *address,
>+    TestUser user);
> 
> void test_kill_pid (GPid pid);
> 
>diff --git a/test/uid-permissions.c b/test/uid-permissions.c
>new file mode 100644
>index 0000000..c200528
>--- /dev/null
>+++ b/test/uid-permissions.c
>@@ -0,0 +1,186 @@
>+/* Integration tests for the dbus-daemon's uid-based hardening
>+ *
>+ * Author: Simon McVittie <simon.mcvittie@collabora.co.uk>
>+ * Copyright Â© 2010-2011 Nokia Corporation
>+ * Copyright Â© 2015 Collabora Ltd.
>+ *
>+ * Permission is hereby granted, free of charge, to any person
>+ * obtaining a copy of this software and associated documentation files
>+ * (the "Software"), to deal in the Software without restriction,
>+ * including without limitation the rights to use, copy, modify, merge,
>+ * publish, distribute, sublicense, and/or sell copies of the Software,
>+ * and to permit persons to whom the Software is furnished to do so,
>+ * subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice shall be
>+ * included in all copies or substantial portions of the Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>+ * SOFTWARE.
>+ */
>+
>+#include <config.h>
>+
>+#include "test-utils-glib.h"
>+
>+typedef struct {
>+    gboolean skip;
>+
>+    TestMainContext *ctx;
>+
>+    DBusError e;
>+    GError *ge;
>+
>+    GPid daemon_pid;
>+
>+    DBusConnection *conn;
>+} Fixture;
>+
>+typedef struct {
>+    const char *config_file;
>+    TestUser user;
>+    gboolean expect_success;
>+} Config;
>+
>+static void
>+setup (Fixture *f,
>+    gconstpointer context)
>+{
>+  const Config *config = context;
>+  gchar *address;
>+
>+  f->ctx = test_main_context_get ();
>+  f->ge = NULL;
>+  dbus_error_init (&f->e);
>+
>+  address = test_get_dbus_daemon (config ? config->config_file : NULL,
>+                                  TEST_USER_MESSAGEBUS,
>+                                  &f->daemon_pid);
>+
>+  if (address == NULL)
>+    {
>+      f->skip = TRUE;
>+      return;
>+    }
>+
>+  f->conn = test_connect_to_bus_as_user (f->ctx, address,
>+      config ? config->user : TEST_USER_ME);
>+
>+  if (f->conn == NULL)
>+    f->skip = TRUE;
>+
>+  g_free (address);
>+}
>+
>+static void
>+test_uae (Fixture *f,
>+    gconstpointer context)
>+{
>+  const Config *config = context;
>+  DBusMessage *m;
>+  DBusPendingCall *pc;
>+  DBusMessageIter args_iter;
>+  DBusMessageIter arr_iter;
>+
>+  if (f->skip)
>+    return;
>+
>+  m = dbus_message_new_method_call (DBUS_SERVICE_DBUS,
>+      DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS, "UpdateActivationEnvironment");
>+
>+  if (m == NULL)
>+    g_error ("OOM");
>+
>+  dbus_message_iter_init_append (m, &args_iter);
>+
>+  /* Append an empty a{ss} (string => string dictionary). */
>+  if (!dbus_message_iter_open_container (&args_iter, DBUS_TYPE_ARRAY,
>+        "{ss}", &arr_iter) ||
>+      !dbus_message_iter_close_container (&args_iter, &arr_iter))
>+    g_error ("OOM");
>+
>+  if (!dbus_connection_send_with_reply (f->conn, m, &pc,
>+                                        DBUS_TIMEOUT_USE_DEFAULT) ||
>+      pc == NULL)
>+    g_error ("OOM");
>+
>+  dbus_message_unref (m);
>+  m = NULL;
>+
>+  if (dbus_pending_call_get_completed (pc))
>+    test_pending_call_store_reply (pc, &m);
>+  else if (!dbus_pending_call_set_notify (pc, test_pending_call_store_reply,
>+                                          &m, NULL))
>+    g_error ("OOM");
>+
>+  while (m == NULL)
>+    test_main_context_iterate (f->ctx, TRUE);
>+
>+  if (config->expect_success)
>+    {
>+      /* it succeeds */
>+      g_assert_cmpint (dbus_message_get_type (m), ==,
>+          DBUS_MESSAGE_TYPE_METHOD_RETURN);
>+    }
>+  else
>+    {
>+      /* it fails, yielding an error message with one string argument */
>+      g_assert_cmpint (dbus_message_get_type (m), ==, DBUS_MESSAGE_TYPE_ERROR);
>+      g_assert_cmpstr (dbus_message_get_error_name (m), ==,
>+          DBUS_ERROR_ACCESS_DENIED);
>+      g_assert_cmpstr (dbus_message_get_signature (m), ==, "s");
>+    }
>+
>+  dbus_message_unref (m);
>+}
>+
>+static void
>+teardown (Fixture *f,
>+    gconstpointer context G_GNUC_UNUSED)
>+{
>+  dbus_error_free (&f->e);
>+  g_clear_error (&f->ge);
>+
>+  test_main_context_unref (f->ctx);
>+}
>+
>+static Config root_fail_config = {
>+    "valid-config-files/multi-user.conf",
>+    TEST_USER_ROOT,
>+    FALSE
>+};
>+
>+static Config messagebus_ok_config = {
>+    "valid-config-files/multi-user.conf",
>+    TEST_USER_MESSAGEBUS,
>+    TRUE
>+};
>+
>+static Config other_fail_config = {
>+    "valid-config-files/multi-user.conf",
>+    TEST_USER_OTHER,
>+    FALSE
>+};
>+
>+int
>+main (int argc,
>+    char **argv)
>+{
>+  g_test_init (&argc, &argv, NULL);
>+  g_test_bug_base ("https://bugs.freedesktop.org/show_bug.cgi?id=");
>+
>+  g_test_add ("/uid-permissions/uae/root", Fixture, &root_fail_config,
>+      setup, test_uae, teardown);
>+  g_test_add ("/uid-permissions/uae/messagebus", Fixture, &messagebus_ok_config,
>+      setup, test_uae, teardown);
>+  g_test_add ("/uid-permissions/uae/other", Fixture, &other_fail_config,
>+      setup, test_uae, teardown);
>+
>+  return g_test_run ();
>+}
>-- 
>2.1.4
>

Actions: View | Splinter Review

Attachments on bug 88810: 112842 | 112843 | 112844 | 112845 | 112846 | 112847 | 112855 | 113053 | 113054 | 113055 | 113056 | 113057 | 113058 | 113059 | 113060 | 113061 | 113062 | 113063

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.