From 736ef6b0862c0a7347cc756d80ffbbb802713496 Mon Sep 17 00:00:00 2001 From: Thijs Schreijer Date: Wed, 10 May 2017 17:05:03 +0200 Subject: [PATCH] no longer lookup ip addresses on the dns server was only cought if the query type was A or AAAA, now it will fail right away for any other type. --- README.md | 4 ++ spec/client_spec.lua | 52 +++++++++++++-- src/resty/dns/client.lua | 133 ++++++++++++++++++++++++++++----------- 3 files changed, 146 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index 2ce903d..9fb6aee 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,10 @@ use the `rbusted` script. History ======= +### 0.5.x (xx-xxx-2017) Bugfixes + +- Fix: no longer lookup ip adresses as names if the query type is not A or AAAA + ### 0.5.0 (25-Apr-2017) implement SEARCH and NDOTS - Removed: BREAKING! stdError function removed. diff --git a/spec/client_spec.lua b/spec/client_spec.lua index da4b55d..c4bed14 100644 --- a/spec/client_spec.lua +++ b/spec/client_spec.lua @@ -27,7 +27,7 @@ describe("DNS client", function() local client before_each(function() - _G._TEST = true + _G._TEST = true client = require("resty.dns.client") end) @@ -493,28 +493,70 @@ describe("DNS client", function() assert.equal(NOT_FOUND_ERROR, err) end) - it("fetching IPv4 address", function() + it("fetching IPv4 address as A type", function() assert(client.init()) local host = "1.2.3.4" - local answers = assert(client.resolve(host)) + local answers = assert(client.resolve(host, { qtype = client.TYPE_A })) assert.are.equal(#answers, 1) assert.are.equal(client.TYPE_A, answers[1].type) assert.are.equal(10*365*24*60*60, answers[1].ttl) -- 10 year ttl end) - it("fetching IPv6 address", function() + it("fetching IPv4 address as SRV type", function() + assert(client.init()) + + -- do a query so we get a resolver object to spy on + local _, _, r, history = client.toip("google.com", 123, false) + local o_query = r.query + r.query = function(self, ...) + print(require("pl.pretty").write({...})) + return o_query(self, ...) + end + + spy.on(r, "query") + + local res, err, r, history = client.resolve( + "1.2.3.4", + { qtype = client.TYPE_SRV }, + false, r) + assert.spy(r.query).was_not.called() + assert.equal(NOT_FOUND_ERROR, err) + end) + + it("fetching IPv6 address as AAAA type", function() assert(client.init()) local host = "1:2::3:4" - local answers = assert(client.resolve(host)) + local answers = assert(client.resolve(host, { qtype = client.TYPE_AAAA })) assert.are.equal(#answers, 1) assert.are.equal(client.TYPE_AAAA, answers[1].type) assert.are.equal(10*365*24*60*60, answers[1].ttl) -- 10 year ttl end) + it("fetching IPv6 address as SRV type", function() + assert(client.init()) + + -- do a query so we get a resolver object to spy on + local _, _, r, history = client.toip("google.com", 123, false) + local o_query = r.query + r.query = function(self, ...) + print(require("pl.pretty").write({...})) + return o_query(self, ...) + end + + spy.on(r, "query") + + local res, err, r, history = client.resolve( + "1:2::3:4", + { qtype = client.TYPE_SRV }, + false, r) + assert.spy(r.query).was_not.called() + assert.equal(NOT_FOUND_ERROR, err) + end) + it("fetching invalid IPv6 address", function() assert(client.init({ resolvConf = { diff --git a/src/resty/dns/client.lua b/src/resty/dns/client.lua index 964bb5f..f1532a1 100644 --- a/src/resty/dns/client.lua +++ b/src/resty/dns/client.lua @@ -25,7 +25,7 @@ local semaphore = require("ngx.semaphore").new local resolver = require("resty.dns.resolver") local time = ngx.now -local log = ngx.log +local ngx_log = ngx.log local log_WARN = ngx.WARN local math_min = math.min @@ -60,6 +60,10 @@ end -- insert our own special value for "last success" _M.TYPE_LAST = -1 +local function log(level, ...) + return ngx_log(level, "[dns-client] ", ...) +end + -- ============================================== -- In memory DNS cache -- ============================================== @@ -360,15 +364,15 @@ _M.init = function(options) if #(options.nameservers or {}) == 0 and resolv.nameserver then options.nameservers = {} -- some systems support port numbers in nameserver entries, so must parse those - for i, address in ipairs(resolv.nameserver) do + for _, address in ipairs(resolv.nameserver) do local ip, port, t = utils.parseHostname(address) if t == "ipv6" and not options.enable_ipv6 then -- should not add this one else if port then - options.nameservers[#options.nameservers+1] = { ip, port } + options.nameservers[#options.nameservers + 1] = { ip, port } else - options.nameservers[#options.nameservers+1] = ip + options.nameservers[#options.nameservers + 1] = ip end end end @@ -526,7 +530,17 @@ local function try_status(self, status) return self end -local function check_ipv6(qname, r, try_list) +local function check_ipv6(qname, qtype, r, try_list) + try_list = try_add(try_list, qname, qtype, "IPv6") + + -- check cache and always use "cacheonly" to not alter it as IP addresses are + -- long lived in the cache anyway + local record = cachelookup(qname, qtype, true) + if record then + try_status(try_list, "cached") + return record, nil, r, try_list + end + local check = qname if check:sub(1,1) == ":" then check = "0"..check end if check:sub(-1,-1) == ":" then check = check.."0" end @@ -536,40 +550,59 @@ local function check_ipv6(qname, r, try_list) local ins = ":"..string.rep("0:", 8 - count) check = check:gsub("::", ins, 1) -- replace only 1 occurence! end - local record - if not check:match("^%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?$") then - -- not a valid IPv6 address - -- return a "server error" as a bad IPv4 would be looked up on - -- the server and return a server error as well, for consistency. + if qtype == _M.TYPE_AAAA and + check:match("^%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?:%x%x?%x?%x?$") then + try_status(try_list, "validated") + record = {{ + address = qname, + type = _M.TYPE_AAAA, + class = 1, + name = qname, + ttl = 10 * 365 * 24 * 60 * 60 -- TTL = 10 years + }} + else + -- not a valid IPv6 address, or a bad type (non ipv6) + -- return a "server error" try_status(try_list, "bad IPv6") record = { errcode = 3, errstr = "name error", } - else - try_status(try_list, "IPv6") + end + cacheinsert(record, qname, qtype) + return record, nil, r, try_list +end + +local function check_ipv4(qname, qtype, r, try_list) + try_list = try_add(try_list, qname, qtype, "IPv4") + + -- check cache and always use "cacheonly" to not alter it as IP addresses are + -- long lived in the cache anyway + local record = cachelookup(qname, qtype, true) + if record then + try_status(try_list, "cached") + return record, nil, r, try_list + end + + if qtype == _M.TYPE_A then + try_status(try_list, "validated") record = {{ address = qname, - type = _M.TYPE_AAAA, + type = _M.TYPE_A, class = 1, name = qname, ttl = 10 * 365 * 24 * 60 * 60 -- TTL = 10 years }} + else + -- bad query type for this ipv4 address + -- return a "server error" + try_status(try_list, "bad IPv4") + record = { + errcode = 3, + errstr = "name error", + } end - cacheinsert(record, qname, _M.TYPE_AAAA) - return record, nil, r, try_list -end - -local function check_ipv4(qname, r, try_list) - try_status(try_list, "IPv4") - local record = {{ - address = qname, - type = _M.TYPE_A, - class = 1, - name = qname, - ttl = 10 * 365 * 24 * 60 * 60 -- TTL = 10 years - }} - cacheinsert(record, qname, _M.TYPE_A) + cacheinsert(record, qname, qtype) return record, nil, r, try_list end @@ -586,17 +619,6 @@ local function lookup(qname, r_opts, dnsCacheOnly, r, try_list) return record, nil, r, try_list end try_list = try_add(try_list, qname, qtype) - if not expect_ttl_0 then -- so no record, and no expected ttl, so not seen - -- this one recently, this is the only time we do the expensive checks - -- for ip addresses, as they will be inserted with a ttl of 10years and - -- hence never hit this code branch again - if (qtype == _M.TYPE_AAAA) and qname:find(":") then - return check_ipv6(qname, r, try_list) -- IPv6 or invalid - end - if (qtype == _M.TYPE_A) and qname:match("^%d%d?%d?%.%d%d?%d?%.%d%d?%d?%.%d%d?%d?$") then - return check_ipv4(qname, r, try_list) -- IPv4 address - end - end if dnsCacheOnly then -- no active lookups allowed, so return error -- NOTE: this error response should never be cached, because it is caused @@ -611,6 +633,10 @@ local function lookup(qname, r_opts, dnsCacheOnly, r, try_list) -- not found in our cache, so perform query on dns servers local answers, err answers, err, r = synchronizedQuery(qname, r_opts, r, expect_ttl_0) +--print("============================================================") +--print("Lookup: ",qname,":",r_opts.qtype) +--print("Error : ", tostring(err)) +--print(require("pl.pretty").write(answers or {})) if not answers then try_status(try_list, tostring(err)) return answers, err, r, try_list @@ -623,6 +649,7 @@ local function lookup(qname, r_opts, dnsCacheOnly, r, try_list) for i = #answers, 1, -1 do -- we're deleting entries, so reverse the traversal local answer = answers[i] if (answer.type ~= qtype) or (answer.name ~= qname) then +--print("removing: ",answer.type,":",answer.name, " (name or type mismatch)") local key = answer.type..":"..answer.name local lst = others[key] if not lst then @@ -649,6 +676,9 @@ local function lookup(qname, r_opts, dnsCacheOnly, r, try_list) -- now insert actual target record in cache try_status(try_list, "queried") cacheinsert(answers, qname, qtype) +--print("------------------------------------------------------------") +--print(require("pl.pretty").write(answers or {})) +--print("============================================================") return answers, nil, r, try_list end @@ -764,6 +794,27 @@ local function resolve(qname, r_opts, dnsCacheOnly, r, try_list) for k,v in pairs(r_opts) do opts[k] = v end -- copy the options table end + -- check for qname being an ip address + local name_type = utils.hostnameType(qname) + if name_type ~= "name" then + if name_type == "ipv4" then + -- if no qtype is given, we're supposed to search, and hence we add TYPE_A as type + records, err, r, try_list = check_ipv4(qname, qtype or _M.TYPE_A, r, try_list) + else + -- must be 'ipv6' + -- if no qtype is given, we're supposed to search, and hence we add TYPE_AAAA as type + records, err, r, try_list = check_ipv6(qname, qtype or _M.TYPE_AAAA, r, try_list) + end + if records.errcode then + -- the query type didn't match the ip address, or a bad ip address + return nil, + ("dns server error: %s %s"):format(records.errcode, records.errstr), + r, try_list + end + -- valid ip + return records, nil, r, try_list + end + -- go try a sequence of record types for try_name, try_type in search_iter(qname, qtype) do @@ -997,6 +1048,12 @@ end -- - `127.0.0.1, 80` -- - `127.0.0.1, 80` (completes WRR 1st level, 2nd run, with different order as WRR is randomized) -- +-- __Debugging__: +-- +-- This function both takes and returns a `try_list`. This is an internal object +-- representing the entire resolution history for a call. To prevent unnecessary +-- string concatenations on a hot code path, it is not logged in this module. +-- If you need to log it, just log `tostring(try_list)` from the caller code. -- @function toip -- @param qname hostname to resolve -- @param port (optional) default port number to return if none was found in