From 1ad65fc10ef361352ade35d4ad9c29d5d35a25ed Mon Sep 17 00:00:00 2001 From: hippoz <10706925-hippoz@users.noreply.gitlab.com> Date: Mon, 16 Jan 2023 19:47:40 +0200 Subject: [PATCH] fix broadcasting, notably fixes xfconf and thunar --- match.c | 34 +++++++++++++++++++++++++++-- match.h | 14 ++---------- server.c | 66 +++++++++++++++++++++++++++++++++++--------------------- server.h | 17 ++++++++++++--- wire.c | 5 ++++- 5 files changed, 93 insertions(+), 43 deletions(-) diff --git a/match.c b/match.c index c35aefe..d061d57 100644 --- a/match.c +++ b/match.c @@ -6,6 +6,7 @@ #include "util.h" #include "wire.h" #include "try.h" +#include "server.h" void match_rule_free(match_rule_t *rule) { @@ -177,13 +178,42 @@ fail: } \ } while(0) -int match_rule_check(match_rule_t *rule, wire_message_t *msg, wire_context_t *ctx) +int match_check_sender(bus_t *s, int sender_index, match_rule_t *rule) +{ + bus_client_t *sender_client = &s->clients[sender_index]; + if (rule->sender) { + if (sender_client->unique_name_index >= 0) { + if (strcmp(rule->sender, s->names[sender_client->unique_name_index].name) == 0) { + return 1; + } + } + for (int i = 0; i < BUS_NAMES_PER_CLIENT; i++) { + if (sender_client->owned_names[i] >= 0 && strcmp(rule->sender, s->names[sender_client->owned_names[i]].name) == 0) { + return 1; + } + } + } + + return -1; +} + +int match_rule_check(bus_t *s, int sender_index, match_rule_t *rule, wire_message_t *msg, wire_context_t *ctx) { if (rule->type && msg->type != rule->type) { return -1; } - _check_header_field_str(sender, DBUS_HEADER_FIELD_SENDER); + if (sender_index < 0) { + // if the sender is negative, we assume the message is coming from the message bus + if (rule->sender && strcmp(rule->sender, "org.freedesktop.DBus") != 0) { + return -1; + } + } else { + if (match_check_sender(s, sender_index, rule) < 0) { + return -1; + } + } + _check_header_field_str(interface, DBUS_HEADER_FIELD_INTERFACE); _check_header_field_str(member, DBUS_HEADER_FIELD_MEMBER); _check_header_field_str(path, DBUS_HEADER_FIELD_PATH); diff --git a/match.h b/match.h index 63ca924..3ccc16f 100644 --- a/match.h +++ b/match.h @@ -4,20 +4,10 @@ #include #include #include "wire.h" - -#define MATCH_RULE_MAX 1024 -#define MATCH_RULE_MAX_ARG 12 - -typedef struct match_rule { - uint8_t type; - bool eavesdrop; - char *sender, *interface, *member, *path, *path_namespace, *destination, *arg0namespace, *rule_string; - char *arg[MATCH_RULE_MAX_ARG]; /* TODO: spec states that indexes 0 to 63 should be supported */ - char *arg_path[MATCH_RULE_MAX_ARG]; /* TODO: spec states that indexes 0 to 63 should be supported */ -} match_rule_t; +#include "server.h" void match_rule_free(match_rule_t *rule); match_rule_t *match_rule_from_string(char *d); -int match_rule_check(match_rule_t *rule, wire_message_t *msg, wire_context_t *ctx); +int match_rule_check(bus_t *s, int sender_index, match_rule_t *rule, wire_message_t *msg, wire_context_t *ctx); #endif // _JITTERBUG__MATCH_H diff --git a/server.c b/server.c index 517d6a7..4c70778 100644 --- a/server.c +++ b/server.c @@ -73,7 +73,6 @@ bus_t *bus_create(const char *socket_path) for (int i = 0; i < BUS_MAX_CLIENTS; i++) { s->clients[i].fd = -1; - s->clients[i].owned_name_index = -1; s->clients[i].unique_name_index = -1; s->clients[i].state = BUS_CLIENT_STATE_NONE; s->clients[i].match_count = 0; @@ -83,6 +82,9 @@ bus_t *bus_create(const char *socket_path) for (int j = 0; j < BUS_MAX_MATCH; j++) { s->clients[i].matches[j] = NULL; } + for (int j = 0; j < BUS_NAMES_PER_CLIENT; j++) { + s->clients[i].owned_names[j] = -1; + } } for (int i = 0; i < BUS_MAX_NAMES; i++) { @@ -104,7 +106,6 @@ int bus_client_add(bus_t *s, int fd) for (int i = 0; i < BUS_MAX_CLIENTS; i++) { if (s->clients[i].fd < 0) { s->clients[i].fd = fd; - s->clients[i].owned_name_index = -1; s->clients[i].unique_name_index = -1; s->clients[i].state = BUS_CLIENT_STATE_WAIT_AUTH; s->fds[i].fd = fd; @@ -157,10 +158,17 @@ int bus_name_add(bus_t *s, char *name, int client_index) for (int i = bucket; i < bucket + 12 && i < BUS_MAX_NAMES; i++) { if (s->names[i].client_index == -1) { - s->names[i].client_index = client_index; - s->names[i].name = name; - s->names_count++; - return i; + for (int j = 0; j < BUS_NAMES_PER_CLIENT; j++) { + if (s->clients[client_index].owned_names[j] < 0) { + s->clients[client_index].owned_names[j] = i; + s->names[i].client_index = client_index; + s->names[i].name = name; + s->names_count++; + return i; + } + } + + return -1; } } @@ -220,12 +228,11 @@ int bus_client_assign_unique_name(bus_t *s, int i) int bus_client_assign_own_name(bus_t *s, int i, char *name) { bus_client_t *c = &s->clients[i]; - if (c->owned_name_index != -1 || !name || *name == ':' || *name == '\0') { + if (!name || *name == ':' || *name == '\0') { return -1; } int name_index = TRYST(bus_name_add(s, name, i)); - c->owned_name_index = name_index; return 0; } @@ -255,10 +262,14 @@ void bus_client_remove(bus_t *s, int i) c->matches[j] = NULL; } } - bus_name_remove(s, c->unique_name_index); - bus_name_remove(s, c->owned_name_index); + for (int j = 0; j < BUS_NAMES_PER_CLIENT; j++) { + if (c->owned_names[j] >= 0) { + bus_name_remove(s, c->owned_names[j]); + c->owned_names[j] = -1; + } + } + c->unique_name_index = -1; - c->owned_name_index = -1; c->match_count = 0; c->fd = -1; c->state = BUS_CLIENT_STATE_NONE; @@ -275,16 +286,20 @@ void bus_client_error(bus_t *s, int i, const char *msg) bus_client_remove(s, i); } -int bus_broadcast_message(bus_t *s, wire_message_t *msg, wire_context_t *ctx, char *sender_unique_name, wire_context_t *reply_ctx) +int bus_broadcast_message(bus_t *s, int sender_index, wire_message_t *msg, wire_context_t *ctx, wire_context_t *reply_ctx) { uint32_t body_end = ctx->byte_cursor + msg->body_length; if (body_end >= ctx->data_len) { return -1; } + bus_client_t *sender_client = &s->clients[sender_index]; + + printf("broadcasting message of %d bytes\n", body_end); + int left = s->clients_count; for (int i = 0; i < BUS_MAX_CLIENTS && left > 0; i++) { - if (s->clients[i].match_count <= 0) { + if (s->clients[i].state != BUS_CLIENT_STATE_READY) { continue; } @@ -298,16 +313,17 @@ int bus_broadcast_message(bus_t *s, wire_message_t *msg, wire_context_t *ctx, ch } match_left--; - if (match_rule_check(c->matches[j], msg, ctx) >= 0) { - uint32_t previous_cursor = ctx->byte_cursor; - TRYST(wire_compose_unicast_reply(reply_ctx, ctx, msg, sender_unique_name)); + + uint32_t previous_cursor = ctx->byte_cursor; + if (match_rule_check(s, sender_index, c->matches[j], msg, ctx) >= 0) { + printf("---------> sending to %d\n", c->fd); + TRYST(wire_compose_unicast_reply(reply_ctx, ctx, msg, s->names[sender_client->unique_name_index].name)); TRYST(send(c->fd, reply_ctx->data, reply_ctx->byte_cursor, 0)); - ctx->byte_cursor = previous_cursor; // TODO? - reply_ctx->byte_cursor = 0; memset(reply_ctx->data, 0, reply_ctx->data_len); - break; } + ctx->byte_cursor = previous_cursor; + reply_ctx->byte_cursor = 0; } } @@ -317,7 +333,7 @@ int bus_broadcast_message(bus_t *s, wire_message_t *msg, wire_context_t *ctx, ch return 0; } -int bus_broadcast_signal(bus_t *s, wire_context_t *ctx, wire_message_t *msg) +int bus_broadcast_signal(bus_t *s, int sender_index, wire_context_t *ctx, wire_message_t *msg) { int left = s->clients_count; for (int i = 0; i < BUS_MAX_CLIENTS && left > 0; i++) { @@ -335,7 +351,7 @@ int bus_broadcast_signal(bus_t *s, wire_context_t *ctx, wire_message_t *msg) } match_left--; - if (match_rule_check(c->matches[j], msg, ctx) >= 0) { + if (match_rule_check(s, sender_index, c->matches[j], msg, ctx) >= 0) { uint32_t previous_cursor = ctx->byte_cursor; TRYST(send(c->fd, ctx->data, ctx->byte_cursor, 0)); ctx->byte_cursor = previous_cursor; @@ -370,7 +386,7 @@ int bus_unicast_message(bus_t *s, wire_message_t *msg, wire_context_t *ctx, char #define _signal_end() \ *signal_body_length = signal_reply_ctx.byte_cursor - signal_body_start; \ - TRYST(bus_broadcast_signal(s, &signal_reply_ctx, &signal_reply_msg)); \ + TRYST(bus_broadcast_signal(s, -1, &signal_reply_ctx, &signal_reply_msg)); \ #define _reply_begin(M_sig) \ @@ -434,9 +450,9 @@ int handle_request_name(bus_t *s, int i, wire_message_t *msg, wire_context_t *ct if (bus_client_assign_own_name(s, i, name_str) < 0) { // TODO: report the actual error - status_code = DBUS_REQUEST_NAME_REPLY_EXISTS; + status_code = DBUS_REQUEST_NAME_REPLY_IN_QUEUE; - WARN("client '%s' (index=%d) couldn't acquire name '%s'\n", s->names[s->clients[i].owned_name_index].name, i, name_str); + WARN("client '%s' (index=%d) couldn't acquire name '%s'\n", s->names[s->clients[i].unique_name_index].name, i, name_str); free(name_str); } @@ -730,7 +746,7 @@ int bus_client_process_message(bus_t *s, int i, wire_context_t *ctx) goto end_nonfatal; } } else { - if (bus_broadcast_message(s, &msg, ctx, s->names[client->unique_name_index].name, &reply_ctx) < 0) { + if (bus_broadcast_message(s, i, &msg, ctx, &reply_ctx) < 0) { _process_message_reply_error("xyz.hippoz.jitterbug.BroadcastFailed"); goto end_nonfatal; } diff --git a/server.h b/server.h index 29a42d9..3d45b21 100644 --- a/server.h +++ b/server.h @@ -1,15 +1,26 @@ #ifndef _JITTERBUG__SERVER_H #define _JITTERBUG__SERVER_H -#include "match.h" #include "wire.h" #include #include +#define MATCH_RULE_MAX 1024 +#define MATCH_RULE_MAX_ARG 12 + +typedef struct match_rule { + uint8_t type; + bool eavesdrop; + char *sender, *interface, *member, *path, *path_namespace, *destination, *arg0namespace, *rule_string; + char *arg[MATCH_RULE_MAX_ARG]; /* TODO: spec states that indexes 0 to 63 should be supported */ + char *arg_path[MATCH_RULE_MAX_ARG]; /* TODO: spec states that indexes 0 to 63 should be supported */ +} match_rule_t; + // TODO: dynamically size the arrays #define BUS_MAX_CLIENTS 256 #define BUS_MAX_NAMES 512 #define BUS_MAX_MATCH 12 +#define BUS_NAMES_PER_CLIENT 4 #define BUS_BACKLOG 12 enum { @@ -21,11 +32,11 @@ enum { typedef struct bus_client { int fd; - uint8_t state; int16_t unique_name_index; - int16_t owned_name_index; + uint8_t state; int8_t match_count; match_rule_t *matches[BUS_MAX_MATCH]; + int16_t owned_names[BUS_NAMES_PER_CLIENT]; } bus_client_t; typedef struct bus_name { diff --git a/wire.c b/wire.c index fe40fe7..8043265 100644 --- a/wire.c +++ b/wire.c @@ -407,6 +407,7 @@ int wire_compose_unicast_reply(wire_context_t *c, wire_context_t *msg_c, wire_me uint32_t header_fields_start = c->byte_cursor; switch (msg->type) { + case DBUS_MESSAGE_SIGNAL: /* through */ case DBUS_MESSAGE_METHOD_CALL: { // these fields are required for DBUS_MESSAGE_METHOD_CALL, so we must (hackily) check for their presence // interface is not strictly required by the spec, however we require it here anyways @@ -581,7 +582,7 @@ int wire_collect_strings(wire_context_t *c, wire_message_t *msg, wire_message_bo } break; default: { - return -1; + goto end_nonfatal; } } @@ -589,6 +590,8 @@ int wire_collect_strings(wire_context_t *c, wire_message_t *msg, wire_message_bo sig++; } +end_nonfatal: + c->byte_cursor = body_cursor; return stri;