diff --git a/app/lib/dns_resolver.rb b/app/lib/dns_resolver.rb index 3444120..1f7189a 100644 --- a/app/lib/dns_resolver.rb +++ b/app/lib/dns_resolver.rb @@ -4,20 +4,22 @@ require "resolv" class DNSResolver + class LocalResolversUnavailableError < StandardError + end + attr_reader :nameservers attr_reader :timeout - def initialize(nameservers: nil, timeout: 5) + def initialize(nameservers) @nameservers = nameservers - @timeout = timeout end # Return all A records for the given name # # @param [String] name # @return [Array] - def a(name) - get_resources(name, Resolv::DNS::Resource::IN::A).map do |s| + def a(name, **options) + get_resources(name, Resolv::DNS::Resource::IN::A, **options).map do |s| s.address.to_s end end @@ -26,8 +28,8 @@ class DNSResolver # # @param [String] name # @return [Array] - def aaaa(name) - get_resources(name, Resolv::DNS::Resource::IN::AAAA).map do |s| + def aaaa(name, **options) + get_resources(name, Resolv::DNS::Resource::IN::AAAA, **options).map do |s| s.address.to_s end end @@ -36,8 +38,8 @@ class DNSResolver # # @param [String] name # @return [Array] - def txt(name) - get_resources(name, Resolv::DNS::Resource::IN::TXT).map do |s| + def txt(name, **options) + get_resources(name, Resolv::DNS::Resource::IN::TXT, **options).map do |s| s.data.to_s.strip end end @@ -46,8 +48,8 @@ class DNSResolver # # @param [String] name # @return [Array] - def cname(name) - get_resources(name, Resolv::DNS::Resource::IN::CNAME).map do |s| + def cname(name, **options) + get_resources(name, Resolv::DNS::Resource::IN::CNAME, **options).map do |s| s.name.to_s.downcase end end @@ -56,8 +58,8 @@ class DNSResolver # # @param [String] name # @return [Array>] - def mx(name) - records = get_resources(name, Resolv::DNS::Resource::IN::MX).map do |m| + def mx(name, **options) + records = get_resources(name, Resolv::DNS::Resource::IN::MX, **options).map do |m| [m.preference.to_i, m.exchange.to_s] end records.sort do |a, b| @@ -73,13 +75,13 @@ class DNSResolver # # @param [String] name # @return [Array] - def effective_ns(name) + def effective_ns(name, **options) records = [] parts = name.split(".") (parts.size - 1).times do |n| d = parts[n, parts.size - n + 1].join(".") - records = get_resources(d, Resolv::DNS::Resource::IN::NS).map do |s| + records = get_resources(d, Resolv::DNS::Resource::IN::NS, **options).map do |s| s.name.to_s end @@ -94,27 +96,31 @@ class DNSResolver # # @param [String] ip_address # @return [String] - def ip_to_hostname(ip_address) - dns do |dns| + def ip_to_hostname(ip_address, **options) + dns(**options) do |dns| dns.getname(ip_address)&.to_s end - rescue Resolv::ResolvError + rescue Resolv::ResolvError => e + raise if e.message =~ /timeout/ && options[:raise_timeout_errors] + ip_address end private - def dns - kwargs = @nameservers ? { nameserver: @nameservers } : {} - Resolv::DNS.open(**kwargs) do |dns| - dns.timeouts = [@timeout, @timeout / 2] + def dns(raise_timeout_errors: false) + Resolv::DNS.open(nameserver: @nameservers, + raise_timeout_errors: raise_timeout_errors) do |dns| + dns.timeouts = [Postal::Config.dns.timeout, + Postal::Config.dns.timeout / 2, + Postal::Config.dns.timeout / 2] yield dns end end - def get_resources(name, type) + def get_resources(name, type, **options) encoded_name = DomainName::Punycode.encode_hostname(name) - dns do |dns| + dns(**options) do |dns| dns.getresources(encoded_name, type) end end @@ -126,19 +132,28 @@ class DNSResolver # @param [String] name # @return [DNSResolver] def for_domain(name) - resolver = new - nameservers = resolver.effective_ns(name) + nameservers = local.effective_ns(name) ips = nameservers.map do |ns| - resolver.a(ns) + local.a(ns) end.flatten.uniq - new(nameservers: ips) + new(ips) end # Return a local resolver to use for lookups # # @return [DNSResolver] def local - @local ||= new + @local ||= begin + resolv_conf_path = Postal::Config.dns.resolv_conf_path + raise LocalResolversUnavailableError, "No resolver config found at #{resolv_conf_path}" unless File.file?(resolv_conf_path) + + resolv_conf = Resolv::DNS::Config.parse_resolv_conf(resolv_conf_path) + if resolv_conf.nil? || resolv_conf[:nameserver].nil? || resolv_conf[:nameserver].empty? + raise LocalResolversUnavailableError, "Could not find nameservers in #{resolv_conf_path}" + end + + new(resolv_conf[:nameserver]) + end end end diff --git a/app/senders/smtp_sender.rb b/app/senders/smtp_sender.rb index ddf01e0..440fa7c 100644 --- a/app/senders/smtp_sender.rb +++ b/app/senders/smtp_sender.rb @@ -162,7 +162,7 @@ class SMTPSender < BaseSender # # @return [Array] def resolve_mx_records_for_domain - hostnames = DNSResolver.local.mx(@domain).map(&:last) + hostnames = DNSResolver.local.mx(@domain, raise_timeout_errors: true).map(&:last) return [SMTPClient::Server.new(@domain)] if hostnames.empty? hostnames.map { |hostname| SMTPClient::Server.new(hostname) } diff --git a/doc/config/environment-variables.md b/doc/config/environment-variables.md index 63739a6..2a8cc53 100644 --- a/doc/config/environment-variables.md +++ b/doc/config/environment-variables.md @@ -63,6 +63,8 @@ This document contains all the environment variables which are available for thi | `DNS_DKIM_IDENTIFIER` | String | The identifier to use for DKIM keys in DNS records | postal | | `DNS_DOMAIN_VERIFY_PREFIX` | String | The prefix to add before TXT record verification string | postal-verification | | `DNS_CUSTOM_RETURN_PATH_PREFIX` | String | The domain to use on external domains which points to the Postal return path domain | psrp | +| `DNS_TIMEOUT` | Integer | The timeout to wait for DNS resolution | 5 | +| `DNS_RESOLV_CONF_PATH` | String | The path to the resolv.conf file containing addresses for local nameservers | /etc/resolv.conf | | `SMTP_HOST` | String | The hostname to send application-level e-mails to | 127.0.0.1 | | `SMTP_PORT` | Integer | The port number to send application-level e-mails to | 25 | | `SMTP_USERNAME` | String | The username to use when authentication to the SMTP server | | diff --git a/doc/config/yaml.yml b/doc/config/yaml.yml index 381358c..63f34de 100644 --- a/doc/config/yaml.yml +++ b/doc/config/yaml.yml @@ -137,6 +137,10 @@ dns: domain_verify_prefix: postal-verification # The domain to use on external domains which points to the Postal return path domain custom_return_path_prefix: psrp + # The timeout to wait for DNS resolution + timeout: 5 + # The path to the resolv.conf file containing addresses for local nameservers + resolv_conf_path: /etc/resolv.conf smtp: # The hostname to send application-level e-mails to diff --git a/lib/postal/config_schema.rb b/lib/postal/config_schema.rb index fdf334b..69b4a17 100644 --- a/lib/postal/config_schema.rb +++ b/lib/postal/config_schema.rb @@ -329,6 +329,16 @@ module Postal description "The domain to use on external domains which points to the Postal return path domain" default "psrp" end + + integer :timeout do + description "The timeout to wait for DNS resolution" + default 5 + end + + string :resolv_conf_path do + description "The path to the resolv.conf file containing addresses for local nameservers" + default "/etc/resolv.conf" + end end group :smtp do diff --git a/spec/lib/dns_resolver_spec.rb b/spec/lib/dns_resolver_spec.rb index 8ad8017..dfce0a9 100644 --- a/spec/lib/dns_resolver_spec.rb +++ b/spec/lib/dns_resolver_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" RSpec.describe DNSResolver do - subject(:resolver) { described_class.new } + subject(:resolver) { described_class.local } # Now, we could mock everything in here which would give us some comfort # but I do think that we'll benefit more from having a full E2E test here @@ -14,15 +14,52 @@ RSpec.describe DNSResolver do it "returns a list of IP addresses" do expect(resolver.a("www.test.postalserver.io").sort).to eq ["1.2.3.4", "2.3.4.5"] end + it "resolves a domain name containing an emoji" do expect(resolver.a("☺.test.postalserver.io").sort).to eq ["3.4.5.6"] end + + it "returns an empty array when timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect(resolver.a("www.test.postalserver.io")).to eq [] + end + + context "when raise_timeout_errors is true" do + it "returns a list of IP addresses" do + expect(resolver.a("www.test.postalserver.io", raise_timeout_errors: true).sort).to eq ["1.2.3.4", "2.3.4.5"] + end + + it "raises an error when the timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect do + resolver.a("www.test.postalserver.io", raise_timeout_errors: true) + end.to raise_error(Resolv::ResolvError, /timeout/) + end + end end describe "#aaaa" do it "returns a list of IP addresses" do expect(resolver.aaaa("www.test.postalserver.io").sort).to eq ["2a00:67a0:a::1", "2a00:67a0:a::2"] end + + it "returns an empty array when timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect(resolver.aaaa("www.test.postalserver.io")).to eq [] + end + + context "when raise_timeout_errors is true" do + it "returns a list of IP addresses" do + expect(resolver.aaaa("www.test.postalserver.io", raise_timeout_errors: true).sort).to eq ["2a00:67a0:a::1", "2a00:67a0:a::2"] + end + + it "raises an error when the timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect do + resolver.aaaa("www.test.postalserver.io", raise_timeout_errors: true) + end.to raise_error(Resolv::ResolvError, /timeout/) + end + end end describe "#txt" do @@ -32,12 +69,51 @@ RSpec.describe DNSResolver do "another example" ] end + + it "returns an empty array when timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect(resolver.txt("test.postalserver.io")).to eq [] + end + + context "when raise_timeout_errors is true" do + it "returns a list of TXT records" do + expect(resolver.txt("test.postalserver.io", raise_timeout_errors: true).sort).to eq [ + "an example txt record", + "another example" + ] + end + + it "raises an error when the timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect do + resolver.txt("test.postalserver.io", raise_timeout_errors: true) + end.to raise_error(Resolv::ResolvError, /timeout/) + end + end end describe "#cname" do it "returns a list of CNAME records" do expect(resolver.cname("cname.test.postalserver.io")).to eq ["www.test.postalserver.io"] end + + it "returns an empty array when timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect(resolver.cname("cname.test.postalserver.io")).to eq [] + end + + context "when raise_timeout_errors is true" do + it "returns a list of CNAME records" do + expect(resolver.cname("cname.test.postalserver.io", raise_timeout_errors: true)).to eq ["www.test.postalserver.io"] + end + + it "raises an error when the timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect do + resolver.cname("cname.test.postalserver.io", raise_timeout_errors: true) + end.to raise_error(Resolv::ResolvError, /timeout/) + end + end end describe "#mx" do @@ -47,6 +123,27 @@ RSpec.describe DNSResolver do [20, "mx2.test.postalserver.io"] ] end + + it "returns an empty array when timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect(resolver.mx("test.postalserver.io")).to eq [] + end + + context "when raise_timeout_errors is true" do + it "returns a list of MX records" do + expect(resolver.mx("test.postalserver.io", raise_timeout_errors: true)).to eq [ + [10, "mx1.test.postalserver.io"], + [20, "mx2.test.postalserver.io"] + ] + end + + it "raises an error when the timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect do + resolver.mx("test.postalserver.io", raise_timeout_errors: true) + end.to raise_error(Resolv::ResolvError, /timeout/) + end + end end describe "#effective_ns" do @@ -56,12 +153,51 @@ RSpec.describe DNSResolver do "the-cake-is-a-lie.katapultdns.com" ] end + + it "returns an empty array when timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect(resolver.effective_ns("postalserver.io")).to eq [] + end + + context "when raise_timeout_errors is true" do + it "returns a list of NS records" do + expect(resolver.effective_ns("postalserver.io", raise_timeout_errors: true).sort).to eq [ + "prestigious-honeybadger.katapultdns.com", + "the-cake-is-a-lie.katapultdns.com" + ] + end + + it "raises an error when the timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect do + resolver.effective_ns("postalserver.io", raise_timeout_errors: true) + end.to raise_error(Resolv::ResolvError, /timeout/) + end + end end describe "#ip_to_hostname" do it "returns the hostname for the given IP" do expect(resolver.ip_to_hostname("151.252.1.100")).to eq "ns1.katapultdns.com" end + + it "returns the IP when the timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect(resolver.ip_to_hostname("151.252.1.100")).to eq "151.252.1.100" + end + + context "when raise_timeout_errors is true" do + it "returns the hostname for the given IP" do + expect(resolver.ip_to_hostname("151.252.1.100", raise_timeout_errors: true)).to eq "ns1.katapultdns.com" + end + + it "raises an error when the timeout is exceeded" do + allow(Postal::Config.dns).to receive(:timeout).and_return(0.00001) + expect do + resolver.ip_to_hostname("151.252.1.100", raise_timeout_errors: true) + end.to raise_error(Resolv::ResolvError, /timeout/) + end + end end describe ".for_domain" do @@ -72,9 +208,40 @@ RSpec.describe DNSResolver do end describe ".local" do - it "returns a resolver with no nameservers" do + after do + # Remove all cached values for the local resolver + DNSResolver.instance_variable_set(:@local, nil) + end + + it "returns a resolver with the local machine's resolvers" do resolver = described_class.local - expect(resolver.nameservers).to be nil + expect(resolver.nameservers).to be_a Array + expect(resolver.nameservers).to_not be_empty + end + + context "when there is no resolv.conf" do + it "raises an error" do + allow(File).to receive(:file?).with("/etc/resolv.conf").and_return(false) + expect { described_class.local }.to raise_error(DNSResolver::LocalResolversUnavailableError, + /no resolver config found at/i) + end + end + + context "when no nameservers are found in resolv.conf" do + it "raises an error" do + allow(Resolv::DNS::Config).to receive(:parse_resolv_conf).with("/etc/resolv.conf").and_return({}) + expect { described_class.local }.to raise_error(DNSResolver::LocalResolversUnavailableError, + /could not find nameservers in/i) + end + end + + context "when an empty array of nameserver is found in resolv.conf" do + it "raises an error" do + allow(Resolv::DNS::Config).to receive(:parse_resolv_conf).with("/etc/resolv.conf") + .and_return({ nameserver: [] }) + expect { described_class.local }.to raise_error(DNSResolver::LocalResolversUnavailableError, + /could not find nameservers in/i) + end end end diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index 4b76faa..a3689fb 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -301,7 +301,7 @@ describe Domain do context "when local nameservers should not be used" do it "uses the a resolver for this domain" do - allow(DNSResolver).to receive(:for_domain).with(domain.name).and_return(DNSResolver.new(nameservers: ["1.2.3.4"])) + allow(DNSResolver).to receive(:for_domain).with(domain.name).and_return(DNSResolver.new(["1.2.3.4"])) expect(domain.resolver).to be_a DNSResolver expect(domain.resolver.nameservers).to eq ["1.2.3.4"] end