From aef94cd0e71d45955df09223096d6c33e15584b0 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Tue, 25 Aug 2026 18:07:15 +0100 Subject: [PATCH] http: don't leak configured headers to other hosts or over plaintext on redirect GHSA-486v-q2wf-fp2r CVE-PENDING The headers set with --http-headers are documented for passing credentials such as Authorization or Cookie. The backend used the default net/http redirect policy which copies all but a handful of well known headers to any redirect target, so a redirect from the configured server to another host would send those credentials to that host, and a redirect from https to http would send them in plaintext. When headers are configured this installs a CheckRedirect function which: - removes the configured headers from every hop once the redirect chain has left the originally requested host - refuses a redirect from https to http with an error --- backend/http/http.go | 52 ++++++++- backend/http/http_internal_test.go | 179 +++++++++++++++++++++++++++++ 2 files changed, 230 insertions(+), 1 deletion(-) diff --git a/backend/http/http.go b/backend/http/http.go index f1afa602b..673826b65 100644 --- a/backend/http/http.go +++ b/backend/http/http.go @@ -57,7 +57,13 @@ The input format is comma separated list of key,value pairs. Standard For example, to set a Cookie use 'Cookie,name=value', or '"Cookie","name=value"'. -You can set multiple headers, e.g. '"Cookie","name=value","Authorization","xxx"'.`, +You can set multiple headers, e.g. '"Cookie","name=value","Authorization","xxx"'. + +The headers are only sent to the host in the configured URL. If the +server redirects to another host (including a subdomain or a different +port) the headers are not sent to it, or to any further hop in that +redirect chain. When headers are set, a redirect from https to http is +refused as it would send them in cleartext.`, Default: fs.CommaSepList{}, Advanced: true, }, { @@ -283,6 +289,11 @@ func (f *Fs) httpConnection(ctx context.Context, opt *Options) (isFile bool, err } client := fshttp.NewClient(ctx) + // Without configured headers keep the default policy which + // public mirrors that redirect from https to http rely on. + if len(opt.Headers) > 0 { + client.CheckRedirect = checkRedirect(opt) + } endpoint, isFile := getFsEndpoint(ctx, client, u.String(), opt) fs.Debugf(nil, "Root: %s", endpoint) @@ -510,6 +521,45 @@ func addHeaders(req *http.Request, opt *Options) { } } +// checkRedirect returns an http.Client.CheckRedirect function which +// follows redirects but refuses an HTTPS to HTTP downgrade with +// rest.ErrHTTPSDowngrade and strips the configured headers when the +// redirect chain has left the originally requested host at any point. +func checkRedirect(opt *Options) func(req *http.Request, via []*http.Request) error { + return func(req *http.Request, via []*http.Request) error { + if err := rest.RefuseHTTPSDowngradeRedirectFn(req, via); err != nil { + if errors.Is(err, rest.ErrHTTPSDowngrade) { + err = fmt.Errorf("%w (the configured headers would be sent to the plaintext target)", err) + } + return err + } + if redirectLeavesHost(req, via) { + for i := 0; i < len(opt.Headers); i += 2 { + req.Header.Del(opt.Headers[i]) + } + } + return nil + } +} + +// redirectLeavesHost reports whether any hop in the redirect chain +// via plus the pending request req is to a different host from the +// original request via[0]. +// +// net/http copies the headers afresh from the original request for +// every hop, so once the chain has visited another host the headers +// must be stripped from every subsequent hop, even one back to the +// original host, as the other host chose the URL. +func redirectLeavesHost(req *http.Request, via []*http.Request) bool { + origin := via[0].URL + for _, hop := range via { + if !rest.SameHost(hop.URL, origin) { + return true + } + } + return !rest.SameHost(req.URL, origin) +} + // Adds the configured headers to the request if any func (f *Fs) addHeaders(req *http.Request) { addHeaders(req, &f.opt) diff --git a/backend/http/http_internal_test.go b/backend/http/http_internal_test.go index 57d72631f..01e637cc5 100644 --- a/backend/http/http_internal_test.go +++ b/backend/http/http_internal_test.go @@ -581,3 +581,182 @@ func TestFsNoSlashRoots(t *testing.T) { } } } + +// readRedirected makes an http remote with the configured headers +// pointing at url and reads file.txt from it, checking the content +func readRedirected(t *testing.T, url string) { + configfile.Install() + m := configmap.Simple{ + "type": "http", + "url": url + "/", + "headers": strings.Join(headers, ","), + } + f, err := NewFs(context.Background(), remoteName, "", m) + require.NoError(t, err) + + o, err := f.NewObject(context.Background(), "file.txt") + require.NoError(t, err) + fd, err := o.Open(context.Background()) + require.NoError(t, err) + data, err := io.ReadAll(fd) + require.NoError(t, err) + require.NoError(t, fd.Close()) + assert.Equal(t, "hello", string(data)) +} + +func TestRedirectKeepsHeadersOnSameHost(t *testing.T) { + var got http.Header + var ts *httptest.Server + ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.HasPrefix(r.URL.Path, "/moved/") { + http.Redirect(w, r, ts.URL+"/moved"+r.URL.Path, http.StatusFound) + return + } + got = r.Header.Clone() + _, _ = w.Write([]byte("hello")) + })) + defer ts.Close() + + readRedirected(t, ts.URL) + + require.NotNil(t, got) + for i := 0; i < len(headers); i += 2 { + assert.Equal(t, headers[i+1], got.Get(headers[i])) + } +} + +func TestRedirectRefusesHTTPSDowngrade(t *testing.T) { + // A plaintext server to be redirected to + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("hello")) + })) + defer target.Close() + + // An HTTPS server which redirects to plaintext HTTP + ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL+r.URL.Path, http.StatusFound) + })) + defer ts.Close() + + // open makes a remote pointing at ts with or without headers + // and opens file.txt on it + open := func(t *testing.T, withHeaders bool) (io.ReadCloser, error) { + configfile.Install() + m := configmap.Simple{ + "type": "http", + "url": ts.URL + "/", + "no_head": "true", + } + if withHeaders { + m.Set("headers", strings.Join(headers, ",")) + } + ctx, ci := fs.AddConfig(context.Background()) + ci.InsecureSkipVerify = true + f, err := NewFs(ctx, remoteName, "", m) + require.NoError(t, err) + + // no_head means NewObject doesn't make a request so use Open to make one + o, err := f.NewObject(ctx, "file.txt") + require.NoError(t, err) + return o.Open(ctx) + } + + t.Run("WithHeaders", func(t *testing.T) { + // Headers could leak so the downgrade must be refused + _, err := open(t, true) + require.ErrorIs(t, err, rest.ErrHTTPSDowngrade) + }) + + t.Run("WithoutHeaders", func(t *testing.T) { + // Nothing to leak so the downgrade is followed + fd, err := open(t, false) + require.NoError(t, err) + data, err := io.ReadAll(fd) + require.NoError(t, err) + require.NoError(t, fd.Close()) + assert.Equal(t, "hello", string(data)) + }) +} + +func TestRedirectKeepsHeadersOnSameHostDifferentCase(t *testing.T) { + // The redirect spells the host differently but it is the same + // host so the headers must be kept + var got http.Header + var ts *httptest.Server + ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.HasPrefix(r.URL.Path, "/moved/") { + http.Redirect(w, r, "http://LOCALHOST:"+port(t, ts)+"/moved"+r.URL.Path, http.StatusFound) + return + } + got = r.Header.Clone() + _, _ = w.Write([]byte("hello")) + })) + defer ts.Close() + + readRedirected(t, "http://localhost:"+port(t, ts)) + + require.NotNil(t, got) + for i := 0; i < len(headers); i += 2 { + assert.Equal(t, headers[i+1], got.Get(headers[i])) + } +} + +// port returns the port ts is listening on +func port(t *testing.T, ts *httptest.Server) string { + u, err := url.Parse(ts.URL) + require.NoError(t, err) + return u.Port() +} + +func TestRedirectStripsHeadersAfterHostChange(t *testing.T) { + // The configured server redirects to a second server which + // redirects back to the configured server. The headers must not + // be sent on the way back as the other host chose the path. + var got http.Header + var ts *httptest.Server + bouncer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, ts.URL+"/moved"+r.URL.Path, http.StatusFound) + })) + defer bouncer.Close() + + ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.HasPrefix(r.URL.Path, "/moved/") { + http.Redirect(w, r, bouncer.URL+r.URL.Path, http.StatusFound) + return + } + got = r.Header.Clone() + _, _ = w.Write([]byte("hello")) + })) + defer ts.Close() + + readRedirected(t, ts.URL) + + require.NotNil(t, got) + for i := 0; i < len(headers); i += 2 { + assert.Empty(t, got.Values(headers[i]), "header %q sent after cross-host redirect", headers[i]) + } +} + +func TestRedirectStripsHeadersOnHostChange(t *testing.T) { + // Serve the final response from a second server. httptest servers + // listen on 127.0.0.1:port so the two servers have different hosts + // for the purposes of the redirect check. + var got http.Header + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = r.Header.Clone() + _, _ = w.Write([]byte("hello")) + })) + defer target.Close() + + redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL+r.URL.Path, http.StatusFound) + })) + defer redirector.Close() + + readRedirected(t, redirector.URL) + + require.NotNil(t, got) + for i := 0; i < len(headers); i += 2 { + assert.Empty(t, got.Values(headers[i]), "header %q leaked to redirect target", headers[i]) + } +}