From bc8f0208aabebbe5cdc4d574954365574c9bdf9d Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Thu, 11 Oct 2018 13:02:20 +0100 Subject: [PATCH] rest: Remove auth headers on HTTP redirect Before this change the rest package would forward all the headers on an HTTP redirect, including the Authorization: header. This caused problems when forwarded to a signed S3 URL ("Only one auth mechanism allowed") as well as being a potential security risk. After we use the go1.8+ mechanism for doing this instead of using our own which does it correctly removing the Authorization: header when redirecting to a different host. This hasn't fixed the behaviour for rclone compiled with go1.7. Fixes #2635 --- lib/rest/rest.go | 24 ---------------------- lib/rest/rest_header_reset.go | 15 ++++++++++++++ lib/rest/rest_header_reset_go17.go | 33 ++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 24 deletions(-) create mode 100644 lib/rest/rest_header_reset.go create mode 100644 lib/rest/rest_header_reset_go17.go diff --git a/lib/rest/rest.go b/lib/rest/rest.go index 76eda84ff..2f473026a 100644 --- a/lib/rest/rest.go +++ b/lib/rest/rest.go @@ -166,30 +166,6 @@ func DecodeXML(resp *http.Response, result interface{}) (err error) { return decoder.Decode(result) } -// ClientWithHeaderReset makes a new http client which resets the -// headers passed in on redirect -// -// FIXME This is now unecessary with go1.8 -func ClientWithHeaderReset(c *http.Client, headers map[string]string) *http.Client { - if len(headers) == 0 { - return c - } - clientCopy := *c - clientCopy.CheckRedirect = func(req *http.Request, via []*http.Request) error { - if len(via) >= 10 { - return errors.New("stopped after 10 redirects") - } - // Reset the headers in the new request - for k, v := range headers { - if v != "" { - req.Header.Set(k, v) - } - } - return nil - } - return &clientCopy -} - // ClientWithNoRedirects makes a new http client which won't follow redirects func ClientWithNoRedirects(c *http.Client) *http.Client { clientCopy := *c diff --git a/lib/rest/rest_header_reset.go b/lib/rest/rest_header_reset.go new file mode 100644 index 000000000..599473f40 --- /dev/null +++ b/lib/rest/rest_header_reset.go @@ -0,0 +1,15 @@ +//+build go1.8 + +package rest + +import ( + "net/http" +) + +// ClientWithHeaderReset makes a new http client which resets the +// headers passed in on redirect +// +// This is now unecessary with go1.8 so becomes a no-op +func ClientWithHeaderReset(c *http.Client, headers map[string]string) *http.Client { + return c +} diff --git a/lib/rest/rest_header_reset_go17.go b/lib/rest/rest_header_reset_go17.go new file mode 100644 index 000000000..7b3109b57 --- /dev/null +++ b/lib/rest/rest_header_reset_go17.go @@ -0,0 +1,33 @@ +//+build !go1.8 + +package rest + +import ( + "net/http" + + "github.com/pkg/errors" +) + +// ClientWithHeaderReset makes a new http client which resets the +// headers passed in on redirect +// +// This is only needed for go < go1.8 +func ClientWithHeaderReset(c *http.Client, headers map[string]string) *http.Client { + if len(headers) == 0 { + return c + } + clientCopy := *c + clientCopy.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return errors.New("stopped after 10 redirects") + } + // Reset the headers in the new request + for k, v := range headers { + if v != "" { + req.Header.Set(k, v) + } + } + return nil + } + return &clientCopy +}