Skip to content

Commit 2415554

Browse files
hperlclaude
authored andcommitted
fix: enforce a DNS-label boundary on wildcard return URLs and CORS origins
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> GitOrigin-RevId: d7d270e36899919f596f7d0789d1114338bfa0ac
1 parent 150e16f commit 2415554

4 files changed

Lines changed: 110 additions & 7 deletions

File tree

driver/configuration/provider_koanf_public_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,10 @@ func TestKoanfProvider(t *testing.T) {
167167
assert.True(t, p.CORSEnabled("proxy"))
168168
assert.True(t, p.CORSEnabled("api"))
169169

170+
proxyCORS := p.CORSOptions("proxy")
171+
// configx sets AllowOriginVaryRequestFunc (boundary-safe matching); a
172+
// func value is not comparable, so drop it before the struct equality.
173+
proxyCORS.AllowOriginVaryRequestFunc = nil
170174
assert.Equal(t, cors.Options{
171175
AllowedOrigins: []string{"https://example.com", "https://*.example.com"},
172176
AllowedMethods: []string{"POST", "GET", "PUT", "PATCH", "DELETE"},
@@ -176,8 +180,10 @@ func TestKoanfProvider(t *testing.T) {
176180
AllowCredentials: true,
177181
OptionsPassthrough: false,
178182
Debug: true,
179-
}, p.CORSOptions("proxy"))
183+
}, proxyCORS)
180184

185+
apiCORS := p.CORSOptions("api")
186+
apiCORS.AllowOriginVaryRequestFunc = nil
181187
assert.Equal(t, cors.Options{
182188
AllowedOrigins: []string{"https://example.org", "https://*.example.org"},
183189
AllowedMethods: []string{"GET", "PUT", "PATCH", "DELETE"},
@@ -187,7 +193,7 @@ func TestKoanfProvider(t *testing.T) {
187193
AllowCredentials: true,
188194
OptionsPassthrough: false,
189195
Debug: true,
190-
}, p.CORSOptions("api"))
196+
}, apiCORS)
191197
})
192198

193199
t.Run("group=tls", func(t *testing.T) {

oryx/configx/cors.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ package configx
55

66
import (
77
_ "embed"
8+
"net/http"
89

10+
"github.com/ory/x/corsx"
911
"github.com/rs/cors"
1012
)
1113

@@ -16,8 +18,13 @@ var CORSConfigSchema []byte
1618

1719
func (p *Provider) CORS(prefix string, defaults cors.Options) (cors.Options, bool) {
1820
prefix = cleanPrefix(prefix)
19-
21+
allowedOrigins := p.StringsF(prefix+"cors.allowed_origins", defaults.AllowedOrigins)
22+
allowInsecureOrigins := p.BoolF("feature_flags.legacy_allow_insecure_origins", false)
2023
return cors.Options{
24+
// Populated even though rs/cors ignores it once AllowOriginVaryRequestFunc
25+
// is set: some consumers read AllowedOrigins directly — notably hydra's
26+
// oauth2cors.Middleware (which builds its own matcher and treats an empty
27+
// list as "allow all", a CORS bypass) and oathkeeper's address detection.
2128
AllowedOrigins: p.StringsF(prefix+"cors.allowed_origins", defaults.AllowedOrigins),
2229
AllowedMethods: p.StringsF(prefix+"cors.allowed_methods", defaults.AllowedMethods),
2330
AllowedHeaders: p.StringsF(prefix+"cors.allowed_headers", defaults.AllowedHeaders),
@@ -26,5 +33,8 @@ func (p *Provider) CORS(prefix string, defaults cors.Options) (cors.Options, boo
2633
OptionsPassthrough: p.BoolF(prefix+"cors.options_passthrough", defaults.OptionsPassthrough),
2734
MaxAge: p.IntF(prefix+"cors.max_age", defaults.MaxAge),
2835
Debug: p.BoolF(prefix+"cors.debug", defaults.Debug),
36+
AllowOriginVaryRequestFunc: func(_ *http.Request, origin string) (bool, []string) {
37+
return corsx.CheckOrigin(allowedOrigins, origin, allowInsecureOrigins), nil
38+
},
2939
}, p.Bool(prefix + "cors.enabled")
3040
}

oryx/corsx/check_origin.go

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,25 @@
33

44
package corsx
55

6-
import "strings"
6+
import (
7+
"strings"
8+
9+
"golang.org/x/net/publicsuffix"
10+
)
711

812
// CheckOrigin is a function that can be used well with cors.Options.AllowOriginRequestFunc.
913
// It checks whether the origin is allowed following the same behavior as github.com/rs/cors.
1014
//
15+
// When legacyAllowInsecureOrigins is false (the default), wildcard patterns are
16+
// only honored when ClassifyOrigin reports them as bounded at a registrable
17+
// domain. Pass true to opt into legacy (trusting) behavior for unbounded
18+
// wildcards.
19+
//
20+
// TODO: legacyAllowInsecureOrigins grandfathers a fixed set of projects through
21+
// a time-boxed migration window (feature_flags.legacy_allow_insecure_origins).
22+
// Once those projects move to bounded wildcards and the entitlement is revoked,
23+
// drop this parameter and always enforce the boundary.
24+
//
1125
// Recommended usage for hot-reloadable origins:
1226
//
1327
// func (p *Config) cors(ctx context.Context, prefix string) (cors.Options, bool) {
@@ -20,11 +34,11 @@ import "strings"
2034
// opts.AllowOriginRequestFunc = func(r *http.Request, origin string) bool {
2135
// // load the origins from the config on every request to allow hot-reloading
2236
// allowedOrigins := p.GetProvider(r.Context()).Strings(prefix + ".cors.allowed_origins")
23-
// return corsx.CheckOrigin(allowedOrigins, origin)
37+
// return corsx.CheckOrigin(allowedOrigins, origin, false)
2438
// }
2539
// return opts, enabled
2640
// }
27-
func CheckOrigin(allowedOrigins []string, origin string) bool {
41+
func CheckOrigin(allowedOrigins []string, origin string, legacyAllowInsecureOrigins bool) bool {
2842
if len(allowedOrigins) == 0 {
2943
return true
3044
}
@@ -45,10 +59,83 @@ func CheckOrigin(allowedOrigins []string, origin string) bool {
4559
}
4660
continue
4761
}
62+
// Only honor wildcards bounded at a registrable domain unless the caller
63+
// explicitly opts into legacy insecure matching. See ClassifyOrigin.
64+
if !legacyAllowInsecureOrigins && ClassifyOrigin(o).IsUnsafeWildcard() {
65+
continue
66+
}
4867
// inspired by https://github.com/rs/cors/blob/066574eebbd0f5f1b6cd1154a160cc292ac1835e/utils.go#L15
4968
if len(origin) >= len(prefix)+len(suffix) && strings.HasPrefix(origin, prefix) && strings.HasSuffix(origin, suffix) {
5069
return true
5170
}
5271
}
5372
return false
5473
}
74+
75+
// OriginPattern describes a CORS origin or return-URL host pattern: whether it
76+
// uses a wildcard and, if so, whether that wildcard is safely bounded at a
77+
// registrable domain.
78+
type OriginPattern struct {
79+
// HasWildcard reports whether the pattern contains a "*".
80+
HasWildcard bool
81+
82+
// BoundedWildcard reports whether the "*" is confined to a subdomain label
83+
// and the fixed domain that follows it is a registrable domain (an eTLD+1,
84+
// e.g. "example.com" or "example.co.uk"). Every host the pattern can match
85+
// then shares that one customer-owned registrable domain, so an attacker
86+
// cannot register a matching host. Always false when HasWildcard is false.
87+
BoundedWildcard bool
88+
89+
// Base is the fixed domain that follows the wildcard label — "example.com"
90+
// for "*.example.com", "com" for "*.com". It is empty for non-wildcards and
91+
// for bare or trailing wildcards where no domain follows the "*". When
92+
// BoundedWildcard is false, Base names the offending suffix, which is the
93+
// actionable signal for reporting why a wildcard was rejected.
94+
Base string
95+
}
96+
97+
// IsUnsafeWildcard reports whether the pattern is a wildcard that is NOT bounded
98+
// at a registrable domain. Such a wildcard would match an attacker-registrable
99+
// host (e.g. "https://*foo.com" matches "https://evilfoo.com"), so it must be
100+
// rejected unless the caller explicitly opts into legacy insecure matching. This
101+
// is the dominant question at call sites that gate, drop, or reject wildcard
102+
// origins and return URLs.
103+
func (p OriginPattern) IsUnsafeWildcard() bool {
104+
return p.HasWildcard && !p.BoundedWildcard
105+
}
106+
107+
// ClassifyOrigin inspects a CORS origin or bare host pattern and reports whether
108+
// it is a wildcard and, if so, whether the wildcard is safely bounded at a
109+
// registrable domain. Only the text from the last "*" onward is inspected, so
110+
// the result is identical whether pattern carries a scheme or is a bare host; a
111+
// trailing ":port" is ignored. Examples:
112+
//
113+
// - "https://*.example.com" → {HasWildcard: true, BoundedWildcard: true, Base: "example.com"}
114+
// - "https://*foo.com" → {HasWildcard: true, Base: "com"} (dot-less; base is a public suffix)
115+
// - "https://*.com" → {HasWildcard: true, Base: "com"} (public suffix, not registrable)
116+
// - "https://www.ory.*" → {HasWildcard: true} (trailing; no domain follows)
117+
// - "https://exact.foo.com" → {} (no wildcard)
118+
func ClassifyOrigin(pattern string) OriginPattern {
119+
i := strings.LastIndexByte(pattern, '*')
120+
if i < 0 {
121+
return OriginPattern{}
122+
}
123+
p := OriginPattern{HasWildcard: true}
124+
// The fixed domain is everything after the first "." that follows the last
125+
// "*" — i.e. the label containing the wildcard is dropped. Without such a dot
126+
// the wildcard is trailing or bare, so no registrable domain follows.
127+
_, base, found := strings.Cut(pattern[i:], ".")
128+
if !found {
129+
return p
130+
}
131+
base = strings.ToLower(base)
132+
if host, _, found := strings.Cut(base, ":"); found {
133+
base = host // Drop a trailing ":port".
134+
}
135+
p.Base = base
136+
// EffectiveTLDPlusOne returns an error when base is itself a public suffix
137+
// (e.g. "com", "co.uk", "vercel.app") or otherwise has no registrable domain.
138+
_, err := publicsuffix.EffectiveTLDPlusOne(base)
139+
p.BoundedWildcard = err == nil
140+
return p
141+
}

oryx/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ require (
8585
go.uber.org/goleak v1.3.0
8686
go.uber.org/mock v0.5.2
8787
golang.org/x/crypto v0.53.0
88+
golang.org/x/net v0.55.0
8889
golang.org/x/oauth2 v0.36.0
8990
golang.org/x/sync v0.21.0
9091
google.golang.org/grpc v1.81.1
@@ -192,7 +193,6 @@ require (
192193
go.yaml.in/yaml/v2 v2.4.4 // indirect
193194
golang.org/x/exp v0.0.0-20251023183803-a4bb9ffd2546 // indirect
194195
golang.org/x/mod v0.36.0 // indirect
195-
golang.org/x/net v0.55.0 // indirect
196196
golang.org/x/sys v0.46.0 // indirect
197197
golang.org/x/telemetry v0.0.0-20260508192327-42602be52be6 // indirect
198198
golang.org/x/text v0.38.0 // indirect

0 commit comments

Comments
 (0)