From 7d524df80eea6f335995af85d4de1fa50b944759 Mon Sep 17 00:00:00 2001 From: Igor Pstyga Date: Tue, 23 Apr 2024 21:04:52 -0700 Subject: [PATCH 1/2] Fix TCP fallback with multiple nameservers Under the following conditions the exception `Resolv::DNS::Requester::RequestError: host/port don't match` is raised: - Multiple nameservers are configured for Resolv::DNS - A nameserver falls back from UDP to TCP - TCP request hits Resolv::DNS timeout - Resolv::DNS retries the next nameserver More details here https://bugs.ruby-lang.org/issues/8285 Co-authored-by: Julian Mehnle --- lib/resolv.rb | 47 ++++++++++------- test/resolv/test_dns.rb | 113 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 20 deletions(-) diff --git a/lib/resolv.rb b/lib/resolv.rb index e36dbce..2aee310 100644 --- a/lib/resolv.rb +++ b/lib/resolv.rb @@ -513,22 +513,32 @@ def each_resource(name, typeclass, &proc) def fetch_resource(name, typeclass) lazy_initialize - begin - requester = make_udp_requester - rescue Errno::EACCES - # fall back to TCP - end + protocols = {} + requesters = {} senders = {} begin - @config.resolv(name) {|candidate, tout, nameserver, port| - requester ||= make_tcp_requester(nameserver, port) + @config.resolv(name) do |candidate, tout, nameserver, port| msg = Message.new msg.rd = 1 msg.add_question(candidate, typeclass) - unless sender = senders[[candidate, nameserver, port]] + + protocol = protocols[candidate] ||= :udp + requester = requesters[[protocol, nameserver]] ||= + case protocol + when :udp + begin + make_udp_requester + rescue Errno::EACCES + make_tcp_requester(nameserver, port) + end + when :tcp + make_tcp_requester(nameserver, port) + end + + unless sender = senders[[candidate, requester, nameserver, port]] sender = requester.sender(msg, candidate, nameserver, port) next if !sender - senders[[candidate, nameserver, port]] = sender + senders[[candidate, requester, nameserver, port]] = sender end reply, reply_name = requester.request(sender, tout) case reply.rcode @@ -536,12 +546,7 @@ def fetch_resource(name, typeclass) if reply.tc == 1 and not Requester::TCP === requester requester.close # Retry via TCP: - requester = make_tcp_requester(nameserver, port) - senders = {} - # This will use TCP for all remaining candidates (assuming the - # current candidate does not already respond successfully via - # TCP). This makes sense because we already know the full - # response will not fit in an untruncated UDP packet. + protocols[candidate] = :tcp redo else yield(reply, reply_name) @@ -552,9 +557,9 @@ def fetch_resource(name, typeclass) else raise Config::OtherResolvError.new(reply_name.to_s) end - } + end ensure - requester&.close + requesters.each_value { |requester| requester&.close } end end @@ -569,6 +574,11 @@ def make_udp_requester # :nodoc: def make_tcp_requester(host, port) # :nodoc: return Requester::TCP.new(host, port) + rescue Errno::ECONNREFUSED + # Treat a refused TCP connection attempt to a nameserver like a timeout, + # as Resolv::DNS::Config#resolv considers ResolvTimeout exceptions as a + # hint to try the next nameserver: + raise ResolvTimeout end def extract_resources(msg, name, typeclass) # :nodoc: @@ -1800,7 +1810,6 @@ def canonical_key(key) # :nodoc: end end - ## # Base class for SvcParam. [RFC9460] @@ -2499,7 +2508,6 @@ def initialize(version, ssize, hprecision, vprecision, latitude, longitude, alti attr_reader :altitude - def encode_rdata(msg) # :nodoc: msg.put_bytes(@version) msg.put_bytes(@ssize.scalar) @@ -3439,4 +3447,3 @@ def DefaultResolver.replace_resolvers new_resolvers AddressRegex = /(?:#{IPv4::Regex})|(?:#{IPv6::Regex})/ end - diff --git a/test/resolv/test_dns.rb b/test/resolv/test_dns.rb index 40c5406..9064e7a 100644 --- a/test/resolv/test_dns.rb +++ b/test/resolv/test_dns.rb @@ -698,4 +698,117 @@ def test_unreachable_server ensure sock&.close end + + def test_multiple_servers_with_timeout_and_truncated_tcp_fallback + begin + OpenSSL + rescue LoadError + skip 'autoload problem. see [ruby-dev:45021][Bug #5786]' + end if defined?(OpenSSL) + + num_records = 50 + + with_udp_and_tcp('127.0.0.1', 0) do |u1, t1| + with_tcp('0.0.0.0', 0) do |t2| + _, server1_port, _, server1_address = u1.addr + _, server2_port, _, server2_address = t2.addr + + client_thread = Thread.new do + Resolv::DNS.open(nameserver_port: [[server1_address, server1_port], [server2_address, server2_port]]) do |dns| + dns.timeouts = [0.1, 0.2] + dns.getresources('foo.example.org', Resolv::DNS::Resource::IN::A) + end + end + + udp_server1_thread = Thread.new do + msg, (_, client_port, _, client_address) = Timeout.timeout(5) { u1.recvfrom(4096) } + id, word2, _qdcount, _ancount, _nscount, _arcount = msg.unpack('nnnnnn') + opcode = (word2 & 0x7800) >> 11 + rd = (word2 & 0x0100) >> 8 + name = [3, 'foo', 7, 'example', 3, 'org', 0].pack('Ca*Ca*Ca*C') + qr = 1 + aa = 0 + tc = 1 + ra = 1 + z = 0 + rcode = 0 + qdcount = 0 + ancount = num_records + nscount = 0 + arcount = 0 + word2 = (qr << 15) | + (opcode << 11) | + (aa << 10) | + (tc << 9) | + (rd << 8) | + (ra << 7) | + (z << 4) | + rcode + msg = [id, word2, qdcount, ancount, nscount, arcount].pack('nnnnnn') + type = 1 + klass = 1 + ttl = 3600 + rdlength = 4 + num_records.times do |i| + rdata = [192, 0, 2, i].pack('CCCC') # 192.0.2.x (TEST-NET address) RFC 3330 + rr = [name, type, klass, ttl, rdlength, rdata].pack('a*nnNna*') + msg << rr + end + u1.send(msg[0...512], 0, client_address, client_port) + end + + tcp_server1_thread = Thread.new { t1.accept } + + tcp_server2_thread = Thread.new do + ct = t2.accept + msg = ct.recv(512) + msg.slice!(0..1) # Size (only for TCP) + id, word2, _qdcount, _ancount, _nscount, _arcount = msg.unpack('nnnnnn') + rd = (word2 & 0x0100) >> 8 + opcode = (word2 & 0x7800) >> 11 + name = [3, 'foo', 7, 'example', 3, 'org', 0].pack('Ca*Ca*Ca*C') + qr = 1 + aa = 0 + tc = 0 + ra = 1 + z = 0 + rcode = 0 + qdcount = 0 + ancount = num_records + nscount = 0 + arcount = 0 + word2 = (qr << 15) | + (opcode << 11) | + (aa << 10) | + (tc << 9) | + (rd << 8) | + (ra << 7) | + (z << 4) | + rcode + msg = [id, word2, qdcount, ancount, nscount, arcount].pack('nnnnnn') + type = 1 + klass = 1 + ttl = 3600 + rdlength = 4 + num_records.times do |i| + rdata = [192, 0, 2, i].pack('CCCC') # 192.0.2.x (TEST-NET address) RFC 3330 + rr = [name, type, klass, ttl, rdlength, rdata].pack('a*nnNna*') + msg << rr + end + msg = "#{[msg.bytesize].pack('n')}#{msg}" # Prefix with size + ct.send(msg, 0) + ct.close + end + result, = assert_join_threads([client_thread, udp_server1_thread, tcp_server1_thread, tcp_server2_thread]) + assert_instance_of(Array, result) + assert_equal(50, result.length) + result.each_with_index do |rr, i| + assert_instance_of(Resolv::DNS::Resource::IN::A, rr) + assert_instance_of(Resolv::IPv4, rr.address) + assert_equal("192.0.2.#{i}", rr.address.to_s) + assert_equal(3600, rr.ttl) + end + end + end + end end From 9bf1b08f5c531a5c4ec4eb557efa1a6b77b1eaf4 Mon Sep 17 00:00:00 2001 From: Kasumi Hanazuki Date: Sat, 1 Jun 2024 10:07:46 +0000 Subject: [PATCH 2/2] Reuse open TCP connection [RFC7766] Section 5 recommends stub resolvers to reuse open TCP connections to a nameserver. [RFC7766]: https://datatracker.ietf.org/doc/html/rfc7766 --- lib/resolv.rb | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/resolv.rb b/lib/resolv.rb index 2aee310..0574ce6 100644 --- a/lib/resolv.rb +++ b/lib/resolv.rb @@ -513,27 +513,28 @@ def each_resource(name, typeclass, &proc) def fetch_resource(name, typeclass) lazy_initialize - protocols = {} + truncated = {} requesters = {} + udp_requester = begin + make_udp_requester + rescue Errno::EACCES + # fall back to TCP + end senders = {} + begin @config.resolv(name) do |candidate, tout, nameserver, port| msg = Message.new msg.rd = 1 msg.add_question(candidate, typeclass) - protocol = protocols[candidate] ||= :udp - requester = requesters[[protocol, nameserver]] ||= - case protocol - when :udp - begin - make_udp_requester - rescue Errno::EACCES - make_tcp_requester(nameserver, port) - end - when :tcp - make_tcp_requester(nameserver, port) + requester = requesters.fetch([nameserver, port]) do + if !truncated[candidate] && udp_requester + udp_requester + else + requesters[[nameserver, port]] = make_tcp_requester(nameserver, port) end + end unless sender = senders[[candidate, requester, nameserver, port]] sender = requester.sender(msg, candidate, nameserver, port) @@ -544,9 +545,8 @@ def fetch_resource(name, typeclass) case reply.rcode when RCode::NoError if reply.tc == 1 and not Requester::TCP === requester - requester.close # Retry via TCP: - protocols[candidate] = :tcp + truncated[candidate] = true redo else yield(reply, reply_name) @@ -559,6 +559,7 @@ def fetch_resource(name, typeclass) end end ensure + udp_requester&.close requesters.each_value { |requester| requester&.close } end end