resolved: actually unregister DNS-SD services when the D-Bus client vanishes (#43578)

Since the D-Bus DNS-SD API was introduced in c3036641f0ed (v236),
`RegisterService()` has created an `sd_bus_track` object to destroy the
registered service when the registering client disconnects from the bus.
However, the track object is a `_cleanup_(sd_bus_track_unrefp)` local in
`bus_method_register_service()` that is never stored anywhere, so it is
destroyed again as soon as the method handler returns. The vanish
handler (`dnssd_registered_service_on_bus_track()`) has thus been dead
code for the past ~8 years.

The observable effect: services registered via D-Bus silently persist
after the client disconnects — unowned, advertised until resolved exits,
and impossible to re-register since the identifier stays taken
(`DNSSD_SERVICE_EXISTS`). At the same time, the registration doesn't
survive a resolved restart, so the current behavior is neither reliably
ephemeral nor reliably persistent. Noticed while reviewing a project
that (unknowingly relying on this bug) registers a service with a
one-shot `gdbus call` and expects it to persist.

This PR:
1. splits the goodbye/zone-removal logic out of
`bus_dnssd_method_unregister()` into a helper — the vanish path needs
the full teardown, not just `dnssd_registered_service_free()`, which
would leave the service's RRs published in the per-link mDNS zones;
2. stores the track object in `DnssdRegisteredService`, making client
tracking work as originally intended;
3. documents the lifetime semantics in `org.freedesktop.resolve1(5)`
(matching the equivalent Avahi D-Bus API, whose entry groups are
likewise bound to the client connection) and points to `*.dnssd` files
for registrations independent of a client's lifetime;
4. adds a TEST-75 case covering the connection-bound lifetime.

Note this is a behavioral change for fire-and-forget clients (one-shot
`busctl`/`gdbus` invocations). Arguably those were never working
reliably (registrations silently vanished on any resolved restart, and
stale entries blocked re-registration), but if this is deemed too risky,
the alternative would be to drop the dead tracking code and codify the
persist-until-exit behavior instead — I'd argue tracking as originally
intended is the better contract.

Verification: code compiles; the new integration test is included.

NEWS

@@ -4,6 +4,19 @@ CHANGES WITH 262 in spe:
 
         Feature Removals and Incompatible Changes:
 
+        * DNS-SD services registered via systemd-resolved's D-Bus method
+          org.freedesktop.resolve1.Manager.RegisterService() are now
+          unregistered automatically when the client that registered them
+          disconnects from the bus, as originally intended and matching the
+          behavior of the equivalent Avahi API. Due to a bug the client
+          tracking never worked, and such services used to stay registered
+          until systemd-resolved exited (but did not survive a restart of the
+          daemon either). Clients must now keep their bus connection open for
+          as long as the service shall remain registered; one-shot
+          registrations, e.g. via busctl or gdbus, will be withdrawn again as
+          soon as the tool exits. To register a DNS-SD service independently
+          of a client's lifetime, use *.dnssd files, see systemd.dnssd(5).
+
         * Meson options '-Dlibiptc=' (deprecated in v259), '-Dlibidn=',
           '-Drc-local=', '-Dsysvinit-path=', and '-Dsysvrcnd-path=' (deprecated
           in v260) have been removed.

man/org.freedesktop.resolve1.xml

@@ -450,6 +450,18 @@ node /org/freedesktop/resolve1 {
       DNS-SD SRV record. Finally, it takes an array of TXT record data. It returns an object path which may be
       used as handle to the registered service.</para>
 
+      <para>The lifetime of a service registered via this method is bound to the D-Bus connection of the
+      client that registered it: when the client disconnects from the bus the service is automatically
+      unregistered and goodbye packets are sent out, as if <function>UnregisterService()</function> had been
+      called. Clients hence must keep their bus connection open for as long as the service shall remain
+      registered. The registration also does not survive <filename>systemd-resolved</filename> losing its
+      own bus connection, in which case tracking clients is not possible anymore and all services registered
+      via this method are unregistered. To register a DNS-SD service independently of a client's lifetime,
+      use <filename>*.dnssd</filename> files, see
+      <citerefentry><refentrytitle>systemd.dnssd</refentrytitle><manvolnum>5</manvolnum></citerefentry>. Note
+      that prior to version 262, due to a bug, the client's connection was not tracked and services
+      registered via this method stayed registered until the daemon exited.</para>
+
       <para>The <function>UnregisterService()</function> method undoes the effect of
       <function>RegisterService()</function> and deletes a DNS-SD service previously created via IPC
       again.</para>

src/resolve/resolved-bus.c

@@ -1894,8 +1894,8 @@ static int dnssd_registered_service_on_bus_track(sd_bus_track *t, void *userdata
 
         assert(t);
 
-        log_debug("Client of active request vanished, destroying DNS-SD service.");
-        dnssd_registered_service_free(s);
+        log_debug("Client of DNS-SD service '%s' vanished, unregistering.", s->id);
+        dnssd_registered_service_unregister(s);
 
         return 0;
 }
@@ -2063,6 +2063,10 @@ static int bus_method_register_service(sd_bus_message *message, void *userdata,
         if (r < 0)
                 return r;
 
+        /* The service is unregistered automatically when the client that registered it disconnects from the
+         * bus, hence keep the bus track object pinned for the lifetime of the service. */
+        service->bus_track = TAKE_PTR(bus_track);
+
         service->manager = m;
 
         r = hashmap_ensure_put(&m->dnssd_registered_services, &string_hash_ops, service->id, service);

src/resolve/resolved-dns-scope.c

@@ -1533,6 +1533,42 @@ static int on_announcement_timeout(sd_event_source *s, usec_t usec, void *userda
         return 0;
 }
 
+int dns_scope_send_goodbye(DnsScope *scope, DnsAnswer *answer) {
+        _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
+        int r;
+
+        assert(scope);
+        assert(answer);
+
+        /* Sends an unsolicited response withdrawing the specified RRs, which are expected to be flagged as
+         * goodbye (TTL 0), without touching anything else published in the zone. */
+
+        if (scope->protocol != DNS_PROTOCOL_MDNS)
+                return 0;
+
+        r = sd_event_get_state(scope->manager->event);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to get event loop state: %m");
+
+        /* If this is called on exit, through manager_free() -> link_free(), then we cannot announce. */
+        if (r == SD_EVENT_FINISHED)
+                return 0;
+
+        if (dns_answer_isempty(answer))
+                return 0;
+
+        r = dns_scope_make_reply_packet(scope, /* id= */ 0, DNS_RCODE_SUCCESS, /* q= */ NULL, answer,
+                                        /* soa= */ NULL, /* tentative= */ false, &p);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to build reply packet: %m");
+
+        r = dns_scope_emit_udp(scope, -EBADF, AF_UNSPEC, p);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to send reply packet: %m");
+
+        return 0;
+}
+
 int dns_scope_announce(DnsScope *scope, bool goodbye) {
         _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL;
         _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;

src/resolve/resolved-dns-scope.h

@@ -114,6 +114,7 @@ int dns_scope_ifindex(DnsScope *s);
 const char* dns_scope_ifname(DnsScope *s);
 
 int dns_scope_announce(DnsScope *scope, bool goodbye);
+int dns_scope_send_goodbye(DnsScope *scope, DnsAnswer *answer);
 
 int dns_scope_add_dnssd_registered_services(DnsScope *scope);
 int dns_scope_remove_dnssd_registered_services(DnsScope *scope);

src/resolve/resolved-dnssd-bus.c

@@ -6,18 +6,14 @@
 #include "bus-object.h"
 #include "bus-polkit.h"
 #include "hashmap.h"
-#include "log.h"
-#include "resolved-dns-scope.h"
 #include "resolved-dnssd.h"
 #include "resolved-dnssd-bus.h"
-#include "resolved-link.h"
 #include "resolved-manager.h"
 #include "strv.h"
 
 int bus_dnssd_method_unregister(sd_bus_message *message, void *userdata, sd_bus_error *error) {
         DnssdRegisteredService *s = ASSERT_PTR(userdata);
         Manager *m;
-        Link *l;
         int r;
 
         assert(message);
@@ -38,35 +34,7 @@ int bus_dnssd_method_unregister(sd_bus_message *message, void *userdata, sd_bus_
         if (r == 0)
                 return 1; /* Polkit will call us back */
 
-        HASHMAP_FOREACH(l, m->links) {
-                if (l->mdns_ipv4_scope) {
-                        r = dns_scope_announce(l->mdns_ipv4_scope, true);
-                        if (r < 0)
-                                log_warning_errno(r, "Failed to send goodbye messages in IPv4 scope: %m");
-
-                        dns_zone_remove_rr(&l->mdns_ipv4_scope->zone, s->ptr_rr);
-                        dns_zone_remove_rr(&l->mdns_ipv4_scope->zone, s->sub_ptr_rr);
-                        dns_zone_remove_rr(&l->mdns_ipv4_scope->zone, s->srv_rr);
-                        LIST_FOREACH(items, txt_data, s->txt_data_items)
-                                dns_zone_remove_rr(&l->mdns_ipv4_scope->zone, txt_data->rr);
-                }
-
-                if (l->mdns_ipv6_scope) {
-                        r = dns_scope_announce(l->mdns_ipv6_scope, true);
-                        if (r < 0)
-                                log_warning_errno(r, "Failed to send goodbye messages in IPv6 scope: %m");
-
-                        dns_zone_remove_rr(&l->mdns_ipv6_scope->zone, s->ptr_rr);
-                        dns_zone_remove_rr(&l->mdns_ipv6_scope->zone, s->sub_ptr_rr);
-                        dns_zone_remove_rr(&l->mdns_ipv6_scope->zone, s->srv_rr);
-                        LIST_FOREACH(items, txt_data, s->txt_data_items)
-                                dns_zone_remove_rr(&l->mdns_ipv6_scope->zone, txt_data->rr);
-                }
-        }
-
-        dnssd_registered_service_free(s);
-
-        manager_refresh_rrs(m);
+        dnssd_registered_service_unregister(s);
 
         return sd_bus_reply_method_return(message, NULL);
 }

src/resolve/resolved-dnssd.c

@@ -6,15 +6,19 @@
 #include "conf-files.h"
 #include "conf-parser.h"
 #include "constants.h"
+#include "dns-answer.h"
 #include "dns-domain.h"
 #include "dns-rr.h"
 #include "extract-word.h"
 #include "hashmap.h"
 #include "hexdecoct.h"
+#include "log.h"
 #include "path-util.h"
 #include "resolved-conf.h"
+#include "resolved-dns-scope.h"
 #include "resolved-dns-zone.h"
 #include "resolved-dnssd.h"
+#include "resolved-link.h"
 #include "resolved-manager.h"
 #include "specifier.h"
 #include "string-util.h"
@@ -53,6 +57,8 @@ DnssdRegisteredService *dnssd_registered_service_free(DnssdRegisteredService *se
         if (service->manager)
                 hashmap_remove(service->manager->dnssd_registered_services, service->id);
 
+        sd_bus_track_unref(service->bus_track);
+
         dns_resource_record_unref(service->ptr_rr);
         dns_resource_record_unref(service->sub_ptr_rr);
         dns_resource_record_unref(service->srv_rr);
@@ -68,6 +74,105 @@ DnssdRegisteredService *dnssd_registered_service_free(DnssdRegisteredService *se
         return mfree(service);
 }
 
+static int dnssd_registered_service_make_goodbye(
+                DnssdRegisteredService *service,
+                DnsScope *scope,
+                DnsAnswer **ret) {
+        _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL;
+        size_t n_rrs = 3;
+        int r;
+
+        assert(service);
+        assert(scope);
+        assert(ret);
+
+        /* Collects those of the service's RRs that are established in the scope's zone (or under
+         * re-verification, in which case they are still served from the zone), flagged as goodbye (TTL 0),
+         * so that only this service is withdrawn, and only where we actually asserted ownership of the
+         * records, i.e. not while a record is still being probed for uniqueness, was withdrawn again after
+         * a conflict, or was never published in this scope in the first place. */
+
+        LIST_FOREACH(items, txt_data, service->txt_data_items)
+                n_rrs++;
+
+        answer = dns_answer_new(n_rrs);
+        if (!answer)
+                return -ENOMEM;
+
+        DnsResourceRecord *rrs[] = { service->ptr_rr, service->sub_ptr_rr, service->srv_rr };
+
+        FOREACH_ELEMENT(rr, rrs) {
+                if (!*rr)
+                        continue;
+
+                DnsZoneItem *i = dns_zone_get(&scope->zone, *rr);
+                if (!i || !IN_SET(i->state, DNS_ZONE_ITEM_ESTABLISHED, DNS_ZONE_ITEM_VERIFYING))
+                        continue;
+
+                r = dns_answer_add(answer, *rr, /* ifindex= */ 0,
+                                   dns_resource_key_is_dnssd_ptr((*rr)->key) ?
+                                   DNS_ANSWER_GOODBYE : (DNS_ANSWER_GOODBYE|DNS_ANSWER_CACHE_FLUSH),
+                                   /* rrsig= */ NULL);
+                if (r < 0)
+                        return r;
+        }
+
+        LIST_FOREACH(items, txt_data, service->txt_data_items) {
+                if (!txt_data->rr)
+                        continue;
+
+                DnsZoneItem *i = dns_zone_get(&scope->zone, txt_data->rr);
+                if (!i || !IN_SET(i->state, DNS_ZONE_ITEM_ESTABLISHED, DNS_ZONE_ITEM_VERIFYING))
+                        continue;
+
+                r = dns_answer_add(answer, txt_data->rr, /* ifindex= */ 0,
+                                   DNS_ANSWER_GOODBYE|DNS_ANSWER_CACHE_FLUSH, /* rrsig= */ NULL);
+                if (r < 0)
+                        return r;
+        }
+
+        *ret = TAKE_PTR(answer);
+        return 0;
+}
+
+void dnssd_registered_service_unregister(DnssdRegisteredService *service) {
+        Link *l;
+        int r;
+
+        assert(service);
+
+        Manager *m = ASSERT_PTR(service->manager);
+
+        /* Takes the service out of service: sends goodbye messages for it, removes its RRs from all mDNS
+         * zones, and drops it from the manager. Note that this also frees the passed object. */
+
+        HASHMAP_FOREACH(l, m->links)
+                FOREACH_ELEMENT(scope, ((DnsScope*[]) { l->mdns_ipv4_scope, l->mdns_ipv6_scope })) {
+                        if (!*scope)
+                                continue;
+
+                        _cleanup_(dns_answer_unrefp) DnsAnswer *goodbye = NULL;
+
+                        r = dnssd_registered_service_make_goodbye(service, *scope, &goodbye);
+                        if (r >= 0)
+                                r = dns_scope_send_goodbye(*scope, goodbye);
+                        if (r < 0)
+                                log_warning_errno(r, "Failed to send goodbye messages, ignoring: %m");
+
+                        dns_zone_remove_rr(&(*scope)->zone, service->ptr_rr);
+                        dns_zone_remove_rr(&(*scope)->zone, service->sub_ptr_rr);
+                        dns_zone_remove_rr(&(*scope)->zone, service->srv_rr);
+                        LIST_FOREACH(items, txt_data, service->txt_data_items)
+                                dns_zone_remove_rr(&(*scope)->zone, txt_data->rr);
+                }
+
+        /* This drops the service from the manager's hashmap, hence the subsequent refresh will not simply
+         * add its RRs back. */
+        dnssd_registered_service_free(service);
+
+        manager_refresh_rrs(m);
+}
+
 void dnssd_registered_service_clear_on_reload(Hashmap *services) {
         DnssdRegisteredService *service;
 

src/resolve/resolved-dnssd.h

@@ -39,6 +39,10 @@ typedef struct DnssdRegisteredService {
 
         Manager *manager;
 
+        /* Tracks the D-Bus client that registered the service, so that it can be unregistered when the
+         * client disconnects from the bus. Only set for services registered via D-Bus. */
+        sd_bus_track *bus_track;
+
         /* Services registered via D-Bus are not removed on reload */
         ResolveConfigSource config_source;
 
@@ -47,6 +51,7 @@ typedef struct DnssdRegisteredService {
 } DnssdRegisteredService;
 
 DnssdRegisteredService *dnssd_registered_service_free(DnssdRegisteredService *service);
+void dnssd_registered_service_unregister(DnssdRegisteredService *service);
 DnssdTxtData *dnssd_txtdata_free(DnssdTxtData *txt_data);
 DnssdTxtData *dnssd_txtdata_free_all(DnssdTxtData *txt_data);
 void dnssd_registered_service_clear_on_reload(Hashmap *services);

test/units/TEST-75-RESOLVED.sh

@@ -232,7 +232,7 @@ manual_testcase_01_resolvectl() {
     ip link add hoge.foo type dummy
 
     # Cleanup
-    # shellcheck disable=SC2317
+    # shellcheck disable=SC2317,SC2329
     cleanup() {
         ip link del hoge
         ip link del hoge.foo
@@ -349,6 +349,7 @@ manual_testcase_02_mdns_llmnr() {
     ip link add hoge.foo type dummy
 
     # Cleanup
+    # shellcheck disable=SC2317,SC2329
     cleanup() {
         rm -f /run/systemd/resolved.conf.d/90-mdns-llmnr.conf
         ip link del hoge
@@ -436,6 +437,78 @@ manual_testcase_02_mdns_llmnr() {
     assert_in 'no' "$(resolvectl llmnr hoge)"
 }
 
+# Test for the D-Bus RegisterService()/UnregisterService() API
+manual_testcase_03_register_service() {
+    local cursor_file="" invocation_id
+
+    # Cleanup
+    # shellcheck disable=SC2317,SC2329
+    cleanup() {
+        rm -f /run/systemd/resolved.conf.d/90-mdns.conf "${cursor_file:-}"
+        ip link del mdns99 2>/dev/null || :
+        systemctl reload systemd-resolved.service
+    }
+
+    trap cleanup RETURN ERR
+
+    mkdir -p /run/systemd/resolved.conf.d
+    {
+        echo "[Resolve]"
+        echo "MulticastDNS=yes"
+    } >/run/systemd/resolved.conf.d/90-mdns.conf
+    restart_resolved
+    resolvectl log-level debug
+
+    # resolved is restarted automatically if it crashes (Restart=always), which would let the checks below
+    # pass for the wrong reason, so remember the invocation and verify it stayed the same at the end.
+    invocation_id=$(systemctl show -P InvocationID systemd-resolved.service)
+
+    # Make sure at least one link carries mDNS scopes, so that unregistration actually runs the
+    # per-scope goodbye/zone teardown.
+    ip link add mdns99 type dummy
+    ip link set mdns99 up
+    resolvectl mdns mdns99 yes
+
+    cursor_file=$(mktemp)
+    journalctl -n 0 --cursor-file="$cursor_file"
+
+    # The lifetime of a registered service is bound to the bus connection of the client that registered
+    # it. busctl disconnects right after the call, so the service should be unregistered again shortly
+    # after. Wait for the vanish handler log message as proof that the client tracking triggered the
+    # unregistration. See testcase_15_wait_online_dns() for why journalctl -f | grep -m1 is not used here.
+    assert_in '/org/freedesktop/resolve1/dnssd/testservice' \
+              "$(busctl call org.freedesktop.resolve1 /org/freedesktop/resolve1 org.freedesktop.resolve1.Manager \
+                     RegisterService 'sssqqqaa{say}' testservice '%H' _testservice._tcp 1234 0 0 0)"
+
+    timeout 30 bash -c "until journalctl --after-cursor=\"\$(cat \"$cursor_file\")\" SYSLOG_IDENTIFIER=systemd-resolved --grep \"Client of DNS-SD service 'testservice' vanished, unregistering.\" >/dev/null 2>&1; do sleep .5; done"
+
+    assert_not_in 'dnssd/testservice' "$(busctl tree org.freedesktop.resolve1)"
+
+    # Unregistering it again should fail specifically because the service is gone (busctl prints the
+    # D-Bus error message, not the error name), i.e. not because resolved crashed or the bus connection
+    # failed.
+    assert_in "DNS-SD service 'testservice' not known" \
+              "$(busctl call org.freedesktop.resolve1 /org/freedesktop/resolve1 org.freedesktop.resolve1.Manager \
+                     UnregisterService 'o' /org/freedesktop/resolve1/dnssd/testservice 2>&1 || :)"
+
+    # Re-registering the same identifier must succeed: before client tracking worked, the identifier
+    # stayed taken by the vanished client's service and this failed with DNSSD_SERVICE_EXISTS. Take a
+    # fresh cursor so the wait below cannot match the first round's identical message.
+    rm -f "$cursor_file"
+    journalctl -n 0 --cursor-file="$cursor_file"
+    assert_in '/org/freedesktop/resolve1/dnssd/testservice' \
+              "$(busctl call org.freedesktop.resolve1 /org/freedesktop/resolve1 org.freedesktop.resolve1.Manager \
+                     RegisterService 'sssqqqaa{say}' testservice '%H' _testservice._tcp 1234 0 0 0)"
+
+    timeout 30 bash -c "until journalctl --after-cursor=\"\$(cat \"$cursor_file\")\" SYSLOG_IDENTIFIER=systemd-resolved --grep \"Client of DNS-SD service 'testservice' vanished, unregistering.\" >/dev/null 2>&1; do sleep .5; done"
+
+    assert_not_in 'dnssd/testservice' "$(busctl tree org.freedesktop.resolve1)"
+
+    # resolved must not have crashed and gotten restarted along the way.
+    assert_eq "$(systemctl show -P InvocationID systemd-resolved.service)" "$invocation_id"
+    systemctl is-active systemd-resolved.service
+}
+
 testcase_03_23951() {
     : "--- nss-resolve/nss-myhostname tests"
 
@@ -866,7 +939,7 @@ testcase_08_resolved() {
 
 testcase_09_resolvectl_showcache() {
     # Cleanup
-    # shellcheck disable=SC2317
+    # shellcheck disable=SC2317,SC2329
     cleanup() {
         rm -f /run/systemd/resolved.conf.d/90-resolved.conf
         rm -f /run/systemd/network/10-dns2.netdev
@@ -928,7 +1001,7 @@ testcase_10_resolvectl_json() {
     local status_json
 
     # Cleanup
-    # shellcheck disable=SC2317
+    # shellcheck disable=SC2317,SC2329
     cleanup() {
         rm -f /run/systemd/resolved.conf.d/90-fallback.conf
         systemctl reload systemd-resolved.service
@@ -1099,7 +1172,7 @@ testcase_11_nft() {
 # Test resolvectl show-server-state
 testcase_12_resolvectl2() {
     # Cleanup
-    # shellcheck disable=SC2317
+    # shellcheck disable=SC2317,SC2329
     cleanup() {
         rm -f /run/systemd/resolved.conf.d/90-reload.conf
         systemctl reload systemd-resolved.service
@@ -1201,7 +1274,7 @@ testcase_13_varlink_subscribe_dns_configuration() {
     fi
 
     # Cleanup
-    # shellcheck disable=SC2317
+    # shellcheck disable=SC2317,SC2329
     cleanup() {
         echo "===== io.systemd.Resolve.Monitor.SubscribeDNSConfiguration output: ====="
         cat "$tmpfile"
@@ -1278,7 +1351,7 @@ testcase_13_varlink_subscribe_dns_configuration() {
 
 # Test RefuseRecordTypes
 testcase_14_refuse_record_types() {
-    # shellcheck disable=SC2317
+    # shellcheck disable=SC2317,SC2329
     cleanup() {
         rm -f /run/systemd/resolved.conf.d/90-refuserecords.conf
         restart_resolved
@@ -1440,7 +1513,7 @@ testcase_14_refuse_record_types() {
 # Test systemd-networkd-wait-online interactions with systemd-resolved
 testcase_15_wait_online_dns() {
     # Cleanup
-    # shellcheck disable=SC2317
+    # shellcheck disable=SC2317,SC2329
     cleanup() {
         echo "===== journalctl -u $unit ====="
         journalctl -b --no-pager --no-hostname --full -u "$unit"
@@ -1725,6 +1798,7 @@ systemctl enable --now systemd-resolved.service
 # Need to be run before SETUP, otherwise things will break
 manual_testcase_01_resolvectl
 manual_testcase_02_mdns_llmnr
+manual_testcase_03_register_service
 
 # Run setup
 setup