swift: fix password being resent to another host on auth redirect GHSA-m97j-7x65-wh93 CVE-PENDING

A v2 or v3 auth request posts the user's password, or the application
credential secret, to the auth server in a JSON body which net/http
can rewind. On a 307 or 308 redirect to another host it resent that
body, password included, to the new host.

The transport now refuses to follow a redirect which leaves the host
and port of the original request if it would resend the request body.

backend/swift/swift.go

@@ -490,15 +490,22 @@ func (o *Object) split() (container, containerPath string) {
 // forwarded when a redirect leaves the host of the original request.
 var authHeaders = []string{"X-Auth-Token", "X-Auth-Key", "X-Auth-User"}
 
+// errRedirectResendsBody is returned when following a redirect which
+// leaves the host of the original request would resend that request's
+// body, such as the password in a v2 or v3 auth request, to the new
+// host.
+var errRedirectResendsBody = errors.New("refusing to follow redirect which leaves the original host: would resend the request body")
+
 // transport is the http.RoundTripper for the swift connection.
 //
 // The swift library makes its own http.Client with the default
 // redirect policy and does not let the caller replace it, so the
 // redirect policy is applied here at the transport level instead. A
 // redirect which downgrades HTTPS to plaintext HTTP is refused with
-// rest.ErrHTTPSDowngrade and the auth headers are removed from any
-// hop once the redirect chain has left the host and port of the
-// original request.
+// rest.ErrHTTPSDowngrade. Once the redirect chain has left the host
+// and port of the original request the auth headers are removed from
+// every further hop, and a hop which would resend the request body is
+// refused with errRedirectResendsBody.
 type transport struct {
 	*fshttp.Transport
 }
@@ -521,6 +528,13 @@ func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) {
 			return nil, fmt.Errorf("redirect to %s: %w", req.URL.Redacted(), rest.ErrHTTPSDowngrade)
 		}
 		if leftHost {
+			// net/http resends the body on a 307 or 308 if it
+			// can be rewound, which for an auth request means
+			// the password, so those redirects stop here.
+			if req.Body != nil && req.Body != http.NoBody {
+				_ = req.Body.Close()
+				return nil, fmt.Errorf("redirect to %s leaves %s: %w", req.URL.Redacted(), origin.URL.Redacted(), errRedirectResendsBody)
+			}
 			for _, header := range authHeaders {
 				if req.Header.Get(header) == "" {
 					continue

backend/swift/swift_internal_test.go

@@ -2,6 +2,7 @@ package swift
 
 import (
 	"context"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -236,3 +237,40 @@ func TestAuthRedirectStripsCredentialsOnCrossHost(t *testing.T) {
 	assert.Equal(t, int32(1), originHits.Load())
 	assert.Equal(t, int32(1), otherHits.Load())
 }
+
+// TestAuthRedirectRefusesToResendPassword checks a v2 or v3 auth
+// request, which carries the password in its body, is not resent to
+// another host the auth server redirects it to.
+func TestAuthRedirectRefusesToResendPassword(t *testing.T) {
+	for _, authVersion := range []string{"2", "3"} {
+		t.Run("v"+authVersion, func(t *testing.T) {
+			var originHits, otherHits atomic.Int32
+			other := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				otherHits.Add(1)
+				body, _ := io.ReadAll(r.Body)
+				assert.NotContains(t, string(body), "test-key", "password was resent to another host")
+			}))
+			defer other.Close()
+
+			origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				originHits.Add(1)
+				body, _ := io.ReadAll(r.Body)
+				assert.Contains(t, string(body), "test-key")
+				http.Redirect(w, r, other.URL+r.URL.Path, http.StatusTemporaryRedirect)
+			}))
+			defer origin.Close()
+
+			m := configmap.Simple{
+				"auth":         origin.URL + "/v" + authVersion,
+				"auth_version": authVersion,
+				"user":         "test-user",
+				"key":          "test-key",
+				"chunk_size":   "5Gi",
+			}
+			_, err := NewFs(context.Background(), "swift-test", "container", m)
+			require.ErrorIs(t, err, errRedirectResendsBody)
+			assert.Equal(t, int32(1), originHits.Load())
+			assert.Equal(t, int32(0), otherHits.Load())
+		})
+	}
+}