diff --git a/lib/rex/proto/dns/cache.rb b/lib/rex/proto/dns/cache.rb index 87c3ea941dd1a..0a2ba1900f765 100644 --- a/lib/rex/proto/dns/cache.rb +++ b/lib/rex/proto/dns/cache.rb @@ -57,7 +57,7 @@ def cache_record(record, expire: true) end unless record.name.to_s.match(MATCH_HOSTNAME) - raise "Invalid record for cache entry (invalid hostname) - #{record.inspect}" + return # skip non-cacheable record: " - #{record.inspect}" end add(record, expire ? (::Time.now.to_i + record.ttl) : 0) diff --git a/lib/rex/proto/dns/server.rb b/lib/rex/proto/dns/server.rb index 36de1e135acc7..79f6c1a3b3c4d 100644 --- a/lib/rex/proto/dns/server.rb +++ b/lib/rex/proto/dns/server.rb @@ -163,32 +163,36 @@ def default_dispatch_request(cli,data) return if data.strip.empty? req = Packet.encode_drb(data) forward = req.dup - # Find cached items, remove request from forwarded packet + # Dnsruby#dup is shallow - req and forward share the same @question Array. + # Give forward its own copy so deleting forwarded questions doesn't also + # empty req.question, which we need intact when echoing it in the response. + forward.instance_variable_set(:@question, req.question.dup) + answers = [] + # Find cached items, remove question from forwarded packet req.question.each do |ques| cached = self.cache.find(ques.qname, ques.qtype) - if cached.empty? - next - else - req.instance_variable_set(:@answer, (req.answer + cached).uniq) + unless cached.empty? + answers.concat(cached) forward.question.delete(ques) end end # Forward remaining requests, cache responses - if forward.question.count > 0 and @fwd_res + if forward.question.count > 0 && @fwd_res forwarded = self.fwd_res.send(forward) - req.instance_variable_set(:@answer, (req.answer + forwarded.answer).uniq) - forwarded.answer.each do |ans| - self.cache.cache_record(ans) - end - req.header.ra = true # Set recursion bit - end - # Finalize answers in response - # Check for empty response prior to sending - if req.answer.size < 1 - req.header.rcode = Dnsruby::RCode::NOERROR + answers.concat(forwarded.answer) + forwarded.answer.each { |ans| self.cache.cache_record(ans) } end - req.header.qr = true # Set response bit - send_response(cli, req.encode) + # Build a fresh response message to avoid a Dnsruby stale-state encoding bug + # where instance_variable_set(:@answer) on a decoded request appends answer + # bytes after the packet end rather than inside the answer section. + resp = Dnsruby::Message.new + resp.header.id = req.header.id + resp.header.qr = true + resp.header.rd = req.header.rd # echo Recursion Desired back to the client + resp.header.ra = !self.fwd_res.nil? # Recursion Available when we can forward upstream + req.question.each { |q| resp.add_question(q.qname, q.qtype, q.qclass) } + answers.uniq.each { |a| resp.add_answer(a) } + send_response(cli, resp.encode) end # @@ -220,8 +224,12 @@ def monitor_listener r,_,_ = ::IO.select(rds,wds,eds,1) if (r != nil and r[0] == self.udp_sock) - buf, addr = self.udp_sock.recvfrom(65535) - host, port = addr[3], addr[1] + buf, addr, source_port = self.udp_sock.recvfrom(65535) + if source_port + host, port = addr, source_port + else + host, port = addr[3], addr[1] + end # Mock up a client object for sending back data cli = MockDnsClient.new(host, port, r[0]) dispatch_request(cli, buf) diff --git a/spec/lib/rex/proto/dns/server_spec.rb b/spec/lib/rex/proto/dns/server_spec.rb index 95b5da04ce940..68a0dbcf8df1a 100644 --- a/spec/lib/rex/proto/dns/server_spec.rb +++ b/spec/lib/rex/proto/dns/server_spec.rb @@ -36,5 +36,65 @@ def query_for(name) server.default_dispatch_request(cli, query_for('empty.example.com')) end end + + context 'when the forwarded response carries an answer' do + let(:answer) do + Dnsruby::RR.create(name: 'poisoned.example.com.', type: 'A', address: '192.0.2.10') + end + + before do + # the resolver forwards the query and returns one answer, which is the + # path that has to survive re-encoding intact + forwarded = Dnsruby::Message.new + forwarded.add_answer(answer) + server.fwd_res = double('resolver', send: forwarded) + end + + it 'encodes the answer inside the answer section rather than past the packet end' do + # Regression: mutating a decoded request's @answer and re-encoding it + # appended the answer bytes after the packet, so the reply decoded with + # zero answers. Building a fresh response keeps the answer readable. + expect(cli).to receive(:write) do |data| + reply = Dnsruby::Message.decode(data) + expect(reply.answer.count).to eq(1) + expect(reply.answer.first.address.to_s).to eq('192.0.2.10') + end + server.default_dispatch_request(cli, query_for('poisoned.example.com')) + end + end + + context 'when one of several questions is served from cache' do + it 'forwards only the uncached question and keeps the request questions intact' do + # Regression: Dnsruby#dup is shallow, so deleting a served question from + # the forwarded packet also emptied the original request's question list. + # Only the uncached question should be forwarded, and both questions must + # still be echoed back in the response. + cached_answer = Dnsruby::RR.create(name: 'a.example.com.', type: 'A', address: '192.0.2.30') + allow(server.cache).to receive(:find) do |qname, _qtype| + qname.to_s == 'a.example.com' ? [cached_answer] : [] + end + + forwarded = Dnsruby::Message.new + forwarded.add_answer(Dnsruby::RR.create(name: 'b.example.com.', type: 'A', address: '192.0.2.20')) + resolver = double('resolver') + server.fwd_res = resolver + + query = Dnsruby::Message.new + query.add_question(Dnsruby::Name.create('a.example.com.'), Dnsruby::Types::A) + query.add_question(Dnsruby::Name.create('b.example.com.'), Dnsruby::Types::A) + + expect(resolver).to receive(:send) do |fwd| + expect(fwd.question.map { |q| q.qname.to_s }).to eq(['b.example.com']) + forwarded + end + + expect(cli).to receive(:write) do |data| + reply = Dnsruby::Message.decode(data) + expect(reply.question.map { |q| q.qname.to_s }).to contain_exactly('a.example.com', 'b.example.com') + expect(reply.answer.map { |a| a.address.to_s }).to contain_exactly('192.0.2.30', '192.0.2.20') + end + server.default_dispatch_request(cli, query.encode) + end + end end end