diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index 47448f925..60d851fec 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/caddyconfig/httpcaddyfile/httptype.go @@ -25,6 +25,7 @@ import ( "strconv" "strings" + "github.com/caddyserver/certmagic" "go.uber.org/zap" "github.com/caddyserver/caddy/v2" @@ -292,7 +293,8 @@ func (st ServerType) Setup( if filesystems, ok := options["filesystem"].(caddy.Module); ok { cfg.AppsRaw["caddy.filesystems"] = caddyconfig.JSON( filesystems, - &warnings) + &warnings, + ) } if storageCvtr, ok := options["storage"].(caddy.StorageConverter); ok { @@ -719,6 +721,7 @@ func (st *ServerType) serversFromPairings( }) var hasCatchAllTLSConnPolicy, addressQualifiesForTLS bool + var emptyConnPolicies caddytls.ConnectionPolicies autoHTTPSWillAddConnPolicy := srv.AutoHTTPS == nil || !srv.AutoHTTPS.Disabled // if needed, the ServerLogConfig is initialized beforehand so @@ -824,8 +827,25 @@ func (st *ServerType) serversFromPairings( if !cp.SettingsEmpty() || mapContains(forceAutomatedNames, hosts) { srv.TLSConnPolicies = append(srv.TLSConnPolicies, cp) hasCatchAllTLSConnPolicy = len(hosts) == 0 + } else if len(hosts) > 0 { + emptyConnPolicies = append(emptyConnPolicies, cp) } } + } else if specificHosts := slices.DeleteFunc( + slices.Clone(hosts), + func(h string) bool { return h == "" || strings.Contains(h, "*") }, + ); len(specificHosts) > 0 { + // site blocks with no TLS connection policy of their own may + // still need an empty policy hoisted above a wildcard policy + // that requires client authentication, or that requirement + // would apply to their more specific hostnames too - see the + // hoisting loop below and issue #7860 + slices.Sort(specificHosts) + emptyConnPolicies = append(emptyConnPolicies, &caddytls.ConnectionPolicy{ + MatchersRaw: caddy.ModuleMap{ + "sni": caddyconfig.JSON(specificHosts, warnings), + }, + }) } for _, addr := range sblock.parsedKeys { @@ -876,7 +896,8 @@ func (st *ServerType) serversFromPairings( listenerWrapper, "wrapper", listenerWrapper.(caddy.Module).CaddyModule().ID.Name(), - warnings) + warnings, + ) srv.ListenerWrappersRaw = append(srv.ListenerWrappersRaw, jsonListenerWrapper) } @@ -890,7 +911,8 @@ func (st *ServerType) serversFromPairings( packetConnWrapper, "wrapper", packetConnWrapper.(caddy.Module).CaddyModule().ID.Name(), - warnings) + warnings, + ) srv.PacketConnWrappersRaw = append(srv.PacketConnWrappersRaw, jsonPacketConnWrapper) } @@ -990,6 +1012,72 @@ func (st *ServerType) serversFromPairings( return nil, err } + // hostnames whose site blocks configure no TLS settings of their own + // still need shielding if a policy matching a wildcard hostname would + // otherwise impose CLIENT AUTHENTICATION on them. Because connection + // policies are first-match, the shield hoisted above the wildcard + // policy must be a COPY of that policy with only client_authentication + // removed: an empty policy would not just lift the client-auth + // requirement but suppress every other setting the wildcard policy + // carries (certificate selection, protocol bounds, ALPN, ...), which + // the covered hostname does want to inherit - see issue #7860 + // the wildcard names each client-auth policy matches, decoded once and + // index-aligned with srv.TLSConnPolicies; nil for a policy that needs + // no client auth or matches no wildcard + clientAuthWildcards := make([][]string, len(srv.TLSConnPolicies)) + for i, cp := range srv.TLSConnPolicies { + if cp.ClientAuthentication == nil { + continue + } + names, ok := sniNames(cp, "of existing connection policy ", warnings) + if !ok { + continue + } + clientAuthWildcards[i] = slices.DeleteFunc(names, func(name string) bool { + return !strings.Contains(name, "*") + }) + } + + // each covering policy needs its OWN shield matching only the + // hostnames it covers: the hostnames of a single site block may be + // covered by DIFFERENT wildcards, and a shield hoisted above one + // policy must not match the hostnames belonging to another, or it + // would lift their client-auth requirement too + shieldedHosts := make([][]string, len(srv.TLSConnPolicies)) + for _, ecp := range emptyConnPolicies { + hosts, ok := sniNames(ecp, "", warnings) + if !ok { + continue // no SNI matcher to reason about + } + for _, host := range hosts { + // connection policies are first-match, so only the first + // covering policy is ever reached for this hostname + for i, wildcards := range clientAuthWildcards { + if slices.ContainsFunc(wildcards, func(name string) bool { + return certmagic.MatchWildcard(host, name) + }) { + shieldedHosts[i] = append(shieldedHosts[i], host) + break + } + } + } + } + + // hoist the shields last to last, so that inserting one does not + // shift the index of a covering policy still to be shielded + for i, hosts := range slices.Backward(shieldedHosts) { + if len(hosts) == 0 { + continue + } + slices.Sort(hosts) + shield := *srv.TLSConnPolicies[i] + shield.ClientAuthentication = nil + shield.MatchersRaw = caddy.ModuleMap{ + "sni": caddyconfig.JSON(slices.Compact(hosts), warnings), + } + srv.TLSConnPolicies = slices.Insert(srv.TLSConnPolicies, i, &shield) + } + // a catch-all TLS conn policy is necessary to ensure TLS can // be offered to all hostnames of the server; even though only // one policy is needed to enable TLS for the server, that @@ -1028,6 +1116,25 @@ func (st *ServerType) serversFromPairings( return servers, nil } +// sniNames returns the server names a connection policy's sni matcher matches. +// The bool is false when the policy has no sni matcher, or when it does not +// decode - the latter is unexpected enough to warn about rather than silently +// skip, since callers use it to decide whether a hostname needs shielding. +func sniNames(cp *caddytls.ConnectionPolicy, what string, warnings *[]caddyconfig.Warning) ([]string, bool) { + raw, ok := cp.MatchersRaw["sni"] + if !ok { + return nil, false + } + var sni caddytls.MatchServerName + if err := json.Unmarshal(raw, &sni); err != nil { + *warnings = append(*warnings, caddyconfig.Warning{ + Message: fmt.Sprintf("decoding sni matcher %swhile checking wildcard coverage: %v", what, err), + }) + return nil, false + } + return sni, true +} + func detectConflictingSchemes(srv *caddyhttp.Server, serverBlocks []serverBlock, options map[string]any) error { httpPort := strconv.Itoa(caddyhttp.DefaultHTTPPort) if hp, ok := options["http_port"].(int); ok { diff --git a/caddytest/integration/caddyfile_adapt/tls_client_auth_wildcard_not_inherited_by_specific_host.caddyfiletest b/caddytest/integration/caddyfile_adapt/tls_client_auth_wildcard_not_inherited_by_specific_host.caddyfiletest new file mode 100644 index 000000000..2dfb1a3e4 --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/tls_client_auth_wildcard_not_inherited_by_specific_host.caddyfiletest @@ -0,0 +1,107 @@ +*.example.com { + tls { + client_auth { + mode require_and_verify + trust_pool file { + pem_file ../caddy.ca.cer + } + } + } + respond "wildcard" +} + +public.example.com { + respond "public" +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "public.example.com" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "public", + "handler": "static_response" + } + ] + } + ] + } + ], + "terminal": true + }, + { + "match": [ + { + "host": [ + "*.example.com" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "wildcard", + "handler": "static_response" + } + ] + } + ] + } + ], + "terminal": true + } + ], + "tls_connection_policies": [ + { + "match": { + "sni": [ + "public.example.com" + ] + } + }, + { + "match": { + "sni": [ + "*.example.com" + ] + }, + "client_authentication": { + "ca": { + "pem_files": [ + "../caddy.ca.cer" + ], + "provider": "file" + }, + "mode": "require_and_verify" + } + }, + {} + ] + } + } + } + } +} diff --git a/caddytest/integration/caddyfile_adapt/tls_client_auth_wildcard_shield_preserves_other_settings.caddyfiletest b/caddytest/integration/caddyfile_adapt/tls_client_auth_wildcard_shield_preserves_other_settings.caddyfiletest new file mode 100644 index 000000000..7dd3063bb --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/tls_client_auth_wildcard_shield_preserves_other_settings.caddyfiletest @@ -0,0 +1,121 @@ +*.example.com { + tls { + protocols tls1.2 tls1.3 + alpn h2 http/1.1 + client_auth { + mode require_and_verify + trust_pool file { + pem_file ../caddy.ca.cer + } + } + } + respond "wildcard" +} + +public.example.com { + respond "public" +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "public.example.com" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "public", + "handler": "static_response" + } + ] + } + ] + } + ], + "terminal": true + }, + { + "match": [ + { + "host": [ + "*.example.com" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "wildcard", + "handler": "static_response" + } + ] + } + ] + } + ], + "terminal": true + } + ], + "tls_connection_policies": [ + { + "match": { + "sni": [ + "public.example.com" + ] + }, + "alpn": [ + "h2", + "http/1.1" + ], + "protocol_min": "tls1.2", + "protocol_max": "tls1.3" + }, + { + "match": { + "sni": [ + "*.example.com" + ] + }, + "alpn": [ + "h2", + "http/1.1" + ], + "protocol_min": "tls1.2", + "protocol_max": "tls1.3", + "client_authentication": { + "ca": { + "pem_files": [ + "../caddy.ca.cer" + ], + "provider": "file" + }, + "mode": "require_and_verify" + } + }, + {} + ] + } + } + } + } +} diff --git a/caddytest/integration/caddyfile_adapt/tls_client_auth_wildcards_shield_each_covering_policy.caddyfiletest b/caddytest/integration/caddyfile_adapt/tls_client_auth_wildcards_shield_each_covering_policy.caddyfiletest new file mode 100644 index 000000000..1a23949a6 --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/tls_client_auth_wildcards_shield_each_covering_policy.caddyfiletest @@ -0,0 +1,168 @@ +*.example.com { + tls { + client_auth { + mode require_and_verify + trust_pool file { + pem_file ../caddy.ca.cer + } + } + } + respond "example wildcard" +} + +*.other.com { + tls { + client_auth { + mode require + trust_pool file { + pem_file ../caddy.ca.cer + } + } + } + respond "other wildcard" +} + +public.example.com, public.other.com { + respond "public" +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":443" + ], + "routes": [ + { + "match": [ + { + "host": [ + "public.example.com", + "public.other.com" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "public", + "handler": "static_response" + } + ] + } + ] + } + ], + "terminal": true + }, + { + "match": [ + { + "host": [ + "*.example.com" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "example wildcard", + "handler": "static_response" + } + ] + } + ] + } + ], + "terminal": true + }, + { + "match": [ + { + "host": [ + "*.other.com" + ] + } + ], + "handle": [ + { + "handler": "subroute", + "routes": [ + { + "handle": [ + { + "body": "other wildcard", + "handler": "static_response" + } + ] + } + ] + } + ], + "terminal": true + } + ], + "tls_connection_policies": [ + { + "match": { + "sni": [ + "public.example.com" + ] + } + }, + { + "match": { + "sni": [ + "*.example.com" + ] + }, + "client_authentication": { + "ca": { + "pem_files": [ + "../caddy.ca.cer" + ], + "provider": "file" + }, + "mode": "require_and_verify" + } + }, + { + "match": { + "sni": [ + "public.other.com" + ] + } + }, + { + "match": { + "sni": [ + "*.other.com" + ] + }, + "client_authentication": { + "ca": { + "pem_files": [ + "../caddy.ca.cer" + ], + "provider": "file" + }, + "mode": "require" + } + }, + {} + ] + } + } + } + } +} diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index 47c09ee12..d8077463b 100644 --- a/modules/caddyhttp/server.go +++ b/modules/caddyhttp/server.go @@ -436,8 +436,7 @@ func (s *Server) provisionUnderscoreHeaders() error { return nil } var err error - s.underscoreExactAllow, s.underscoreExactDrop, s.underscorePrefixRules, err = - provisionHeaderAliasAllowlist(s.ExpectedUnderscoreHeaders, '_', "expected_underscore_headers") + s.underscoreExactAllow, s.underscoreExactDrop, s.underscorePrefixRules, err = provisionHeaderAliasAllowlist(s.ExpectedUnderscoreHeaders, '_', "expected_underscore_headers") return err } @@ -449,8 +448,7 @@ func (s *Server) provisionDotHeaders() error { return nil } var err error - s.dotExactAllow, s.dotExactDrop, s.dotPrefixRules, err = - provisionHeaderAliasAllowlist(s.ExpectedDotHeaders, '.', "expected_dot_headers") + s.dotExactAllow, s.dotExactDrop, s.dotPrefixRules, err = provisionHeaderAliasAllowlist(s.ExpectedDotHeaders, '.', "expected_dot_headers") return err } @@ -905,7 +903,7 @@ func (s *Server) findLastRouteWithHostMatcher() int { for i, route := range s.Routes { // since we want to break out of an inner loop, use a closure // to allow us to use 'return' when we found a host matcher - found := (func() bool { + found := func() bool { for _, sets := range route.MatcherSets { for _, matcher := range sets { switch matcher.(type) { @@ -916,7 +914,7 @@ func (s *Server) findLastRouteWithHostMatcher() int { } } return false - })() + }() // if we found the host matcher, change the lastIndex to // just after the current route @@ -1164,7 +1162,8 @@ func (s *Server) logRequest( fieldCount := 6 fields = make([]zapcore.Field, 0, fieldCount+len(extra.fields)) - fields = append(fields, + fields = append( + fields, zap.Int("bytes_read", reqBodyLength), zap.String("user_id", userID), zap.Duration("duration", *duration),