From b0d920f7b63b553b5f849b837d17dbfd733f5622 Mon Sep 17 00:00:00 2001 From: enp7s0d <75983347+Pushpenderrathore@users.noreply.github.com> Date: Tue, 1 Sep 2026 00:45:19 +0530 Subject: [PATCH 1/2] Fix DNS forward/cache path for relay-driven poisoning Three issues surfaced while running the DNS server under the Kerberos relay coercion workflow (identified during jheysel's ESC8 testing): - Rex::Proto::DNS::Cache#cache_record raised on any forwarded record whose name did not match MATCH_HOSTNAME, killing the dispatch thread. Skip the non-cacheable record instead. - Server#default_dispatch_request duplicated the request with Dnsruby::Message#dup, which is shallow, so req.question and forward.question shared one Array. Deleting a cache-served question from the forwarded packet also emptied the original request's question list. Give forward its own copy. - The response was built by mutating the decoded request's @answer via instance_variable_set and re-encoding it, which appended the answer bytes past the packet end so clients decoded zero answers ("bad DNS packet" on Windows). Build a fresh Dnsruby::Message for the response instead. Also handle the udp recvfrom returning an explicit source port so the reply goes back to the right host/port across socket variants. Adds regression specs for the shallow-dup question handling and the answer-encoding fix. --- lib/rex/proto/dns/cache.rb | 2 +- lib/rex/proto/dns/server.rb | 47 ++++++++++++--------- spec/lib/rex/proto/dns/server_spec.rb | 60 +++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 21 deletions(-) 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..1873d44cde6cc 100644 --- a/lib/rex/proto/dns/server.rb +++ b/lib/rex/proto/dns/server.rb @@ -163,32 +163,35 @@ 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.ra = req.header.rd + 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 +223,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 From 76fd57ac4e9ffa8f2106a61c56fb943ba17422cb Mon Sep 17 00:00:00 2001 From: enp7s0d <75983347+Pushpenderrathore@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:29:30 +0530 Subject: [PATCH 2/2] Echo RD and set RA from forwarder in fresh DNS response The fresh Dnsruby::Message dropped the request RD bit and set RA from the client RD. Echo RD back and derive RA from whether a forwarder is configured, so recursive queries get a spec-correct header. --- lib/rex/proto/dns/server.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rex/proto/dns/server.rb b/lib/rex/proto/dns/server.rb index 1873d44cde6cc..79f6c1a3b3c4d 100644 --- a/lib/rex/proto/dns/server.rb +++ b/lib/rex/proto/dns/server.rb @@ -188,7 +188,8 @@ def default_dispatch_request(cli,data) resp = Dnsruby::Message.new resp.header.id = req.header.id resp.header.qr = true - resp.header.ra = req.header.rd + 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)