From a92ea352a51059aee8ada3670ec23b9bfd848978 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 7 Jan 2024 02:58:15 +0900 Subject: [PATCH 01/13] resolve: update rcode table and align enum definitions --- src/resolve/resolved-dns-packet.c | 1 + src/resolve/resolved-dns-packet.h | 144 ++++++++++++++++-------------- 2 files changed, 76 insertions(+), 69 deletions(-) diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index a031ffeccae..8e23b7afef4 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -2732,6 +2732,7 @@ static const char* const dns_rcode_table[_DNS_RCODE_MAX_DEFINED] = { [DNS_RCODE_NXRRSET] = "NXRRSET", [DNS_RCODE_NOTAUTH] = "NOTAUTH", [DNS_RCODE_NOTZONE] = "NOTZONE", + [DNS_RCODE_DSOTYPENI] = "DSOTYPENI", [DNS_RCODE_BADVERS] = "BADVERS", [DNS_RCODE_BADKEY] = "BADKEY", [DNS_RCODE_BADTIME] = "BADTIME", diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h index 8f062d24ece..b6ac0e06553 100644 --- a/src/resolve/resolved-dns-packet.h +++ b/src/resolve/resolved-dns-packet.h @@ -259,88 +259,94 @@ int dns_packet_has_nsid_request(DnsPacket *p); /* https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-6 */ enum { - DNS_RCODE_SUCCESS = 0, - DNS_RCODE_FORMERR = 1, - DNS_RCODE_SERVFAIL = 2, - DNS_RCODE_NXDOMAIN = 3, - DNS_RCODE_NOTIMP = 4, - DNS_RCODE_REFUSED = 5, - DNS_RCODE_YXDOMAIN = 6, - DNS_RCODE_YXRRSET = 7, - DNS_RCODE_NXRRSET = 8, - DNS_RCODE_NOTAUTH = 9, - DNS_RCODE_NOTZONE = 10, - DNS_RCODE_BADVERS = 16, - DNS_RCODE_BADSIG = 16, /* duplicate value! */ - DNS_RCODE_BADKEY = 17, - DNS_RCODE_BADTIME = 18, - DNS_RCODE_BADMODE = 19, - DNS_RCODE_BADNAME = 20, - DNS_RCODE_BADALG = 21, - DNS_RCODE_BADTRUNC = 22, - DNS_RCODE_BADCOOKIE = 23, + DNS_RCODE_SUCCESS = 0, + DNS_RCODE_FORMERR = 1, + DNS_RCODE_SERVFAIL = 2, + DNS_RCODE_NXDOMAIN = 3, + DNS_RCODE_NOTIMP = 4, + DNS_RCODE_REFUSED = 5, + DNS_RCODE_YXDOMAIN = 6, + DNS_RCODE_YXRRSET = 7, + DNS_RCODE_NXRRSET = 8, + DNS_RCODE_NOTAUTH = 9, + DNS_RCODE_NOTZONE = 10, + DNS_RCODE_DSOTYPENI = 11, + /* 12-15 are unassigned. */ + DNS_RCODE_BADVERS = 16, + DNS_RCODE_BADSIG = 16, /* duplicate value! */ + DNS_RCODE_BADKEY = 17, + DNS_RCODE_BADTIME = 18, + DNS_RCODE_BADMODE = 19, + DNS_RCODE_BADNAME = 20, + DNS_RCODE_BADALG = 21, + DNS_RCODE_BADTRUNC = 22, + DNS_RCODE_BADCOOKIE = 23, + /* 24-3840 are unassigned. */ + /* 3841-4095 are for private use. */ + /* 4096-65534 are unassigned. */ _DNS_RCODE_MAX_DEFINED, - _DNS_RCODE_MAX = 4095 /* 4 bit rcode in the header plus 8 bit rcode in OPT, makes 12 bit */ + _DNS_RCODE_MAX = 65535, /* reserved */ + _DNS_RCODE_INVALID = -EINVAL, }; /* https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-11 */ enum { - DNS_EDNS_OPT_RESERVED = 0, /* RFC 6891 */ - DNS_EDNS_OPT_LLQ = 1, /* RFC 8764 */ - DNS_EDNS_OPT_UL = 2, - DNS_EDNS_OPT_NSID = 3, /* RFC 5001 */ - /* DNS_EDNS_OPT_RESERVED = 4 */ - DNS_EDNS_OPT_DAU = 5, /* RFC 6975 */ - DNS_EDNS_OPT_DHU = 6, /* RFC 6975 */ - DNS_EDNS_OPT_N3U = 7, /* RFC 6975 */ + DNS_EDNS_OPT_RESERVED = 0, /* RFC 6891 */ + DNS_EDNS_OPT_LLQ = 1, /* RFC 8764 */ + DNS_EDNS_OPT_UL = 2, + DNS_EDNS_OPT_NSID = 3, /* RFC 5001 */ + /* DNS_EDNS_OPT_RESERVED = 4 */ + DNS_EDNS_OPT_DAU = 5, /* RFC 6975 */ + DNS_EDNS_OPT_DHU = 6, /* RFC 6975 */ + DNS_EDNS_OPT_N3U = 7, /* RFC 6975 */ DNS_EDNS_OPT_CLIENT_SUBNET = 8, /* RFC 7871 */ - DNS_EDNS_OPT_EXPIRE = 9, /* RFC 7314 */ - DNS_EDNS_OPT_COOKIE = 10, /* RFC 7873 */ + DNS_EDNS_OPT_EXPIRE = 9, /* RFC 7314 */ + DNS_EDNS_OPT_COOKIE = 10, /* RFC 7873 */ DNS_EDNS_OPT_TCP_KEEPALIVE = 11, /* RFC 7828 */ - DNS_EDNS_OPT_PADDING = 12, /* RFC 7830 */ - DNS_EDNS_OPT_CHAIN = 13, /* RFC 7901 */ - DNS_EDNS_OPT_KEY_TAG = 14, /* RFC 8145 */ - DNS_EDNS_OPT_EXT_ERROR = 15, /* RFC 8914 */ - DNS_EDNS_OPT_CLIENT_TAG = 16, - DNS_EDNS_OPT_SERVER_TAG = 17, + DNS_EDNS_OPT_PADDING = 12, /* RFC 7830 */ + DNS_EDNS_OPT_CHAIN = 13, /* RFC 7901 */ + DNS_EDNS_OPT_KEY_TAG = 14, /* RFC 8145 */ + DNS_EDNS_OPT_EXT_ERROR = 15, /* RFC 8914 */ + DNS_EDNS_OPT_CLIENT_TAG = 16, + DNS_EDNS_OPT_SERVER_TAG = 17, _DNS_EDNS_OPT_MAX_DEFINED, - _DNS_EDNS_OPT_INVALID = -EINVAL + _DNS_EDNS_OPT_INVALID = -EINVAL, }; /* https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#extended-dns-error-codes */ enum { - DNS_EDE_RCODE_OTHER = 0, /* RFC 8914, Section 4.1 */ - DNS_EDE_RCODE_UNSUPPORTED_DNSKEY_ALG = 1, /* RFC 8914, Section 4.2 */ - DNS_EDE_RCODE_UNSUPPORTED_DS_DIGEST = 2, /* RFC 8914, Section 4.3 */ - DNS_EDE_RCODE_STALE_ANSWER = 3, /* RFC 8914, Section 4.4 */ - DNS_EDE_RCODE_FORGED_ANSWER = 4, /* RFC 8914, Section 4.5 */ - DNS_EDE_RCODE_DNSSEC_INDETERMINATE = 5, /* RFC 8914, Section 4.6 */ - DNS_EDE_RCODE_DNSSEC_BOGUS = 6, /* RFC 8914, Section 4.7 */ - DNS_EDE_RCODE_SIG_EXPIRED = 7, /* RFC 8914, Section 4.8 */ - DNS_EDE_RCODE_SIG_NOT_YET_VALID = 8, /* RFC 8914, Section 4.9 */ - DNS_EDE_RCODE_DNSKEY_MISSING = 9, /* RFC 8914, Section 4.10 */ - DNS_EDE_RCODE_RRSIG_MISSING = 10, /* RFC 8914, Section 4.11 */ - DNS_EDE_RCODE_NO_ZONE_KEY_BIT = 11, /* RFC 8914, Section 4.12 */ - DNS_EDE_RCODE_NSEC_MISSING = 12, /* RFC 8914, Section 4.13 */ - DNS_EDE_RCODE_CACHED_ERROR = 13, /* RFC 8914, Section 4.14 */ - DNS_EDE_RCODE_NOT_READY = 14, /* RFC 8914, Section 4.15 */ - DNS_EDE_RCODE_BLOCKED = 15, /* RFC 8914, Section 4.16 */ - DNS_EDE_RCODE_CENSORED = 16, /* RFC 8914, Section 4.17 */ - DNS_EDE_RCODE_FILTERED = 17, /* RFC 8914, Section 4.18 */ - DNS_EDE_RCODE_PROHIBITIED = 18, /* RFC 8914, Section 4.19 */ - DNS_EDE_RCODE_STALE_NXDOMAIN_ANSWER = 19, /* RFC 8914, Section 4.20 */ - DNS_EDE_RCODE_NOT_AUTHORITATIVE = 20, /* RFC 8914, Section 4.21 */ - DNS_EDE_RCODE_NOT_SUPPORTED = 21, /* RFC 8914, Section 4.22 */ - DNS_EDE_RCODE_UNREACH_AUTHORITY = 22, /* RFC 8914, Section 4.23 */ - DNS_EDE_RCODE_NET_ERROR = 23, /* RFC 8914, Section 4.24 */ - DNS_EDE_RCODE_INVALID_DATA = 24, /* RFC 8914, Section 4.25 */ - DNS_EDE_RCODE_SIG_NEVER = 25, - DNS_EDE_RCODE_TOO_EARLY = 26, /* RFC 9250 */ - DNS_EDE_RCODE_UNSUPPORTED_NSEC3_ITER = 27, /* RFC 9276 */ - DNS_EDE_RCODE_TRANSPORT_POLICY = 28, - DNS_EDE_RCODE_SYNTHESIZED = 29, + DNS_EDE_RCODE_OTHER = 0, /* RFC 8914, Section 4.1 */ + DNS_EDE_RCODE_UNSUPPORTED_DNSKEY_ALG = 1, /* RFC 8914, Section 4.2 */ + DNS_EDE_RCODE_UNSUPPORTED_DS_DIGEST = 2, /* RFC 8914, Section 4.3 */ + DNS_EDE_RCODE_STALE_ANSWER = 3, /* RFC 8914, Section 4.4 */ + DNS_EDE_RCODE_FORGED_ANSWER = 4, /* RFC 8914, Section 4.5 */ + DNS_EDE_RCODE_DNSSEC_INDETERMINATE = 5, /* RFC 8914, Section 4.6 */ + DNS_EDE_RCODE_DNSSEC_BOGUS = 6, /* RFC 8914, Section 4.7 */ + DNS_EDE_RCODE_SIG_EXPIRED = 7, /* RFC 8914, Section 4.8 */ + DNS_EDE_RCODE_SIG_NOT_YET_VALID = 8, /* RFC 8914, Section 4.9 */ + DNS_EDE_RCODE_DNSKEY_MISSING = 9, /* RFC 8914, Section 4.10 */ + DNS_EDE_RCODE_RRSIG_MISSING = 10, /* RFC 8914, Section 4.11 */ + DNS_EDE_RCODE_NO_ZONE_KEY_BIT = 11, /* RFC 8914, Section 4.12 */ + DNS_EDE_RCODE_NSEC_MISSING = 12, /* RFC 8914, Section 4.13 */ + DNS_EDE_RCODE_CACHED_ERROR = 13, /* RFC 8914, Section 4.14 */ + DNS_EDE_RCODE_NOT_READY = 14, /* RFC 8914, Section 4.15 */ + DNS_EDE_RCODE_BLOCKED = 15, /* RFC 8914, Section 4.16 */ + DNS_EDE_RCODE_CENSORED = 16, /* RFC 8914, Section 4.17 */ + DNS_EDE_RCODE_FILTERED = 17, /* RFC 8914, Section 4.18 */ + DNS_EDE_RCODE_PROHIBITIED = 18, /* RFC 8914, Section 4.19 */ + DNS_EDE_RCODE_STALE_NXDOMAIN_ANSWER = 19, /* RFC 8914, Section 4.20 */ + DNS_EDE_RCODE_NOT_AUTHORITATIVE = 20, /* RFC 8914, Section 4.21 */ + DNS_EDE_RCODE_NOT_SUPPORTED = 21, /* RFC 8914, Section 4.22 */ + DNS_EDE_RCODE_UNREACH_AUTHORITY = 22, /* RFC 8914, Section 4.23 */ + DNS_EDE_RCODE_NET_ERROR = 23, /* RFC 8914, Section 4.24 */ + DNS_EDE_RCODE_INVALID_DATA = 24, /* RFC 8914, Section 4.25 */ + DNS_EDE_RCODE_SIG_NEVER = 25, + DNS_EDE_RCODE_TOO_EARLY = 26, /* RFC 9250 */ + DNS_EDE_RCODE_UNSUPPORTED_NSEC3_ITER = 27, /* RFC 9276 */ + DNS_EDE_RCODE_TRANSPORT_POLICY = 28, + DNS_EDE_RCODE_SYNTHESIZED = 29, _DNS_EDE_RCODE_MAX_DEFINED, - _DNS_EDE_RCODE_INVALID = -EINVAL + _DNS_EDE_RCODE_INVALID = -EINVAL, }; const char* dns_rcode_to_string(int i) _const_; From 71682ac6bb6579376ad4be55c93bbab4e5ad7714 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 7 Jan 2024 03:28:19 +0900 Subject: [PATCH 02/13] resolve: cleanup dns_packet_ede_rcode() This makes the following: - make dns_packet_ede_rcode() return -EINVAL when EDE code not found. Otherwise, the caller may be confused that the packet has an unknown error code. - make the function escape EDE message only when non-utf8 message is received. - the message handling logic is applied even if the error code is unknown, as there is no reason that we escape EDE message only when an known error code is received. - reduce scope of variables, - drop redundant 'else', - append full stop to the log messages, - drop redundant log message in the caller, - split out error in the function and returned EDE error code. Follow-up for ac6844460ca1c01eaf2cb209ffa21c200d21a8f8. --- src/resolve/resolved-dns-packet.c | 51 +++++++++++++++----------- src/resolve/resolved-dns-packet.h | 2 +- src/resolve/resolved-dns-transaction.c | 8 +--- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index 8e23b7afef4..44e1e4faabf 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -2588,17 +2588,15 @@ bool dns_packet_equal(const DnsPacket *a, const DnsPacket *b) { return dns_packet_compare_func(a, b) == 0; } -int dns_packet_ede_rcode(DnsPacket *p, char **ret_ede_msg) { - assert(p); - - _cleanup_free_ char *msg = NULL, *msg_escaped = NULL; - int ede_rcode = _DNS_EDNS_OPT_MAX_DEFINED; - int r; +int dns_packet_ede_rcode(DnsPacket *p, int *ret_ede_rcode, char **ret_ede_msg) { const uint8_t *d; size_t l; + int r; + + assert(p); if (!p->opt) - return _DNS_EDE_RCODE_INVALID; + return -ENOENT; d = p->opt->opt.data; l = p->opt->opt.data_size; @@ -2618,31 +2616,40 @@ int dns_packet_ede_rcode(DnsPacket *p, char **ret_ede_msg) { "Truncated option in EDNS0 variable part."); if (code == DNS_EDNS_OPT_EXT_ERROR) { + _cleanup_free_ char *msg = NULL; + if (length < 2U) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), - "EDNS0 truncated EDE info code."); - ede_rcode = unaligned_read_be16(d + 4); - r = make_cstring((char *)d + 6, length - 2U, MAKE_CSTRING_ALLOW_TRAILING_NUL, &msg); + "EDNS0 truncated EDE info code."); + + r = make_cstring((char *) d + 6, length - 2U, MAKE_CSTRING_ALLOW_TRAILING_NUL, &msg); if (r < 0) - return log_debug_errno(r, "Invalid EDE text in opt"); - else if (!utf8_is_valid(msg)) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Invalid EDE text in opt"); - else if (ede_rcode < _DNS_EDNS_OPT_MAX_DEFINED) { - msg_escaped = cescape(msg); - if (!msg_escaped) - return -ENOMEM; + return log_debug_errno(r, "Invalid EDE text in opt."); + + if (ret_ede_msg) { + if (!utf8_is_valid(msg)) { + _cleanup_free_ char *msg_escaped = NULL; + + msg_escaped = cescape(msg); + if (!msg_escaped) + return log_oom_debug(); + + *ret_ede_msg = TAKE_PTR(msg_escaped); + } else + *ret_ede_msg = TAKE_PTR(msg); } - break; + + if (ret_ede_rcode) + *ret_ede_rcode = unaligned_read_be16(d + 4); + + return 0; } d += 4U + length; l -= 4U + length; } - if (ret_ede_msg) - *ret_ede_msg = TAKE_PTR(msg_escaped); - - return ede_rcode; + return -ENOENT; } bool dns_ede_rcode_is_dnssec(int ede_rcode) { diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h index b6ac0e06553..705fc511d6b 100644 --- a/src/resolve/resolved-dns-packet.h +++ b/src/resolve/resolved-dns-packet.h @@ -253,7 +253,7 @@ int dns_packet_extract(DnsPacket *p); bool dns_packet_equal(const DnsPacket *a, const DnsPacket *b); -int dns_packet_ede_rcode(DnsPacket *p, char **ret_ede_msg); +int dns_packet_ede_rcode(DnsPacket *p, int *ret_ede_rcode, char **ret_ede_msg); bool dns_ede_rcode_is_dnssec(int ede_rcode); int dns_packet_has_nsid_request(DnsPacket *p); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index e3c70e88e27..ea8243a2c82 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1224,17 +1224,11 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p, bool encrypt case DNS_PROTOCOL_DNS: { int ede_rcode; - _cleanup_free_ char *ede_msg = NULL; assert(t->server); - ede_rcode = dns_packet_ede_rcode(p, &ede_msg); - if (ede_rcode < 0 && ede_rcode != -EINVAL) - log_debug_errno(ede_rcode, "Unable to extract EDE error code from packet, ignoring: %m"); - else { + if (dns_packet_ede_rcode(p, &ede_rcode, &t->answer_ede_msg) >= 0) t->answer_ede_rcode = ede_rcode; - t->answer_ede_msg = TAKE_PTR(ede_msg); - } if (!t->bypass && IN_SET(DNS_PACKET_RCODE(p), DNS_RCODE_FORMERR, DNS_RCODE_SERVFAIL, DNS_RCODE_NOTIMP)) { From c08fd7becae0996ef87d4cef53167fe6c637667e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 7 Jan 2024 03:57:11 +0900 Subject: [PATCH 03/13] resolve: several follow-ups for ac6844460ca1c01eaf2cb209ffa21c200d21a8f8 - fix indentation, - drop incorrect comment, dns_packet_ede_rcode() can handle unknown EDE rcode. - 0 is a valid EDE rcode, - DnsTransaction.answer_ede_msg may be NULL, so needs to use strempty() on logging, - drop redundant ede_rcode temporary variable. Follow-up for ac6844460ca1c01eaf2cb209ffa21c200d21a8f8. --- src/resolve/resolved-dns-transaction.c | 40 ++++++++++++-------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index ea8243a2c82..aeae08f2f61 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1223,38 +1223,35 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p, bool encrypt switch (t->scope->protocol) { case DNS_PROTOCOL_DNS: { - int ede_rcode; - assert(t->server); - if (dns_packet_ede_rcode(p, &ede_rcode, &t->answer_ede_msg) >= 0) - t->answer_ede_rcode = ede_rcode; + (void) dns_packet_ede_rcode(p, &t->answer_ede_rcode, &t->answer_ede_msg); if (!t->bypass && IN_SET(DNS_PACKET_RCODE(p), DNS_RCODE_FORMERR, DNS_RCODE_SERVFAIL, DNS_RCODE_NOTIMP)) { /* If the server has replied with detailed error data, using a degraded feature set * will likely not help anyone. Examine the detailed error to determine the best * course of action. */ - if (ede_rcode >= 0 && DNS_PACKET_RCODE(p) == DNS_RCODE_SERVFAIL) { + if (t->answer_ede_rcode >= 0 && DNS_PACKET_RCODE(p) == DNS_RCODE_SERVFAIL) { /* These codes are related to DNSSEC configuration errors. If accurate, * this is the domain operator's problem, and retrying won't help. */ - if (dns_ede_rcode_is_dnssec(ede_rcode)) { + if (dns_ede_rcode_is_dnssec(t->answer_ede_rcode)) { log_debug("Server returned error: %s (%s%s%s). Lookup failed.", - FORMAT_DNS_RCODE(DNS_PACKET_RCODE(p)), - FORMAT_DNS_EDE_RCODE(ede_rcode), - isempty(t->answer_ede_msg) ? "" : ": ", - t->answer_ede_msg); + FORMAT_DNS_RCODE(DNS_PACKET_RCODE(p)), + FORMAT_DNS_EDE_RCODE(t->answer_ede_rcode), + isempty(t->answer_ede_msg) ? "" : ": ", + strempty(t->answer_ede_msg)); dns_transaction_complete(t, DNS_TRANSACTION_UPSTREAM_DNSSEC_FAILURE); return; } /* These codes probably indicate a transient error. Let's try again. */ - if (IN_SET(ede_rcode, DNS_EDE_RCODE_NOT_READY, DNS_EDE_RCODE_NET_ERROR)) { + if (IN_SET(t->answer_ede_rcode, DNS_EDE_RCODE_NOT_READY, DNS_EDE_RCODE_NET_ERROR)) { log_debug("Server returned error: %s (%s%s%s), retrying transaction.", - FORMAT_DNS_RCODE(DNS_PACKET_RCODE(p)), - FORMAT_DNS_EDE_RCODE(ede_rcode), - isempty(t->answer_ede_msg) ? "" : ": ", - t->answer_ede_msg); + FORMAT_DNS_RCODE(DNS_PACKET_RCODE(p)), + FORMAT_DNS_EDE_RCODE(t->answer_ede_rcode), + isempty(t->answer_ede_msg) ? "" : ": ", + strempty(t->answer_ede_msg)); dns_transaction_retry(t, false); return; } @@ -1262,11 +1259,12 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p, bool encrypt /* OK, the query failed, but we still shouldn't degrade the feature set for * this server. */ log_debug("Server returned error: %s (%s%s%s)", - FORMAT_DNS_RCODE(DNS_PACKET_RCODE(p)), - FORMAT_DNS_EDE_RCODE(ede_rcode), - isempty(t->answer_ede_msg) ? "" : ": ", t->answer_ede_msg); + FORMAT_DNS_RCODE(DNS_PACKET_RCODE(p)), + FORMAT_DNS_EDE_RCODE(t->answer_ede_rcode), + isempty(t->answer_ede_msg) ? "" : ": ", + strempty(t->answer_ede_msg)); break; - } /* No EDE rcode, or EDE rcode we don't understand */ + } /* Request failed, immediately try again with reduced features */ @@ -1323,9 +1321,9 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p, bool encrypt if (DNS_PACKET_RCODE(p) == DNS_RCODE_REFUSED) { /* This server refused our request? If so, try again, use a different server */ - if (ede_rcode > 0) + if (t->answer_ede_rcode >= 0) log_debug("Server returned REFUSED (%s), switching servers, and retrying.", - FORMAT_DNS_EDE_RCODE(ede_rcode)); + FORMAT_DNS_EDE_RCODE(t->answer_ede_rcode)); else log_debug("Server returned REFUSED, switching servers, and retrying."); From 9ae51762d7be5efc0553cf4039fa7483a2799e45 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 10 Jan 2024 10:10:06 +0900 Subject: [PATCH 04/13] resolve: drop DNS_TRANSACTION_UPSTREAM_DNSSEC_FAILURE This partially reverts commit 9ca133e97a0c8795b1f293ccea4965b4ad1accc4. Not only there is no reason to introduce a new transaction failure state, but also the commit introduces several severe issues. Fixes #30776, #30779. --- src/resolve/resolved-bus.c | 5 ----- src/resolve/resolved-dns-transaction.c | 17 +---------------- src/resolve/resolved-dns-transaction.h | 1 - 3 files changed, 1 insertion(+), 22 deletions(-) diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 8c9475cd3de..ef3f5237a9e 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -189,11 +189,6 @@ static int reply_query_state(DnsQuery *q) { return sd_bus_reply_method_error(req, &error); } - case DNS_TRANSACTION_UPSTREAM_DNSSEC_FAILURE: - return reply_method_errorf(q, BUS_ERROR_DNSSEC_FAILED, "DNSSEC validation failed upstream: %s%s%s", - dns_ede_rcode_to_string(q->answer_ede_rcode), - isempty(q->answer_ede_msg) ? "" : ": ", q->answer_ede_msg); - case DNS_TRANSACTION_NULL: case DNS_TRANSACTION_PENDING: case DNS_TRANSACTION_VALIDATING: diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index aeae08f2f61..12c48deb662 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -411,21 +411,6 @@ void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state) { "DNS_SERVER_FEATURE_LEVEL=%s", dns_server_feature_level_to_string(t->server->possible_feature_level)); } - if (state == DNS_TRANSACTION_UPSTREAM_DNSSEC_FAILURE) { - dns_resource_key_to_string(dns_transaction_key(t), key_str, sizeof key_str); - - log_struct(LOG_NOTICE, - "MESSAGE_ID=" SD_MESSAGE_DNSSEC_FAILURE_STR, - LOG_MESSAGE("Upstream resolver reported failure for question %s: %s%s%s", - key_str, dns_ede_rcode_to_string(t->answer_ede_rcode), - isempty(t->answer_ede_msg) ? "" : ": ", t->answer_ede_msg), - "DNS_TRANSACTION=%" PRIu16, t->id, - "DNS_QUESTION=%s", key_str, - "DNS_EDE_RCODE=%s", dns_ede_rcode_to_string(t->answer_ede_rcode), - "DNS_SERVER=%s", strna(dns_server_string_full(t->server)), - "DNS_SERVER_FEATURE_LEVEL=%s", dns_server_feature_level_to_string(t->server->possible_feature_level)); - } - /* Note that this call might invalidate the query. Callers * should hence not attempt to access the query or transaction * after calling this function. */ @@ -1241,7 +1226,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p, bool encrypt FORMAT_DNS_EDE_RCODE(t->answer_ede_rcode), isempty(t->answer_ede_msg) ? "" : ": ", strempty(t->answer_ede_msg)); - dns_transaction_complete(t, DNS_TRANSACTION_UPSTREAM_DNSSEC_FAILURE); + dns_transaction_complete(t, DNS_TRANSACTION_DNSSEC_FAILED); return; } diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index 1188708d8f5..6de4cdd7491 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -20,7 +20,6 @@ enum DnsTransactionState { DNS_TRANSACTION_PENDING, DNS_TRANSACTION_VALIDATING, DNS_TRANSACTION_RCODE_FAILURE, - DNS_TRANSACTION_UPSTREAM_DNSSEC_FAILURE, DNS_TRANSACTION_SUCCESS, DNS_TRANSACTION_NO_SERVERS, DNS_TRANSACTION_TIMEOUT, From 41398e87930902d62a3f6d11e4ff8b4af54fc69e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 7 Jan 2024 06:01:05 +0900 Subject: [PATCH 05/13] resolve: several follow-ups for 9ca133e97a0c8795b1f293ccea4965b4ad1accc4. - add missing initialization for DnsQuery.answer_ede_rcode, - clear EDE code and message in dns_transaction_reset_answer(), otherwise the previous EDE code or message may be mistakenly reused on restart. This fixes memory leak of DnsTransaction.answer_ede_msg. - also clear EDE code and message in dns_query_reset_answer(), otherwise ede message is leaked if dns_query_accept() is called multiple times for the same DnsQuery. Follow-up for 9ca133e97a0c8795b1f293ccea4965b4ad1accc4. Fixes #30752. --- src/resolve/resolved-dns-query.c | 20 +++++++------------- src/resolve/resolved-dns-query.h | 2 +- src/resolve/resolved-dns-transaction.c | 4 ++-- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index a7bc45838f7..861a2db2cce 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -368,6 +368,8 @@ static void dns_query_reset_answer(DnsQuery *q) { q->answer = dns_answer_unref(q->answer); q->answer_rcode = 0; + q->answer_ede_rcode = _DNS_EDE_RCODE_INVALID; + q->answer_ede_msg = mfree(q->answer_ede_msg); q->answer_dnssec_result = _DNSSEC_RESULT_INVALID; q->answer_errno = 0; q->answer_query_flags = 0; @@ -421,8 +423,6 @@ DnsQuery *dns_query_free(DnsQuery *q) { dns_answer_unref(q->reply_authoritative); dns_answer_unref(q->reply_additional); - free(q->answer_ede_msg); - if (q->request_stream) { /* Detach the stream from our query, in case something else keeps a reference to it. */ (void) set_remove(q->request_stream->queries, q); @@ -516,6 +516,7 @@ int dns_query_new( .question_bypass = dns_packet_ref(question_bypass), .ifindex = ifindex, .flags = flags, + .answer_ede_rcode = _DNS_EDE_RCODE_INVALID, .answer_dnssec_result = _DNSSEC_RESULT_INVALID, .answer_protocol = _DNS_PROTOCOL_INVALID, .answer_family = AF_UNSPEC, @@ -898,20 +899,13 @@ static void dns_query_accept(DnsQuery *q, DnsQueryCandidate *c) { !FLAGS_SET(t->answer_query_flags, SD_RESOLVED_AUTHENTICATED)) continue; - char *answer_ede_msg = NULL; - if (t->answer_ede_msg) { - answer_ede_msg = strdup(t->answer_ede_msg); - if (!answer_ede_msg) { - r = log_oom(); - goto fail; - } - } - DNS_ANSWER_REPLACE(q->answer, dns_answer_ref(t->answer)); q->answer_rcode = t->answer_rcode; - q->answer_dnssec_result = t->answer_dnssec_result; q->answer_ede_rcode = t->answer_ede_rcode; - q->answer_ede_msg = answer_ede_msg; + r = free_and_strdup_warn(&q->answer_ede_msg, t->answer_ede_msg); + if (r < 0) + goto fail; + q->answer_dnssec_result = t->answer_dnssec_result; q->answer_query_flags = t->answer_query_flags | dns_transaction_source_to_query_flags(t->answer_source); q->answer_errno = t->answer_errno; DNS_PACKET_REPLACE(q->answer_full_packet, dns_packet_ref(t->received)); diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index 74ad2c73500..29d7288981f 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -73,9 +73,9 @@ struct DnsQuery { /* Discovered data */ DnsAnswer *answer; int answer_rcode; - DnssecResult answer_dnssec_result; int answer_ede_rcode; char *answer_ede_msg; + DnssecResult answer_dnssec_result; uint64_t answer_query_flags; DnsProtocol answer_protocol; int answer_family; diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 12c48deb662..aabaa129447 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -28,6 +28,8 @@ static void dns_transaction_reset_answer(DnsTransaction *t) { t->received = dns_packet_unref(t->received); t->answer = dns_answer_unref(t->answer); t->answer_rcode = 0; + t->answer_ede_rcode = _DNS_EDE_RCODE_INVALID; + t->answer_ede_msg = mfree(t->answer_ede_msg); t->answer_dnssec_result = _DNSSEC_RESULT_INVALID; t->answer_source = _DNS_TRANSACTION_SOURCE_INVALID; t->answer_query_flags = 0; @@ -166,8 +168,6 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { dns_resource_key_unref(t->key); dns_packet_unref(t->bypass); - free(t->answer_ede_msg); - return mfree(t); } From 0c61995d80d3ba211b50e28349cdc9b121104d41 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 10 Jan 2024 05:05:29 +0900 Subject: [PATCH 06/13] resolvectl: use JSON_ALLOW_EXTENSIONS Follow-up for f0e4244b2fda1b3de3da1c2792ed1cd21c72087b. --- src/resolve/resolvectl.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index afa537f160b..64b829e5e64 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -2715,7 +2715,7 @@ static int print_answer(JsonVariant *answer) { static void monitor_query_dump(JsonVariant *v) { _cleanup_(json_variant_unrefp) JsonVariant *question = NULL, *answer = NULL, *collected_questions = NULL; - int rcode = -1, error = 0, r; + int rcode = -1, error = 0; const char *state = NULL; assert(v); @@ -2730,9 +2730,8 @@ static void monitor_query_dump(JsonVariant *v) { {} }; - r = json_dispatch(v, dispatch_table, 0, NULL); - if (r < 0) - return (void) log_warning("Received malformed monitor message, ignoring."); + if (json_dispatch(v, dispatch_table, JSON_LOG|JSON_ALLOW_EXTENSIONS, NULL) < 0) + return; /* First show the current question */ print_question('Q', ansi_highlight_cyan(), question); @@ -2856,7 +2855,7 @@ static int dump_cache_item(JsonVariant *item) { _cleanup_(dns_resource_key_unrefp) DnsResourceKey *k = NULL; int r, c = 0; - r = json_dispatch(item, dispatch_table, JSON_LOG, &item_info); + r = json_dispatch(item, dispatch_table, JSON_LOG|JSON_ALLOW_EXTENSIONS, &item_info); if (r < 0) return r; @@ -2918,7 +2917,7 @@ static int dump_cache_scope(JsonVariant *scope) { {}, }; - r = json_dispatch(scope, dispatch_table, JSON_LOG, &scope_info); + r = json_dispatch(scope, dispatch_table, JSON_LOG|JSON_ALLOW_EXTENSIONS, &scope_info); if (r < 0) return r; @@ -3034,7 +3033,7 @@ static int dump_server_state(JsonVariant *server) { {}, }; - r = json_dispatch(server, dispatch_table, JSON_LOG|JSON_PERMISSIVE, &server_state); + r = json_dispatch(server, dispatch_table, JSON_LOG|JSON_ALLOW_EXTENSIONS, &server_state); if (r < 0) return r; From a72cf22d065630da74cf8aee12d8a2a23fcfefb6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 10 Jan 2024 11:34:44 +0900 Subject: [PATCH 07/13] resolve: introduce DNSSEC_UPSTREAM_FAILURE and include EDE code and message in the error messages. This replaces 9ca133e97a0c8795b1f293ccea4965b4ad1accc4, and implements originally suggested at https://github.com/systemd/systemd/pull/30513#discussion_r1433823737 --- src/resolve/resolved-bus.c | 17 ++++++++++++++--- src/resolve/resolved-dns-dnssec.c | 1 + src/resolve/resolved-dns-dnssec.h | 3 ++- src/resolve/resolved-dns-transaction.c | 19 +++++++++++++++++-- src/resolve/resolved-varlink.c | 12 ++++++++++-- src/shared/varlink-io.systemd.Resolve.c | 8 ++++++-- 6 files changed, 50 insertions(+), 10 deletions(-) diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index ef3f5237a9e..d9d967ec400 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -145,8 +145,13 @@ static int reply_query_state(DnsQuery *q) { return reply_method_errorf(q, BUS_ERROR_ABORTED, "Query aborted"); case DNS_TRANSACTION_DNSSEC_FAILED: - return reply_method_errorf(q, BUS_ERROR_DNSSEC_FAILED, "DNSSEC validation failed: %s", - dnssec_result_to_string(q->answer_dnssec_result)); + return reply_method_errorf(q, BUS_ERROR_DNSSEC_FAILED, "DNSSEC validation failed: %s%s%s%s%s%s", + dnssec_result_to_string(q->answer_dnssec_result), + q->answer_ede_rcode >= 0 ? " (" : "", + q->answer_ede_rcode >= 0 ? FORMAT_DNS_EDE_RCODE(q->answer_ede_rcode) : "", + (q->answer_ede_rcode >= 0 && !isempty(q->answer_ede_msg)) ? ": " : "", + q->answer_ede_rcode >= 0 ? strempty(q->answer_ede_msg) : "", + q->answer_ede_rcode >= 0 ? ")" : ""); case DNS_TRANSACTION_NO_TRUST_ANCHOR: return reply_method_errorf(q, BUS_ERROR_NO_TRUST_ANCHOR, "No suitable trust anchor known"); @@ -183,7 +188,13 @@ static int reply_query_state(DnsQuery *q) { rc = FORMAT_DNS_RCODE(q->answer_rcode); n = strjoina(_BUS_ERROR_DNS, rc); - sd_bus_error_setf(&error, n, "Could not resolve '%s', server or network returned error %s", dns_query_string(q), rc); + sd_bus_error_setf(&error, n, "Could not resolve '%s', server or network returned error: %s%s%s%s%s%s", + dns_query_string(q), rc, + q->answer_ede_rcode >= 0 ? " (" : "", + q->answer_ede_rcode >= 0 ? FORMAT_DNS_EDE_RCODE(q->answer_ede_rcode) : "", + (q->answer_ede_rcode >= 0 && !isempty(q->answer_ede_msg)) ? ": " : "", + q->answer_ede_rcode >= 0 ? strempty(q->answer_ede_msg) : "", + q->answer_ede_rcode >= 0 ? ")" : ""); } return sd_bus_reply_method_error(req, &error); diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 3c9b90c89b2..8788bd6b0bf 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -2564,6 +2564,7 @@ static const char* const dnssec_result_table[_DNSSEC_RESULT_MAX] = { [DNSSEC_FAILED_AUXILIARY] = "failed-auxiliary", [DNSSEC_NSEC_MISMATCH] = "nsec-mismatch", [DNSSEC_INCOMPATIBLE_SERVER] = "incompatible-server", + [DNSSEC_UPSTREAM_FAILURE] = "upstream-failure", }; DEFINE_STRING_TABLE_LOOKUP(dnssec_result, DnssecResult); diff --git a/src/resolve/resolved-dns-dnssec.h b/src/resolve/resolved-dns-dnssec.h index 954bb3ef9de..2f93a7f5852 100644 --- a/src/resolve/resolved-dns-dnssec.h +++ b/src/resolve/resolved-dns-dnssec.h @@ -20,11 +20,12 @@ enum DnssecResult { DNSSEC_NO_SIGNATURE, DNSSEC_MISSING_KEY, - /* These two are added by the DnsTransaction logic */ + /* These five are added by the DnsTransaction logic */ DNSSEC_UNSIGNED, DNSSEC_FAILED_AUXILIARY, DNSSEC_NSEC_MISMATCH, DNSSEC_INCOMPATIBLE_SERVER, + DNSSEC_UPSTREAM_FAILURE, _DNSSEC_RESULT_MAX, _DNSSEC_RESULT_INVALID = -EINVAL, diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index aabaa129447..4ec58dc1c82 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -888,8 +888,21 @@ static int dns_transaction_dnssec_ready(DnsTransaction *t) { /* We handle DNSSEC failures different from other errors, as we care about the DNSSEC * validation result */ - log_debug("Auxiliary DNSSEC RR query failed validation: %s", dnssec_result_to_string(dt->answer_dnssec_result)); - t->answer_dnssec_result = dt->answer_dnssec_result; /* Copy error code over */ + log_debug("Auxiliary DNSSEC RR query failed validation: %s%s%s%s%s%s", + dnssec_result_to_string(dt->answer_dnssec_result), + dt->answer_ede_rcode >= 0 ? " (" : "", + dt->answer_ede_rcode >= 0 ? FORMAT_DNS_EDE_RCODE(dt->answer_ede_rcode) : "", + (dt->answer_ede_rcode >= 0 && !isempty(dt->answer_ede_msg)) ? ": " : "", + dt->answer_ede_rcode >= 0 ? strempty(dt->answer_ede_msg) : "", + dt->answer_ede_rcode >= 0 ? ")" : ""); + + /* Copy error code over */ + t->answer_dnssec_result = dt->answer_dnssec_result; + t->answer_ede_rcode = dt->answer_ede_rcode; + r = free_and_strdup(&t->answer_ede_msg, dt->answer_ede_msg); + if (r < 0) + log_oom_debug(); + dns_transaction_complete(t, DNS_TRANSACTION_DNSSEC_FAILED); return 0; @@ -1226,6 +1239,8 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p, bool encrypt FORMAT_DNS_EDE_RCODE(t->answer_ede_rcode), isempty(t->answer_ede_msg) ? "" : ": ", strempty(t->answer_ede_msg)); + + t->answer_dnssec_result = DNSSEC_UPSTREAM_FAILURE; dns_transaction_complete(t, DNS_TRANSACTION_DNSSEC_FAILED); return; } diff --git a/src/resolve/resolved-varlink.c b/src/resolve/resolved-varlink.c index 3e178a69f35..4e7e91bdfb7 100644 --- a/src/resolve/resolved-varlink.c +++ b/src/resolve/resolved-varlink.c @@ -49,7 +49,11 @@ static int reply_query_state(DnsQuery *q) { case DNS_TRANSACTION_DNSSEC_FAILED: return varlink_errorb(q->varlink_request, "io.systemd.Resolve.DNSSECValidationFailed", - JSON_BUILD_OBJECT(JSON_BUILD_PAIR("result", JSON_BUILD_STRING(dnssec_result_to_string(q->answer_dnssec_result))))); + JSON_BUILD_OBJECT(JSON_BUILD_PAIR("result", JSON_BUILD_STRING(dnssec_result_to_string(q->answer_dnssec_result))), + JSON_BUILD_PAIR_CONDITION(q->answer_ede_rcode >= 0, + "extendedDNSErrorCode", JSON_BUILD_INTEGER(q->answer_ede_rcode)), + JSON_BUILD_PAIR_CONDITION(q->answer_ede_rcode >= 0 && !isempty(q->answer_ede_msg), + "extendedDNSErrorMessage", JSON_BUILD_STRING(q->answer_ede_msg)))); case DNS_TRANSACTION_NO_TRUST_ANCHOR: return varlink_error(q->varlink_request, "io.systemd.Resolve.NoTrustAnchor", NULL); @@ -74,7 +78,11 @@ static int reply_query_state(DnsQuery *q) { case DNS_TRANSACTION_RCODE_FAILURE: return varlink_errorb(q->varlink_request, "io.systemd.Resolve.DNSError", - JSON_BUILD_OBJECT(JSON_BUILD_PAIR("rcode", JSON_BUILD_INTEGER(q->answer_rcode)))); + JSON_BUILD_OBJECT(JSON_BUILD_PAIR("rcode", JSON_BUILD_INTEGER(q->answer_rcode)), + JSON_BUILD_PAIR_CONDITION(q->answer_ede_rcode >= 0, + "extendedDNSErrorCode", JSON_BUILD_INTEGER(q->answer_ede_rcode)), + JSON_BUILD_PAIR_CONDITION(q->answer_ede_rcode >= 0 && !isempty(q->answer_ede_msg), + "extendedDNSErrorMessage", JSON_BUILD_STRING(q->answer_ede_msg)))); case DNS_TRANSACTION_NULL: case DNS_TRANSACTION_PENDING: diff --git a/src/shared/varlink-io.systemd.Resolve.c b/src/shared/varlink-io.systemd.Resolve.c index 0d8ad281fa6..627b062ab0c 100644 --- a/src/shared/varlink-io.systemd.Resolve.c +++ b/src/shared/varlink-io.systemd.Resolve.c @@ -40,7 +40,9 @@ static VARLINK_DEFINE_ERROR(InvalidReply); static VARLINK_DEFINE_ERROR(QueryAborted); static VARLINK_DEFINE_ERROR( DNSSECValidationFailed, - VARLINK_DEFINE_FIELD(result, VARLINK_STRING, 0)); + VARLINK_DEFINE_FIELD(result, VARLINK_STRING, 0), + VARLINK_DEFINE_FIELD(extendedDNSErrorCode, VARLINK_INT, VARLINK_NULLABLE), + VARLINK_DEFINE_FIELD(extendedDNSErrorMessage, VARLINK_STRING, VARLINK_NULLABLE)); static VARLINK_DEFINE_ERROR(NoTrustAnchor); static VARLINK_DEFINE_ERROR(ResourceRecordTypeUnsupported); static VARLINK_DEFINE_ERROR(NetworkDown); @@ -48,7 +50,9 @@ static VARLINK_DEFINE_ERROR(NoSource); static VARLINK_DEFINE_ERROR(StubLoop); static VARLINK_DEFINE_ERROR( DNSError, - VARLINK_DEFINE_FIELD(rcode, VARLINK_INT, 0)); + VARLINK_DEFINE_FIELD(rcode, VARLINK_INT, 0), + VARLINK_DEFINE_FIELD(extendedDNSErrorCode, VARLINK_INT, VARLINK_NULLABLE), + VARLINK_DEFINE_FIELD(extendedDNSErrorMessage, VARLINK_STRING, VARLINK_NULLABLE)); static VARLINK_DEFINE_ERROR(CNAMELoop); static VARLINK_DEFINE_ERROR(BadAddressSize); From 37baa00fe061c967ebb773c2d4bbc332fac10d6a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 10 Jan 2024 13:01:31 +0900 Subject: [PATCH 08/13] resolve: also read EDE code and message from cached packet --- src/resolve/resolved-dns-transaction.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 4ec58dc1c82..307630f3c7a 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1821,8 +1821,12 @@ static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) { t->answer_source = DNS_TRANSACTION_CACHE; if (t->answer_rcode == DNS_RCODE_SUCCESS) dns_transaction_complete(t, DNS_TRANSACTION_SUCCESS); - else + else { + if (t->received) + (void) dns_packet_ede_rcode(t->received, &t->answer_ede_rcode, &t->answer_ede_msg); + dns_transaction_complete(t, DNS_TRANSACTION_RCODE_FAILURE); + } return 0; } } From d9f9b8ce8a76dbebbb84f7db194ad30a05822a42 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 10 Jan 2024 12:07:19 +0900 Subject: [PATCH 09/13] resolve: make manager_monitor_send() take DnsQuery* --- src/resolve/resolved-dns-query.c | 2 +- src/resolve/resolved-manager.c | 28 +++++++++------------------- src/resolve/resolved-manager.h | 2 +- 3 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 861a2db2cce..938dd61a6a7 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -589,7 +589,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->question_bypass, q->collected_questions, q->answer); + (void) manager_monitor_send(q->manager, q); dns_query_stop(q); if (q->complete) diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index a0251b4b97b..34f2268d820 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -1098,17 +1098,7 @@ static int dns_question_to_json(DnsQuestion *q, JsonVariant **ret) { return 0; } -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_monitor_send(Manager *m, DnsQuery *q) { _cleanup_(json_variant_unrefp) JsonVariant *jquestion = NULL, *jcollected_questions = NULL, *janswer = NULL; _cleanup_(dns_question_unrefp) DnsQuestion *merged = NULL; Varlink *connection; @@ -1121,14 +1111,14 @@ int manager_monitor_send( return 0; /* Merge all questions into one */ - r = dns_question_merge(question_idna, question_utf8, &merged); + r = dns_question_merge(q->question_idna, q->question_utf8, &merged); if (r < 0) return log_error_errno(r, "Failed to merge UTF8/IDNA questions: %m"); - if (question_bypass) { + if (q->question_bypass) { _cleanup_(dns_question_unrefp) DnsQuestion *merged2 = NULL; - r = dns_question_merge(merged, question_bypass->question, &merged2); + r = dns_question_merge(merged, q->question_bypass->question, &merged2); if (r < 0) return log_error_errno(r, "Failed to merge UTF8/IDNA questions and DNS packet question: %m"); @@ -1142,11 +1132,11 @@ int manager_monitor_send( return log_error_errno(r, "Failed to convert question to JSON: %m"); /* Generate a JSON array of the questions preceding the current one in the CNAME chain */ - r = dns_question_to_json(collected_questions, &jcollected_questions); + r = dns_question_to_json(q->collected_questions, &jcollected_questions); if (r < 0) return log_error_errno(r, "Failed to convert question to JSON: %m"); - DNS_ANSWER_FOREACH_ITEM(rri, answer) { + DNS_ANSWER_FOREACH_ITEM(rri, q->answer) { _cleanup_(json_variant_unrefp) JsonVariant *v = NULL; r = dns_resource_record_to_json(rri->rr, &v); @@ -1169,9 +1159,9 @@ int manager_monitor_send( SET_FOREACH(connection, m->varlink_subscription) { r = varlink_notifyb(connection, - JSON_BUILD_OBJECT(JSON_BUILD_PAIR("state", JSON_BUILD_STRING(dns_transaction_state_to_string(state))), - JSON_BUILD_PAIR_CONDITION(state == DNS_TRANSACTION_RCODE_FAILURE, "rcode", JSON_BUILD_INTEGER(rcode)), - JSON_BUILD_PAIR_CONDITION(state == DNS_TRANSACTION_ERRNO, "errno", JSON_BUILD_INTEGER(error)), + JSON_BUILD_OBJECT(JSON_BUILD_PAIR("state", JSON_BUILD_STRING(dns_transaction_state_to_string(q->state))), + JSON_BUILD_PAIR_CONDITION(q->state == DNS_TRANSACTION_RCODE_FAILURE, "rcode", JSON_BUILD_INTEGER(q->answer_rcode)), + JSON_BUILD_PAIR_CONDITION(q->state == DNS_TRANSACTION_ERRNO, "errno", JSON_BUILD_INTEGER(q->answer_errno)), JSON_BUILD_PAIR("question", JSON_BUILD_VARIANT(jquestion)), JSON_BUILD_PAIR_CONDITION(jcollected_questions, "collectedQuestions", JSON_BUILD_VARIANT(jcollected_questions)), JSON_BUILD_PAIR_CONDITION(janswer, "answer", JSON_BUILD_VARIANT(janswer)))); diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h index 5cd5e834d39..8b08d0cebf0 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, DnsPacket *question_bypass, DnsQuestion *collected_questions, DnsAnswer *answer); +int manager_monitor_send(Manager *m, DnsQuery *q); 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 a068e06da67e1608226dffb8bff2caebb0116a9b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 10 Jan 2024 12:37:36 +0900 Subject: [PATCH 10/13] resolve: notify DNSSEC failure, EDE code, and EDE message --- src/resolve/resolvectl.c | 32 +++++++++++++------ src/resolve/resolved-manager.c | 24 +++++++++++--- .../varlink-io.systemd.Resolve.Monitor.c | 3 ++ 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 64b829e5e64..b086a67010f 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -2715,18 +2715,21 @@ static int print_answer(JsonVariant *answer) { static void monitor_query_dump(JsonVariant *v) { _cleanup_(json_variant_unrefp) JsonVariant *question = NULL, *answer = NULL, *collected_questions = NULL; - int rcode = -1, error = 0; - const char *state = NULL; + int rcode = -1, error = 0, ede_code = -1; + const char *state = NULL, *result = NULL, *ede_msg = NULL; assert(v); JsonDispatch dispatch_table[] = { - { "question", JSON_VARIANT_ARRAY, json_dispatch_variant, PTR_TO_SIZE(&question), JSON_MANDATORY }, - { "answer", JSON_VARIANT_ARRAY, json_dispatch_variant, PTR_TO_SIZE(&answer), 0 }, - { "collectedQuestions", JSON_VARIANT_ARRAY, json_dispatch_variant, PTR_TO_SIZE(&collected_questions), 0 }, - { "state", JSON_VARIANT_STRING, json_dispatch_const_string, PTR_TO_SIZE(&state), JSON_MANDATORY }, - { "rcode", _JSON_VARIANT_TYPE_INVALID, json_dispatch_int, PTR_TO_SIZE(&rcode), 0 }, - { "errno", _JSON_VARIANT_TYPE_INVALID, json_dispatch_int, PTR_TO_SIZE(&error), 0 }, + { "question", JSON_VARIANT_ARRAY, json_dispatch_variant, PTR_TO_SIZE(&question), JSON_MANDATORY }, + { "answer", JSON_VARIANT_ARRAY, json_dispatch_variant, PTR_TO_SIZE(&answer), 0 }, + { "collectedQuestions", JSON_VARIANT_ARRAY, json_dispatch_variant, PTR_TO_SIZE(&collected_questions), 0 }, + { "state", JSON_VARIANT_STRING, json_dispatch_const_string, PTR_TO_SIZE(&state), JSON_MANDATORY }, + { "result", JSON_VARIANT_STRING, json_dispatch_const_string, PTR_TO_SIZE(&result), 0 }, + { "rcode", _JSON_VARIANT_TYPE_INVALID, json_dispatch_int, PTR_TO_SIZE(&rcode), 0 }, + { "errno", _JSON_VARIANT_TYPE_INVALID, json_dispatch_int, PTR_TO_SIZE(&error), 0 }, + { "extendedDNSErrorCode", _JSON_VARIANT_TYPE_INVALID, json_dispatch_int, PTR_TO_SIZE(&ede_code), 0 }, + { "extendedDNSErrorMessage", JSON_VARIANT_STRING, json_dispatch_const_string, PTR_TO_SIZE(&ede_msg), 0 }, {} }; @@ -2739,7 +2742,7 @@ static void monitor_query_dump(JsonVariant *v) { /* And then show the questions that led to this one in case this was a CNAME chain */ print_question('C', ansi_highlight_grey(), collected_questions); - printf("%s%s S%s: %s\n", + printf("%s%s S%s: %s", streq_ptr(state, "success") ? ansi_highlight_green() : ansi_highlight_red(), special_glyph(SPECIAL_GLYPH_ARROW_LEFT), ansi_normal(), @@ -2747,6 +2750,17 @@ static void monitor_query_dump(JsonVariant *v) { streq_ptr(state, "rcode-failure") ? dns_rcode_to_string(rcode) : state)); + if (!isempty(result)) + printf(": %s", result); + + if (ede_code >= 0) + printf(" (%s%s%s)", + FORMAT_DNS_EDE_RCODE(ede_code), + !isempty(ede_msg) ? ": " : "", + strempty(ede_msg)); + + puts(""); + print_answer(answer); } diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 34f2268d820..fcd269f1cef 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -1160,11 +1160,27 @@ int manager_monitor_send(Manager *m, DnsQuery *q) { SET_FOREACH(connection, m->varlink_subscription) { r = varlink_notifyb(connection, JSON_BUILD_OBJECT(JSON_BUILD_PAIR("state", JSON_BUILD_STRING(dns_transaction_state_to_string(q->state))), - JSON_BUILD_PAIR_CONDITION(q->state == DNS_TRANSACTION_RCODE_FAILURE, "rcode", JSON_BUILD_INTEGER(q->answer_rcode)), - JSON_BUILD_PAIR_CONDITION(q->state == DNS_TRANSACTION_ERRNO, "errno", JSON_BUILD_INTEGER(q->answer_errno)), + JSON_BUILD_PAIR_CONDITION(q->state == DNS_TRANSACTION_DNSSEC_FAILED, + "result", JSON_BUILD_STRING(dnssec_result_to_string(q->answer_dnssec_result))), + JSON_BUILD_PAIR_CONDITION(q->state == DNS_TRANSACTION_RCODE_FAILURE, + "rcode", JSON_BUILD_INTEGER(q->answer_rcode)), + JSON_BUILD_PAIR_CONDITION(q->state == DNS_TRANSACTION_ERRNO, + "errno", JSON_BUILD_INTEGER(q->answer_errno)), + JSON_BUILD_PAIR_CONDITION(IN_SET(q->state, + DNS_TRANSACTION_DNSSEC_FAILED, + DNS_TRANSACTION_RCODE_FAILURE) && + q->answer_ede_rcode >= 0, + "extendedDNSErrorCode", JSON_BUILD_INTEGER(q->answer_ede_rcode)), + JSON_BUILD_PAIR_CONDITION(IN_SET(q->state, + DNS_TRANSACTION_DNSSEC_FAILED, + DNS_TRANSACTION_RCODE_FAILURE) && + q->answer_ede_rcode >= 0 && !isempty(q->answer_ede_msg), + "extendedDNSErrorMessage", JSON_BUILD_STRING(q->answer_ede_msg)), JSON_BUILD_PAIR("question", JSON_BUILD_VARIANT(jquestion)), - JSON_BUILD_PAIR_CONDITION(jcollected_questions, "collectedQuestions", JSON_BUILD_VARIANT(jcollected_questions)), - JSON_BUILD_PAIR_CONDITION(janswer, "answer", JSON_BUILD_VARIANT(janswer)))); + JSON_BUILD_PAIR_CONDITION(jcollected_questions, + "collectedQuestions", JSON_BUILD_VARIANT(jcollected_questions)), + JSON_BUILD_PAIR_CONDITION(janswer, + "answer", JSON_BUILD_VARIANT(janswer)))); if (r < 0) log_debug_errno(r, "Failed to send monitor event, ignoring: %m"); } diff --git a/src/shared/varlink-io.systemd.Resolve.Monitor.c b/src/shared/varlink-io.systemd.Resolve.Monitor.c index d95b613eafb..96a58ca768d 100644 --- a/src/shared/varlink-io.systemd.Resolve.Monitor.c +++ b/src/shared/varlink-io.systemd.Resolve.Monitor.c @@ -78,8 +78,11 @@ VARLINK_DEFINE_METHOD( VARLINK_DEFINE_OUTPUT(ready, VARLINK_BOOL, VARLINK_NULLABLE), /* Subsequent replies */ VARLINK_DEFINE_OUTPUT(state, VARLINK_STRING, VARLINK_NULLABLE), + VARLINK_DEFINE_OUTPUT(result, VARLINK_STRING, VARLINK_NULLABLE), VARLINK_DEFINE_OUTPUT(rcode, VARLINK_INT, VARLINK_NULLABLE), VARLINK_DEFINE_OUTPUT(errno, VARLINK_INT, VARLINK_NULLABLE), + VARLINK_DEFINE_OUTPUT(extendedDNSErrorCode, VARLINK_INT, VARLINK_NULLABLE), + VARLINK_DEFINE_OUTPUT(extendedDNSErrorMessage, VARLINK_STRING, VARLINK_NULLABLE), VARLINK_DEFINE_OUTPUT_BY_TYPE(question, ResourceKey, VARLINK_NULLABLE|VARLINK_ARRAY), VARLINK_DEFINE_OUTPUT_BY_TYPE(collectedQuestions, ResourceKey, VARLINK_NULLABLE|VARLINK_ARRAY), VARLINK_DEFINE_OUTPUT_BY_TYPE(answer, Answer, VARLINK_NULLABLE|VARLINK_ARRAY)); From ed6c51781f1cd319875ff413538d30a46ed64bb4 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Mon, 8 Jan 2024 19:23:06 +0100 Subject: [PATCH 11/13] resolve: export sendmsg_loop() So we can use it in tests. --- src/resolve/resolved-manager.c | 2 +- src/resolve/resolved-manager.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index fcd269f1cef..5a14e64fe5c 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -894,7 +894,7 @@ int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) { return 1; } -static int sendmsg_loop(int fd, struct msghdr *mh, int flags) { +int sendmsg_loop(int fd, struct msghdr *mh, int flags) { usec_t end; int r; diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h index 8b08d0cebf0..bd0e0532e7d 100644 --- a/src/resolve/resolved-manager.h +++ b/src/resolve/resolved-manager.h @@ -178,6 +178,7 @@ uint32_t manager_find_mtu(Manager *m); int manager_monitor_send(Manager *m, DnsQuery *q); +int sendmsg_loop(int fd, struct msghdr *mh, int flags); 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); int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret); From f1caa5d6e7770fef7fa033f9eb68a580e8555f42 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Mon, 8 Jan 2024 14:20:30 +0100 Subject: [PATCH 12/13] test: introduce a dummy DNS test server Introduce a _very_ simple DNS server using our internal DNS-related code, that responds to queries with specifically crafted packets, to cover scenarios that are difficult to reproduce with well-behaving DNS servers. Also, hide the test DNS server behind Knot using the dnsproxy module, so we don't have to switch DNS servers during tests. --- src/resolve/meson.build | 14 + src/resolve/test-resolved-dummy-server.c | 428 +++++++++++++++++++++++ test/knot-data/knot.conf | 17 + test/units/testsuite-75.sh | 85 ++++- 4 files changed, 543 insertions(+), 1 deletion(-) create mode 100644 src/resolve/test-resolved-dummy-server.c diff --git a/src/resolve/meson.build b/src/resolve/meson.build index e7867e2f85a..d855ded91a3 100644 --- a/src/resolve/meson.build +++ b/src/resolve/meson.build @@ -196,6 +196,20 @@ executables += [ ], 'include_directories' : resolve_includes, }, + test_template + { + 'sources' : [ + files('test-resolved-dummy-server.c'), + basic_dns_sources, + systemd_resolved_sources, + ], + 'dependencies' : [ + lib_openssl_or_gcrypt, + libm, + systemd_resolved_dependencies, + ], + 'include_directories' : resolve_includes, + 'type' : 'manual', + }, resolve_fuzz_template + { 'sources' : files('fuzz-dns-packet.c'), }, diff --git a/src/resolve/test-resolved-dummy-server.c b/src/resolve/test-resolved-dummy-server.c new file mode 100644 index 00000000000..4fc4734df77 --- /dev/null +++ b/src/resolve/test-resolved-dummy-server.c @@ -0,0 +1,428 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "sd-daemon.h" + +#include "fd-util.h" +#include "iovec-util.h" +#include "log.h" +#include "resolved-dns-packet.h" +#include "resolved-manager.h" +#include "socket-netlink.h" +#include "socket-util.h" + +/* Taken from resolved-dns-stub.c */ +#define ADVERTISE_DATAGRAM_SIZE_MAX (65536U-14U-20U-8U) + +/* This is more or less verbatim manager_recv() from resolved-manager.c, sans the manager stuff */ +static int server_recv(int fd, DnsPacket **ret) { + _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL; + CMSG_BUFFER_TYPE(CMSG_SPACE(MAXSIZE(struct in_pktinfo, struct in6_pktinfo)) + + CMSG_SPACE(int) /* ttl/hoplimit */ + + EXTRA_CMSG_SPACE /* kernel appears to require extra buffer space */) control; + union sockaddr_union sa; + struct iovec iov; + struct msghdr mh = { + .msg_name = &sa.sa, + .msg_namelen = sizeof(sa), + .msg_iov = &iov, + .msg_iovlen = 1, + .msg_control = &control, + .msg_controllen = sizeof(control), + }; + struct cmsghdr *cmsg; + ssize_t ms, l; + int r; + + assert(fd >= 0); + assert(ret); + + ms = next_datagram_size_fd(fd); + if (ms < 0) + return ms; + + r = dns_packet_new(&p, DNS_PROTOCOL_DNS, ms, DNS_PACKET_SIZE_MAX); + if (r < 0) + return r; + + iov = IOVEC_MAKE(DNS_PACKET_DATA(p), p->allocated); + + l = recvmsg_safe(fd, &mh, 0); + if (ERRNO_IS_NEG_TRANSIENT(l)) + return 0; + if (l <= 0) + return l; + + assert(!(mh.msg_flags & MSG_TRUNC)); + + p->size = (size_t) l; + + p->family = sa.sa.sa_family; + p->ipproto = IPPROTO_UDP; + if (p->family == AF_INET) { + p->sender.in = sa.in.sin_addr; + p->sender_port = be16toh(sa.in.sin_port); + } else if (p->family == AF_INET6) { + p->sender.in6 = sa.in6.sin6_addr; + p->sender_port = be16toh(sa.in6.sin6_port); + p->ifindex = sa.in6.sin6_scope_id; + } else + return -EAFNOSUPPORT; + + p->timestamp = now(CLOCK_BOOTTIME); + + CMSG_FOREACH(cmsg, &mh) { + + if (cmsg->cmsg_level == IPPROTO_IPV6) { + assert(p->family == AF_INET6); + + switch (cmsg->cmsg_type) { + + case IPV6_PKTINFO: { + struct in6_pktinfo *i = CMSG_TYPED_DATA(cmsg, struct in6_pktinfo); + + if (p->ifindex <= 0) + p->ifindex = i->ipi6_ifindex; + + p->destination.in6 = i->ipi6_addr; + break; + } + + case IPV6_HOPLIMIT: + p->ttl = *CMSG_TYPED_DATA(cmsg, int); + break; + + case IPV6_RECVFRAGSIZE: + p->fragsize = *CMSG_TYPED_DATA(cmsg, int); + break; + } + } else if (cmsg->cmsg_level == IPPROTO_IP) { + assert(p->family == AF_INET); + + switch (cmsg->cmsg_type) { + + case IP_PKTINFO: { + struct in_pktinfo *i = CMSG_TYPED_DATA(cmsg, struct in_pktinfo); + + if (p->ifindex <= 0) + p->ifindex = i->ipi_ifindex; + + p->destination.in = i->ipi_addr; + break; + } + + case IP_TTL: + p->ttl = *CMSG_TYPED_DATA(cmsg, int); + break; + + case IP_RECVFRAGSIZE: + p->fragsize = *CMSG_TYPED_DATA(cmsg, int); + break; + } + } + } + + /* The Linux kernel sets the interface index to the loopback + * device if the packet came from the local host since it + * avoids the routing table in such a case. Let's unset the + * interface index in such a case. */ + if (p->ifindex == LOOPBACK_IFINDEX) + p->ifindex = 0; + + log_debug("Received DNS UDP packet of size %zu, ifindex=%i, ttl=%u, fragsize=%zu, sender=%s, destination=%s", + p->size, p->ifindex, p->ttl, p->fragsize, + IN_ADDR_TO_STRING(p->family, &p->sender), + IN_ADDR_TO_STRING(p->family, &p->destination)); + + *ret = TAKE_PTR(p); + return 1; +} + +/* Same as above, see manager_ipv4_send() in resolved-manager.c */ +static int server_ipv4_send( + int fd, + const struct in_addr *destination, + uint16_t port, + const struct in_addr *source, + DnsPacket *packet) { + + union sockaddr_union sa; + struct iovec iov; + struct msghdr mh = { + .msg_iov = &iov, + .msg_iovlen = 1, + .msg_name = &sa.sa, + .msg_namelen = sizeof(sa.in), + }; + + assert(fd >= 0); + assert(destination); + assert(port > 0); + assert(packet); + + iov = IOVEC_MAKE(DNS_PACKET_DATA(packet), packet->size); + + sa = (union sockaddr_union) { + .in.sin_family = AF_INET, + .in.sin_addr = *destination, + .in.sin_port = htobe16(port), + }; + + return sendmsg_loop(fd, &mh, 0); +} + +static int make_reply_packet(DnsPacket *packet, DnsPacket **ret) { + _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL; + int r; + + assert(packet); + assert(ret); + + r = dns_packet_new(&p, DNS_PROTOCOL_DNS, 0, DNS_PACKET_PAYLOAD_SIZE_MAX(packet)); + if (r < 0) + return r; + + r = dns_packet_append_question(p, packet->question); + if (r < 0) + return r; + + DNS_PACKET_HEADER(p)->id = DNS_PACKET_ID(packet); + DNS_PACKET_HEADER(p)->qdcount = htobe16(dns_question_size(packet->question)); + + *ret = TAKE_PTR(p); + return 0; +} + +static int reply_append_edns(DnsPacket *packet, DnsPacket *reply, const char *extra_text, size_t rcode, uint16_t ede_code) { + size_t saved_size; + int r; + + assert(packet); + assert(reply); + + /* Append EDNS0 stuff (inspired by dns_packet_append_opt() from resolved-dns-packet.c). + * + * Relevant headers from RFC 6891: + * + * +------------+--------------+------------------------------+ + * | Field Name | Field Type | Description | + * +------------+--------------+------------------------------+ + * | NAME | domain name | MUST be 0 (root domain) | + * | TYPE | u_int16_t | OPT (41) | + * | CLASS | u_int16_t | requestor's UDP payload size | + * | TTL | u_int32_t | extended RCODE and flags | + * | RDLEN | u_int16_t | length of all RDATA | + * | RDATA | octet stream | {attribute,value} pairs | + * +------------+--------------+------------------------------+ + * + * +0 (MSB) +1 (LSB) + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ + * 0: | OPTION-CODE | + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ + * 2: | OPTION-LENGTH | + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ + * 4: | | + * / OPTION-DATA / + * / / + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ + * + * And from RFC 8914: + * + * 1 1 1 1 1 1 + * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ + * 0: | OPTION-CODE | + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ + * 2: | OPTION-LENGTH | + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ + * 4: | INFO-CODE | + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ + * 6: / EXTRA-TEXT ... / + * +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ + */ + + saved_size = reply->size; + + /* empty name */ + r = dns_packet_append_uint8(reply, 0, NULL); + if (r < 0) + return r; + + /* type */ + r = dns_packet_append_uint16(reply, DNS_TYPE_OPT, NULL); + if (r < 0) + return r; + + /* class: maximum udp packet that can be received */ + r = dns_packet_append_uint16(reply, ADVERTISE_DATAGRAM_SIZE_MAX, NULL); + if (r < 0) + return r; + + /* extended RCODE and VERSION */ + r = dns_packet_append_uint16(reply, ((uint16_t) rcode & 0x0FF0) << 4, NULL); + if (r < 0) + return r; + + /* flags: DNSSEC OK (DO), see RFC3225 */ + r = dns_packet_append_uint16(reply, 0, NULL); + if (r < 0) + return r; + + /* RDATA */ + + size_t extra_text_len = isempty(extra_text) ? 0 : strlen(extra_text); + /* RDLENGTH (OPTION CODE + OPTION LENGTH + INFO-CODE + EXTRA-TEXT) */ + r = dns_packet_append_uint16(reply, 2 + 2 + 2 + extra_text_len, NULL); + if (r < 0) + return 0; + + /* OPTION-CODE: 15 for EDE */ + r = dns_packet_append_uint16(reply, 15, NULL); + if (r < 0) + return r; + + /* OPTION-LENGTH: INFO-CODE + EXTRA-TEXT */ + r = dns_packet_append_uint16(reply, 2 + extra_text_len, NULL); + if (r < 0) + return r; + + /* INFO-CODE: EDE code */ + r = dns_packet_append_uint16(reply, ede_code, NULL); + if (r < 0) + return r; + + /* EXTRA-TEXT */ + if (extra_text_len > 0) { + /* From RFC 8914: + * EDE text may be null terminated but MUST NOT be assumed to be; the length MUST be derived + * from the OPTION-LENGTH field + * + * Let's exercise our code on the receiving side and not NUL-terminate the EXTRA-TEXT field + */ + r = dns_packet_append_blob(reply, extra_text, extra_text_len, NULL); + if (r < 0) + return r; + } + + DNS_PACKET_HEADER(reply)->arcount = htobe16(DNS_PACKET_ARCOUNT(reply) + 1); + reply->opt_start = saved_size; + reply->opt_size = reply->size - saved_size; + + /* Order: qr, opcode, aa, tc, rd, ra, ad, cd, rcode */ + DNS_PACKET_HEADER(reply)->flags = htobe16(DNS_PACKET_MAKE_FLAGS( + 1, 0, 0, 0, DNS_PACKET_RD(packet), 1, 0, 1, rcode)); + return 0; +} + +static void server_fail(DnsPacket *packet, DnsPacket *reply, int rcode) { + assert(reply); + + /* Order: qr, opcode, aa, tc, rd, ra, ad, cd, rcode */ + DNS_PACKET_HEADER(reply)->flags = htobe16(DNS_PACKET_MAKE_FLAGS( + 1, 0, 0, 0, DNS_PACKET_RD(packet), 1, 0, 1, rcode)); +} + +static int server_handle_edns_bogus_dnssec(DnsPacket *packet, DnsPacket *reply) { + assert(packet); + assert(reply); + + return reply_append_edns(packet, reply, NULL, DNS_RCODE_SERVFAIL, DNS_EDE_RCODE_DNSSEC_BOGUS); +} + +static int server_handle_edns_extra_text(DnsPacket *packet, DnsPacket *reply) { + assert(packet); + assert(reply); + + return reply_append_edns(packet, reply, "Nothing to see here!", DNS_RCODE_SERVFAIL, DNS_EDE_RCODE_CENSORED); +} + +static int server_handle_edns_invalid_code(DnsPacket *packet, DnsPacket *reply, const char *extra_text) { + assert(packet); + assert(reply); + assert_cc(_DNS_EDE_RCODE_MAX_DEFINED < UINT16_MAX); + + return reply_append_edns(packet, reply, extra_text, DNS_RCODE_SERVFAIL, _DNS_EDE_RCODE_MAX_DEFINED + 1); +} + +static int server_handle_edns_code_zero(DnsPacket *packet, DnsPacket *reply) { + assert(packet); + assert(reply); + assert_cc(DNS_EDE_RCODE_OTHER == 0); + + return reply_append_edns(packet, reply, "\xF0\x9F\x90\xB1", DNS_RCODE_SERVFAIL, DNS_EDE_RCODE_OTHER); +} + +int main(int argc, char *argv[]) { + _cleanup_close_ int fd = -EBADF; + int r; + + log_parse_environment(); + log_open(); + + if (argc != 2) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "This program takes one argument in format ip_address:port"); + + fd = make_socket_fd(LOG_DEBUG, argv[1], SOCK_DGRAM, SOCK_CLOEXEC); + if (fd < 0) + return log_error_errno(fd, "Failed to listen on address '%s': %m", argv[1]); + + (void) sd_notify(/* unset_environment=false */ false, "READY=1"); + + for (;;) { + _cleanup_(dns_packet_unrefp) DnsPacket *packet = NULL; + _cleanup_(dns_packet_unrefp) DnsPacket *reply = NULL; + const char *name; + + r = server_recv(fd, &packet); + if (r < 0) { + log_debug_errno(r, "Failed to receive packet, ignoring: %m"); + continue; + } + + r = dns_packet_validate_query(packet); + if (r < 0) { + log_debug_errno(r, "Invalid DNS UDP packet, ignoring."); + continue; + } + + r = dns_packet_extract(packet); + if (r < 0) { + log_debug_errno(r, "Failed to extract DNS packet, ignoring: %m"); + continue; + } + + name = dns_question_first_name(packet->question); + log_info("Processing question for name '%s'", name); + + (void) dns_question_dump(packet->question, stdout); + + r = make_reply_packet(packet, &reply); + if (r < 0) { + log_debug_errno(r, "Failed to make reply packet: %m"); + break; + } + + if (streq_ptr(name, "edns-bogus-dnssec.forwarded.test")) + r = server_handle_edns_bogus_dnssec(packet, reply); + else if (streq_ptr(name, "edns-extra-text.forwarded.test")) + r = server_handle_edns_extra_text(packet, reply); + else if (streq_ptr(name, "edns-invalid-code.forwarded.test")) + r = server_handle_edns_invalid_code(packet, reply, NULL); + else if (streq_ptr(name, "edns-invalid-code-with-extra-text.forwarded.test")) + r = server_handle_edns_invalid_code(packet, reply, "Hello [#]$%~ World"); + else if (streq_ptr(name, "edns-code-zero.forwarded.test")) + r = server_handle_edns_code_zero(packet, reply); + else + r = log_debug_errno(SYNTHETIC_ERRNO(EFAULT), "Unhandled name '%s', ignoring.", name); + + if (r < 0) + server_fail(packet, reply, DNS_RCODE_NXDOMAIN); + + r = server_ipv4_send(fd, &packet->sender.in, packet->sender_port, &packet->destination.in, reply); + if (r < 0) + log_debug_errno(r, "Failed to send reply: %m"); + + } + + return 0; +} diff --git a/test/knot-data/knot.conf b/test/knot-data/knot.conf index 245fa75cf7c..69c2082e48c 100644 --- a/test/knot-data/knot.conf +++ b/test/knot-data/knot.conf @@ -29,6 +29,9 @@ remote: address: 10.0.0.1@53 address: fd00:dead:beef:cafe::1@53 + - id: forwarded + address: 10.99.0.1@53 + submission: - id: parent_zone_sbm check-interval: 2s @@ -69,6 +72,11 @@ policy: - id: manual manual: on +mod-dnsproxy: + - id: forwarded + remote: forwarded + fallback: off + template: # Sign everything by default and propagate the respective DS records to the parent - id: default @@ -86,6 +94,11 @@ template: semantic-checks: on storage: "/var/lib/knot/zones" + - id: forwarded + dnssec-signing: off + module: mod-dnsproxy/forwarded + zonefile-load: none + zone: # Create our own DNSSEC-aware root zone, so we can test the whole chain of # trust. This needs a ZSK/KSK keypair to be generated before running knot + @@ -119,3 +132,7 @@ zone: # An unsigned zone - domain: unsigned.test template: unsigned + + # Forward all queries for this zone to our dummy test server + - domain: forwarded.test + template: forwarded diff --git a/test/units/testsuite-75.sh b/test/units/testsuite-75.sh index a4e2e0547bb..f1fb5d943a5 100755 --- a/test/units/testsuite-75.sh +++ b/test/units/testsuite-75.sh @@ -197,6 +197,25 @@ DNSSEC=allow-downgrade DNS=10.0.0.1 DNS=fd00:dead:beef:cafe::1 EOF +cat >/etc/systemd/network/10-dns1.netdev </etc/systemd/network/10-dns1.network < Date: Wed, 10 Jan 2024 04:56:09 +0900 Subject: [PATCH 13/13] test-resolved-dummy-server: several modernization - use sd-event, - use DEFINE_MAIN_FUNCTION(), - use log_setup(). --- src/resolve/test-resolved-dummy-server.c | 138 +++++++++++++---------- 1 file changed, 80 insertions(+), 58 deletions(-) diff --git a/src/resolve/test-resolved-dummy-server.c b/src/resolve/test-resolved-dummy-server.c index 4fc4734df77..58257d78322 100644 --- a/src/resolve/test-resolved-dummy-server.c +++ b/src/resolve/test-resolved-dummy-server.c @@ -5,6 +5,7 @@ #include "fd-util.h" #include "iovec-util.h" #include "log.h" +#include "main-func.h" #include "resolved-dns-packet.h" #include "resolved-manager.h" #include "socket-netlink.h" @@ -351,12 +352,71 @@ static int server_handle_edns_code_zero(DnsPacket *packet, DnsPacket *reply) { return reply_append_edns(packet, reply, "\xF0\x9F\x90\xB1", DNS_RCODE_SERVFAIL, DNS_EDE_RCODE_OTHER); } -int main(int argc, char *argv[]) { +static int on_dns_packet(sd_event_source *s, int fd, uint32_t revents, void *userdata) { + _cleanup_(dns_packet_unrefp) DnsPacket *packet = NULL; + _cleanup_(dns_packet_unrefp) DnsPacket *reply = NULL; + const char *name; + int r; + + assert(fd >= 0); + + r = server_recv(fd, &packet); + if (r < 0) { + log_debug_errno(r, "Failed to receive packet, ignoring: %m"); + return 0; + } + + r = dns_packet_validate_query(packet); + if (r < 0) { + log_debug_errno(r, "Invalid DNS UDP packet, ignoring."); + return 0; + } + + r = dns_packet_extract(packet); + if (r < 0) { + log_debug_errno(r, "Failed to extract DNS packet, ignoring: %m"); + return 0; + } + + name = dns_question_first_name(packet->question); + log_info("Processing question for name '%s'", name); + + (void) dns_question_dump(packet->question, stdout); + + r = make_reply_packet(packet, &reply); + if (r < 0) { + log_debug_errno(r, "Failed to make reply packet, ignoring: %m"); + return 0; + } + + if (streq_ptr(name, "edns-bogus-dnssec.forwarded.test")) + r = server_handle_edns_bogus_dnssec(packet, reply); + else if (streq_ptr(name, "edns-extra-text.forwarded.test")) + r = server_handle_edns_extra_text(packet, reply); + else if (streq_ptr(name, "edns-invalid-code.forwarded.test")) + r = server_handle_edns_invalid_code(packet, reply, NULL); + else if (streq_ptr(name, "edns-invalid-code-with-extra-text.forwarded.test")) + r = server_handle_edns_invalid_code(packet, reply, "Hello [#]$%~ World"); + else if (streq_ptr(name, "edns-code-zero.forwarded.test")) + r = server_handle_edns_code_zero(packet, reply); + else + r = log_debug_errno(SYNTHETIC_ERRNO(EFAULT), "Unhandled name '%s', ignoring.", name); + if (r < 0) + server_fail(packet, reply, DNS_RCODE_NXDOMAIN); + + r = server_ipv4_send(fd, &packet->sender.in, packet->sender_port, &packet->destination.in, reply); + if (r < 0) + log_debug_errno(r, "Failed to send reply, ignoring: %m"); + + return 0; +} + +static int run(int argc, char *argv[]) { + _cleanup_(sd_event_unrefp) sd_event *event = NULL; _cleanup_close_ int fd = -EBADF; int r; - log_parse_environment(); - log_open(); + log_setup(); if (argc != 2) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), @@ -366,63 +426,25 @@ int main(int argc, char *argv[]) { if (fd < 0) return log_error_errno(fd, "Failed to listen on address '%s': %m", argv[1]); + r = sd_event_default(&event); + if (r < 0) + return log_error_errno(r, "Failed to allocate event: %m"); + + r = sd_event_add_io(event, NULL, fd, EPOLLIN, on_dns_packet, NULL); + if (r < 0) + return log_error_errno(r, "Failed to add IO event source: %m"); + + r = sd_event_set_signal_exit(event, true); + if (r < 0) + return log_error_errno(r, "Failed to install SIGINT/SIGTERM handlers: %m"); + (void) sd_notify(/* unset_environment=false */ false, "READY=1"); - for (;;) { - _cleanup_(dns_packet_unrefp) DnsPacket *packet = NULL; - _cleanup_(dns_packet_unrefp) DnsPacket *reply = NULL; - const char *name; - - r = server_recv(fd, &packet); - if (r < 0) { - log_debug_errno(r, "Failed to receive packet, ignoring: %m"); - continue; - } - - r = dns_packet_validate_query(packet); - if (r < 0) { - log_debug_errno(r, "Invalid DNS UDP packet, ignoring."); - continue; - } - - r = dns_packet_extract(packet); - if (r < 0) { - log_debug_errno(r, "Failed to extract DNS packet, ignoring: %m"); - continue; - } - - name = dns_question_first_name(packet->question); - log_info("Processing question for name '%s'", name); - - (void) dns_question_dump(packet->question, stdout); - - r = make_reply_packet(packet, &reply); - if (r < 0) { - log_debug_errno(r, "Failed to make reply packet: %m"); - break; - } - - if (streq_ptr(name, "edns-bogus-dnssec.forwarded.test")) - r = server_handle_edns_bogus_dnssec(packet, reply); - else if (streq_ptr(name, "edns-extra-text.forwarded.test")) - r = server_handle_edns_extra_text(packet, reply); - else if (streq_ptr(name, "edns-invalid-code.forwarded.test")) - r = server_handle_edns_invalid_code(packet, reply, NULL); - else if (streq_ptr(name, "edns-invalid-code-with-extra-text.forwarded.test")) - r = server_handle_edns_invalid_code(packet, reply, "Hello [#]$%~ World"); - else if (streq_ptr(name, "edns-code-zero.forwarded.test")) - r = server_handle_edns_code_zero(packet, reply); - else - r = log_debug_errno(SYNTHETIC_ERRNO(EFAULT), "Unhandled name '%s', ignoring.", name); - - if (r < 0) - server_fail(packet, reply, DNS_RCODE_NXDOMAIN); - - r = server_ipv4_send(fd, &packet->sender.in, packet->sender_port, &packet->destination.in, reply); - if (r < 0) - log_debug_errno(r, "Failed to send reply: %m"); - - } + r = sd_event_loop(event); + if (r < 0) + return log_error_errno(r, "Failed to run event loop: %m"); return 0; } + +DEFINE_MAIN_FUNCTION(run);