From: David Rheinsberg Date: Thu, 14 Mar 2019 12:34:13 +0000 (+0100) Subject: sd-bus: skip sending formatted UIDs via SASL X-Git-Tag: v242-rc1~129^2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1ed4723d38cd0d1423c8fe650f90fa86007ddf55;p=thirdparty%2Fsystemd.git sd-bus: skip sending formatted UIDs via SASL The dbus external authentication takes as optional argument the UID the sender wants to authenticate as. This uid is purely optional. The AF_UNIX socket already conveys the same information through the auxiliary socket data, so we really don't have to provide that information. Unfortunately, there is no way to send empty arguments, since they are interpreted as "missing argument", which has a different meaning. The SASL negotiation thus changes from: AUTH EXTERNAL NEGOTIATE_UNIX_FD (optional) BEGIN to: AUTH EXTERNAL DATA NEGOTIATE_UNIX_FD (optional) BEGIN And thus the replies we expect as a client change from: OK AGREE_UNIX_FD (optional) to: DATA OK AGREE_UNIX_FD (optional) Since the old sd-bus server implementation used the wrong reply for "AUTH" requests that do not carry the arguments inlined, we decided to make sd-bus clients accept this as well. Hence, sd-bus now allows "OK \r\n" replies instead of "DATA\r\n" replies. Signed-off-by: David Rheinsberg --- diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index fd85960a41d..790b64010a5 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -158,17 +158,25 @@ static int bus_socket_write_auth(sd_bus *b) { } static int bus_socket_auth_verify_client(sd_bus *b) { - char *e, *f, *start; + char *d, *e, *f, *start; sd_id128_t peer; unsigned i; int r; assert(b); - /* We expect two response lines: "OK" and possibly - * "AGREE_UNIX_FD" */ + /* + * We expect three response lines: + * "DATA\r\n" + * "OK \r\n" + * "AGREE_UNIX_FD\r\n" (optional) + */ - e = memmem_safe(b->rbuffer, b->rbuffer_size, "\r\n", 2); + d = memmem_safe(b->rbuffer, b->rbuffer_size, "\r\n", 2); + if (!d) + return 0; + + e = memmem(d + 2, b->rbuffer_size - (d - (char*) b->rbuffer) - 2, "\r\n", 2); if (!e) return 0; @@ -183,13 +191,30 @@ static int bus_socket_auth_verify_client(sd_bus *b) { start = e + 2; } - /* Nice! We got all the lines we need. First check the OK - * line */ + /* Nice! We got all the lines we need. First check the DATA line. */ + + if (d - (char*) b->rbuffer == 4) { + if (memcmp(b->rbuffer, "DATA", 4)) + return -EPERM; + } else if (d - (char*) b->rbuffer == 3 + 32) { + /* + * Old versions of the server-side implementation of `sd-bus` replied with "OK " to + * "AUTH" requests from a client, even if the "AUTH" line did not contain inlined + * arguments. Therefore, we also accept "OK " here, even though it is technically the + * wrong reply. We ignore the "" parameter, though, since it has no real value. + */ + if (memcmp(b->rbuffer, "OK ", 3)) + return -EPERM; + } else { + return -EPERM; + } + + /* Now check the OK line. */ - if (e - (char*) b->rbuffer != 3 + 32) + if (e - d != 2 + 3 + 32) return -EPERM; - if (memcmp(b->rbuffer, "OK ", 3)) + if (memcmp(d + 2, "OK ", 3)) return -EPERM; b->auth = b->anonymous_auth ? BUS_AUTH_ANONYMOUS : BUS_AUTH_EXTERNAL; @@ -197,8 +222,8 @@ static int bus_socket_auth_verify_client(sd_bus *b) { for (i = 0; i < 32; i += 2) { int x, y; - x = unhexchar(((char*) b->rbuffer)[3 + i]); - y = unhexchar(((char*) b->rbuffer)[3 + i + 1]); + x = unhexchar(d[2 + 3 + i]); + y = unhexchar(d[2 + 3 + i + 1]); if (x < 0 || y < 0) return -EINVAL; @@ -212,7 +237,7 @@ static int bus_socket_auth_verify_client(sd_bus *b) { b->server_id = peer; - /* And possibly check the second line, too */ + /* And possibly check the third line, too */ if (f) b->can_fds = @@ -611,39 +636,39 @@ static void bus_get_peercred(sd_bus *b) { } static int bus_socket_start_auth_client(sd_bus *b) { - size_t l; - const char *auth_suffix, *auth_prefix; + static const char sasl_auth_anonymous[] = { + /* + * We use an arbitrary trace-string for the ANONYMOUS authentication. It can be used by the + * message broker to aid debugging of clients. We fully anonymize the connection and use a + * static default. + */ + "\0AUTH ANONYMOUS\r\n" + /* HEX a n o n y m o u s */ + "DATA 616e6f6e796d6f7573\r\n" + }; + static const char sasl_auth_external[] = { + "\0AUTH EXTERNAL\r\n" + "DATA\r\n" + }; + static const char sasl_negotiate_unix_fd[] = { + "NEGOTIATE_UNIX_FD\r\n" + }; + static const char sasl_begin[] = { + "BEGIN\r\n" + }; + size_t i = 0; assert(b); - if (b->anonymous_auth) { - auth_prefix = "\0AUTH ANONYMOUS "; - - /* For ANONYMOUS auth we send some arbitrary "trace" string */ - l = 9; - b->auth_buffer = hexmem("anonymous", l); - } else { - char text[DECIMAL_STR_MAX(uid_t) + 1]; - - auth_prefix = "\0AUTH EXTERNAL "; - - xsprintf(text, UID_FMT, geteuid()); - - l = strlen(text); - b->auth_buffer = hexmem(text, l); - } - - if (!b->auth_buffer) - return -ENOMEM; + if (b->anonymous_auth) + b->auth_iovec[i++] = IOVEC_MAKE((char*) sasl_auth_anonymous, sizeof(sasl_auth_anonymous) - 1); + else + b->auth_iovec[i++] = IOVEC_MAKE((char*) sasl_auth_external, sizeof(sasl_auth_external) - 1); if (b->accept_fd) - auth_suffix = "\r\nNEGOTIATE_UNIX_FD\r\nBEGIN\r\n"; - else - auth_suffix = "\r\nBEGIN\r\n"; + b->auth_iovec[i++] = IOVEC_MAKE_STRING(sasl_negotiate_unix_fd); - b->auth_iovec[0] = IOVEC_MAKE((void*) auth_prefix, 1 + strlen(auth_prefix + 1)); - b->auth_iovec[1] = IOVEC_MAKE(b->auth_buffer, l * 2); - b->auth_iovec[2] = IOVEC_MAKE_STRING(auth_suffix); + b->auth_iovec[i++] = IOVEC_MAKE_STRING(sasl_begin); return bus_socket_write_auth(b); }