Merge pull request #106525 from dgrisonnet/sanitize-forwarded-uri

apimachinery/pkg/util/proxy: escape forwarded URI
This commit is contained in:
Kubernetes Prow Robot 2022-03-07 09:22:53 -08:00 committed by GitHub
commit 170a9c050f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 56 additions and 14 deletions

View File

@ -83,7 +83,7 @@ type Transport struct {
// RoundTrip implements the http.RoundTripper interface
func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
// Add reverse proxy headers.
forwardedURI := path.Join(t.PathPrepend, req.URL.Path)
forwardedURI := path.Join(t.PathPrepend, req.URL.EscapedPath())
if strings.HasSuffix(req.URL.Path, "/") {
forwardedURI = forwardedURI + "/"
}
@ -106,7 +106,11 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
}
if redirect := resp.Header.Get("Location"); redirect != "" {
resp.Header.Set("Location", t.rewriteURL(redirect, req.URL, req.Host))
targetURL, err := url.Parse(redirect)
if err != nil {
return nil, errors.NewInternalError(fmt.Errorf("error trying to parse Location header: %v", err))
}
resp.Header.Set("Location", t.rewriteURL(targetURL, req.URL, req.Host))
return resp, nil
}
@ -128,14 +132,8 @@ func (rt *Transport) WrappedRoundTripper() http.RoundTripper {
// rewriteURL rewrites a single URL to go through the proxy, if the URL refers
// to the same host as sourceURL, which is the page on which the target URL
// occurred, or if the URL matches the sourceRequestHost. If any error occurs (e.g.
// parsing), it returns targetURL.
func (t *Transport) rewriteURL(targetURL string, sourceURL *url.URL, sourceRequestHost string) string {
url, err := url.Parse(targetURL)
if err != nil {
return targetURL
}
// occurred, or if the URL matches the sourceRequestHost.
func (t *Transport) rewriteURL(url *url.URL, sourceURL *url.URL, sourceRequestHost string) string {
// Example:
// When API server processes a proxy request to a service (e.g. /api/v1/namespace/foo/service/bar/proxy/),
// the sourceURL.Host (i.e. req.URL.Host) is the endpoint IP address of the service. The
@ -151,7 +149,7 @@ func (t *Transport) rewriteURL(targetURL string, sourceURL *url.URL, sourceReque
isDifferentHost := url.Host != "" && url.Host != sourceURL.Host && url.Host != sourceRequestHost
isRelative := !strings.HasPrefix(url.Path, "/")
if isDifferentHost || isRelative {
return targetURL
return url.String()
}
// Do not rewrite scheme and host if the Transport has empty scheme and host
@ -178,7 +176,7 @@ func (t *Transport) rewriteURL(targetURL string, sourceURL *url.URL, sourceReque
// rewriteHTML scans the HTML for tags with url-valued attributes, and updates
// those values with the urlRewriter function. The updated HTML is output to the
// writer.
func rewriteHTML(reader io.Reader, writer io.Writer, urlRewriter func(string) string) error {
func rewriteHTML(reader io.Reader, writer io.Writer, urlRewriter func(*url.URL) string) error {
// Note: This assumes the content is UTF-8.
tokenizer := html.NewTokenizer(reader)
@ -193,7 +191,14 @@ func rewriteHTML(reader io.Reader, writer io.Writer, urlRewriter func(string) st
if urlAttrs, ok := atomsToAttrs[token.DataAtom]; ok {
for i, attr := range token.Attr {
if urlAttrs.Has(attr.Key) {
token.Attr[i].Val = urlRewriter(attr.Val)
url, err := url.Parse(attr.Val)
if err != nil {
// Do not rewrite the URL if it isn't valid. It is intended not
// to error here to prevent the inability to understand the
// content of the body to cause a fatal error.
continue
}
token.Attr[i].Val = urlRewriter(url)
}
}
}
@ -248,7 +253,7 @@ func (t *Transport) rewriteResponse(req *http.Request, resp *http.Response) (*ht
return resp, nil
}
urlRewriter := func(targetUrl string) string {
urlRewriter := func(targetUrl *url.URL) string {
return t.rewriteURL(targetUrl, req.URL, req.Host)
}
err := rewriteHTML(reader, writer, urlRewriter)

View File

@ -197,6 +197,43 @@ func TestProxyTransport(t *testing.T) {
contentType: "text/html",
forwardedURI: "/proxy/node/node1:10250/logs/log.log",
},
"forwarded URI must be escaped": {
input: "<html></html>",
sourceURL: "http://mynode.com/logs/log.log%00<script>alert(1)</script>",
transport: testTransport,
output: "<html></html>",
contentType: "text/html",
forwardedURI: "/proxy/node/node1:10250/logs/log.log%00%3Cscript%3Ealert%281%29%3C/script%3E",
},
"redirect rel must be escaped": {
sourceURL: "http://mynode.com/redirect",
transport: testTransport,
redirect: "/redirected/target/%00<script>alert(1)</script>/",
redirectWant: "http://foo.com/proxy/node/node1:10250/redirected/target/%00%3Cscript%3Ealert%281%29%3C/script%3E/",
forwardedURI: "/proxy/node/node1:10250/redirect",
},
"redirect abs same host must be escaped": {
sourceURL: "http://mynode.com/redirect",
transport: testTransport,
redirect: "http://mynode.com/redirected/target/%00<script>alert(1)</script>/",
redirectWant: "http://foo.com/proxy/node/node1:10250/redirected/target/%00%3Cscript%3Ealert%281%29%3C/script%3E/",
forwardedURI: "/proxy/node/node1:10250/redirect",
},
"redirect abs other host must be escaped": {
sourceURL: "http://mynode.com/redirect",
transport: testTransport,
redirect: "http://example.com/redirected/target/%00<script>alert(1)</script>/",
redirectWant: "http://example.com/redirected/target/%00%3Cscript%3Ealert%281%29%3C/script%3E/",
forwardedURI: "/proxy/node/node1:10250/redirect",
},
"redirect abs use reqHost no host no scheme must be escaped": {
sourceURL: "http://mynode.com/redirect",
transport: emptyHostAndSchemeTransport,
redirect: "http://10.0.0.1:8001/redirected/target/%00<script>alert(1)</script>/",
redirectWant: "http://10.0.0.1:8001/proxy/node/node1:10250/redirected/target/%00%3Cscript%3Ealert%281%29%3C/script%3E/",
forwardedURI: "/proxy/node/node1:10250/redirect",
reqHost: "10.0.0.1:8001",
},
}
testItem := func(name string, item *Item) {