Handle invalid nameservers in Resolver setup

Author: alexdalitz

lib/dnsruby/cache.rb

@@ -68,14 +68,12 @@ def add(message)
     #  This method "fixes up" the response, so that the header and ttls are OK
     #  The resolver will still need to copy the flags and ID across from the query
     def find(qname, qtype, qclass = Classes.IN)
-#      print "CACHE find : #{qname}, #{qtype}\n"
       qn = Name.create(qname)
       qn.absolute = true
       key = CacheKey.new(qn, qtype, qclass).to_s
       @mutex.synchronize {
         data = @cache[key]
         if (!data)
-#          print "CACHE lookup failed\n"
           return nil
         end
         if (data.expiration <= Time.now.to_i)

lib/dnsruby/packet_sender.rb

@@ -221,7 +221,7 @@ def initialize(*args)
               else
                 send(attr.to_s + "=", arg[attr])
               end
-           end
+            end
           rescue Exception => e
             Dnsruby.log.error { "PacketSender : Argument #{attr}, #{arg[attr]} not valid : #{e}\n" }
           end
@@ -236,7 +236,7 @@ def initialize(*args)
           rescue ArgumentError
             return
           end
-          else
+        else
             @server=Config.resolve_server(@server)
         end
 

lib/dnsruby/resolver.rb

@@ -401,7 +401,7 @@ def send_async(msg, client_queue, client_query_id = nil)
       if @single_resolvers.length == 0
         Thread.start {
           sleep(@query_timeout == 0 ? 1 : @query_timeout)
-          client_queue.push([client_query_id, nil, ResolvTimeout.new('Query timed out - no nameservers configured')])
+          client_queue.push([client_query_id, nil, ResolvTimeout.new('Query timed out - no valid nameservers configured')])
         }
       end
       client_query_id
@@ -436,7 +436,7 @@ def close
     #  * :tcp_pipelining
     #  * :tcp_pipelining_max_queries - can be a number or :infinite symbol
     def initialize(*args)
-      #  @TODO@ Should we allow :namesver to be an RRSet of NS records? Would then need to randomly order them?
+      #  @TODO@ Should we allow :nameserver to be an RRSet of NS records? Would then need to randomly order them?
       @resolver_ruby = nil
       @src_address = nil
       @src_address6 = nil
@@ -506,10 +506,16 @@ def add_config_nameservers # :nodoc: all
       end
 
       new_resolvers = threads.map(&:value).compact
+      new_resolvers = new_resolvers.select do |resolver|
+        !resolver.nil? && !resolver.server.nil?
+      end
       if new_resolvers.empty?
-        throw new ArgumentError, "No valid nameservers found in config"
+        raise ArgumentError.new("No valid nameservers found in config")
       end
       @single_res_mutex.synchronize { @single_resolvers.concat(new_resolvers) }
+      if @single_resolvers.nil? || @single_resolvers.empty?
+        raise ArgumentError.new("No valid nameservers found")
+      end
     end
 
     def set_config_nameserver(n)

lib/dnsruby/timetest.rb

@@ -0,0 +1,53 @@
+class TestDomain
+  ROOT_NAMESERVERS = %w[
+      a.root-servers.net
+      b.root-servers.net
+      c.root-servers.net
+      d.root-servers.net
+      e.root-servers.net
+      f.root-servers.net
+      g.root-servers.net
+      h.root-servers.net
+      i.root-servers.net
+      j.root-servers.net
+      k.root-servers.net
+      l.root-servers.net
+      m.root-servers.net
+    ].freeze
+
+  def name
+    "example.com"
+  end
+
+  def recursive_query(type, ns: ROOT_NAMESERVERS)
+    return [] unless ns
+
+    msg = nil
+    puts "Time: #{Benchmark.realtime { msg = resolve_query(type, ns: ns, recurse: false, cache: ROOT_NAMESERVERS.intersect?(ns)) }}"
+    return [] unless msg
+
+    # The answer is received.
+    return msg.answer.map { |rr| rr.rdata_to_string } if msg.answer
+
+    # Another layer of intermediaries.
+    authority = msg.authority.
+      find_all { |rr| rr.type == Dnsruby::Types.NS }.
+      map(&:rdata).map(&:to_s).reject(&:blank?)
+    return [] unless authority
+
+    recursive_query(type, ns: authority)
+  end
+
+  private
+
+  def resolve_query(type, ns: nil, recurse: true, cache: true)
+    puts "Caching: #{cache} | NS: #{ns.first}"
+
+    resolver = Dnsruby::Resolver.
+      new(recurse: recurse, do_caching: cache, retry_times: 2)
+
+    resolver.nameserver = [ns].flatten.shift
+
+    resolver.query(name, type)
+  end
+end

test/tc_res_config.rb

@@ -92,4 +92,33 @@ def test_dns
     end;
   end
 
+  def test_bad_ns
+    begin
+      res = Dnsruby::Resolver.new(:nameserver => ["ns32.domiancontrol.com."])
+      res.query("example.com")
+      assert(false, "Should not query with bad nameserver")
+    rescue Exception => e
+      # OK
+    end
+  end
+
+  def test_bad_ns_post_init
+    begin
+      resolver = Dnsruby::Resolver.new(recurse: false, do_caching: false, retry_times: 2)
+      resolver.nameserver = ["ns32.domiancontrol.com."]
+      assert(false, "Should not allow bad nameserver")
+      resolver.query("example.com")
+    rescue ArgumentError
+    end
+  end
+
+  def test_one_good_ns_rest_bad
+    begin
+      resolver = Dnsruby::Resolver.new(recurse: false, do_caching: false, retry_times: 2)
+      resolver.nameserver = ["ns32.domiancontrol.com.", "1.1.1.1"]
+      resolver.query("example.com")
+    rescue ArgumentError
+      assert(false, "Should allow one good nameserver")
+    end
+  end
 end