Repository navigation
fix(transport): refuse cross-origin redirects in default HTTP clients (GO-2026-6911) #1192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
390448b
fix(transport): refuse cross-origin redirects in default HTTP clients
hymns 8ce69db
fix(transport): tighten origin checks and cover OAuth credential requ…
hymns 41a7375
test(transport): fix names of the dotted capital I cases
hymns 8d778b5
fix(transport): compare IP literals in canonical form
hymns 4c8da10
fix(transport): keep IPv6 zone case when comparing origins
hymns c7131c9
test(transport): cover RefreshToken against a cross-origin token redi…
hymns File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| package transport | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "net/http" | ||
| "net/netip" | ||
| "net/url" | ||
| "strconv" | ||
| "strings" | ||
| ) | ||
|
|
||
| // ErrCrossOriginRedirect is returned when a server redirects a request to a | ||
| // different origin (scheme, host or port) and the redirect policy does not | ||
| // allow it. The redirected request would otherwise carry the configured | ||
| // headers, the OAuth bearer token and, for 307 and 308, the request body to a | ||
| // destination the server chose. | ||
| var ErrCrossOriginRedirect = errors.New("refusing cross-origin redirect") | ||
|
|
||
| // ErrInsecureRedirect is returned when a server redirects an https request to | ||
| // a plain http URL. | ||
| var ErrInsecureRedirect = errors.New("refusing redirect from https to http") | ||
|
|
||
| // ErrTooManyRedirects is returned when a redirect chain reaches maxRedirects. | ||
| var ErrTooManyRedirects = errors.New("too many redirects") | ||
|
|
||
| // maxRedirects matches the limit net/http applies when CheckRedirect is nil. | ||
| const maxRedirects = 10 | ||
|
|
||
| // newDefaultHTTPClient returns the HTTP client a transport uses when the | ||
| // caller does not supply one. It only follows same-origin redirects. | ||
| func newDefaultHTTPClient() *http.Client { | ||
| return &http.Client{CheckRedirect: sameOriginRedirectPolicy} | ||
| } | ||
|
|
||
| // sameOriginRedirectPolicy is an http.Client CheckRedirect function that | ||
| // follows a redirect only when the new URL has the origin of the request that | ||
| // started the chain, and at most maxRedirects times. | ||
| func sameOriginRedirectPolicy(req *http.Request, via []*http.Request) error { | ||
| if len(via) >= maxRedirects { | ||
| return fmt.Errorf("%w: stopped after %d redirects", ErrTooManyRedirects, maxRedirects) | ||
| } | ||
| if len(via) == 0 { | ||
| return nil | ||
| } | ||
| from := via[0].URL | ||
| if !sameOrigin(from, req.URL) { | ||
| return fmt.Errorf("%w: %s://%s to %s://%s", | ||
| ErrCrossOriginRedirect, from.Scheme, from.Host, req.URL.Scheme, req.URL.Host) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // oauthRedirectPolicy is the CheckRedirect function of the OAuth handler's | ||
| // default client. Requests that carry credentials (token, refresh and client | ||
| // registration requests, which are not GET or HEAD) follow only same-origin | ||
| // redirects. Metadata discovery requests, which carry no credentials, may be | ||
| // redirected to another origin but never from https to http. | ||
| func oauthRedirectPolicy(req *http.Request, via []*http.Request) error { | ||
| if len(via) == 0 { | ||
| return nil | ||
| } | ||
| first := via[0] | ||
| if first.Method != http.MethodGet && first.Method != http.MethodHead { | ||
| return sameOriginRedirectPolicy(req, via) | ||
| } | ||
| if len(via) >= maxRedirects { | ||
| return fmt.Errorf("%w: stopped after %d redirects", ErrTooManyRedirects, maxRedirects) | ||
| } | ||
| prev := via[len(via)-1].URL | ||
| if asciiLower(prev.Scheme) == "https" && asciiLower(req.URL.Scheme) != "https" { | ||
| return fmt.Errorf("%w: %s to %s", ErrInsecureRedirect, prev.Redacted(), req.URL.Redacted()) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // sameOrigin reports whether a and b have the same scheme, host and port. | ||
| func sameOrigin(a, b *url.URL) bool { | ||
| return origin(a) == origin(b) | ||
| } | ||
|
|
||
| // origin returns a normalized scheme://host:port string. The scheme and a | ||
| // host name are lowercased (ASCII only), an IP literal is written in its | ||
| // canonical form (so "[0:0::1]" and "[::1]" compare equal) with its zone | ||
| // unchanged, a missing port becomes the | ||
| // scheme's default, and a numeric port loses leading zeros, so ":0443" and | ||
| // ":443" compare equal as net/http dials them. | ||
| func origin(u *url.URL) string { | ||
| scheme := asciiLower(u.Scheme) | ||
| host := u.Hostname() | ||
| if addr, err := netip.ParseAddr(host); err == nil { | ||
| // netip writes hex digits in lowercase and keeps the zone as is: | ||
| // zone names are interface names, which can differ only in case. | ||
| host = addr.String() | ||
| } else { | ||
| host = asciiLower(host) | ||
| } | ||
| port := u.Port() | ||
| if n, err := strconv.ParseUint(port, 10, 16); err == nil { | ||
| port = strconv.FormatUint(n, 10) | ||
| } | ||
| if port == "" { | ||
| port = defaultPort(scheme) | ||
| } | ||
| return scheme + "://[" + host + "]:" + port | ||
| } | ||
|
|
||
| // asciiLower lowercases only ASCII letters. Unicode case mapping must not be | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| // used on hosts: strings.ToLower maps "İ" to "i" and strings.EqualFold matches | ||
| // "K" (Kelvin sign) with "k", while net/http dials those as different hosts. | ||
| func asciiLower(s string) string { | ||
| return strings.Map(func(r rune) rune { | ||
| if 'A' <= r && r <= 'Z' { | ||
| return r + ('a' - 'A') | ||
| } | ||
| return r | ||
| }, s) | ||
| } | ||
|
|
||
| // defaultPort returns the port net/http dials for scheme when a URL has none, | ||
| // or "" for a scheme it does not know. | ||
| func defaultPort(scheme string) string { | ||
| switch scheme { | ||
| case "http": | ||
| return "80" | ||
| case "https": | ||
| return "443" | ||
| } | ||
| return "" | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.