diff --git a/lib/rest/rest.go b/lib/rest/rest.go index 771d33263..e8c1ec953 100644 --- a/lib/rest/rest.go +++ b/lib/rest/rest.go @@ -16,6 +16,7 @@ import ( "net/http" "net/textproto" "net/url" + "strings" "sync" "github.com/rclone/rclone/fs" @@ -221,13 +222,38 @@ func ClientWithNoRedirects(c *http.Client) *http.Client { var ErrHTTPSDowngrade = errors.New("refusing to follow HTTPS to HTTP redirect: would send credentials in cleartext") // isHTTPSDowngrade reports whether following the redirect to req would -// move from an https:// URL to a plaintext http:// URL. +// move a request which was originally made to an https:// URL to a +// plaintext http:// URL. func isHTTPSDowngrade(req *http.Request, via []*http.Request) bool { if len(via) == 0 { return false } - prev := via[len(via)-1] - return prev.URL.Scheme == "https" && req.URL.Scheme == "http" + return via[0].URL.Scheme == "https" && req.URL.Scheme == "http" +} + +// SameHost reports whether a and b address the same host and port. +// +// Host names are compared case insensitively and a port which is +// the default for the URL's scheme is treated the same as no port, +// so a server which redirects "https://example.com/" to +// "https://EXAMPLE.com:443/" is not taken to be a different host. +func SameHost(a, b *url.URL) bool { + return strings.EqualFold(a.Hostname(), b.Hostname()) && portOf(a) == portOf(b) +} + +// portOf returns the port of u, filling in the default for the +// scheme if none is given. +func portOf(u *url.URL) string { + if port := u.Port(); port != "" { + return port + } + switch strings.ToLower(u.Scheme) { + case "http": + return "80" + case "https": + return "443" + } + return "" } // PreserveMethodRedirectFn is a CheckRedirect function that diff --git a/lib/rest/rest_test.go b/lib/rest/rest_test.go index 94fa6dfd1..39582e649 100644 --- a/lib/rest/rest_test.go +++ b/lib/rest/rest_test.go @@ -65,6 +65,18 @@ func TestRefuseHTTPSDowngradeRedirectFn(t *testing.T) { next := mkRedirectReq(t, "https://example.com/b", "GET") assert.NoError(t, RefuseHTTPSDowngradeRedirectFn(next, []*http.Request{orig})) }) + t.Run("RefusesDowngradeViaOtherHost", func(t *testing.T) { + orig := mkRedirectReq(t, "https://example.com/a", "GET") + mid := mkRedirectReq(t, "https://other.example/b", "GET") + next := mkRedirectReq(t, "http://example.com/c", "GET") + assert.ErrorIs(t, RefuseHTTPSDowngradeRedirectFn(next, []*http.Request{orig, mid}), ErrHTTPSDowngrade) + }) + t.Run("AllowsPlaintextOriginViaHTTPS", func(t *testing.T) { + orig := mkRedirectReq(t, "http://example.com/a", "GET") + mid := mkRedirectReq(t, "https://other.example/b", "GET") + next := mkRedirectReq(t, "http://example.com/c", "GET") + assert.NoError(t, RefuseHTTPSDowngradeRedirectFn(next, []*http.Request{orig, mid})) + }) t.Run("TooManyRedirects", func(t *testing.T) { next := mkRedirectReq(t, "https://example.com/b", "GET") via := make([]*http.Request, 10) @@ -72,6 +84,30 @@ func TestRefuseHTTPSDowngradeRedirectFn(t *testing.T) { }) } +func TestSameHost(t *testing.T) { + for _, test := range []struct { + a, b string + want bool + }{ + {"https://example.com/a", "https://example.com/b", true}, + {"https://example.com/", "https://EXAMPLE.com/", true}, + {"https://example.com/", "https://example.com:443/", true}, + {"http://example.com/", "http://example.com:80/", true}, + {"https://example.com/", "http://example.com/", false}, + {"https://example.com:8443/", "https://example.com:8444/", false}, + {"https://example.com/", "https://www.example.com/", false}, + {"https://example.com/", "https://example.com.evil/", false}, + {"http://[::1]:8080/", "http://[::1]:8080/", true}, + {"http://[::1]:8080/", "http://[::1]:8081/", false}, + } { + a, err := url.Parse(test.a) + require.NoError(t, err) + b, err := url.Parse(test.b) + require.NoError(t, err) + assert.Equal(t, test.want, SameHost(a, b), "%s vs %s", test.a, test.b) + } +} + // newDowngradeServers returns an HTTPS server that redirects every // request to a plaintext HTTP server on the same host, together with a // flag that records whether the plaintext server ever received an