caddytls: Fix handling of IP-only TLS configs and empty-SNI handshakes

caddy/caddymain/run.go

@@ -46,6 +46,7 @@ func init() {
 
 	flag.BoolVar(&certmagic.Agreed, "agree", false, "Agree to the CA's Subscriber Agreement")
 	flag.StringVar(&certmagic.CA, "ca", certmagic.CA, "URL to certificate authority's ACME server directory")
+	flag.StringVar(&certmagic.DefaultServerName, "default-sni", certmagic.DefaultServerName, "If a ClientHello ServerName is empty, use this ServerName to choose a TLS certificate")
 	flag.BoolVar(&certmagic.DisableHTTPChallenge, "disable-http-challenge", certmagic.DisableHTTPChallenge, "Disable the ACME HTTP challenge")
 	flag.BoolVar(&certmagic.DisableTLSALPNChallenge, "disable-tls-alpn-challenge", certmagic.DisableTLSALPNChallenge, "Disable the ACME TLS-ALPN challenge")
 	flag.StringVar(&disabledMetrics, "disabled-metrics", "", "Comma-separated list of telemetry metrics to disable")
@@ -108,7 +109,7 @@ func Run() {
 		}
 	}
 
-	//Load all additional envs as soon as possible
+	// load all additional envs as soon as possible
 	if err := LoadEnvFromFile(envFile); err != nil {
 		mustLogFatalf("%v", err)
 	}

caddytls/config.go

@@ -93,7 +93,7 @@ type Config struct {
 }
 
 // NewConfig returns a new Config with a pointer to the instance's
-// certificate cache. You will usually need to set Other fields on
+// certificate cache. You will usually need to set other fields on
 // the returned Config for successful practical use.
 func NewConfig(inst *caddy.Instance) *Config {
 	inst.StorageMu.RLock()
@@ -257,9 +257,13 @@ func MakeTLSConfig(configs []*Config) (*tls.Config, error) {
 		// configs with the same hostname pattern; should
 		// be OK since we already asserted they are roughly
 		// the same); during TLS handshakes, configs are
-		// loaded based on the hostname pattern, according
-		// to client's SNI
-		configMap[cfg.Hostname] = cfg
+		// loaded based on the hostname pattern according
+		// to client's ServerName (SNI) value
+		if cfg.Hostname == "0.0.0.0" || cfg.Hostname == "::" {
+			configMap[""] = cfg
+		} else {
+			configMap[cfg.Hostname] = cfg
+		}
 	}
 
 	// Is TLS disabled? By now, we know that all

caddytls/handshake.go

@@ -17,6 +17,8 @@ package caddytls
 import (
 	"crypto/tls"
 	"fmt"
+	"log"
+	"net"
 	"strings"
 
 	"github.com/mholt/caddy/telemetry"
@@ -38,14 +40,31 @@ type configGroup map[string]*Config
 // This function follows nearly the same logic to lookup
 // a hostname as the getCertificate function uses.
 func (cg configGroup) getConfig(hello *tls.ClientHelloInfo) *Config {
-	name := certmagic.CertNameFromClientHello(hello)
+	name := certmagic.NormalizedName(hello.ServerName)
+	if name == "" {
+		name = certmagic.NormalizedName(certmagic.DefaultServerName)
+	}
+
+	// if SNI is empty, prefer matching IP address (it is
+	// more specific than a "catch-all" configuration)
+	if name == "" && hello.Conn != nil {
+		addr := hello.Conn.LocalAddr().String()
+		ip, _, err := net.SplitHostPort(addr)
+		if err == nil {
+			addr = ip
+		}
+		if config, ok := cg[addr]; ok {
+			return config
+		}
+	}
 
-	// exact match? great, let's use it
+	// otherwise, try an exact match
 	if config, ok := cg[name]; ok {
 		return config
 	}
 
-	// try replacing labels in the name with wildcards until we get a match
+	// then try replacing labels in the name with
+	// wildcards until we get a match
 	labels := strings.Split(name, ".")
 	for i := range labels {
 		labels[i] = "*"
@@ -55,14 +74,25 @@ func (cg configGroup) getConfig(hello *tls.ClientHelloInfo) *Config {
 		}
 	}
 
-	// try a config that serves all names (the above
-	// loop doesn't try empty string; for hosts defined
-	// with only a port, for instance, like ":443") -
-	// also known as the default config
+	// try a config that matches all names - this
+	// is needed to match configs defined without
+	// a specific host, like ":443", when SNI is
+	// a non-empty value
 	if config, ok := cg[""]; ok {
 		return config
 	}
 
+	// failover with a random config: this is necessary
+	// because we might be needing to solve a TLS-ALPN
+	// ACME challenge for a name that we don't have a
+	// TLS configuration for; any config will do for
+	// this purpose
+	for _, config := range cg {
+		return config
+	}
+
+	log.Printf("[ERROR] No TLS configuration available for ClientHello with ServerName: %s", hello.ServerName)
+
 	return nil
 }
 

caddytls/setup.go

@@ -54,18 +54,6 @@ func setupTLS(c *caddy.Controller) error {
 
 	config.Enabled = true
 
-	// a single certificate cache is used by the whole caddy.Instance; get a pointer to it
-	certCache, ok := c.Get(CertCacheInstStorageKey).(*certmagic.Cache)
-	if !ok || certCache == nil {
-		certCache = certmagic.NewCache(certmagic.DefaultStorage)
-		c.OnShutdown(func() error {
-			certCache.Stop()
-			return nil
-		})
-		c.Set(CertCacheInstStorageKey, certCache)
-	}
-	config.Manager = certmagic.NewWithCache(certCache, certmagic.Config{})
-
 	// we use certmagic events to collect metrics for telemetry
 	config.Manager.OnEvent = func(event string, data interface{}) {
 		switch event {

caddytls/setup_test.go

@@ -47,11 +47,18 @@ func TestMain(m *testing.M) {
 }
 
 func TestSetupParseBasic(t *testing.T) {
-	cfg := &Config{Manager: &certmagic.Config{}}
+	tmpdir, err := ioutil.TempDir("", "caddytls_setup_test_")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpdir)
+
+	certCache := certmagic.NewCache(&certmagic.FileStorage{Path: tmpdir})
+	cfg := &Config{Manager: certmagic.NewWithCache(certCache, certmagic.Config{})}
 	RegisterConfigGetter("", func(c *caddy.Controller) *Config { return cfg })
 	c := caddy.NewTestController("", `tls `+certFile+` `+keyFile+``)
 
-	err := setupTLS(c)
+	err = setupTLS(c)
 	if err != nil {
 		t.Errorf("Expected no errors, got: %v", err)
 	}
@@ -126,11 +133,18 @@ func TestSetupParseWithOptionalParams(t *testing.T) {
             alpn http/1.1
         }`
 
-	cfg := &Config{Manager: &certmagic.Config{}}
+	tmpdir, err := ioutil.TempDir("", "caddytls_setup_test_")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpdir)
+
+	certCache := certmagic.NewCache(&certmagic.FileStorage{Path: tmpdir})
+	cfg := &Config{Manager: certmagic.NewWithCache(certCache, certmagic.Config{})}
 	RegisterConfigGetter("", func(c *caddy.Controller) *Config { return cfg })
 	c := caddy.NewTestController("", params)
 
-	err := setupTLS(c)
+	err = setupTLS(c)
 	if err != nil {
 		t.Errorf("Expected no errors, got: %v", err)
 	}