libink: a broker peer is not an ordinary client

libink was written peer to peer, where one connection is one client
and one principal.  Attaching to a message bus breaks both halves of
that, and two things followed from it.

Signals never reached the system bus.  Fan-out is gated on the peer
having sent AddMatch, but a broker subscribes for its own clients and
never sends us one, so every ServiceStateChanged was dropped on the
floor.  A connection attached with LINK_ATTACH_BROKER gets them all.

Hello, AddMatch and RemoveMatch write per-connection state.  Shared by
every caller behind a broker, that lets one sender exhaust the match
cap or drop another's rule, so we leave all three to the bus, whose
job they are.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

libink/builtin.c

@@ -459,6 +459,20 @@ static int handle_remove_match(link_connection_t *conn, const struct link_msg *m
 
 int __handle_builtin(link_connection_t *conn, const struct link_msg *m)
 {
+	/* Hello and AddMatch/RemoveMatch are per-connection state, and on
+	 * a broker link the connection is shared by every caller: one
+	 * sender could exhaust the match cap or drop another's rule.
+	 * Naming and subscription belong to the broker for its own
+	 * clients, so we do not answer these there. */
+	if (conn->broker && m->member &&
+	    (!strcmp(m->member, "Hello") ||
+	     !strcmp(m->member, "AddMatch") ||
+	     !strcmp(m->member, "RemoveMatch"))) {
+		return __send_error(conn, m,
+			"org.freedesktop.DBus.Error.AccessDenied",
+			"Handled by the message bus, not by this peer");
+	}
+
 	if (member_is(m, "org.freedesktop.DBus", "Hello") &&
 	    m->path && strcmp(m->path, "/org/freedesktop/DBus") == 0)
 		return handle_hello(conn, m);

libink/dispatch.c

@@ -187,12 +187,14 @@ int link_connection_emit_signal(link_connection_t *conn,
 	if (conn->auth != LINK_AUTH_DONE)
 		return 0;	/* peer hasn't finished the SASL phase */
 
-	for (i = 0; i < conn->matches_count; i++) {
+	/* A broker routes to whoever subscribed with it, so it wants
+	 * every signal and never sends us AddMatch of its own. */
+	matched = conn->broker;
+
+	for (i = 0; !matched && i < conn->matches_count; i++) {
 		if (__match_matches(conn->matches[i], path,
-				       interface, member)) {
+				       interface, member))
 			matched = 1;
-			break;
-		}
 	}
 	if (!matched)
 		return 0;	/* peer didn't subscribe — nothing to do */

libink/internal.h

@@ -91,9 +91,11 @@ struct link_connection {
 
 	/* Match rules registered via org.freedesktop.DBus.AddMatch.
 	 * Bounded for PID 1 hygiene; a peer that exceeds the cap gets
-	 * a LimitsExceeded error reply. */
+	 * a LimitsExceeded error reply.  A broker never registers any,
+	 * it matches for its own clients, so `broker` bypasses them. */
 	struct link_match   *matches[LINK_MATCH_PEER_CAP];
 	size_t              matches_count;
+	int                 broker;
 
 	uint8_t             rxbuf[LINK_RX_BUF_SIZE];
 	size_t              rxlen;

libink/link.h

@@ -105,8 +105,16 @@ int   link_server_accept(link_server_t *server, link_connection_t **conn);
  *
  * On success the connection takes ownership of `fd`.  On any failure
  * `fd` is closed before the function returns NULL, so callers never
- * have to track partial state. */
-link_connection_t *link_server_attach(link_server_t *server, int fd, uid_t peer_uid);
+ * have to track partial state.
+ *
+ * LINK_ATTACH_BROKER says the peer is a message bus rather than an
+ * ordinary client.  A broker subscribes on behalf of its own clients
+ * and never sends us AddMatch, so signals go to it unconditionally
+ * instead of being filtered by this connection's match rules. */
+#define LINK_ATTACH_BROKER  0x01
+
+link_connection_t *link_server_attach(link_server_t *server, int fd, uid_t peer_uid,
+				      unsigned int attach_flags);
 
 int   link_connection_get_fd  (const link_connection_t *conn);
 uid_t link_connection_get_uid (const link_connection_t *conn);

libink/server.c

@@ -162,7 +162,8 @@ int link_server_accept(link_server_t *srv, link_connection_t **out)
 	return 0;
 }
 
-link_connection_t *link_server_attach(link_server_t *srv, int fd, uid_t peer_uid)
+link_connection_t *link_server_attach(link_server_t *srv, int fd, uid_t peer_uid,
+				      unsigned int attach_flags)
 {
 	link_connection_t *conn;
 	int flags;
@@ -194,6 +195,7 @@ link_connection_t *link_server_attach(link_server_t *srv, int fd, uid_t peer_uid
 	conn->auth     = LINK_AUTH_DONE;	/* caller already handshook */
 	conn->server   = srv;
 	conn->peer_uid = peer_uid;
+	conn->broker   = !!(attach_flags & LINK_ATTACH_BROKER);
 	__auth_generate_guid(conn->guid);
 
 	return conn;

src/dbus.c

@@ -1280,7 +1280,8 @@ static int try_attach_system_bus(uev_ctx_t *ctx)
 	/* link_server_attach owns the fd from this point on whether it
 	 * succeeds or fails, so the steal-then-attach pair has no leak
 	 * window. */
-	conn = link_server_attach(server, link_client_steal_fd(c), (uid_t)-1);
+	conn = link_server_attach(server, link_client_steal_fd(c), (uid_t)-1,
+				  LINK_ATTACH_BROKER);
 	if (!conn)
 		return -1;