From ae55c9c0aed1578efd981a9fe79135112e643575 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 31 Oct 2023 23:00:41 +0100 Subject: [PATCH 1/2] resolved: make sure "resolvectl monitor" can properly deal with stub queries If we receive a query via the two stubs we store the original packet instead of just the question object. Hence when we send monitor info to subscribed clients we need to extract its question and also include it in the returned data. Fixes: #29580 --- src/resolve/resolved-dns-query.c | 2 +- src/resolve/resolved-manager.c | 14 +++++++++++++- src/resolve/resolved-manager.h | 2 +- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 58a7b2d878..7eb6b9736e 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -586,7 +586,7 @@ void dns_query_complete(DnsQuery *q, DnsTransactionState state) { q->state = state; - (void) manager_monitor_send(q->manager, q->state, q->answer_rcode, q->answer_errno, q->question_idna, q->question_utf8, q->collected_questions, q->answer); + (void) manager_monitor_send(q->manager, q->state, q->answer_rcode, q->answer_errno, q->question_idna, q->question_utf8, q->question_bypass, q->collected_questions, q->answer); dns_query_stop(q); if (q->complete) diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index fd800cedfe..fc8d7412fb 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -1105,6 +1105,7 @@ int manager_monitor_send( int error, DnsQuestion *question_idna, DnsQuestion *question_utf8, + DnsPacket *question_bypass, DnsQuestion *collected_questions, DnsAnswer *answer) { @@ -1119,11 +1120,22 @@ int manager_monitor_send( if (set_isempty(m->varlink_subscription)) return 0; - /* Merge both questions format into one */ + /* Merge all questions into one */ r = dns_question_merge(question_idna, question_utf8, &merged); if (r < 0) return log_error_errno(r, "Failed to merge UTF8/IDNA questions: %m"); + if (question_bypass) { + _cleanup_(dns_question_unrefp) DnsQuestion *merged2 = NULL; + + r = dns_question_merge(merged, question_bypass->question, &merged2); + if (r < 0) + return log_error_errno(r, "Failed to merge UTF8/IDNA questions and DNS packet question: %m"); + + dns_question_unref(merged); + merged = TAKE_PTR(merged2); + } + /* Convert the current primary question to JSON */ r = dns_question_to_json(merged, &jquestion); if (r < 0) diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h index 16b883bd21..5cd5e834d3 100644 --- a/src/resolve/resolved-manager.h +++ b/src/resolve/resolved-manager.h @@ -176,7 +176,7 @@ int manager_start(Manager *m); uint32_t manager_find_mtu(Manager *m); -int manager_monitor_send(Manager *m, int state, int rcode, int error, DnsQuestion *question_idna, DnsQuestion *question_utf8, DnsQuestion *collected_questions, DnsAnswer *answer); +int manager_monitor_send(Manager *m, int state, int rcode, int error, DnsQuestion *question_idna, DnsQuestion *question_utf8, DnsPacket *question_bypass, DnsQuestion *collected_questions, DnsAnswer *answer); int manager_write(Manager *m, int fd, DnsPacket *p); int manager_send(Manager *m, int fd, int ifindex, int family, const union in_addr_union *destination, uint16_t port, const union in_addr_union *source, DnsPacket *p); From a0e000076a6f8722c2fd7b1accc9404ba08996de Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Thu, 2 Nov 2023 18:08:30 +0100 Subject: [PATCH 2/2] test: check that `resolvectl monitor --json` generates valid JSON Provides coverage for #29580. --- test/units/testsuite-75.sh | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/test/units/testsuite-75.sh b/test/units/testsuite-75.sh index 7e73d2e952..b34045fcb5 100755 --- a/test/units/testsuite-75.sh +++ b/test/units/testsuite-75.sh @@ -40,7 +40,7 @@ monitor_check_rr() ( # displayed. We turn off pipefail for this, since we don't care about the # lhs of this pipe expression, we only care about the rhs' result to be # clean - timeout -v 30s journalctl -u resmontest.service --since "$since" -f --full | grep -m1 "$match" + timeout -v 30s journalctl -u resolvectl-monitor.service --since "$since" -f --full | grep -m1 "$match" ) # Test for resolvectl, resolvconf @@ -238,7 +238,8 @@ resolvectl status resolvectl log-level debug # Start monitoring queries -systemd-run -u resmontest.service -p Type=notify resolvectl monitor +systemd-run -u resolvectl-monitor.service -p Type=notify resolvectl monitor +systemd-run -u resolvectl-monitor-json.service -p Type=notify resolvectl monitor --json=short # Check if all the zones are valid (zone-check always returns 0, so let's check # if it produces any errors/warnings) @@ -529,7 +530,25 @@ grep -qF "fd00:dead:beef:cafe::123" "$RUN_OUT" #run dig +dnssec this.does.not.exist.untrusted.test #grep -qF "status: NXDOMAIN" "$RUN_OUT" -systemctl stop resmontest.service +# Issue: https://github.com/systemd/systemd/issues/29580 (part #1) +dig @127.0.0.54 signed.test + +systemctl stop resolvectl-monitor.service +systemctl stop resolvectl-monitor-json.service + +# Issue: https://github.com/systemd/systemd/issues/29580 (part #2) +# +# Check for any warnings regarding malformed messages +(! journalctl -u resolvectl-monitor.service -u reseolvectl-monitor-json.service -p warning --grep malformed) +# Verify that all queries recorded by `resolvectl monitor --json` produced a valid JSON +# with expected fields +journalctl -p info -o cat _SYSTEMD_UNIT="resolvectl-monitor-json.service" | while read -r line; do + # Check that both "question" and "answer" fields are arrays + # + # The expression is slightly more complicated due to the fact that the "answer" field is optional, + # so we need to select it only if it's present, otherwise the type == "array" check would fail + echo "$line" | jq -e '[. | .question, (select(has("answer")) | .answer) | type == "array"] | all' +done # Test serve stale feature and NFTSet= if nftables is installed if command -v nft >/dev/null; then