From bddc8e406284029bbdc842f3453e97ba4db9dd0c Mon Sep 17 00:00:00 2001
From: Mechiel Lukkien <mechiel@ueber.net>
Date: Fri, 10 Mar 2023 17:55:37 +0100
Subject: [PATCH] also configure acme validation with http-01, and fix a bug
 that caused tls cert refresh at startup to not always run

we already do acme tls-alpn-01 validation, and still require it (we could relax
this at some point). http-01 is easy to add.

the bug was that the list of acme managers and hosts to refresh was overwritten
by another listener. the listeners are a map, and we range over it, so the
order we handle them is random. if the public listener was handled first, and
an internal handler later, the list was reset again.
---
 autotls/autotls.go      | 14 +++++++++-----
 autotls/autotls_test.go |  3 +++
 http/web.go             | 26 ++++++++++++++++----------
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/autotls/autotls.go b/autotls/autotls.go
index 12be5eb..74cd726 100644
--- a/autotls/autotls.go
+++ b/autotls/autotls.go
@@ -2,11 +2,9 @@
 // requesting certificates with ACME, typically from Let's Encrypt.
 package autotls
 
-// We only do tls-alpn-01. For http-01, we would have to start another
-// listener. For DNS we would need a third party tool with an API that can make
-// the DNS changes, as we don't want to link in dozens of bespoke API's for DNS
-// record manipulation into mox. We can do http-01 relatively easily. It could
-// be useful to not depend on a single mechanism.
+// We do tls-alpn-01, and also http-01. For DNS we would need a third party tool
+// with an API that can make the DNS changes, as we don't want to link in dozens of
+// bespoke API's for DNS record manipulation into mox.
 
 import (
 	"bytes"
@@ -272,6 +270,12 @@ func (m *Manager) HostPolicy(ctx context.Context, host string) (rerr error) {
 	default:
 	}
 
+	xhost, _, err := net.SplitHostPort(host)
+	if err == nil {
+		// For http-01, host may include a port number.
+		host = xhost
+	}
+
 	d, err := dns.ParseDomain(host)
 	if err != nil {
 		return fmt.Errorf("invalid host: %v", err)
diff --git a/autotls/autotls_test.go b/autotls/autotls_test.go
index 6cd179a..8c5f406 100644
--- a/autotls/autotls_test.go
+++ b/autotls/autotls_test.go
@@ -36,6 +36,9 @@ func TestAutotls(t *testing.T) {
 	if err := m.HostPolicy(context.Background(), "mox.example"); err != nil {
 		t.Fatalf("hostpolicy, got err %v, expected no error", err)
 	}
+	if err := m.HostPolicy(context.Background(), "mox.example:80"); err != nil {
+		t.Fatalf("hostpolicy, got err %v, expected no error", err)
+	}
 	if err := m.HostPolicy(context.Background(), "other.mox.example"); err == nil || !errors.Is(err, errHostNotAllowed) {
 		t.Fatalf("hostpolicy, got err %v, expected errHostNotAllowed", err)
 	}
diff --git a/http/web.go b/http/web.go
index afe557a..4b98b79 100644
--- a/http/web.go
+++ b/http/web.go
@@ -189,7 +189,7 @@ type pathHandler struct {
 	Handle    http.HandlerFunc
 }
 type serve struct {
-	Kinds        []string // Type of handler and protocol (http/https).
+	Kinds        []string // Type of handler and protocol (e.g. acme-tls-alpn-01, account-http, admin-https).
 	TLSConfig    *tls.Config
 	PathHandlers []pathHandler // Sorted, longest first.
 	Webserver    bool          // Whether serving WebHandler. PathHandlers are always evaluated before WebHandlers.
@@ -319,7 +319,7 @@ func Listen() {
 
 		if l.TLS != nil && l.TLS.ACME != "" && (l.SMTP.Enabled && !l.SMTP.NoSTARTTLS || l.Submissions.Enabled || l.IMAPS.Enabled) {
 			port := config.Port(mox.Conf.Static.ACME[l.TLS.ACME].Port, 443)
-			ensureServe(true, port, "acme-tls-alpn01")
+			ensureServe(true, port, "acme-tls-alpn-01")
 		}
 
 		if l.AccountHTTP.Enabled {
@@ -405,16 +405,16 @@ func Listen() {
 			srv.Webserver = true
 		}
 
-		// We'll explicitly ensure these TLS certs exist (e.g. are created with ACME)
-		// immediately after startup. We only do so for our explicit listener hostnames,
-		// not for mta-sts DNS records, it can be requested on demand (perhaps never). We
-		// do request autoconfig, otherwise clients may run into their timeouts waiting for
-		// the certificate to be given during the first https connection.
-		ensureManagerHosts = map[*autotls.Manager]map[dns.Domain]struct{}{}
-
 		if l.TLS != nil && l.TLS.ACME != "" {
 			m := mox.Conf.Static.ACME[l.TLS.ACME].Manager
 
+			// If we are listening on port 80 for plain http, also register acme http-01
+			// validation handler.
+			if srv, ok := portServe[80]; ok && srv.TLSConfig == nil {
+				srv.Kinds = append(srv.Kinds, "acme-http-01")
+				srv.HandleFunc("acme-http-01", nil, "/.well-known/acme-challenge/", m.Manager.HTTPHandler(nil).ServeHTTP)
+			}
+
 			hosts := map[dns.Domain]struct{}{
 				mox.Conf.Static.HostnameDomain: {},
 			}
@@ -467,7 +467,13 @@ func adminIndex(w http.ResponseWriter, r *http.Request) {
 
 // functions to be launched in goroutine that will serve on a listener.
 var servers []func()
-var ensureManagerHosts map[*autotls.Manager]map[dns.Domain]struct{}
+
+// We'll explicitly ensure these TLS certs exist (e.g. are created with ACME)
+// immediately after startup. We only do so for our explicit listener hostnames,
+// not for mta-sts DNS records, it can be requested on demand (perhaps never). We
+// do request autoconfig, otherwise clients may run into their timeouts waiting for
+// the certificate to be given during the first https connection.
+var ensureManagerHosts = map[*autotls.Manager]map[dns.Domain]struct{}{}
 
 // listen prepares a listener, and adds it to "servers", to be launched (if not running as root) through Serve.
 func listen1(ip string, port int, tlsConfig *tls.Config, name string, kinds []string, handler http.Handler) {