for incoming smtp deliveries with starttls, use cert of hostname if s…

…ni hostname is unknown

instead of failing the connection because no certificates are available.

this may improve interoperability. perhaps the remote smtp client that's doing
the delivery will decide they do like the tls cert for our (mx) hostname after
all.

this only applies to incoming smtp deliveries. for other tls connections
(https, imaps/submissions and imap/submission with starttls) we still cause
connections for unknown sni hostnames to fail. if case no sni was present, we
were already falling back to a cert for the (listener/mx) hostname, that
behaviour hasn't changed.

for issue #206 by RobSlgm

autotls/autotls.go

@@ -54,7 +54,6 @@ var (
 // certificates for allowlisted hosts.
 type Manager struct {
 	ACMETLSConfig *tls.Config // For serving HTTPS on port 443, which is required for certificate requests to succeed.
-	TLSConfig     *tls.Config // For all TLS servers not used for validating ACME requests. Like SMTP and IMAP (including with STARTTLS) and HTTPS on ports other than 443.
 	Manager       *autocert.Manager
 
 	shutdown <-chan struct{}
@@ -158,50 +157,79 @@ func Load(name, acmeDir, contactEmail, directoryURL string, eabKeyID string, eab
 		}
 	}
 
-	loggingGetCertificate := func(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
-		log := mlog.New("autotls", nil).WithContext(hello.Context())
-
-		// We handle missing invalid hostnames/ip's by returning a nil certificate and nil
-		// error, which crypto/tls turns into a TLS alert "unrecognized name", which can be
-		// interpreted by clients as a hint that they are using the wrong hostname, or a
-		// certificate is missing.
-
-		// Handle missing SNI to prevent logging an error below.
-		// At startup, during config initialization, we already adjust the tls config to
-		// inject the listener hostname if there isn't one in the TLS client hello. This is
-		// common for SMTP STARTTLS connections, which often do not care about the
-		// verification of the certificate.
-		if hello.ServerName == "" {
-			log.Debug("tls request without sni servername, rejecting", slog.Any("localaddr", hello.Conn.LocalAddr()), slog.Any("supportedprotos", hello.SupportedProtos))
-			return nil, nil
-		}
+	a := &Manager{
+		Manager:  m,
+		shutdown: shutdown,
+		hosts:    map[dns.Domain]struct{}{},
+	}
+	m.HostPolicy = a.HostPolicy
+	acmeTLSConfig := *m.TLSConfig()
+	acmeTLSConfig.GetCertificate = func(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
+		return a.loggingGetCertificate(hello, dns.Domain{}, false, false)
+	}
+	a.ACMETLSConfig = &acmeTLSConfig
+	return a, nil
+}
+
+// logigngGetCertificate is a helper to implement crypto/tls.Config.GetCertificate,
+// optionally falling back to a certificate for fallbackHostname in case SNI is
+// absent or for an unknown hostname.
+func (m *Manager) loggingGetCertificate(hello *tls.ClientHelloInfo, fallbackHostname dns.Domain, fallbackNoSNI, fallbackUnknownSNI bool) (*tls.Certificate, error) {
+	log := mlog.New("autotls", nil).WithContext(hello.Context())
+
+	// If we can't find a certificate (depending on fallback parameters), we return a
+	// nil certificate and nil error, which crypto/tls turns into a TLS alert
+	// "unrecognized name", which can be interpreted by clients as a hint that they are
+	// using the wrong hostname, or a certificate is missing.
+
+	if hello.ServerName == "" && fallbackNoSNI {
+		hello.ServerName = fallbackHostname.ASCII
+	}
+
+	// Handle missing SNI to prevent logging an error below.
+	if hello.ServerName == "" {
+		log.Debug("tls request without sni servername, rejecting", slog.Any("localaddr", hello.Conn.LocalAddr()), slog.Any("supportedprotos", hello.SupportedProtos))
+		return nil, nil
+	}
 
-		cert, err := m.GetCertificate(hello)
-		if err != nil && errors.Is(err, errHostNotAllowed) {
+	cert, err := m.Manager.GetCertificate(hello)
+	if err != nil && errors.Is(err, errHostNotAllowed) {
+		if !fallbackUnknownSNI {
 			log.Debugx("requesting certificate", err, slog.String("host", hello.ServerName))
 			return nil, nil
-		} else if err != nil {
-			log.Errorx("requesting certificate", err, slog.String("host", hello.ServerName))
 		}
-		return cert, err
-	}
-
-	acmeTLSConfig := *m.TLSConfig()
-	acmeTLSConfig.GetCertificate = loggingGetCertificate
 
-	tlsConfig := tls.Config{
-		GetCertificate: loggingGetCertificate,
+		log.Debug("certificate for unknown hostname, using fallback hostname", slog.String("host", hello.ServerName))
+		hello.ServerName = fallbackHostname.ASCII
+		cert, err = m.Manager.GetCertificate(hello)
+		if err != nil {
+			log.Errorx("requesting certificate for fallback hostname", err, slog.String("host", hello.ServerName))
+		} else {
+			log.Debugx("requesting certificate for fallback hostname", err, slog.String("host", hello.ServerName))
+		}
+		return cert, err
+	} else if err != nil {
+		log.Errorx("requesting certificate", err, slog.String("host", hello.ServerName))
 	}
+	return cert, err
+}
 
-	a := &Manager{
-		ACMETLSConfig: &acmeTLSConfig,
-		TLSConfig:     &tlsConfig,
-		Manager:       m,
-		shutdown:      shutdown,
-		hosts:         map[dns.Domain]struct{}{},
+// TLSConfig returns a TLS server config that optionally returns a certificate for
+// fallbackHostname if no SNI was done, or for an unknown hostname.
+//
+// If fallbackNoSNI is set, TLS connections without SNI will use a certificate for
+// fallbackHostname. Otherwise, connections without SNI will fail with a message
+// that no TLS certificate is available.
+//
+// If fallbackUnknownSNI is set, TLS connections with an SNI hostname that is not
+// allowlisted will instead use a certificate for fallbackHostname. Otherwise, such
+// TLS connections will fail.
+func (m *Manager) TLSConfig(fallbackHostname dns.Domain, fallbackNoSNI, fallbackUnknownSNI bool) *tls.Config {
+	return &tls.Config{
+		GetCertificate: func(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
+			return m.loggingGetCertificate(hello, fallbackHostname, fallbackNoSNI, fallbackUnknownSNI)
+		},
 	}
-	m.HostPolicy = a.HostPolicy
-	return a, nil
 }
 
 // CertAvailable checks whether a non-expired ECDSA certificate is available in the

config/config.go

@@ -517,7 +517,8 @@ type TLS struct {
 	MinVersion          string    `sconf:"optional" sconf-doc:"Minimum TLS version. Default: TLSv1.2."`
 	HostPrivateKeyFiles []string  `sconf:"optional" sconf-doc:"Private keys used for ACME certificates. Specified explicitly so DANE TLSA DNS records can be generated, even before the certificates are requested. DANE is a mechanism to authenticate remote TLS certificates based on a public key or certificate specified in DNS, protected with DNSSEC. DANE is opportunistic and attempted when delivering SMTP with STARTTLS. The private key files must be in PEM format. PKCS8 is recommended, but PKCS1 and EC private keys are recognized as well. Only RSA 2048 bit and ECDSA P-256 keys are currently used. The first of each is used when requesting new certificates through ACME."`
 
-	Config                   *tls.Config     `sconf:"-" json:"-"` // TLS config for non-ACME-verification connections, i.e. SMTP and IMAP, and not port 443.
+	Config                   *tls.Config     `sconf:"-" json:"-"` // TLS config for non-ACME-verification connections, i.e. SMTP and IMAP, and not port 443. Connections without SNI will use a certificate for the hostname of the listener, connections with an SNI hostname that isn't allowed will be rejected.
+	ConfigFallback           *tls.Config     `sconf:"-" json:"-"` // Like Config, but uses the certificate for the listener hostname when the requested SNI hostname is not allowed, instead of causing the connection to fail.
 	ACMEConfig               *tls.Config     `sconf:"-" json:"-"` // TLS config that handles ACME verification, for serving on port 443.
 	HostPrivateRSA2048Keys   []crypto.Signer `sconf:"-" json:"-"` // Private keys for new TLS certificates for listener host name, for new certificates with ACME, and for DANE records.
 	HostPrivateECDSAP256Keys []crypto.Signer `sconf:"-" json:"-"`

mox-/config.go

@@ -709,28 +709,26 @@ func PrepareStaticConfig(ctx context.Context, log mlog.Log, configFile string, c
 
 				// If only checking or with missing ACME definition, we don't have an acme manager,
 				// so set an empty tls config to continue.
-				var tlsconfig *tls.Config
+				var tlsconfig, tlsconfigFallback *tls.Config
 				if checkOnly || acme.Manager == nil {
 					tlsconfig = &tls.Config{}
+					tlsconfigFallback = &tls.Config{}
 				} else {
-					tlsconfig = acme.Manager.TLSConfig.Clone()
-					l.TLS.ACMEConfig = acme.Manager.ACMETLSConfig
-
-					// SMTP STARTTLS connections are commonly made without SNI, because certificates
-					// often aren't verified.
 					hostname := c.HostnameDomain
 					if l.Hostname != "" {
 						hostname = l.HostnameDomain
 					}
-					getCert := tlsconfig.GetCertificate
-					tlsconfig.GetCertificate = func(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
-						if hello.ServerName == "" {
-							hello.ServerName = hostname.ASCII
-						}
-						return getCert(hello)
-					}
+					// If SNI is absent, we will use the listener hostname, but reject connections with
+					// an SNI hostname that is not allowlisted.
+					// Incoming SMTP deliveries use tlsconfigFallback for interoperability. TLS
+					// connections for unknown SNI hostnames fall back to a certificate for the
+					// listener hostname instead of causing the TLS connection to fail.
+					tlsconfig = acme.Manager.TLSConfig(hostname, true, false)
+					tlsconfigFallback = acme.Manager.TLSConfig(hostname, true, true)
+					l.TLS.ACMEConfig = acme.Manager.ACMETLSConfig
 				}
 				l.TLS.Config = tlsconfig
+				l.TLS.ConfigFallback = tlsconfigFallback
 			} else if len(l.TLS.KeyCerts) != 0 {
 				if doLoadTLSKeyCerts {
 					if err := loadTLSKeyCerts(configFile, "listener "+name, l.TLS); err != nil {
@@ -793,6 +791,9 @@ func PrepareStaticConfig(ctx context.Context, log mlog.Log, configFile string, c
 			if l.TLS.Config != nil {
 				l.TLS.Config.MinVersion = minVersion
 			}
+			if l.TLS.ConfigFallback != nil {
+				l.TLS.ConfigFallback.MinVersion = minVersion
+			}
 			if l.TLS.ACMEConfig != nil {
 				l.TLS.ACMEConfig.MinVersion = minVersion
 			}
@@ -1921,6 +1922,7 @@ func loadTLSKeyCerts(configFile, kind string, ctls *config.TLS) error {
 	ctls.Config = &tls.Config{
 		Certificates: certs,
 	}
+	ctls.ConfigFallback = ctls.Config
 	return nil
 }
 

smtpserver/server.go

@@ -190,9 +190,13 @@ func Listen() {
 	for _, name := range names {
 		listener := mox.Conf.Static.Listeners[name]
 
-		var tlsConfig *tls.Config
+		var tlsConfig, tlsConfigDelivery *tls.Config
 		if listener.TLS != nil {
 			tlsConfig = listener.TLS.Config
+			// For SMTP delivery, if we get a TLS handshake for an SNI hostname that we don't
+			// allow, we'll fallback to a certificate for the listener hostname instead of
+			// causing the connection to fail. May improve interoperability.
+			tlsConfigDelivery = listener.TLS.ConfigFallback
 		}
 
 		maxMsgSize := listener.SMTPMaxMessageSize
@@ -208,7 +212,7 @@ func Listen() {
 			port := config.Port(listener.SMTP.Port, 25)
 			for _, ip := range listener.IPs {
 				firstTimeSenderDelay := durationDefault(listener.SMTP.FirstTimeSenderDelay, firstTimeSenderDelayDefault)
-				listen1("smtp", name, ip, port, hostname, tlsConfig, false, false, maxMsgSize, false, listener.SMTP.RequireSTARTTLS, !listener.SMTP.NoRequireTLS, listener.SMTP.DNSBLZones, firstTimeSenderDelay)
+				listen1("smtp", name, ip, port, hostname, tlsConfigDelivery, false, false, maxMsgSize, false, listener.SMTP.RequireSTARTTLS, !listener.SMTP.NoRequireTLS, listener.SMTP.DNSBLZones, firstTimeSenderDelay)
 			}
 		}
 		if listener.Submission.Enabled {