rest: add SameHost and check HTTPS downgrades against the original request GHSA-486v-q2wf-fp2r CVE-PENDING

SameHost compares two URLs by host name (case insensitively) and port
(treating the scheme's default port as no port) so redirect policies
can tell a real change of host from a server spelling its own host
differently, e.g. redirecting "https://example.com/" to
"https://EXAMPLE.com:443/".

The HTTPS downgrade check now compares the redirect target against
the original request rather than the previous hop, so a chain which
started on plaintext http, passed through an https server and came
back to http is no longer refused - nothing is being downgraded
relative to what the user asked for. A chain which started on https
and reaches http via any number of hops is still refused.

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 via[0].URL.Scheme == "https" && req.URL.Scheme == "http"
-	return prev.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

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