Changes from code review

This commit is contained in:
Ashley Donaldson 2023-11-22 15:45:03 +11:00
parent 34bd661d3f
commit ef9a165d22
No known key found for this signature in database
GPG Key ID: D4BCDC8C892F7477
5 changed files with 80 additions and 32 deletions

View File

@ -84,6 +84,7 @@ class Framework
if options.include?('CustomDnsResolver')
self.dns_resolver = options['CustomDnsResolver']
self.dns_resolver.set_framework(self)
Rex::Socket._install_global_resolver(self.dns_resolver)
end

View File

@ -41,19 +41,19 @@ class DNS
# Tab completion for the dns command
#
# @param str [String] the string currently being typed before tab was hit
# @param words [Array<String>] the previously completed words on the command line. words is always
# at least 1 when tab completion has reached this stage since the command itself has been completed
# @param words [Array<String>] the previously completed words on the command line. The array
# contains at least one entry when tab completion has reached this stage since the command itself has been completed
def cmd_dns_tabs(str, words)
return if driver.framework.dns_resolver.nil?
if words.length == 1
options = ['add','del','remove','flush','print']
options = ['add','del','remove','purge','print']
return options.select { |opt| opt.start_with?(str) }
end
cmd = words[1]
case cmd
when 'flush','print'
when 'purge','print'
# These commands don't have any arguments
return
when 'add'
@ -95,33 +95,41 @@ class DNS
end
def cmd_dns_help
print_line 'Usage: dns'
print_line
print_line "Manage Metasploit's DNS resolution behaviour"
print_line
print_line "Usage:"
print_line " dns [add/remove] [--session <session_id>] [--rule <wildcard DNS entry>] <IP Address> <IP Address> ..."
print_line " dns [get] <hostname>"
print_line " dns [flush]"
print_line " dns [add] [--session <session_id>] [--rule <wildcard DNS entry>] <IP Address> <IP Address> ..."
print_line " dns [remove/del] -i <entry id> [-i <entry id> ...]"
print_line " dns [purge]"
print_line " dns [print]"
print_line
print_line "Subcommands:"
print_line " add - add a DNS resolution entry to resolve certain domain names through a particular DNS server"
print_line " remove - delete a DNS resolution entry; 'del' is an alias"
print_line " flush - remove all DNS resolution entries"
print_line " purge - remove all DNS resolution entries"
print_line " print - show all active DNS resolution entries"
print_line
print_line "Examples:"
print_line " Set the DNS server to be used for *.metasploit.com to 192.168.1.10"
print_line " Display all current DNS nameserver entries"
print_line " dns"
print_line " dns print"
print_line
print_line " Set the DNS server(s) to be used for *.metasploit.com to 192.168.1.10"
print_line " route add --rule *.metasploit.com 192.168.1.10"
print_line
print_line " Set the DNS server to be used for *.metasploit.com to 192.168.1.10, but specifically to go through session 2"
print_line " Add multiple entries at once"
print_line " route add --rule *.metasploit.com --rule *.google.com 192.168.1.10 192.168.1.11"
print_line
print_line " Set the DNS server(s) to be used for *.metasploit.com to 192.168.1.10, but specifically to go through session 2"
print_line " route add --session 2 --rule *.metasploit.com 192.168.1.10"
print_line
print_line " Delete the above DNS resolution rule"
print_line " route remove --session 2 --rule *.metasploit.com 192.168.1.10"
print_line " Delete the DNS resolution rule with ID 3"
print_line " route remove -i 3"
print_line
print_line " Set the DNS server to be used for all requests that match no rules"
print_line " Delete multiple entries in one command"
print_line " route remove -i 3 -i 4 -i 5"
print_line
print_line " Set the DNS server(s) to be used for all requests that match no rules"
print_line " route add 8.8.8.8 8.8.4.4"
print_line
end
@ -217,7 +225,7 @@ class DNS
servers.each do |server|
driver.framework.dns_resolver.add_nameserver(rules, server, comm_obj)
end
print_good("DNS #{servers.length > 1 ? 'entries' : 'entry'} added")
print_good("#{servers.length} DNS #{servers.length > 1 ? 'entries' : 'entry'} added")
end
#
@ -238,7 +246,7 @@ class DNS
print_warning("Some entries were not removed: #{difference.join(', ')}") unless difference.empty?
if removed.length > 0
print_good("#{removed.length} DNS #{removed.length > 1 ? 'entries' : 'entry'} removed")
print_dns_set('Deleted entries', ['ID', 'Rules(s)', 'DNS Server', 'Commm channel'], removed.map {|hash| [hash[:id], hash[:wildcard_rules].join(','), hash[:dns_server], prettify_comm(hash[:comm], hash[:dns_server])]})
print_dns_set('Deleted entries', removed)
end
end
@ -256,11 +264,13 @@ class DNS
def print_dns
results = driver.framework.dns_resolver.nameserver_entries
columns = ['ID','Rule(s)', 'DNS Server', 'Comm channel']
print_dns_set('Custom nameserver rules', columns, results[0].map {|hash| [hash[:id], hash[:wildcard_rules].join(','), hash[:dns_server], prettify_comm(hash[:comm], hash[:dns_server])]})
print_dns_set('Custom nameserver rules', results[0])
# Default nameservers don't include a rule
columns = ['ID', 'DNS Server', 'Comm channel']
print_dns_set('Default nameservers', columns, results[1].map {|hash| [hash[:id], hash[:dns_server], prettify_comm(hash[:comm], hash[:dns_server])]})
print_dns_set('Default nameservers', results[1])
print_line('No custom DNS nameserver entries configured') if results[0].length + results[1].length == 0
end
private
@ -285,7 +295,14 @@ class DNS
end
end
def print_dns_set(heading, columns, result_set)
def print_dns_set(heading, result_set)
return if result_set.length == 0
if result_set[0][:wildcard_rules].any?
columns = ['ID', 'Rules(s)', 'DNS Server', 'Commm channel']
else
columns = ['ID', 'DNS Server', 'Commm channel']
end
tbl = Table.new(
Table::Style::Default,
'Header' => heading,
@ -293,8 +310,12 @@ class DNS
'Postfix' => "\n",
'Columns' => columns
)
result_set.each do |row|
tbl << row
result_set.each do |hash|
if columns.size == 4
tbl << [hash[:id], hash[:wildcard_rules].join(','), hash[:dns_server], prettify_comm(hash[:comm], hash[:dns_server])]
else
tbl << [hash[:id], hash[:dns_server], prettify_comm(hash[:comm], hash[:dns_server])]
end
end
print(tbl.to_s) if tbl.rows.length > 0

View File

@ -161,6 +161,9 @@ module DNS
# @return [Array<Array>] A list of nameservers, each with Rex::Socket options
#
def nameservers_for_packet(packet)
unless feature_set.enabled?(Msf::FeatureManager::DNS_FEATURE)
return super
end
# Leaky abstraction: a packet could have multiple question entries,
# and each of these could have different nameservers, or travel via
# different comm channels. We can't allow DNS leaks, so for now, we
@ -172,9 +175,9 @@ module DNS
self.entries_with_rules.each do |entry|
entry[:wildcard_rules].each do |rule|
socket_options = {}
socket_options['Comm'] = entry[:comm] unless entry[:comm].nil?
if matches(name, rule)
socket_options = {}
socket_options['Comm'] = entry[:comm] unless entry[:comm].nil?
dns_servers.append([entry[:dns_server], socket_options])
break
end
@ -208,6 +211,10 @@ module DNS
mod.init
end
def set_framework(framework)
self.feature_set = framework.features
end
private
#
# Is the given wildcard DNS entry valid?
@ -228,6 +235,7 @@ module DNS
attr_accessor :entries_with_rules # Set of custom nameserver entries that specify a rule
attr_accessor :entries_without_rules # Set of custom nameserver entries that do not include a rule
attr_accessor :next_id # The next ID to have been allocated to an entry
attr_accessor :feature_set
end
end
end

View File

@ -112,11 +112,11 @@ module DNS
#
# Find the nameservers to use for a given DNS request
# @param dns_message [Dnsruby::Message] The DNS message to be sent
# @param _dns_message [Dnsruby::Message] The DNS message to be sent
#
# @return [Array<Array>] A list of nameservers, each with Rex::Socket options
#
def nameservers_for_packet(dns_message)
def nameservers_for_packet(_dns_message)
@config[:nameservers].map {|ns| [ns, {}]}
end
@ -174,7 +174,7 @@ module DNS
method = :send_tcp
end
ans = self.__send__(method, packet, packet_data)
ans = self.__send__(method, packet, packet_data, nameservers)
unless (ans and ans[0].length > 0)
@logger.fatal "No response from nameservers list: aborting"
@ -203,13 +203,13 @@ module DNS
#
# @param packet [Net::DNS::Packet] Packet associated with packet_data
# @param packet_data [String] Data segment of DNS request packet
# @param nameservers [Array<[String,Hash]>] List of nameservers to use for this request, and their associated socket options
# @param prox [String] Proxy configuration for TCP socket
#
# @return ans [String] Raw DNS reply
def send_tcp(packet,packet_data,prox = @config[:proxies])
def send_tcp(packet, packet_data, nameservers, prox = @config[:proxies])
ans = nil
length = [packet_data.size].pack("n")
nameservers = nameservers_for_packet(packet)
nameservers.each do |ns, socket_options|
begin
socket = nil
@ -307,12 +307,12 @@ module DNS
#
# @param packet [Net::DNS::Packet] Packet associated with packet_data
# @param packet_data [String] Data segment of DNS request packet
# @param nameservers [Array<[String,Hash]>] List of nameservers to use for this request, and their associated socket options
#
# @return ans [String] Raw DNS reply
def send_udp(packet,packet_data)
def send_udp(packet,packet_data, nameservers)
ans = nil
response = ""
nameservers = nameservers_for_packet(packet)
nameservers.each do |ns, socket_options|
catch(:next_ns) do
begin
@ -414,7 +414,7 @@ module DNS
def supports_udp?(nameserver_results)
nameserver_results.each do |nameserver, socket_options|
comm = socket_options.fetch('Comm') { @config.fetch(:comm) { Rex::Socket::SwitchBoard.best_comm(ns) }}
comm = socket_options.fetch('Comm') { @config[:comm] || Rex::Socket::SwitchBoard.best_comm(nameserver) }
next if comm.nil?
return false unless comm.supports_udp?
end

View File

@ -33,6 +33,20 @@ RSpec.describe Rex::Proto::DNS::CustomNameserverProvider do
{:dns_cache_no_start => true}
end
let (:framework_with_dns_enabled) do
framework = Object.new
def framework.features
f = Object.new
def f.enabled?(_name)
true
end
f
end
framework
end
subject(:many_ruled_provider) do
dns_resolver = Rex::Proto::DNS::CachedResolver.new(config)
dns_resolver.extend(Rex::Proto::DNS::CustomNameserverProvider)
@ -41,6 +55,7 @@ RSpec.describe Rex::Proto::DNS::CustomNameserverProvider do
dns_resolver.add_nameserver(['*.metasploit.com'], ruled_nameserver, nil)
dns_resolver.add_nameserver(['*.metasploit.com'], ruled_nameserver2, nil)
dns_resolver.add_nameserver(['*.notmetasploit.com'], ruled_nameserver3, nil)
dns_resolver.set_framework(framework_with_dns_enabled)
dns_resolver
end
@ -51,6 +66,7 @@ RSpec.describe Rex::Proto::DNS::CustomNameserverProvider do
dns_resolver.nameservers = [base_nameserver]
dns_resolver.add_nameserver([], ruleless_nameserver, nil)
dns_resolver.add_nameserver(['*.metasploit.com'], ruled_nameserver, nil)
dns_resolver.set_framework(framework_with_dns_enabled)
dns_resolver
end
@ -60,6 +76,7 @@ RSpec.describe Rex::Proto::DNS::CustomNameserverProvider do
dns_resolver.extend(Rex::Proto::DNS::CustomNameserverProvider)
dns_resolver.nameservers = [base_nameserver]
dns_resolver.add_nameserver([], ruleless_nameserver, nil)
dns_resolver.set_framework(framework_with_dns_enabled)
dns_resolver
end
@ -68,6 +85,7 @@ RSpec.describe Rex::Proto::DNS::CustomNameserverProvider do
dns_resolver = Rex::Proto::DNS::CachedResolver.new(config)
dns_resolver.extend(Rex::Proto::DNS::CustomNameserverProvider)
dns_resolver.nameservers = [base_nameserver]
dns_resolver.set_framework(framework_with_dns_enabled)
dns_resolver
end