From b4258f84aa1b4c15160b93e586a6939496a9f3bb Mon Sep 17 00:00:00 2001 From: Xiaochen Wang Date: Mon, 24 Jun 2024 19:01:18 +0800 Subject: [PATCH] fix(dns): disable additional_section option for r:qeury by default --- ...ignore-records-with-non-matching-types.yml | 2 +- kong/resty/dns/client.lua | 7 +- spec/01-unit/21-dns-client/02-client_spec.lua | 103 ++++++++++++++++++ 3 files changed, 106 insertions(+), 6 deletions(-) diff --git a/changelog/unreleased/kong/fix-dns-ignore-records-with-non-matching-types.yml b/changelog/unreleased/kong/fix-dns-ignore-records-with-non-matching-types.yml index 1cfb60d3dc64..134e2c43ddd4 100644 --- a/changelog/unreleased/kong/fix-dns-ignore-records-with-non-matching-types.yml +++ b/changelog/unreleased/kong/fix-dns-ignore-records-with-non-matching-types.yml @@ -1,3 +1,3 @@ -message: "**DNS Client**: Ignore records with RR types differs from that of the query when parsing answers." +message: "**DNS Client**: Fixed an issue where the Kong DNS client stored records with non-matching domain and type when parsing answers. It now ignores records when the RR type differs from that of the query when parsing answers and disables the `ADDITIONAL SECTION` in DNS responses." type: bugfix scope: Core diff --git a/kong/resty/dns/client.lua b/kong/resty/dns/client.lua index 874515badeb1..6a126727a399 100644 --- a/kong/resty/dns/client.lua +++ b/kong/resty/dns/client.lua @@ -1125,7 +1125,6 @@ end -- @param qname Name to resolve -- @param r_opts Options table, see remark about the `qtype` field above and -- [OpenResty docs](https://github.com/openresty/lua-resty-dns) for more options. --- The field `additional_section` will default to `true` instead of `false`. -- @param dnsCacheOnly Only check the cache, won't do server lookups -- @param try_list (optional) list of tries to add to -- @param force_no_sync force noSynchronisation @@ -1140,10 +1139,8 @@ local function resolve(qname, r_opts, dnsCacheOnly, try_list, force_no_sync) for k,v in pairs(r_opts) do opts[k] = v end -- copy the options table end - -- default the ADDITIONAL SECTION to TRUE - if opts.additional_section == nil then - opts.additional_section = true - end + -- avoid parsing ip addresses that do not match the qname + opts.additional_section = nil -- first check for shortname in the cache -- we do this only to prevent iterating over the SEARCH directive and diff --git a/spec/01-unit/21-dns-client/02-client_spec.lua b/spec/01-unit/21-dns-client/02-client_spec.lua index 622d59f07617..39984c189c03 100644 --- a/spec/01-unit/21-dns-client/02-client_spec.lua +++ b/spec/01-unit/21-dns-client/02-client_spec.lua @@ -1931,4 +1931,107 @@ describe("[DNS client]", function() assert.equal(call_count, 10) end) + it("disable additional section when querying", function() + + local function build_dns_reply(id, name, ip, ns_ip1, ns_ip2) + local function dns_encode_name(name) + local parts = {} + for part in string.gmatch(name, "[^.]+") do + table.insert(parts, string.char(#part) .. part) + end + table.insert(parts, "\0") + return table.concat(parts) + end + + local function ip_to_bytes(ip) + local bytes = { "\x00\x04" } -- RDLENGTH:4bytes (ipv4) + for octet in string.gmatch(ip, "%d+") do + table.insert(bytes, string.char(tonumber(octet))) + end + return table.concat(bytes) + end + + local package = {} + + -- Header + package[#package+1] = id + package[#package+1] = "\x85\x00" -- QR, AA, RD + package[#package+1] = "\x00\x01\x00\x01\x00\x00\x00\x02" -- QD:1 AN:1 NS:0 AR:2 + + -- Question + package[#package+1] = dns_encode_name(name) + package[#package+1] = "\x00\x01\x00\x01" -- QTYPE A; QCLASS IN + + -- Answer + package[#package+1] = dns_encode_name(name) + package[#package+1] = "\x00\x01\x00\x01\x00\x00\x00\x30" -- QTYPE:A; QCLASS:IN TTL:48 + package[#package+1] = ip_to_bytes(ip) + + -- Additional + local function add_additional(name, ip) + package[#package+1] = dns_encode_name(name) + package[#package+1] = "\x00\x01\x00\x01\x00\x00\x00\x30" -- QTYPE:A; QCLASS:IN TTL:48 + package[#package+1] = ip_to_bytes(ip) + end + + add_additional("ns1." .. name, ns_ip1) + add_additional("ns2." .. name, ns_ip2) + + return table.concat(package) + end + + local force_enable_additional_section = false + + -- dns client will ignore additional section + query_func = function(self, original_query_func, name, options) + if options.qtype ~= client.TYPE_A then + return { errcode = 5, errstr = "refused" } + end + + if force_enable_additional_section then + options.additional_section = true + end + + self.tcp_sock = nil -- disable TCP query + + local id + local sock = assert(self.socks[1]) + -- hook send to get id + local orig_sock_send = sock.send + sock.send = function (self, query) + id = query[1] .. query[2] + return orig_sock_send(self, query) + end + -- hook receive to reply raw data + sock.receive = function (self, size) + return build_dns_reply(id, name, "1.1.1.1", "2.2.2.2", "3.3.3.3") + end + + return original_query_func(self, name, options) + end + + local name = "additional-section.test" + + -- no additional_section by default + assert(client.init({ search = {}, })) + local answers = client.resolve(name) + assert.equal(#answers, 1) + assert.same(answers[1].address, "1.1.1.1") + + -- disable the additional_section option in r_opt + assert(client.init({ search = {}, })) + local answers = client.resolve(name, { additional_section = true }) + assert.equal(#answers, 1) + assert.same(answers[1].address, "1.1.1.1") + + -- test the buggy scenario + force_enable_additional_section = true + assert(client.init({ search = {}, })) + local answers = client.resolve(name) + assert.equal(#answers, 3) + assert.same(answers[1].address, "1.1.1.1") + assert.same(answers[2].address, "2.2.2.2") + assert.same(answers[3].address, "3.3.3.3") + end) + end)