Upstream kernels allow unprivileged users to create user namespaces
and change their uid/gid.
These patches update kdbus policy logic to handle this case and
improve our current checks across user namespaces.
So this patch adds:
* kdbus_test_waitpid() to get exit code of childs.
* kdbus_clone_userns_test() that performs the test inside a new
user namespace.
* Converts all the other tests to return CHECK_OK, CHECK_SKIP or
CHECK_ERR so we are consistent.
Currently we fail at kdbus_clone_userns_test() test #8. The next patch
will fix this issue.
Signed-off-by: Djalal Harouni <tix...@opendz.org>
---
test/test-kdbus-policy.c | 503 +++++++++++++++++++++++++++++++++++------------
1 file changed, 375 insertions(+), 128 deletions(-)
diff --git a/test/test-kdbus-policy.c b/test/test-kdbus-policy.c
index 8d7bcb5..a354290 100644
--- a/test/test-kdbus-policy.c
+++ b/test/test-kdbus-policy.c
@@ -12,6 +12,7 @@
#include <fcntl.h>
#include <pthread.h>
#include <poll.h>
+#include <sched.h>
#include <stdlib.h>
#include <stddef.h>
#include <stdint.h>
@@ -22,6 +23,8 @@
#include <sys/wait.h>
#include <sys/prctl.h>
#include <sys/ioctl.h>
+#include <sys/eventfd.h>
+#include <sys/syscall.h>
#include "kdbus-util.h"
#include "kdbus-enum.h"
@@ -29,6 +32,30 @@
#define MAX_CONN 64
#define POLICY_NAME "foo.test.policy-test"
+static int ok_cnt;
+static int skip_cnt;
+static int fail_cnt;
+
+static void print_test_status(int test_status)
+{
+ switch (test_status) {
+ case CHECK_OK:
+ printf("OK");
+ ok_cnt++;
+ break;
+ case CHECK_SKIP:
+ printf("SKIPPED");
+ skip_cnt++;
+ break;
+ case CHECK_ERR:
+ default:
+ printf("ERROR");
+ fail_cnt++;
+ break;
+ }
+
+ printf("\n");
+}
/**
* The purpose of these tests:
@@ -53,35 +80,59 @@ void kdbus_free_conn(struct conn *conn)
}
}
+static void *kdbus_recv_echo(void *ptr)
+{
+ int ret;
+ struct conn *conn = ptr;
+
+ ret = conn_recv(conn);
+
+ return (void *)(long)ret;
+}
+
/* Trigger kdbus_policy_set() */
static int kdbus_set_policy_talk(struct conn *conn,
const char *name,
uid_t id, unsigned int type)
{
+ int ret;
struct kdbus_policy_access access = {
.type = type,
.id = id,
.access = KDBUS_POLICY_TALK,
};
- return conn_update_policy(conn, name, &access, 1);
+ ret = conn_update_policy(conn, name, &access, 1);
+ if (ret < 0)
+ return CHECK_ERR;
+
+ return CHECK_OK;
}
-/* The policy access will be stored in a policy holder connection */
-static int kdbus_register_activator(char *bus, const char *name,
- struct conn **c)
+/* return CHECK_OK or CHECK_ERR on failure */
+static int kdbus_register_same_activator(char *bus, const char *name,
+ struct conn **c)
{
+ int ret;
struct conn *activator;
activator = kdbus_hello_activator(bus, name, NULL, 0);
- if (!activator)
- return -errno;
+ if (activator) {
+ *c = activator;
+ fprintf(stderr, "--- error was able to register name twice
'%s'.\n",
+ name);
+ return CHECK_ERR;
+ }
- *c = activator;
+ ret = -errno;
+ /* -EEXIST means test succeeded */
+ if (ret == -EEXIST)
+ return CHECK_OK;
- return 0;
+ return CHECK_ERR;
}
+/* return CHECK_OK or CHECK_ERR on failure */
static int kdbus_register_policy_holder(char *bus, const char *name,
struct conn **conn)
{
@@ -99,54 +150,88 @@ static int kdbus_register_policy_holder(char *bus, const
char *name,
c = kdbus_hello_registrar(bus, name, access, 2,
KDBUS_HELLO_POLICY_HOLDER);
if (!c)
- return -errno;
+ return CHECK_ERR;
*conn = c;
- return 0;
+ return CHECK_OK;
}
-static void *kdbus_recv_echo(void *ptr)
+/* return CHECK_OK or CHECK_ERR on failure */
+static int kdbus_receiver_acquire_name(char *bus, const char *name,
+ struct conn **conn)
{
int ret;
- int cnt = 3;
- struct pollfd fd;
- struct conn *conn = ptr;
+ struct conn *c;
- fd.fd = conn->fd;
- fd.events = POLLIN | POLLPRI | POLLHUP;
- fd.revents = 0;
+ c = kdbus_hello(bus, 0, NULL, 0);
+ if (!c)
+ return CHECK_ERR;
- while (cnt) {
- cnt--;
- ret = poll(&fd, 1, 2000);
- if (ret == 0) {
- ret = -ETIMEDOUT;
- break;
- }
+ add_match_empty(c->fd);
- if (ret > 0 && fd.revents & POLLIN) {
- printf("-- Connection id: %llu received new message:\n",
- (unsigned long long)conn->id);
- ret = msg_recv(conn);
- }
+ ret = name_acquire(c, name, 0);
+ if (ret < 0)
+ return CHECK_ERR;
- if (ret >= 0 || ret != -EAGAIN)
- break;
+ *conn = c;
+
+ return CHECK_OK;
+}
+
+/**
+ * Return the exit code of the child process and set the test_done
+ * variable if the test was performed:
+ * In case of WIFEXITED():
+ * 1) If exit code != EXIT_FAILURE assume test reached and set the
+ * 'test_done' variable if provided to 1.
+ * 2) If exit code != EXIT_FAILURE and exit code != EXIT_SUCCESS:
+ * Convert the exit code to a proper -errno and return it.
+ * In all other cases:
+ * set the exit code to EXIT_FAILURE and return it.
+ */
+static int kdbus_test_waitpid(pid_t pid, int *test_done)
+{
+ int ret;
+ int status;
+
+ int test_performed = 0;
+
+ ret = waitpid(pid, &status, 0);
+ if (ret < 0) {
+ ret = -errno;
+ fprintf(stderr, "error waitpid: %d (%m)\n", ret);
+ return ret;
}
- return (void *)(long)ret;
+ if (WIFEXITED(status)) {
+ ret = WEXITSTATUS(status);
+ if (ret != EXIT_FAILURE) {
+ if (ret != EXIT_SUCCESS)
+ ret |= -1 << 8; /* get -errno */
+
+ /* test reached */
+ test_performed = 1;
+ }
+ } else {
+ ret = EXIT_FAILURE;
+ }
+
+ if (test_done)
+ *test_done = test_performed;
+
+ return ret;
}
/**
- * Just run a normal test, the 'conn_db' will be populated by
- * newly created connections. Caller should free all allocated
- * connections.
+ * Create new threads for receiving from multiple senders,
+ * The 'conn_db' will be populated by newly created connections.
+ * Caller should free all allocated connections.
*
- * return 0 on success, a non-zero on failure.
+ * return 0 on success, negative errno on failure.
*/
-static int kdbus_normal_test(const char *bus, const char *name,
- struct conn **conn_db)
+static int kdbus_recv_in_threads(const char *bus, const char *name,
+ struct conn **conn_db)
{
int ret;
unsigned int i, tid;
@@ -198,18 +283,38 @@ static int kdbus_normal_test(const char *bus, const char
*name,
return ret;
}
-static int kdbus_fork_test(const char *bus, const char *name,
+/* Return: CHECK_OK or CHECK_ERR on failure */
+static int kdbus_normal_test(const char *bus, const char *name,
struct conn **conn_db)
{
- int ret = 0;
- int status;
+ int ret;
+
+ ret = kdbus_recv_in_threads(bus, name, conn_db);
+ if (ret < 0)
+ return CHECK_ERR;
+
+ return CHECK_OK;
+}
+
+/*
+ * Return: CHECK_OK, CHECK_ERR or CHECK_SKIP
+ * we return CHECK_OK only if the childs return with the expected
+ * 'exit_code' that is specified as an argument.
+ */
+static int kdbus_fork_test(const char *bus, const char *name,
+ struct conn **conn_db, int exit_code)
+{
pid_t pid;
+ int ret = 0;
int test_done = 0;
+ int test_status = CHECK_ERR;
+ setbuf(stdout, NULL);
+ printf("STARTING the (FORK+DROP) test...............\n");
if (geteuid() > 0) {
fprintf(stderr, "error geteuid() != 0, %s() needs root\n",
__func__);
- goto out;
+ return CHECK_SKIP;
}
pid = fork();
@@ -228,7 +333,7 @@ static int kdbus_fork_test(const char *bus, const char
*name,
if (ret < 0)
goto child_fail;
- ret = kdbus_normal_test(bus, POLICY_NAME, conn_db);
+ ret = kdbus_recv_in_threads(bus, name, conn_db);
/*
* Here cached connections belong to child, they will
@@ -240,31 +345,203 @@ child_fail:
_exit(EXIT_FAILURE);
}
- ret = waitpid(pid, &status, 0);
+ ret = kdbus_test_waitpid(pid, &test_done);
+
+out:
+ /* test reached */
+ if (test_done) {
+ if (ret == exit_code)
+ test_status = CHECK_OK;
+ else
+ fprintf(stderr,
+ "error TEST exit code: %d was expecting code:
%d\n",
+ ret, exit_code);
+ }
+
+ return test_status;
+}
+
+/* Return EXIT_SUCCESS, EXIT_FAILURE or negative errno */
+static int __kdbus_clone_userns_test(const char *bus,
+ const char *name,
+ struct conn **conn_db)
+{
+ int efd;
+ pid_t pid;
+ int ret = 0;
+ unsigned int uid = 65534;
+
+ ret = drop_privileges(uid, uid);
+ if (ret < 0)
+ return ret;
+
+ /**
+ * Since we just dropped privileges, the dumpable flag was just
+ * cleared which makes the /proc/$clone_child/uid_map to be
+ * owned by root, hence any userns uid mapping will fail with
+ * -EPERM since the mapping will be done by uid 65534.
+ *
+ * To avoid this set the dumpable flag again which makes procfs
+ * update the /proc/$clone_child/ inodes owner to 65534.
+ *
+ * Using this we will be able write to /proc/$clone_child/uid_map
+ * as uid 65534 and map the uid 65534 to 0 inside the user
+ * namespace.
+ */
+ ret = prctl(PR_SET_DUMPABLE, SUID_DUMP_USER);
if (ret < 0) {
ret = -errno;
- fprintf(stderr, "error waitpid: %d (%m)\n", ret);
+ fprintf(stderr, "error prctl: %d (%m)\n", ret);
+ return ret;
+ }
+
+ /* sync parent/child */
+ efd = eventfd(0, EFD_CLOEXEC);
+ if (efd < 0) {
+ ret = -errno;
+ fprintf(stderr, "error eventfd: %d (%m)\n", ret);
+ return ret;
+ }
+
+ pid = syscall(__NR_clone, SIGCHLD|CLONE_NEWUSER, NULL);
+ if (pid < 0) {
+ ret = -errno;
+ fprintf(stderr, "error clone: %d (%m)\n", ret);
+ /*
+ * Normal user not allowed to create userns,
+ * so nothing to worry about ?
+ */
+ if (ret == -EPERM) {
+ printf("-- CLONE_NEWUSER TEST Failed for uid: %u\n"
+ "-- Make sure that your kernel do not allow
CLONE_NEWUSER for unprivileged users\n"
+ "-- Upstream Commit:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5eaf563e53294d6696e651466697eb9d491f3946\n";,
+ uid);
+ ret = 0;
+ }
+
goto out;
}
- if (WIFEXITED(status)) {
- ret = WEXITSTATUS(status);
- if (ret != EXIT_FAILURE) {
- if (ret != EXIT_SUCCESS)
- ret |= -1 << 8; /* get -errno */
+ if (pid == 0) {
+ struct conn *conn_src;
+ eventfd_t event_status = 0;
- test_done = 1; /* assume test reached */
+ setbuf(stdout, NULL);
+ ret = prctl(PR_SET_PDEATHSIG, SIGKILL);
+ if (ret < 0) {
+ ret = -errno;
+ fprintf(stderr, "error prctl: %d (%m)\n", ret);
+ goto child_fail;
}
+
+ ret = eventfd_read(efd, &event_status);
+ if (ret < 0 || event_status != 1)
+ /* event_stats == 1 to continue */
+ goto child_fail;
+
+ /* ping connection from the new user namespace */
+ conn_src = kdbus_hello(bus, 0, NULL, 0);
+ if (!conn_src)
+ goto child_fail;
+
+ add_match_empty(conn_src->fd);
+ ret = msg_send(conn_src, name, 0xabcd1234,
+ 0, 0, 0, KDBUS_DST_ID_NAME);
+
+ close(conn_src->fd);
+ free(conn_src);
+ _exit(ret);
+
+child_fail:
+ _exit(EXIT_FAILURE);
+ }
+
+ ret = userns_map_uid_gid(pid, "0 65534 1", "0 65534 1");
+ if (ret < 0) {
+ /* send error to child */
+ eventfd_write(efd, 2);
+ fprintf(stderr, "error mapping uid/gid in new user
namespace\n");
+ goto out;
}
+ /* Tell child we are ready */
+ ret = eventfd_write(efd, 1);
+ if (ret < 0) {
+ ret = -errno;
+ fprintf(stderr, "error eventfd_write: %d (%m)\n", ret);
+ goto out;
+ }
+
+ ret = kdbus_test_waitpid(pid, NULL);
+
out:
- /* test not reached, return -EIO and avoid EXIT_FAILURE */
- if (!test_done)
- ret = -EIO;
+ close(efd);
return ret;
}
+static int kdbus_clone_userns_test(const char *bus,
+ const char *name,
+ struct conn **conn_db,
+ int exit_code)
+{
+ pid_t pid;
+ int ret = 0;
+ int test_done = 0;
+ int test_status = CHECK_ERR;
+
+ setbuf(stdout, NULL);
+ printf("STARTING TEST connections in a new user namespace..\n");
+ if (geteuid() > 0) {
+ fprintf(stderr, "error geteuid() != 0, %s() needs root\n",
+ __func__);
+ return CHECK_SKIP;
+ }
+
+ pid = fork();
+ if (pid < 0) {
+ ret = -errno;
+ fprintf(stderr, "error fork(): %d (%m)\n", ret);
+ goto out;
+ }
+
+ if (pid == 0) {
+ ret = prctl(PR_SET_PDEATHSIG, SIGKILL);
+ if (ret < 0)
+ goto child_fail;
+
+ ret = __kdbus_clone_userns_test(bus, name, conn_db);
+ _exit(ret);
+child_fail:
+ _exit(EXIT_FAILURE);
+ }
+
+ /* Receive in the original (root privileged) user namespace */
+ ret = conn_recv(conn_db[0]);
+ if (!ret) {
+ fprintf(stderr,
+ "--- error received packet from unprivileged user
namespace\n");
+ goto out;
+ }
+
+ ret = kdbus_test_waitpid(pid, &test_done);
+
+out:
+ /* test reached */
+ if (test_done) {
+ /* Set to CHECK_OK if we did get the right exit_code */
+ if (ret == exit_code)
+ test_status = CHECK_OK;
+ else
+ fprintf(stderr,
+ "error TEST exit code: %d was expecting code:
%d\n",
+ ret, exit_code);
+ }
+
+ return test_status;
+}
+
+/* Return CHECK_OK, CHECK_ERR or CHECK_SKIP */
static int kdbus_check_policy(char *bus)
{
int i;
@@ -282,107 +559,74 @@ static int kdbus_check_policy(char *bus)
&policy_holder);
printf("-- TEST 1) register a policy holder for '%s' ",
POLICY_NAME);
- if (ret < 0) {
- printf("FAILED\n");
+ print_test_status(ret);
+ if (ret == CHECK_ERR)
goto out_free_connections;
- }
-
- printf("OK\n");
/* Try to register the same name with an activator */
- ret = kdbus_register_activator(bus, POLICY_NAME, &activator);
+ ret = kdbus_register_same_activator(bus, POLICY_NAME,
+ &activator);
printf("-- TEST 2) register again '%s' as an activator ",
POLICY_NAME);
- if (ret == 0) {
- printf("succeeded: TEST FAILED\n");
- fprintf(stderr, "--- error was able to register twice '%s'.\n",
- POLICY_NAME);
- ret = -1;
- goto out_free_connections;
- } else if (ret < 0) {
- /* -EEXIST means test succeeded */
- if (ret == -EEXIST) {
- ret = 0;
- printf("failed: TEST OK\n");
- } else {
- printf("FAILED\n");
- goto out_free_connections;
- }
- }
-
- /* The name receiver */
- conn_db[0] = kdbus_hello(bus, 0, NULL, 0);
- if (!conn_db[0]) {
- ret = -1;
+ print_test_status(ret);
+ if (ret == CHECK_ERR)
goto out_free_connections;
- }
-
- add_match_empty(conn_db[0]->fd);
- ret = name_acquire(conn_db[0], POLICY_NAME, 0);
+ ret = kdbus_receiver_acquire_name(bus, POLICY_NAME, &conn_db[0]);
printf("-- TEST 3) acquire '%s' name..... ", POLICY_NAME);
- if (ret < 0) {
- printf("FAILED\n");
+ print_test_status(ret);
+ if (ret == CHECK_ERR)
goto out_free_connections;
- }
-
- printf("OK\n");
ret = kdbus_normal_test(bus, POLICY_NAME, conn_db);
printf("-- TEST 4) testing connections (NORMAL TEST).... ");
- if (ret != 0) {
- printf("FAILED\n");
+ print_test_status(ret);
+ if (ret == CHECK_ERR)
goto out_free_connections;
- }
-
- printf("OK\n");
name_list(conn_db[0], KDBUS_NAME_LIST_NAMES |
KDBUS_NAME_LIST_UNIQUE |
KDBUS_NAME_LIST_ACTIVATORS |
KDBUS_NAME_LIST_QUEUED);
- ret = kdbus_fork_test(bus, POLICY_NAME, conn_db);
+ ret = kdbus_fork_test(bus, POLICY_NAME, conn_db, EXIT_SUCCESS);
printf("-- TEST 5) testing connections (FORK+DROP)...... ");
- if (ret != 0) {
- printf("FAILED\n");
+ print_test_status(ret);
+ if (ret == CHECK_ERR)
goto out_free_connections;
- }
-
- printf("OK\n");
/*
* Connections that can talk are perhaps being destroyed now.
* Restrict the policy and purge cache entries where the
* conn_db[0] is the destination.
+ *
+ * Now only connections with uid == 0 are allowed to talk.
*/
ret = kdbus_set_policy_talk(policy_holder, POLICY_NAME,
geteuid(), KDBUS_POLICY_ACCESS_USER);
printf("-- TEST 6) restricting '%s' policy TALK access ",
POLICY_NAME);
- if (ret < 0) {
- printf("FAILED\n");
+ print_test_status(ret);
+ if (ret == CHECK_ERR)
goto out_free_connections;
- }
- printf("OK\n");
-
- /* After setting the policy re-check connections */
- ret = kdbus_fork_test(bus, POLICY_NAME, conn_db);
+ printf("-- TEST 7) testing connections (FORK+DROP) again\n");
+ /*
+ * After setting the policy re-check connections
+ * we expect the childs to fail with -EPERM
+ */
+ ret = kdbus_fork_test(bus, POLICY_NAME, conn_db, -EPERM);
printf("-- TEST 7) testing connections (FORK+DROP) again ");
- if (ret == 0) {
- printf("FAILED\n");
- fprintf(stderr, "--- error policy rules: send to all
succeeded.\n");
- ret = -1;
- } else if (ret < 0) {
- /* -EPERM means tests succeeded */
- if (ret == -EPERM) {
- ret = 0;
- printf("OK\n");
- } else {
- printf("FAILED\n");
- }
- }
+ print_test_status(ret);
+ if (ret == CHECK_ERR)
+ goto out_free_connections;
+
+ printf("-- TEST 8) testing connections in a new user namespace\n");
+ /* Check if the name can be reached in a new userns */
+ ret = kdbus_clone_userns_test(bus, POLICY_NAME,
+ conn_db, -EPERM);
+ printf("-- TEST 8) testing connections in a new user namespace ");
+ print_test_status(ret);
out_free_connections:
kdbus_free_conn(activator);
@@ -413,7 +657,7 @@ int main(int argc, char *argv[])
uint64_t n_type;
char name[64];
} bus_make;
- int fdc, ret, i;
+ int fdc, ret;
char *bus;
printf("-- opening /dev/" KBUILD_MODNAME "/control\n");
@@ -451,13 +695,16 @@ int main(int argc, char *argv[])
ret = kdbus_check_policy(bus);
- printf("RUNNING TEST 'policy db check' ");
- for (i = 0; i < 17; i++)
- printf(".");
- printf(" ");
+ printf("\nSUMMARY: %d tests passed, %d skipped%s, %d failed\n",
+ ok_cnt, skip_cnt,
+ skip_cnt ? " (need privileges)" : "", fail_cnt);
- if (ret < 0) {
- printf("FAILED\n");
+ if (skip_cnt > 0)
+ printf("For security reasons make sure to re-run skipped
tests.\n");
+
+ printf("RUNNING TEST 'policy db check'................ ");
+ if (fail_cnt > 0) {
+ printf("Failed\n");
return EXIT_FAILURE;
}
--
1.9.3
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
