replace listener config option IPsNATed with NATIPs, and let autotls …

…check NATIPs

NATIPs lists the public IPs, so we can still do the DNS checks on them. with
IPsNATed, we disabled the checks.

based on feedback by kikoreis in issue #52

config/config.go

@@ -114,7 +114,8 @@ type ACME struct {
 
 type Listener struct {
 	IPs            []string   `sconf-doc:"Use 0.0.0.0 to listen on all IPv4 and/or :: to listen on all IPv6 addresses, but it is better to explicitly specify the IPs you want to use for email, as mox will make sure outgoing connections will only be made from one of those IPs."`
-	IPsNATed       bool       `sconf:"optional" sconf-doc:"Set this if the specified IPs are not the public IPs, but are NATed. This makes the DNS check skip a few checks related to IPs, such as for iprev, mx, spf, autoconfig, autodiscover."`
+	NATIPs         []string   `sconf:"optional" sconf-doc:"If set, the mail server is configured behind a NAT and field IPs are internal instead of the public IPs, while NATIPs lists the public IPs. Used during IP-related DNS self-checks, such as for iprev, mx, spf, autoconfig, autodiscover, and for autotls."`
+	IPsNATed       bool       `sconf:"optional" sconf-doc:"Deprecated, use NATIPs instead. If set, IPs are not the public IPs, but are NATed. Skips IP-related DNS self-checks."`
 	Hostname       string     `sconf:"optional" sconf-doc:"If empty, the config global Hostname is used."`
 	HostnameDomain dns.Domain `sconf:"-" json:"-"` // Set when parsing config.
 

config/doc.go

@@ -110,9 +110,15 @@ describe-static" and "mox config describe-domains":
 			IPs:
 				-
 
-			# Set this if the specified IPs are not the public IPs, but are NATed. This makes
-			# the DNS check skip a few checks related to IPs, such as for iprev, mx, spf,
-			# autoconfig, autodiscover. (optional)
+			# If set, the mail server is configured behind a NAT and field IPs are internal
+			# instead of the public IPs, while NATIPs lists the public IPs. Used during
+			# IP-related DNS self-checks, such as for iprev, mx, spf, autoconfig,
+			# autodiscover, and for autotls. (optional)
+			NATIPs:
+				-
+
+			# Deprecated, use NATIPs instead. If set, IPs are not the public IPs, but are
+			# NATed. Skips IP-related DNS self-checks. (optional)
 			IPsNATed: false
 
 			# If empty, the config global Hostname is used. (optional)

mox-/admin.go

@@ -959,7 +959,8 @@ func ClientConfigDomain(d dns.Domain) (ClientConfig, error) {
 	return c, nil
 }
 
-// return IPs we may be listening/receiving mail on or connecting/sending from to the outside.
+// IPs returns ip addresses we may be listening/receiving mail on or
+// connecting/sending from to the outside.
 func IPs(ctx context.Context, receiveOnly bool) ([]net.IP, error) {
 	log := xlog.WithContext(ctx)
 
@@ -972,7 +973,11 @@ func IPs(ctx context.Context, receiveOnly bool) ([]net.IP, error) {
 		if l.IPsNATed {
 			return nil, nil
 		}
-		for _, s := range l.IPs {
+		check := l.IPs
+		if len(l.NATIPs) > 0 {
+			check = l.NATIPs
+		}
+		for _, s := range check {
 			ip := net.ParseIP(s)
 			if ip.IsUnspecified() {
 				if ip.To4() != nil {

mox-/config.go

@@ -272,7 +272,15 @@ func (c *Config) allowACMEHosts(checkACMEHosts bool) {
 			}
 		}
 
-		m.SetAllowedHostnames(dns.StrictResolver{Pkg: "autotls"}, hostnames, c.Static.Listeners["public"].IPs, checkACMEHosts)
+		public := c.Static.Listeners["public"]
+		ips := public.IPs
+		if len(public.NATIPs) > 0 {
+			ips = public.NATIPs
+		}
+		if public.IPsNATed {
+			ips = nil
+		}
+		m.SetAllowedHostnames(dns.StrictResolver{Pkg: "autotls"}, hostnames, ips, checkACMEHosts)
 	}
 }
 
@@ -629,6 +637,17 @@ func PrepareStaticConfig(ctx context.Context, configFile string, conf *Config, c
 			}
 			l.SMTP.DNSBLZones = append(l.SMTP.DNSBLZones, d)
 		}
+		if l.IPsNATed && len(l.NATIPs) > 0 {
+			addErrorf("listener %q has both IPsNATed and NATIPs (remove deprecated IPsNATed)", name)
+		}
+		for _, ipstr := range l.NATIPs {
+			ip := net.ParseIP(ipstr)
+			if ip == nil {
+				addErrorf("listener %q has invalid ip %q", name, ipstr)
+			} else if ip.IsUnspecified() || ip.IsLoopback() {
+				addErrorf("listener %q has NAT ip that is the unspecified or loopback address %s", name, ipstr)
+			}
+		}
 		checkPath := func(kind string, enabled bool, path string) {
 			if enabled && path != "" && !strings.HasPrefix(path, "/") {
 				addErrorf("listener %q has %s with path %q that must start with a slash", name, kind, path)

quickstart.go

@@ -463,7 +463,15 @@ listed in more DNS block lists, visit:
 configured to listen on 0.0.0.0 (IPv4) and :: (IPv6). If you don't change these
 to your actual public IP addresses, you will likely get "address in use" errors
 when starting mox because the "internal" listener binds to a specific IP
-address on the same port(s).
+address on the same port(s). If you are behind a NAT, instead configure the
+actual public IPs in the listener's "NATIPs" option.
+
+If you are behind a NAT that does not preserve the remote IPs of connections,
+you will likely experience problems accepting email due to IP-based policies.
+For example, SPF is a mechanism that checks if an IP address is allowed to send
+email for a domain, and mox uses IP-based (non)junk classification, and IP-based
+rate-limiting both for accepting email and blocking bad actors (such as with
+too many authentication failures).
 
 `)
 	}

webadmin/admin.go

@@ -455,13 +455,19 @@ func checkDomain(ctx context.Context, resolver dns.Resolver, dialer *net.Dialer,
 		}
 	}
 
-	// If at least one listener with SMTP enabled has specified NATed IPs, we'll skip
+	// If at least one listener with SMTP enabled has unspecified NATed IPs, we'll skip
 	// some checks related to these IPs.
-	var isNAT bool
+	var isNAT, isUnspecifiedNAT bool
 	for _, l := range mox.Conf.Static.Listeners {
-		if l.IPsNATed && l.SMTP.Enabled {
+		if !l.SMTP.Enabled {
+			continue
+		}
+		if l.IPsNATed {
+			isUnspecifiedNAT = true
+			isNAT = true
+		}
+		if len(l.NATIPs) > 0 {
 			isNAT = true
-			break
 		}
 	}
 
@@ -473,16 +479,17 @@ func checkDomain(ctx context.Context, resolver dns.Resolver, dialer *net.Dialer,
 		defer logPanic(ctx)
 		defer wg.Done()
 
-		// For each mox.Conf.SpecifiedSMTPListenIPs, and each address for
+		// For each mox.Conf.SpecifiedSMTPListenIPs and all NATIPs, and each IP for
 		// mox.Conf.HostnameDomain, check if they resolve back to the host name.
 		hostIPs := map[dns.Domain][]net.IP{}
 		ips, err := resolver.LookupIP(ctx, "ip", mox.Conf.Static.HostnameDomain.ASCII+".")
 		if err != nil {
 			addf(&r.IPRev.Errors, "Looking up IPs for hostname: %s", err)
 		}
-		if !isNAT {
+
+		gatherMoreIPs := func(publicIPs []net.IP) {
 		nextip:
-			for _, ip := range mox.Conf.Static.SpecifiedSMTPListenIPs {
+			for _, ip := range publicIPs {
 				for _, xip := range ips {
 					if ip.Equal(xip) {
 						continue nextip
@@ -491,6 +498,19 @@ func checkDomain(ctx context.Context, resolver dns.Resolver, dialer *net.Dialer,
 				ips = append(ips, ip)
 			}
 		}
+		if !isNAT {
+			gatherMoreIPs(mox.Conf.Static.SpecifiedSMTPListenIPs)
+		}
+		for _, l := range mox.Conf.Static.Listeners {
+			if !l.SMTP.Enabled {
+				continue
+			}
+			var natips []net.IP
+			for _, ip := range l.NATIPs {
+				natips = append(natips, net.ParseIP(ip))
+			}
+			gatherMoreIPs(natips)
+		}
 		hostIPs[mox.Conf.Static.HostnameDomain] = ips
 
 		iplist := func(ips []net.IP) string {
@@ -598,7 +618,7 @@ func checkDomain(ctx context.Context, resolver dns.Resolver, dialer *net.Dialer,
 				addf(&r.MX.Errors, "Looking up IPs for mx host %q: %s", mx.Host, err)
 			}
 			r.MX.Records[i].IPs = ips
-			if isNAT {
+			if isUnspecifiedNAT {
 				continue
 			}
 			if len(ourIPs) == 0 {
@@ -765,7 +785,11 @@ func checkDomain(ctx context.Context, resolver dns.Resolver, dialer *net.Dialer,
 				if !l.SMTP.Enabled || l.IPsNATed {
 					continue
 				}
-				for _, ipstr := range l.IPs {
+				ips := l.IPs
+				if len(l.NATIPs) > 0 {
+					ips = l.NATIPs
+				}
+				for _, ipstr := range ips {
 					ip := net.ParseIP(ipstr)
 					checkSPFIP(ip)
 				}
@@ -1135,7 +1159,7 @@ When enabling MTA-STS, or updating a policy, always update the policy first (thr
 		}
 
 		r.Autoconf.IPs = ips
-		if !isNAT {
+		if !isUnspecifiedNAT {
 			if len(ourIPs) == 0 {
 				addf(&r.Autoconf.Errors, "Autoconfig does not point to one of our IPs.")
 			} else if len(notOurIPs) > 0 {
@@ -1171,7 +1195,7 @@ When enabling MTA-STS, or updating a policy, always update the policy first (thr
 			}
 			match = true
 			r.Autodiscover.Records = append(r.Autodiscover.Records, AutodiscoverSRV{*srv, ips})
-			if !isNAT {
+			if !isUnspecifiedNAT {
 				if len(ourIPs) == 0 {
 					addf(&r.Autodiscover.Errors, "SRV target %q does not point to our IPs.", srv.Target)
 				} else if len(notOurIPs) > 0 {