From dedc90f455208d6f41e2c8ec9c7e08b7620aceb3 Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Sun, 5 Mar 2023 16:22:23 +0100 Subject: [PATCH] at startup, with acme, if the config has explicitly configured public ips (the default with the quickstart), lookup the host names allowed for acme validation and warn about ips that mox is not configured to listen on i've seen this cause acme validation failures 3 times now, so give a hint in the logs to new users. also for issue #13. --- autotls/autotls.go | 46 +++++++++++++++++++++++++++++++++++++++-- autotls/autotls_test.go | 4 ++-- config/config.go | 2 +- config/doc.go | 4 +++- mox-/config.go | 8 +------ 5 files changed, 51 insertions(+), 13 deletions(-) diff --git a/autotls/autotls.go b/autotls/autotls.go index 54dbb25..391a4fb 100644 --- a/autotls/autotls.go +++ b/autotls/autotls.go @@ -22,11 +22,13 @@ import ( "errors" "fmt" "io" + "net" "os" "path/filepath" "sort" "strings" "sync" + "time" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" @@ -182,7 +184,11 @@ func Load(name, acmeDir, contactEmail, directoryURL string, shutdown <-chan stru } // SetAllowedHostnames sets a new list of allowed hostnames for automatic TLS. -func (m *Manager) SetAllowedHostnames(hostnames map[dns.Domain]struct{}) { +// After setting the host names, a goroutine is start to check that new host names +// are fully served by publicIPs (only if non-empty and there is no unspecified +// address in the list). If no, log an error with a warning that ACME validation +// may fail. +func (m *Manager) SetAllowedHostnames(resolver dns.Resolver, hostnames map[dns.Domain]struct{}, publicIPs []string) { m.Lock() defer m.Unlock() @@ -195,8 +201,44 @@ func (m *Manager) SetAllowedHostnames(hostnames map[dns.Domain]struct{}) { return l[i].Name() < l[j].Name() }) - xlog.Debug("autotls setting allowed hostnames", mlog.Field("hostnames", l)) + xlog.Debug("autotls setting allowed hostnames", mlog.Field("hostnames", l), mlog.Field("publicips", publicIPs)) + var added []dns.Domain + for h := range hostnames { + if _, ok := m.hosts[h]; !ok { + added = append(added, h) + } + } m.hosts = hostnames + + if len(added) > 0 && len(publicIPs) > 0 { + for _, ip := range publicIPs { + if net.ParseIP(ip).IsUnspecified() { + return + } + } + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + publicIPstrs := map[string]struct{}{} + for _, ip := range publicIPs { + publicIPstrs[ip] = struct{}{} + } + + for _, h := range added { + ips, err := resolver.LookupIP(ctx, "ip", h.ASCII+".") + if err != nil { + xlog.Errorx("warning: acme tls cert validation for host may fail due to dns lookup error", err, mlog.Field("host", h)) + continue + } + for _, ip := range ips { + if _, ok := publicIPstrs[ip.String()]; !ok { + xlog.Error("warning: acme tls cert validation for host is likely to fail because not all its ips are being listened on", mlog.Field("hostname", h), mlog.Field("listenedips", publicIPs), mlog.Field("hostips", ips), mlog.Field("missingip", ip)) + } + } + } + }() + } } // Hostnames returns the allowed host names for use with ACME. diff --git a/autotls/autotls_test.go b/autotls/autotls_test.go index 3ea3cfb..21636a2 100644 --- a/autotls/autotls_test.go +++ b/autotls/autotls_test.go @@ -28,7 +28,7 @@ func TestAutotls(t *testing.T) { if err := m.HostPolicy(context.Background(), "mox.example"); err == nil || !errors.Is(err, errHostNotAllowed) { t.Fatalf("hostpolicy, got err %v, expected errHostNotAllowed", err) } - m.SetAllowedHostnames(map[dns.Domain]struct{}{{ASCII: "mox.example"}: {}}) + m.SetAllowedHostnames(dns.StrictResolver{}, map[dns.Domain]struct{}{{ASCII: "mox.example"}: {}}, nil) l = m.Hostnames() if !reflect.DeepEqual(l, []dns.Domain{{ASCII: "mox.example"}}) { t.Fatalf("hostnames, got %v, expected single mox.example", l) @@ -79,7 +79,7 @@ func TestAutotls(t *testing.T) { t.Fatalf("private key changed after reload") } m.shutdown = make(chan struct{}) - m.SetAllowedHostnames(map[dns.Domain]struct{}{{ASCII: "mox.example"}: {}}) + m.SetAllowedHostnames(dns.StrictResolver{}, map[dns.Domain]struct{}{{ASCII: "mox.example"}: {}}, nil) if err := m.HostPolicy(context.Background(), "mox.example"); err != nil { t.Fatalf("hostpolicy, got err %v, expected no error", err) } diff --git a/config/config.go b/config/config.go index 5128a10..521c3f3 100644 --- a/config/config.go +++ b/config/config.go @@ -47,7 +47,7 @@ type Static struct { } `sconf:"optional" sconf-doc:"Global TLS configuration, e.g. for additional Certificate Authorities."` ACME map[string]ACME `sconf:"optional" sconf-doc:"Automatic TLS configuration with ACME, e.g. through Let's Encrypt. The key is a name referenced in TLS configs, e.g. letsencrypt."` AdminPasswordFile string `sconf:"optional" sconf-doc:"File containing hash of admin password, for authentication in the web admin pages (if enabled)."` - Listeners map[string]Listener `sconf-doc:"Listeners are groups of IP addresses and services enabled on those IP addresses, such as SMTP/IMAP or internal endpoints for administration or Prometheus metrics. All listeners with SMTP/IMAP services enabled will serve all configured domains."` + Listeners map[string]Listener `sconf-doc:"Listeners are groups of IP addresses and services enabled on those IP addresses, such as SMTP/IMAP or internal endpoints for administration or Prometheus metrics. All listeners with SMTP/IMAP services enabled will serve all configured domains. If the listener is named 'public', it will get a few helpful additional configuration checks, for acme automatic tls certificates and monitoring of ips in dnsbls if those are configured."` Postmaster struct { Account string Mailbox string `sconf-doc:"E.g. Postmaster or Inbox."` diff --git a/config/doc.go b/config/doc.go index 86be847..408e1e8 100644 --- a/config/doc.go +++ b/config/doc.go @@ -89,7 +89,9 @@ describe-static" and "mox config describe-domains": # Listeners are groups of IP addresses and services enabled on those IP addresses, # such as SMTP/IMAP or internal endpoints for administration or Prometheus # metrics. All listeners with SMTP/IMAP services enabled will serve all configured - # domains. + # domains. If the listener is named 'public', it will get a few helpful additional + # configuration checks, for acme automatic tls certificates and monitoring of ips + # in dnsbls if those are configured. Listeners: x: diff --git a/mox-/config.go b/mox-/config.go index 1dbb696..b43c157 100644 --- a/mox-/config.go +++ b/mox-/config.go @@ -227,12 +227,6 @@ func (c *Config) allowACMEHosts() { } else { hostnames[d] = struct{}{} } - - if d, err := dns.ParseDomain("autodiscover." + dom.Domain.ASCII); err != nil { - xlog.Errorx("parsing autodiscover domain", err, mlog.Field("domain", dom.Domain)) - } else { - hostnames[d] = struct{}{} - } } if l.MTASTSHTTPS.Enabled && dom.MTASTS != nil && !l.MTASTSHTTPS.NonTLS { @@ -254,7 +248,7 @@ func (c *Config) allowACMEHosts() { } } - m.SetAllowedHostnames(hostnames) + m.SetAllowedHostnames(dns.StrictResolver{Pkg: "autotls"}, hostnames, c.Static.Listeners["public"].IPs) } }