daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
52b84b
From ccdb8883c6ff0e72d5e221768af0718e25cbf177 Mon Sep 17 00:00:00 2001
52b84b
From: David Rheinsberg <david.rheinsberg@gmail.com>
52b84b
Date: Thu, 14 Mar 2019 13:34:13 +0100
52b84b
Subject: [PATCH] sd-bus: skip sending formatted UIDs via SASL
52b84b
52b84b
The dbus external authentication takes as optional argument the UID the
52b84b
sender wants to authenticate as. This uid is purely optional. The
52b84b
AF_UNIX socket already conveys the same information through the
52b84b
auxiliary socket data, so we really don't have to provide that
52b84b
information.
52b84b
52b84b
Unfortunately, there is no way to send empty arguments, since they are
52b84b
interpreted as "missing argument", which has a different meaning. The
52b84b
SASL negotiation thus changes from:
52b84b
52b84b
    AUTH EXTERNAL <uid>
52b84b
    NEGOTIATE_UNIX_FD                   (optional)
52b84b
    BEGIN
52b84b
52b84b
to:
52b84b
52b84b
    AUTH EXTERNAL
52b84b
    DATA
52b84b
    NEGOTIATE_UNIX_FD                   (optional)
52b84b
    BEGIN
52b84b
52b84b
And thus the replies we expect as a client change from:
52b84b
52b84b
    OK <server-id>
52b84b
    AGREE_UNIX_FD                       (optional)
52b84b
52b84b
to:
52b84b
52b84b
    DATA
52b84b
    OK <server-id>
52b84b
    AGREE_UNIX_FD                       (optional)
52b84b
52b84b
Since the old sd-bus server implementation used the wrong reply for
52b84b
"AUTH" requests that do not carry the arguments inlined, we decided to
52b84b
make sd-bus clients accept this as well. Hence, sd-bus now allows
52b84b
"OK <server-id>\r\n" replies instead of "DATA\r\n" replies.
52b84b
52b84b
Signed-off-by: David Rheinsberg <david.rheinsberg@gmail.com>
52b84b
(cherry picked from commit 1ed4723d38cd0d1423c8fe650f90fa86007ddf55)
52b84b
52b84b
Resolves: #1838081
52b84b
---
52b84b
 src/libsystemd/sd-bus/bus-socket.c | 106 +++++++++++++++++------------
52b84b
 1 file changed, 64 insertions(+), 42 deletions(-)
52b84b
52b84b
diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c
52b84b
index e505d43c6b..8813dd5efd 100644
52b84b
--- a/src/libsystemd/sd-bus/bus-socket.c
52b84b
+++ b/src/libsystemd/sd-bus/bus-socket.c
52b84b
@@ -162,17 +162,25 @@ static int bus_socket_write_auth(sd_bus *b) {
52b84b
 }
52b84b
 
52b84b
 static int bus_socket_auth_verify_client(sd_bus *b) {
52b84b
-        char *e, *f, *start;
52b84b
+        char *d, *e, *f, *start;
52b84b
         sd_id128_t peer;
52b84b
         unsigned i;
52b84b
         int r;
52b84b
 
52b84b
         assert(b);
52b84b
 
52b84b
-        /* We expect two response lines: "OK" and possibly
52b84b
-         * "AGREE_UNIX_FD" */
52b84b
+        /*
52b84b
+         * We expect three response lines:
52b84b
+         *   "DATA\r\n"
52b84b
+         *   "OK <server-id>\r\n"
52b84b
+         *   "AGREE_UNIX_FD\r\n"        (optional)
52b84b
+         */
52b84b
 
52b84b
-        e = memmem_safe(b->rbuffer, b->rbuffer_size, "\r\n", 2);
52b84b
+        d = memmem_safe(b->rbuffer, b->rbuffer_size, "\r\n", 2);
52b84b
+        if (!d)
52b84b
+                return 0;
52b84b
+
52b84b
+        e = memmem(d + 2, b->rbuffer_size - (d - (char*) b->rbuffer) - 2, "\r\n", 2);
52b84b
         if (!e)
52b84b
                 return 0;
52b84b
 
52b84b
@@ -187,13 +195,30 @@ static int bus_socket_auth_verify_client(sd_bus *b) {
52b84b
                 start = e + 2;
52b84b
         }
52b84b
 
52b84b
-        /* Nice! We got all the lines we need. First check the OK
52b84b
-         * line */
52b84b
+        /* Nice! We got all the lines we need. First check the DATA line. */
52b84b
+
52b84b
+        if (d - (char*) b->rbuffer == 4) {
52b84b
+                if (memcmp(b->rbuffer, "DATA", 4))
52b84b
+                        return -EPERM;
52b84b
+        } else if (d - (char*) b->rbuffer == 3 + 32) {
52b84b
+                /*
52b84b
+                 * Old versions of the server-side implementation of `sd-bus` replied with "OK <id>" to
52b84b
+                 * "AUTH" requests from a client, even if the "AUTH" line did not contain inlined
52b84b
+                 * arguments. Therefore, we also accept "OK <id>" here, even though it is technically the
52b84b
+                 * wrong reply. We ignore the "<id>" parameter, though, since it has no real value.
52b84b
+                 */
52b84b
+                if (memcmp(b->rbuffer, "OK ", 3))
52b84b
+                        return -EPERM;
52b84b
+        } else {
52b84b
+                return -EPERM;
52b84b
+        }
52b84b
+
52b84b
+        /* Now check the OK line. */
52b84b
 
52b84b
-        if (e - (char*) b->rbuffer != 3 + 32)
52b84b
+        if (e - d != 2 + 3 + 32)
52b84b
                 return -EPERM;
52b84b
 
52b84b
-        if (memcmp(b->rbuffer, "OK ", 3))
52b84b
+        if (memcmp(d + 2, "OK ", 3))
52b84b
                 return -EPERM;
52b84b
 
52b84b
         b->auth = b->anonymous_auth ? BUS_AUTH_ANONYMOUS : BUS_AUTH_EXTERNAL;
52b84b
@@ -201,8 +226,8 @@ static int bus_socket_auth_verify_client(sd_bus *b) {
52b84b
         for (i = 0; i < 32; i += 2) {
52b84b
                 int x, y;
52b84b
 
52b84b
-                x = unhexchar(((char*) b->rbuffer)[3 + i]);
52b84b
-                y = unhexchar(((char*) b->rbuffer)[3 + i + 1]);
52b84b
+                x = unhexchar(d[2 + 3 + i]);
52b84b
+                y = unhexchar(d[2 + 3 + i + 1]);
52b84b
 
52b84b
                 if (x < 0 || y < 0)
52b84b
                         return -EINVAL;
52b84b
@@ -216,7 +241,7 @@ static int bus_socket_auth_verify_client(sd_bus *b) {
52b84b
 
52b84b
         b->server_id = peer;
52b84b
 
52b84b
-        /* And possibly check the second line, too */
52b84b
+        /* And possibly check the third line, too */
52b84b
 
52b84b
         if (f)
52b84b
                 b->can_fds =
52b84b
@@ -616,42 +641,39 @@ static void bus_get_peercred(sd_bus *b) {
52b84b
 }
52b84b
 
52b84b
 static int bus_socket_start_auth_client(sd_bus *b) {
52b84b
-        size_t l;
52b84b
-        const char *auth_suffix, *auth_prefix;
52b84b
+        static const char sasl_auth_anonymous[] = {
52b84b
+                /*
52b84b
+                 * We use an arbitrary trace-string for the ANONYMOUS authentication. It can be used by the
52b84b
+                 * message broker to aid debugging of clients. We fully anonymize the connection and use a
52b84b
+                 * static default.
52b84b
+                 */
52b84b
+                "\0AUTH ANONYMOUS\r\n"
52b84b
+                /* HEX a n o n y m o u s */
52b84b
+                "DATA 616e6f6e796d6f7573\r\n"
52b84b
+        };
52b84b
+        static const char sasl_auth_external[] = {
52b84b
+                "\0AUTH EXTERNAL\r\n"
52b84b
+                "DATA\r\n"
52b84b
+        };
52b84b
+        static const char sasl_negotiate_unix_fd[] = {
52b84b
+                "NEGOTIATE_UNIX_FD\r\n"
52b84b
+        };
52b84b
+        static const char sasl_begin[] = {
52b84b
+                "BEGIN\r\n"
52b84b
+        };
52b84b
+        size_t i = 0;
52b84b
 
52b84b
         assert(b);
52b84b
 
52b84b
-        if (b->anonymous_auth) {
52b84b
-                auth_prefix = "\0AUTH ANONYMOUS ";
52b84b
-
52b84b
-                /* For ANONYMOUS auth we send some arbitrary "trace" string */
52b84b
-                l = 9;
52b84b
-                b->auth_buffer = hexmem("anonymous", l);
52b84b
-        } else {
52b84b
-                char text[DECIMAL_STR_MAX(uid_t) + 1];
52b84b
-
52b84b
-                auth_prefix = "\0AUTH EXTERNAL ";
52b84b
-
52b84b
-                xsprintf(text, UID_FMT, geteuid());
52b84b
-
52b84b
-                l = strlen(text);
52b84b
-                b->auth_buffer = hexmem(text, l);
52b84b
-        }
52b84b
-
52b84b
-        if (!b->auth_buffer)
52b84b
-                return -ENOMEM;
52b84b
+        if (b->anonymous_auth)
52b84b
+                b->auth_iovec[i++] = IOVEC_MAKE((char*) sasl_auth_anonymous, sizeof(sasl_auth_anonymous) - 1);
52b84b
+        else
52b84b
+                b->auth_iovec[i++] = IOVEC_MAKE((char*) sasl_auth_external, sizeof(sasl_auth_external) - 1);
52b84b
 
52b84b
         if (b->accept_fd)
52b84b
-                auth_suffix = "\r\nNEGOTIATE_UNIX_FD\r\nBEGIN\r\n";
52b84b
-        else
52b84b
-                auth_suffix = "\r\nBEGIN\r\n";
52b84b
-
52b84b
-        b->auth_iovec[0].iov_base = (void*) auth_prefix;
52b84b
-        b->auth_iovec[0].iov_len = 1 + strlen(auth_prefix + 1);
52b84b
-        b->auth_iovec[1].iov_base = (void*) b->auth_buffer;
52b84b
-        b->auth_iovec[1].iov_len = l * 2;
52b84b
-        b->auth_iovec[2].iov_base = (void*) auth_suffix;
52b84b
-        b->auth_iovec[2].iov_len = strlen(auth_suffix);
52b84b
+                b->auth_iovec[i++] = IOVEC_MAKE_STRING(sasl_negotiate_unix_fd);
52b84b
+
52b84b
+        b->auth_iovec[i++] = IOVEC_MAKE_STRING(sasl_begin);
52b84b
 
52b84b
         return bus_socket_write_auth(b);
52b84b
 }