From 55d05c6beaba3e14c9931475629f5cd359be4bbf Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Fri, 11 Aug 2023 10:13:17 +0200 Subject: [PATCH] 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 | 3 ++- config/doc.go | 12 +++++++++--- mox-/admin.go | 9 +++++++-- mox-/config.go | 21 ++++++++++++++++++++- quickstart.go | 10 +++++++++- webadmin/admin.go | 46 +++++++++++++++++++++++++++++++++++----------- 6 files changed, 82 insertions(+), 19 deletions(-) diff --git a/config/config.go b/config/config.go index 31aa19a..ee07e16 100644 --- a/config/config.go +++ b/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. diff --git a/config/doc.go b/config/doc.go index cb9a601..59b577a 100644 --- a/config/doc.go +++ b/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) diff --git a/mox-/admin.go b/mox-/admin.go index 1e47df2..7a44eba 100644 --- a/mox-/admin.go +++ b/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 { diff --git a/mox-/config.go b/mox-/config.go index 96dbf67..1767288 100644 --- a/mox-/config.go +++ b/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) diff --git a/quickstart.go b/quickstart.go index faf0bb9..49f2e6a 100644 --- a/quickstart.go +++ b/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). `) } diff --git a/webadmin/admin.go b/webadmin/admin.go index f6b0869..0f4a24d 100644 --- a/webadmin/admin.go +++ b/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 {