fix: raise an error if MX lookup times out during sending

This avoids potentially sending mail to the A record when an MX query times out.

closes #2833

app/lib/dns_resolver.rb

@@ -4,20 +4,22 @@
 
 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<String>]
-  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 @@ def a(name)
   #
   # @param [String] name
   # @return [Array<String>]
-  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 @@ def aaaa(name)
   #
   # @param [String] name
   # @return [Array<String>]
-  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 @@ def txt(name)
   #
   # @param [String] name
   # @return [Array<String>]
-  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 @@ def cname(name)
   #
   # @param [String] name
   # @return [Array<Array<Integer, String>>]
-  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 @@ def mx(name)
   #
   # @param [String] name
   # @return [Array<String>]
-  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 @@ def effective_ns(name)
   #
   # @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 << self
     # @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

app/senders/smtp_sender.rb

@@ -162,7 +162,7 @@ def determine_rcpt_to_for_message(message)
   #
   # @return [Array<String>]
   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) }

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 |  |

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

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