Fixes #13536 - working ptr's for dnscmd

modules/dns_common/dns_common.rb

@@ -1,3 +1,6 @@
+require 'resolv'
+require 'ipaddr'
+
 module Proxy::Dns
   class Error < RuntimeError; end
   class NotFound < RuntimeError; end
@@ -24,5 +27,43 @@ def dns_find key
     rescue Resolv::ResolvError
       false
     end
+
+    def ptr_to_ip ptr
+     if ptr =~ /\.in-addr\.arpa$/
+       ptr.split('.')[0..-3].reverse.join('.')
+     elsif ptr =~ /\.ip6\.arpa$/
+       ptr.split('.')[0..-3].reverse.each_slice(4).inject([]) {|address, word| address << word.join}.join(":")
+     else
+       raise Proxy::Dns::Error.new("Not a PTR record: '#{ptr}'")
+     end
+    end
+
+    # conflict methods return values:
+    # no conflict: -1; conflict: 1, conflict but record / ip matches: 0
+    def a_record_conflicts(fqdn, ip)
+      if ip_addr = to_ipaddress(ip)
+        addresses = resolver.getaddresses(fqdn)
+        return -1 if addresses.empty?
+        return 0 if addresses.any? {|a| IPAddr.new(a.to_s) == ip_addr}
+        1
+      else
+        raise Proxy::Dns::Error.new("Not an IP Address: '#{ip}'")
+      end
+    end
+
+    def ptr_record_conflicts(fqdn, ip)
+      if ip_addr = to_ipaddress(ip)
+        names = resolver.getnames(ip_addr.to_s)
+        return -1 if names.empty?
+        return 0 if names.any? {|n| n.to_s.casecmp(fqdn) == 0}
+        1
+      else
+        raise Proxy::Dns::Error.new("Not an IP Address: '#{ip}'")
+      end
+    end
+
+    def to_ipaddress ip
+      IPAddr.new(ip) rescue false
+    end
   end
 end

modules/dns_dnscmd/dns_dnscmd_main.rb

@@ -1,4 +1,3 @@
-require 'resolv'
 require 'open3'
 require 'dns_common/dns_common'
 
@@ -13,36 +12,49 @@ def initialize(a_server = nil, a_ttl = nil)
     end
 
     def create_a_record(fqdn, ip)
-      if found = dns_find(fqdn)
-        raise(Proxy::Dns::Collision, "#{fqdn} is already used by #{found}") if found != ip
-      else
-        zone = match_zone(fqdn)
-        msg = "Added DNS entry #{fqdn} => #{ip}"
-        cmd = "/RecordAdd #{zone} #{fqdn}. A #{ip}"
-        execute(cmd, msg)
+      case a_record_conflicts(fqdn, ip) #returns -1, 0, 1
+        when 1 then
+          raise(Proxy::Dns::Collision, "'#{fqdn} 'is already in use")
+        when 0 then
+          return nil
+        else
+          zone = match_zone(fqdn, enum_zones)
+          msg = "Added DNS entry #{fqdn} => #{ip}"
+          cmd = "/RecordAdd #{zone} #{fqdn}. A #{ip}"
+          execute(cmd, msg)
+          nil
       end
     end
 
-    # noop
-    def create_ptr_record(fqdn, ip)
-      found = dns_find(ip)
-      raise(Proxy::Dns::Collision, "#{ip} is already used by #{found}") if found && found != fqdn
-      true
+    def create_ptr_record(fqdn, ptr)
+      case ptr_record_conflicts(fqdn, ptr_to_ip(ptr)) #returns -1, 0, 1
+        when 1 then
+          raise(Proxy::Dns::Collision, "'#{ptr}' is already in use")
+        when 0
+          return nil
+        else
+          zone = match_zone(ptr, enum_zones)
+          msg = "Added PTR entry #{ptr} => #{fqdn}"
+          cmd = "/RecordAdd #{zone} #{ptr}. PTR #{fqdn}."
+          execute(cmd, msg)
+          nil
+      end
     end
 
     def remove_a_record(fqdn)
-      ip = dns_find(fqdn)
-      raise Proxy::Dns::NotFound.new("Cannot find DNS entry for #{fqdn}") unless ip
-      zone = match_zone(fqdn)
-      msg = "Removed DNS entry #{fqdn} => #{ip}"
+      zone = match_zone(fqdn, enum_zones)
+      msg = "Removed DNS entry #{fqdn}"
       cmd = "/RecordDelete #{zone} #{fqdn}. A /f"
       execute(cmd, msg)
+      nil
     end
 
-    # noop
-    def remove_ptr_record(ip)
-      raise Proxy::Dns::NotFound.new("Cannot find DNS entry for #{ip}") unless dns_find(ip)
-      true
+    def remove_ptr_record(ptr)
+      zone = match_zone(ptr, enum_zones)
+      msg = "Removed PTR entry #{ptr}"
+      cmd = "/RecordDelete #{zone} #{ptr}. PTR /f"
+      execute(cmd, msg)
+      nil
     end
 
     def execute cmd, msg=nil, error_only=false
@@ -87,15 +99,13 @@ def report msg, response, error_only
       raise Proxy::Dns::Error.new("Unknown error while processing '#{msg}'")
     end
 
-    def match_zone(fqdn)
-      dns_zones = enum_zones
+    def match_zone(record, zone_list)
       weight = 0 # sub zones might be independent from similar named parent zones; use weight for longest suffix match
       matched_zone = nil
-
-      dns_zones.each do |zone|
+      zone_list.each do |zone|
         zone_labels = zone.split(".").reverse
         zone_weight = zone_labels.length
-        fqdn_labels = fqdn.split(".")
+        fqdn_labels = record.split(".")
         fqdn_labels.shift
         is_match = zone_labels.all? { |zone_label| zone_label == fqdn_labels.pop }
         # match only the longest zone suffix
@@ -104,18 +114,18 @@ def match_zone(fqdn)
           weight = zone_weight
         end
       end
-      raise Proxy::Dns::NotFound.new("The DNS server has no authoritative zone for #{fqdn}") unless matched_zone
+      raise Proxy::Dns::NotFound.new("The DNS server has no authoritative zone for #{record}") unless matched_zone
       matched_zone
     end
 
     def enum_zones
       zones = []
-      response = execute('/EnumZones')
+      response = execute '/EnumZones', nil, true
       response.each do |line|
         next unless line =~  / Primary /
         zones << line.sub(/^ +/, '').sub(/ +.*$/, '').chomp("\n")
       end
-      logger.debug "Enumerated dns zones: #{zones}"
+      logger.debug "Enumerated authoritative dns zones: #{zones}"
       zones
     end
   end

test/dns_common/record_test.rb

@@ -16,4 +16,54 @@ def test_dns_find_key_not_found
     Resolv::DNS.any_instance.expects(:getaddress).with('another.host').raises(Resolv::ResolvError.new('DNS result has no information'))
     assert !Proxy::Dns::Record.new.dns_find('another.host')
   end
+
+  def test_ptr_to_ip_ipv4
+    assert_equal('192.168.33.30', Proxy::Dns::Record.new.ptr_to_ip('30.33.168.192.in-addr.arpa'))
+  end
+
+  def test_ptr_to_ip_ipv6
+    assert_equal('2001:0db8:deef:0000:0000:0000:0000:0001', Proxy::Dns::Record.new.ptr_to_ip('1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.f.e.e.d.8.b.d.0.1.0.0.2.ip6.arpa'))
+  end
+
+  def test_ptr_to_ip_without_record_exception
+    assert_raise Proxy::Dns::Error do
+      Proxy::Dns::Record.new.ptr_to_ip('host.example.com')
+    end
+  end
+
+  def test_a_record_conflicts_no_conflict
+    Resolv::DNS.any_instance.expects(:getaddresses).with('some.host').returns([])
+    assert_equal -1, Proxy::Dns::Record.new.a_record_conflicts('some.host', '192.168.33.33')
+  end
+
+  def test_a_record_conflicts_has_conflict
+    Resolv::DNS.any_instance.expects(:getaddresses).with('some.host').returns(['192.168.33.33', '2001:DB8:DEEF::1'])
+    assert_equal 1, Proxy::Dns::Record.new.a_record_conflicts('some.host', '2001:db8:deef:1000::1')
+  end
+
+  def test_a_record_conflicts_but_nothing_todo
+    Resolv::DNS.any_instance.expects(:getaddresses).with('some.host').returns(['192.168.33.33', '2001:DB8:DEEF::1'])
+    assert_equal 0, Proxy::Dns::Record.new.a_record_conflicts('some.host', '192.168.33.33')
+  end
+
+  def test_ptr_record_conflicts_no_conflict
+    Resolv::DNS.any_instance.expects(:getnames).with('192.168.33.33').returns([])
+    assert_equal -1, Proxy::Dns::Record.new.ptr_record_conflicts('some.host', '192.168.33.33')
+  end
+
+  def test_ptr_record_conflicts_has_conflict
+    Resolv::DNS.any_instance.expects(:getnames).with('2001:db8:deef::1').returns(['some.host'])
+    assert_equal 1, Proxy::Dns::Record.new.ptr_record_conflicts('another.host', '2001:db8:deef::1')
+  end
+
+  def test_ptr_record_conflicts_but_nothing_todo
+    Resolv::DNS.any_instance.expects(:getnames).with('192.168.33.33').returns(['some.host'])
+    assert_equal 0, Proxy::Dns::Record.new.ptr_record_conflicts('some.host', '192.168.33.33')
+  end
+
+  def test_validate_ip
+    assert_equal '192.168.33.33', Proxy::Dns::Record.new.to_ipaddress('192.168.33.33').to_s
+    assert_equal '2001:db8:deef::1', Proxy::Dns::Record.new.to_ipaddress('2001:db8:deef::1').to_s
+    assert_equal false, Proxy::Dns::Record.new.to_ipaddress('some.host')
+  end
 end

test/dns_dnscmd/dnscmd_test.rb

@@ -14,7 +14,7 @@ def setup
     @server = DnscmdForTesting.new(["_msdcs.bar.domain.local",
                                     "168.192.in-addr.arpa",
                                     "33.168.192.in-addr.arpa",
-                                    "e.e.d.8.b.d.0.1.0.0.2.ip6.arpa",
+                                    "f.e.e.d.8.b.d.0.1.0.0.2.ip6.arpa",
                                     "bar.domain.local",
                                     "domain.local",
                                     "TrustAnchors"])
@@ -30,79 +30,68 @@ def test_dnscmd_provider_initialization
   end
 
   def test_create_address_record_with_longest_zone_match
-    Proxy::Dns::Dnscmd::Record.any_instance.expects(:dns_find).with('host.foo.bar.domain.local').returns(false)
+    Proxy::Dns::Dnscmd::Record.any_instance.expects(:a_record_conflicts).with('host.foo.bar.domain.local', '192.168.33.33').returns(-1)
     Proxy::Dns::Dnscmd::Record.any_instance.expects(:execute).with('/RecordAdd bar.domain.local host.foo.bar.domain.local. A 192.168.33.33', anything).returns(true)
-    assert @server.create_a_record('host.foo.bar.domain.local', '192.168.33.33')
+    assert_nil @server.create_a_record('host.foo.bar.domain.local', '192.168.33.33')
   end
 
   def test_overwrite_address_record_with_longest_zone_match
-    Proxy::Dns::Dnscmd::Record.any_instance.expects(:dns_find).with('host.foo.bar.domain.local').returns('192.168.33.33')
-    @server.create_a_record('host.foo.bar.domain.local', '192.168.33.33')
+    Proxy::Dns::Dnscmd::Record.any_instance.expects(:a_record_conflicts).with('host.foo.bar.domain.local', '192.168.33.33').returns(0)
+    assert_nil @server.create_a_record('host.foo.bar.domain.local', '192.168.33.33')
   end
 
   def test_create_duplicate_address_record_fails
-    Proxy::Dns::Dnscmd::Record.any_instance.expects(:dns_find).with('host.foo.bar.domain.local').returns('192.168.33.34')
-
+    Proxy::Dns::Dnscmd::Record.any_instance.expects(:a_record_conflicts).with('host.foo.bar.domain.local', '192.168.33.33').returns(1)
     assert_raise Proxy::Dns::Collision do
       @server.create_a_record('host.foo.bar.domain.local', '192.168.33.33')
     end
   end
 
   def test_create_ptr_record
-    Proxy::Dns::Dnscmd::Record.any_instance.expects(:dns_find).with('192.168.33.33').returns(false)
-    assert @server.create_ptr_record('host.foo.bar.domain.local', '192.168.33.33')
+    Proxy::Dns::Dnscmd::Record.any_instance.expects(:ptr_record_conflicts).with('host.foo.bar.domain.local', '192.168.33.33').returns(-1)
+    Proxy::Dns::Dnscmd::Record.any_instance.expects(:execute).with('/RecordAdd 33.168.192.in-addr.arpa 33.33.168.192.in-addr.arpa. PTR host.foo.bar.domain.local.', anything).returns(true)
+    assert_nil @server.create_ptr_record('host.foo.bar.domain.local', '33.33.168.192.in-addr.arpa')
   end
 
   def test_overwrite_ptr_record
-    Proxy::Dns::Dnscmd::Record.any_instance.expects(:dns_find).with('192.168.33.33').returns('host.foo.bar.domain.local')
-    @server.create_ptr_record('host.foo.bar.domain.local', '192.168.33.33')
+    Proxy::Dns::Dnscmd::Record.any_instance.expects(:ptr_record_conflicts).with('host.foo.bar.domain.local', '192.168.33.33').returns(0)
+    assert_nil @server.create_ptr_record('host.foo.bar.domain.local', '33.33.168.192.in-addr.arpa')
   end
 
   def test_create_duplicate_ptr_record_fails
-    Proxy::Dns::Dnscmd::Record.any_instance.expects(:dns_find).with('192.168.33.33').returns('another.host.foo.bar.domain.local')
+    Proxy::Dns::Dnscmd::Record.any_instance.expects(:ptr_record_conflicts).with('host.foo.bar.domain.local', '192.168.33.33').returns(1)
     assert_raise Proxy::Dns::Collision do
-      @server.create_ptr_record('host.foo.bar.domain.local', '192.168.33.33')
+      @server.create_ptr_record('host.foo.bar.domain.local', '33.33.168.192.in-addr.arpa')
     end
   end
 
   def test_remove_address_record_with_longest_zone_match
-    Proxy::Dns::Dnscmd::Record.any_instance.expects(:dns_find).with('host.foo.bar.domain.local').returns(true)
     Proxy::Dns::Dnscmd::Record.any_instance.expects(:execute).with('/RecordDelete bar.domain.local host.foo.bar.domain.local. A /f', anything).returns(true)
-    assert @server.remove_a_record('host.foo.bar.domain.local')
-  end
-
-  def test_remove_non_existent_address_record_raises_exception
-    Proxy::Dns::Dnscmd::Record.any_instance.expects(:dns_find).with('host.foo.bar.domain.local').returns(false)
-    assert_raise Proxy::Dns::NotFound do
-      @server.remove_a_record('host.foo.bar.domain.local')
-    end
+    assert_nil @server.remove_a_record('host.foo.bar.domain.local')
   end
 
   def test_remove_ptr_record
-    Proxy::Dns::Dnscmd::Record.any_instance.expects(:dns_find).with('192.168.33.33').returns(true)
-    assert @server.remove_ptr_record('192.168.33.33')
-  end
-
-  def test_remove_nonexistent_ptr_record_raises_exception
-    Proxy::Dns::Dnscmd::Record.any_instance.expects(:dns_find).with('192.168.33.33').returns(false)
-    assert_raise Proxy::Dns::NotFound do
-      @server.remove_ptr_record('192.168.33.33')
-    end
+    Proxy::Dns::Dnscmd::Record.any_instance.expects(:execute).with('/RecordDelete 33.168.192.in-addr.arpa 33.33.168.192.in-addr.arpa. PTR /f', anything).returns(true)
+    assert_nil @server.remove_ptr_record('33.33.168.192.in-addr.arpa')
   end
 
   def test_dns_zone_matches_second_best_match_if_zone_name_equals_host_name
-    assert_equal('33.168.192.in-addr.arpa', @server.match_zone('33.33.168.192.in-addr.arpa'))
-    assert_equal('domain.local', @server.match_zone('bar.domain.local'))
+    assert_equal('domain.local', @server.match_zone('bar.domain.local', @server.enum_zones))
   end
 
   def test_dns_zone_matches_sole_available_zone
-    server = DnscmdForTesting.new(["sole.domain"])
-    assert_equal('sole.domain', server.match_zone('host.foo.bar.sole.domain'))
+    assert_equal('sole.domain', Proxy::Dns::Dnscmd::Record.new.match_zone('host.foo.bar.sole.domain', ["sole.domain"]))
   end
 
-  def test_dns_zone_no_match_raises_exception
+  def test_dns_non_authoritative_zone_raises_exception
+    assert_raise Proxy::Dns::NotFound do
+      @server.match_zone('host.foo.bar.domain.com', ['domain.local'])
+    end
+    assert_raise Proxy::Dns::NotFound do
+      @server.match_zone('33.33.16.192.in-addr.arpa', ['168.192.in-addr.arpa'])
+    end
     assert_raise Proxy::Dns::NotFound do
-      @server.match_zone('host.foo.bar.domain.com')
+      @server.match_zone('1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.f.e.e.d.8.b.d.1.1.0.0.2.ip6.arpa', ['f.e.e.d.8.b.d.0.1.0.0.2.ip6.arpa'])
     end
   end
 
@@ -117,20 +106,20 @@ def test_dnscmd_enum_zones_parses_primary_zones_only
  _msdcs.bar.domain.local        Primary    AD-Forest       Secure
  168.192.in-addr.arpa           Primary    AD-Domain       Rev
  33.168.192.in-addr.arpa        Primary    AD-Domain       Secure Rev Aging
- e.e.d.8.b.d.0.1.0.0.2.ip6.arpa Primary    AD-Domain       Secure Rev
+ f.e.e.d.8.b.d.0.1.0.0.2.ip6.arpa  Primary    AD-Domain       Secure Rev
  bar.domain.local               Primary    AD-Domain       Secure Aging
  domain.local                   Primary    AD-Domain       Secure
  domain.com                     Secondary    File
  TrustAnchors                   Primary    AD-Forest
 
 
 Command completed successfully.'.split("\n")
-    Proxy::Dns::Dnscmd::Record.any_instance.expects(:execute).with('/EnumZones').returns(to_parse)
+    Proxy::Dns::Dnscmd::Record.any_instance.expects(:execute).with('/EnumZones', nil, true).returns(to_parse)
     assert_equal [
       "_msdcs.bar.domain.local",
       "168.192.in-addr.arpa",
       "33.168.192.in-addr.arpa",
-      "e.e.d.8.b.d.0.1.0.0.2.ip6.arpa",
+      "f.e.e.d.8.b.d.0.1.0.0.2.ip6.arpa",
       "bar.domain.local",
       "domain.local",
       "TrustAnchors"], Proxy::Dns::Dnscmd::Record.new.enum_zones