diff --git a/core/dnsserver/config.go b/core/dnsserver/config.go index 6e47fd8d2..a93bba698 100644 --- a/core/dnsserver/config.go +++ b/core/dnsserver/config.go @@ -70,6 +70,10 @@ type Config struct { // TLSConfig when listening for encrypted connections (gRPC, DNS-over-TLS). TLSConfig *tls.Config + // tlsConfigIdentity identifies dynamic TLS configurations that are known to + // represent the same listener-wide policy even after tls.Config.Clone. + tlsConfigIdentity *TLSConfigIdentity + // MaxQUICStreams defines the maximum number of concurrent QUIC streams for a QUIC server. // This is nil if not specified, allowing for a default to be used. MaxQUICStreams *int @@ -165,6 +169,22 @@ type Config struct { // FilterFunc is a function that filters requests from the Config type FilterFunc func(context.Context, *request.Request) bool +// TLSConfigIdentity is an opaque identity for equivalent dynamic TLS policies. +// Plugins should share one identity only when they can prove that independently +// constructed TLS configs are interchangeable on the same listener. +type TLSConfigIdentity struct { + _ byte +} + +// NewTLSConfigIdentity returns a new opaque TLS policy identity. +func NewTLSConfigIdentity() *TLSConfigIdentity { return &TLSConfigIdentity{} } + +// SetTLSConfigIdentity associates an opaque listener-wide policy identity with +// this config. +func (c *Config) SetTLSConfigIdentity(identity *TLSConfigIdentity) { + c.tlsConfigIdentity = identity +} + // keyForConfig builds a key for identifying the configs during setup time func keyForConfig(blocIndex int, blocKeyIndex int) string { return fmt.Sprintf("%d:%d", blocIndex, blocKeyIndex) diff --git a/core/dnsserver/register.go b/core/dnsserver/register.go index 15a4e2d33..f7efd27fa 100644 --- a/core/dnsserver/register.go +++ b/core/dnsserver/register.go @@ -298,8 +298,10 @@ func propagateConfigParams(configs []*Config) { c.Stacktrace = c.firstConfigInBlock.Stacktrace c.NumSockets = c.firstConfigInBlock.NumSockets - // Fork TLSConfig for each encrypted connection + // Fork TLSConfig for each encrypted connection while preserving the + // listener-wide policy identity used to compare dynamic callbacks. c.TLSConfig = c.firstConfigInBlock.TLSConfig.Clone() + c.tlsConfigIdentity = c.firstConfigInBlock.tlsConfigIdentity c.ReadTimeout = c.firstConfigInBlock.ReadTimeout c.WriteTimeout = c.firstConfigInBlock.WriteTimeout c.IdleTimeout = c.firstConfigInBlock.IdleTimeout diff --git a/core/dnsserver/server_grpc.go b/core/dnsserver/server_grpc.go index 03102f380..9f3b667e6 100644 --- a/core/dnsserver/server_grpc.go +++ b/core/dnsserver/server_grpc.go @@ -50,18 +50,13 @@ type ServergRPC struct { // NewServergRPC returns a new CoreDNS GRPC server and compiles all plugin in to it. func NewServergRPC(addr string, group []*Config) (*ServergRPC, error) { - s, err := NewServer(addr, group) + tlsConfig, err := sharedTLSConfig(addr, group) if err != nil { return nil, err } - // The *tls* plugin must make sure that multiple conflicting - // TLS configuration returns an error: it can only be specified once. - var tlsConfig *tls.Config - for _, z := range s.zones { - for _, conf := range z { - // Should we error if some configs *don't* have TLS? - tlsConfig = conf.TLSConfig - } + s, err := NewServer(addr, group) + if err != nil { + return nil, err } // http/2 is required when using gRPC. We need to specify it in next protos // or the upgrade won't happen. diff --git a/core/dnsserver/server_https.go b/core/dnsserver/server_https.go index 414d0d22f..5d43dfe15 100644 --- a/core/dnsserver/server_https.go +++ b/core/dnsserver/server_https.go @@ -58,18 +58,13 @@ type HTTPRequestKey struct{} // NewServerHTTPS returns a new CoreDNS HTTPS server and compiles all plugins in to it. func NewServerHTTPS(addr string, group []*Config) (*ServerHTTPS, error) { - s, err := NewServer(addr, group) + tlsConfig, err := sharedTLSConfig(addr, group) if err != nil { return nil, err } - // The *tls* plugin must make sure that multiple conflicting - // TLS configuration returns an error: it can only be specified once. - var tlsConfig *tls.Config - for _, z := range s.zones { - for _, conf := range z { - // Should we error if some configs *don't* have TLS? - tlsConfig = conf.TLSConfig - } + s, err := NewServer(addr, group) + if err != nil { + return nil, err } // http/2 is recommended when using DoH. We need to specify it in next protos diff --git a/core/dnsserver/server_https3.go b/core/dnsserver/server_https3.go index a996c3bcc..b22440198 100644 --- a/core/dnsserver/server_https3.go +++ b/core/dnsserver/server_https3.go @@ -77,18 +77,15 @@ func (l *limitQUICListener) Accept(ctx context.Context) (*quic.Conn, error) { // NewServerHTTPS3 builds the HTTP/3 (DoH3) server. func NewServerHTTPS3(addr string, group []*Config) (*ServerHTTPS3, error) { + tlsConfig, err := sharedTLSConfig(addr, group) + if err != nil { + return nil, err + } s, err := NewServer(addr, group) if err != nil { return nil, err } - // Extract TLS config (CoreDNS guarantees it is consistent) - var tlsConfig *tls.Config - for _, z := range s.zones { - for _, conf := range z { - tlsConfig = conf.TLSConfig - } - } if tlsConfig == nil { return nil, fmt.Errorf("DoH3 requires TLS, no TLS config found") } diff --git a/core/dnsserver/server_quic.go b/core/dnsserver/server_quic.go index 07d4334af..438702e0b 100644 --- a/core/dnsserver/server_quic.go +++ b/core/dnsserver/server_quic.go @@ -64,18 +64,13 @@ type ServerQUIC struct { // NewServerQUIC returns a new CoreDNS QUIC server and compiles all plugin in to it. func NewServerQUIC(addr string, group []*Config) (*ServerQUIC, error) { - s, err := NewServer(addr, group) + tlsConfig, err := sharedTLSConfig(addr, group) if err != nil { return nil, err } - // The *tls* plugin must make sure that multiple conflicting - // TLS configuration returns an error: it can only be specified once. - var tlsConfig *tls.Config - for _, z := range s.zones { - for _, conf := range z { - // Should we error if some configs *don't* have TLS? - tlsConfig = conf.TLSConfig - } + s, err := NewServer(addr, group) + if err != nil { + return nil, err } if tlsConfig != nil { diff --git a/core/dnsserver/server_tls.go b/core/dnsserver/server_tls.go index c9b7342bc..8bf6682d7 100644 --- a/core/dnsserver/server_tls.go +++ b/core/dnsserver/server_tls.go @@ -22,18 +22,13 @@ type ServerTLS struct { // NewServerTLS returns a new CoreDNS TLS server and compiles all plugin in to it. func NewServerTLS(addr string, group []*Config) (*ServerTLS, error) { - s, err := NewServer(addr, group) + tlsConfig, err := sharedTLSConfig(addr, group) if err != nil { return nil, err } - // The *tls* plugin must make sure that multiple conflicting - // TLS configuration returns an error: it can only be specified once. - var tlsConfig *tls.Config - for _, z := range s.zones { - for _, conf := range z { - // Should we error if some configs *don't* have TLS? - tlsConfig = conf.TLSConfig - } + s, err := NewServer(addr, group) + if err != nil { + return nil, err } return &ServerTLS{Server: s, tlsConfig: tlsConfig}, nil diff --git a/core/dnsserver/server_tls_test.go b/core/dnsserver/server_tls_test.go index d52471dea..1f9227a51 100644 --- a/core/dnsserver/server_tls_test.go +++ b/core/dnsserver/server_tls_test.go @@ -4,6 +4,7 @@ import ( "crypto/tls" "errors" "net" + "strings" "testing" "time" @@ -29,6 +30,101 @@ func (l *stubListener) Addr() net.Addr { return &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0} } +func TestNewServerTLSRejectsConflictingSharedTLSConfig(t *testing.T) { + certificateA := tls.Certificate{Certificate: [][]byte{[]byte("certificate-a")}} + certificateB := tls.Certificate{Certificate: [][]byte{[]byte("certificate-b")}} + + config := func(zone string, tlsConfig *tls.Config) *Config { + c := testConfig("tls", testPlugin{}) + c.Zone = zone + c.TLSConfig = tlsConfig + return c + } + + tests := []struct { + name string + first *tls.Config + second *tls.Config + wantErr string + }{ + { + name: "client authentication", + first: &tls.Config{ + Certificates: []tls.Certificate{certificateA}, + ClientAuth: tls.RequireAndVerifyClientCert, + }, + second: &tls.Config{ + Certificates: []tls.Certificate{certificateA}, + ClientAuth: tls.NoClientCert, + }, + wantErr: "client authentication policies differ", + }, + { + name: "TLS and plaintext", + first: &tls.Config{ + Certificates: []tls.Certificate{certificateA}, + }, + second: nil, + wantErr: "TLS is configured for only one server block", + }, + { + name: "server certificate", + first: &tls.Config{ + Certificates: []tls.Certificate{certificateA}, + }, + second: &tls.Config{ + Certificates: []tls.Certificate{certificateB}, + }, + wantErr: "server certificates differ", + }, + { + name: "dynamic TLS callbacks", + first: &tls.Config{ + GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) { return nil, nil }, + }, + second: &tls.Config{ + GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) { return nil, nil }, + }, + wantErr: "dynamic TLS callbacks differ", + }, + { + name: "matching policies", + first: &tls.Config{ + Certificates: []tls.Certificate{certificateA}, + ClientAuth: tls.RequireAndVerifyClientCert, + MinVersion: tls.VersionTLS12, + }, + second: &tls.Config{ + Certificates: []tls.Certificate{certificateA}, + ClientAuth: tls.RequireAndVerifyClientCert, + MinVersion: tls.VersionTLS12, + }, + wantErr: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := NewServerTLS("tls://127.0.0.1:0", []*Config{ + config("a.example.test.", tt.first), + config("b.example.test.", tt.second), + }) + if tt.wantErr == "" { + if err != nil { + t.Fatalf("NewServerTLS() failed: %v", err) + } + return + } + if err == nil { + t.Fatalf("NewServerTLS() succeeded, want error containing %q", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("NewServerTLS() error = %q, want substring %q", err, tt.wantErr) + } + }) + } +} + func TestServerTLSSetsTsigSecret(t *testing.T) { server, err := NewServerTLS("tls://127.0.0.1:0", []*Config{testConfig("tls", testPlugin{})}) if err != nil { diff --git a/core/dnsserver/tlsconfig.go b/core/dnsserver/tlsconfig.go new file mode 100644 index 000000000..b2c87e8c8 --- /dev/null +++ b/core/dnsserver/tlsconfig.go @@ -0,0 +1,132 @@ +package dnsserver + +import ( + "bytes" + "crypto/tls" + "crypto/x509" + "fmt" + "slices" +) + +// sharedTLSConfig returns the TLS configuration for a listener shared by group. +// TLS is negotiated before CoreDNS knows which zone will handle the DNS request, +// so listener-wide identity and client-authentication settings must agree. +func sharedTLSConfig(addr string, group []*Config) (*tls.Config, error) { + if len(group) == 0 { + return nil, nil + } + + first := group[0] + if first == nil { + return nil, fmt.Errorf("nil config for shared listener %s", addr) + } + + for _, conf := range group[1:] { + if conf == nil { + return nil, fmt.Errorf("nil config for shared listener %s", addr) + } + if err := compatibleTLSConfig(first, conf); err != nil { + return nil, fmt.Errorf("conflicting TLS configuration for shared listener %s between zones %q and %q: %w", addr, first.Zone, conf.Zone, err) + } + } + + if first.TLSConfig == nil { + return nil, nil + } + return first.TLSConfig.Clone(), nil +} + +func sameServerBlock(a, b *Config) bool { + aFirst := a.firstConfigInBlock + if aFirst == nil { + aFirst = a + } + bFirst := b.firstConfigInBlock + if bFirst == nil { + bFirst = b + } + return aFirst == bFirst +} + +func compatibleTLSConfig(aConfig, bConfig *Config) error { + a := aConfig.TLSConfig + b := bConfig.TLSConfig + if a == nil || b == nil { + if a == b { + return nil + } + return fmt.Errorf("TLS is configured for only one server block") + } + + if a.ClientAuth != b.ClientAuth { + return fmt.Errorf("client authentication policies differ") + } + if !certPoolsEqual(a.ClientCAs, b.ClientCAs) { + return fmt.Errorf("client CA pools differ") + } + if !certificatesEqual(a.Certificates, b.Certificates) { + return fmt.Errorf("server certificates differ") + } + + // Dynamic callbacks cannot be compared directly. Allow them only when the + // configs came from the same server block, are the same config object, or a + // plugin supplied a trusted identity proving the policies are equivalent. + trustedDynamicPolicy := sameServerBlock(aConfig, bConfig) || a == b || + (aConfig.tlsConfigIdentity != nil && aConfig.tlsConfigIdentity == bConfig.tlsConfigIdentity) + if !trustedDynamicPolicy && (a.GetCertificate != nil || b.GetCertificate != nil || + a.GetConfigForClient != nil || b.GetConfigForClient != nil || + a.VerifyPeerCertificate != nil || b.VerifyPeerCertificate != nil || + a.VerifyConnection != nil || b.VerifyConnection != nil) { + return fmt.Errorf("dynamic TLS callbacks differ") + } + + if a.MinVersion != b.MinVersion || a.MaxVersion != b.MaxVersion { + return fmt.Errorf("TLS version policies differ") + } + if !slices.Equal(a.CipherSuites, b.CipherSuites) { + return fmt.Errorf("cipher suite policies differ") + } + if !slices.Equal(a.CurvePreferences, b.CurvePreferences) { + return fmt.Errorf("curve preference policies differ") + } + if a.SessionTicketsDisabled != b.SessionTicketsDisabled { + return fmt.Errorf("session ticket policies differ") + } + + return nil +} + +func certPoolsEqual(a, b *x509.CertPool) bool { + if a == nil || b == nil { + return a == b + } + return a.Equal(b) +} + +func certificatesEqual(a, b []tls.Certificate) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if len(a[i].Certificate) != len(b[i].Certificate) { + return false + } + for j := range a[i].Certificate { + if !bytes.Equal(a[i].Certificate[j], b[i].Certificate[j]) { + return false + } + } + if !bytes.Equal(a[i].OCSPStaple, b[i].OCSPStaple) { + return false + } + if len(a[i].SignedCertificateTimestamps) != len(b[i].SignedCertificateTimestamps) { + return false + } + for j := range a[i].SignedCertificateTimestamps { + if !bytes.Equal(a[i].SignedCertificateTimestamps[j], b[i].SignedCertificateTimestamps[j]) { + return false + } + } + } + return true +} diff --git a/plugin/tls/acme.go b/plugin/tls/acme.go index 41133c5b4..6c5e936ea 100644 --- a/plugin/tls/acme.go +++ b/plugin/tls/acme.go @@ -244,8 +244,9 @@ type acmeBackend interface { type acmeBackendFactory func([]*acmeEntry, *acmeDNS01Solver) (acmeBackend, error) type acmeEntry struct { - options acmeOptions - key acmeConfigKey + options acmeOptions + key acmeConfigKey + tlsConfigIdentity *dnsserver.TLSConfigIdentity mu sync.RWMutex manager certificateManager @@ -330,7 +331,11 @@ func (r *acmeRuntime) add(options acmeOptions) (*acmeEntry, error) { } } - entry := &acmeEntry{options: options, key: key} + entry := &acmeEntry{ + options: options, + key: key, + tlsConfigIdentity: dnsserver.NewTLSConfigIdentity(), + } r.entries[key] = entry for _, domain := range options.domains { r.domainOwners[domain] = key diff --git a/plugin/tls/acme_test.go b/plugin/tls/acme_test.go index f40011956..1d932d1d3 100644 --- a/plugin/tls/acme_test.go +++ b/plugin/tls/acme_test.go @@ -72,6 +72,34 @@ func TestParseACMETLS(t *testing.T) { } } +func TestEquivalentACMEConfigsShareTLSListener(t *testing.T) { + root := t.TempDir() + runtime := newACMERuntime(nil) + + config := func(zone string) *dnsserver.Config { + c := caddy.NewTestController("dns", `tls { + acme dns.example + }`) + c.Set(acmeRuntimeStorageKey{}, runtime) + cfg := dnsserver.GetConfig(c) + cfg.Root = root + cfg.Zone = zone + if err := setup(c); err != nil { + t.Fatalf("setup %s failed: %v", zone, err) + } + return cfg + } + + first := config("a.example.") + second := config("b.example.") + if first.TLSConfig == second.TLSConfig { + t.Fatal("equivalent ACME entries unexpectedly reused the same tls.Config") + } + if _, err := dnsserver.NewServerTLS("tls://127.0.0.1:0", []*dnsserver.Config{first, second}); err != nil { + t.Fatalf("equivalent ACME configs rejected: %v", err) + } +} + func TestACMEDirectiveOrder(t *testing.T) { indexes := make(map[string]int) for i, directive := range dnsserver.Directives { diff --git a/plugin/tls/tls.go b/plugin/tls/tls.go index 929511834..30a0ae057 100644 --- a/plugin/tls/tls.go +++ b/plugin/tls/tls.go @@ -190,6 +190,7 @@ func parseACMETLS(c *caddy.Controller, config *dnsserver.Config) (*ctls.Config, return nil, err } runtime.installChallengeHandlers(c) + config.SetTLSConfigIdentity(entry.tlsConfigIdentity) return entry.tlsConfig(), nil }