From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Sat, 6 Jan 2024 18:28:19 +0000 (+0900)
Subject: resolve: cleanup dns_packet_ede_rcode()
X-Git-Tag: v256-rc1~1205^2~11
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=71682ac6bb6579376ad4be55c93bbab4e5ad7714;p=thirdparty%2Fsystemd.git

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.
---

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)) {