diff --git a/docs/services/routing.md b/docs/services/routing.md index 9eb275e..d749cfa 100644 --- a/docs/services/routing.md +++ b/docs/services/routing.md @@ -229,6 +229,12 @@ This fragment belongs in `config/http.yaml`. List the frontend's exact origin; ` The rules surf applies to the origin answer: - `supports_credentials: true` together with `*` in `allowed_origins` fails the start-up. A credentialed answer that trusted any origin would let every site read responses made with the user's cookies; Laravel's php-cors reflects the origin in that case, surf refuses it. +- With `supports_credentials: true`, three more rules close other spellings of the same exposure, and each refusal names the key with its index, such as `http.cors.allowed_origins[1]`: + - an `allowed_origins` entry with a `*` is accepted only when the scheme is literal and the `*` is the whole leftmost host label, as in `https://*.example.com`; `https://*`, `*://*`, `https://*example.com` and `http*://www.example.com` fail the start-up; + - every `allowed_origins_patterns` entry must be anchored with `^` and `$`, as in `^https://[a-z]+\.example\.com$`; an unanchored pattern matches inside any origin, so `https://.*\.example\.com` would trust `https://x.example.com.attacker.net`; + - no origin rule may match an origin outside the configured domains: surf tries each rule against probe origins such as `https://cors-probe.invalid` and `null`, which refuses `https://*.com` and `^https?://.*$`. + + Without credentials these entries load as before. - An allowed origin is echoed exactly in `Access-Control-Allow-Origin`; `*` is sent only for `allowed_origins: ["*"]` without credentials, and that answer is unchanged from earlier releases. - When the answer depends on the request's `Origin` (with `supports_credentials`, with any pattern, or with more than one listed origin), every response on a CORS path carries `Vary: Origin`, the preflight and a disallowed origin's response included, so a shared cache never serves one origin's answer to another. - An `allowed_origins` entry with a `*` inside, such as `https://*.example.com`, matches like Laravel's `Str::is`: the `*` stands for any characters and the rest is literal. diff --git a/modules/surf/README.md b/modules/surf/README.md index 17bf773..d9019fa 100644 --- a/modules/surf/README.md +++ b/modules/surf/README.md @@ -154,8 +154,8 @@ The generated application `main` wires surf in with `surf.ServeCommand` and `sur | `http.body_limits.upload_bytes` | none, required | Upload body cap in bytes. Must be a whole number of at least 1; it is validated at boot. | | `http.trusted_proxies` | empty | List of CIDRs whose `X-Forwarded-For` header is trusted when resolving the client IP. Malformed entries are skipped. | | `http.cors.paths` | empty | Path globs (`api/*`) that get CORS headers. With no `http.cors` section no CORS headers are sent. | -| `http.cors.allowed_origins` | empty | Allowed origins, echoed exactly; `*` allows any (without credentials only). An entry with a `*` inside, such as `https://*.example.com`, matches any characters there, as Laravel's `Str::is` does. | -| `http.cors.allowed_origins_patterns` | empty | Go regular expressions matched against the origin. A pattern that does not compile, or one still wrapped in PHP delimiters (`#^...$#`), fails boot. | +| `http.cors.allowed_origins` | empty | Allowed origins, echoed exactly; `*` allows any (without credentials only). An entry with a `*` inside, such as `https://*.example.com`, matches any characters there, as Laravel's `Str::is` does. With credentials, such a `*` must be the whole leftmost host label after a literal scheme (`https://*.example.com`; `https://*` and `*://*` fail boot), and no entry may match an origin outside the configured domains. | +| `http.cors.allowed_origins_patterns` | empty | Go regular expressions matched against the origin. A pattern that does not compile, or one still wrapped in PHP delimiters (`#^...$#`), fails boot. With credentials, a pattern must be anchored with `^` and `$` and must not match an origin outside the configured domains (`^https?://.*$` fails boot). | | `http.cors.allowed_methods` | empty | Methods sent in `Access-Control-Allow-Methods`; `*` allows any. | | `http.cors.allowed_headers` | empty | Headers sent in `Access-Control-Allow-Headers`; `*` allows any. | | `http.cors.exposed_headers` | empty | Headers sent in `Access-Control-Expose-Headers`. | diff --git a/modules/surf/cors.go b/modules/surf/cors.go index 0d0dde3..659d422 100644 --- a/modules/surf/cors.go +++ b/modules/surf/cors.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "regexp" + "regexp/syntax" "strconv" "strings" @@ -25,7 +26,10 @@ type CORSConfig struct { // LoadCORSConfig reads http.cors. Missing section yields a zero config (no // path matches, so no CORS headers). The config is validated: credentials // with a "*" origin, an origin pattern that does not compile and a pattern -// still wrapped in PHP delimiters (#^...$#) are errors naming the key. +// still wrapped in PHP delimiters (#^...$#) are errors naming the key. With +// credentials, a wildcard origin must be the whole leftmost host label +// (https://*.example.com), a pattern must be anchored with ^ and $, and no +// origin rule may match an origin outside the configured domains. func LoadCORSConfig(cfg *compass.Config) (CORSConfig, error) { var out CORSConfig if cfg == nil || !cfg.Has("http.cors") { @@ -50,13 +54,111 @@ func validateCORSConfig(prefix string, cfg CORSConfig) error { if phpDelimited(p) { return fmt.Errorf("surf: %s.allowed_origins_patterns[%d] %q is wrapped in PHP delimiters; drop the leading and trailing %q (and any flags): Go patterns take none", prefix, i, p, p[:1]) } - if _, err := regexp.Compile(p); err != nil { + re, err := regexp.Compile(p) + if err != nil { return fmt.Errorf("surf: %s.allowed_origins_patterns[%d] %q does not compile: %v", prefix, i, p, err) } + if !cfg.SupportsCredentials { + continue + } + if !anchoredOriginPattern(p) { + return fmt.Errorf("surf: %s.allowed_origins_patterns[%d] %q is not anchored: with %s.supports_credentials true a pattern must start with ^ and end with $, as in ^https://[a-z]+\\.example\\.com$, or it matches inside any origin", prefix, i, p, prefix) + } + if probe, ok := matchesCORSProbe(re); ok { + return fmt.Errorf("surf: %s.allowed_origins_patterns[%d] %q matches %s, an origin outside any configured domain: with %s.supports_credentials true a pattern must name your own domains", prefix, i, p, probe, prefix) + } + } + if !cfg.SupportsCredentials { + return nil + } + for i, o := range cfg.AllowedOrigins { + if o == "*" || !strings.Contains(o, "*") { + continue + } + if !wildcardHostLabel(o) { + return fmt.Errorf("surf: %s.allowed_origins[%d] %q: with %s.supports_credentials true a wildcard must be the whole leftmost host label after a literal scheme, as in https://*.example.com", prefix, i, o, prefix) + } + if probe, ok := matchesCORSProbe(regexp.MustCompile(wildcardOriginPattern(o))); ok { + return fmt.Errorf("surf: %s.allowed_origins[%d] %q matches %s, an origin outside any configured domain: with %s.supports_credentials true a wildcard must sit in front of your own domain, as in https://*.example.com", prefix, i, o, probe, prefix) + } } return nil } +// corsProbeOrigins are origins no credentialed origin rule may match: a +// rule that matches them trusts origins outside the configured domains +// (https://*, https://*.com, ^https?://.*$). +var corsProbeOrigins = []string{ + "https://cors-probe.invalid", + "http://cors-probe.invalid", + "https://cors-probe.com", + "null", +} + +// matchesCORSProbe reports the first probe origin re matches. +func matchesCORSProbe(re *regexp.Regexp) (string, bool) { + for _, probe := range corsProbeOrigins { + if re.MatchString(probe) { + return probe, true + } + } + return "", false +} + +// wildcardHostLabel reports an allowed_origins entry whose only "*" is the +// whole leftmost host label after a literal scheme: https://*.example.com. +func wildcardHostLabel(origin string) bool { + scheme, host, ok := strings.Cut(origin, "://") + if !ok || scheme == "" || strings.Contains(scheme, "*") { + return false + } + rest, ok := strings.CutPrefix(host, "*.") + return ok && rest != "" && !strings.Contains(rest, "*") +} + +// anchoredOriginPattern reports a pattern anchored at both ends: it starts +// with ^ and ends with $, and every alternative of the parsed expression +// stays anchored, so ^a|b$ is refused. +func anchoredOriginPattern(p string) bool { + if !strings.HasPrefix(p, "^") || !strings.HasSuffix(p, "$") { + return false + } + re, err := syntax.Parse(p, syntax.Perl) + if err != nil { + return false + } + return anchoredAt(re, true) && anchoredAt(re, false) +} + +// anchoredAt reports whether every match of re begins (start true) or ends +// at the text boundary. +func anchoredAt(re *syntax.Regexp, start bool) bool { + switch re.Op { + case syntax.OpBeginText: + return start + case syntax.OpEndText: + return !start + case syntax.OpCapture: + return anchoredAt(re.Sub[0], start) + case syntax.OpConcat: + if len(re.Sub) == 0 { + return false + } + if start { + return anchoredAt(re.Sub[0], start) + } + return anchoredAt(re.Sub[len(re.Sub)-1], start) + case syntax.OpAlternate: + for _, sub := range re.Sub { + if !anchoredAt(sub, start) { + return false + } + } + return len(re.Sub) > 0 + } + return false +} + // phpDelimited reports a preg pattern such as #^https://x$# or /^x$/i: the // first byte is # or /, the same byte closes the pattern and only flag // letters follow it. An origin pattern never starts with either byte. diff --git a/modules/surf/cors_profile_test.go b/modules/surf/cors_profile_test.go index 8dbbfbc..8d2d246 100644 --- a/modules/surf/cors_profile_test.go +++ b/modules/surf/cors_profile_test.go @@ -71,6 +71,25 @@ cors: `)); err != nil { t.Fatalf("credentials with exact origins: %v", err) } + + // A wildcard host and an unanchored pattern are the same exposure under + // another spelling (WR-04): refused with credentials, loaded without. + for _, tc := range []struct{ key, value, want string }{ + {"allowed_origins", "https://*", "http.cors.allowed_origins[1]"}, + {"allowed_origins_patterns", `https://.*\.example\.com`, "http.cors.allowed_origins_patterns[1]"}, + } { + list := `["https://www.example.com", '` + tc.value + `']` + if tc.key == "allowed_origins_patterns" { + list = `['^https://www\.example\.com$', '` + tc.value + `']` + } + yaml := "cors:\n paths: [\"api/*\"]\n " + tc.key + ": " + list + "\n" + if _, err := LoadCORSConfig(writeHTTPConfig(t, yaml+" supports_credentials: true\n")); err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("%s with credentials: error = %v, want one naming %s", tc.value, err, tc.want) + } + if _, err := LoadCORSConfig(writeHTTPConfig(t, yaml)); err != nil { + t.Errorf("%s without credentials: %v", tc.value, err) + } + } } func TestCORSPatternErrors(t *testing.T) {