[bugfix] httpclient not signing subsequent redirect requests (#2798)

* move http request signing to transport

* actually hook up the http roundtripper ...

* add code comments for the new gtscontext functions

internal/gtscontext/context.go

@@ -19,6 +19,7 @@
 
 import (
 	"context"
+	"net/http"
 	"net/url"
 
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
@@ -42,6 +43,7 @@
 	httpSigKey
 	httpSigPubKeyIDKey
 	dryRunKey
+	httpClientSignFnKey
 )
 
 // DryRun returns whether the "dryrun" context key has been set. This can be
@@ -127,6 +129,19 @@ func SetOtherIRIs(ctx context.Context, iris []*url.URL) context.Context {
 	return context.WithValue(ctx, otherIRIsKey, iris)
 }
 
+// HTTPClientSignFunc returns an httpclient signing function for the current client
+// request context. This can be used to resign a request as calling transport's user.
+func HTTPClientSignFunc(ctx context.Context) func(*http.Request) error {
+	fn, _ := ctx.Value(httpClientSignFnKey).(func(*http.Request) error)
+	return fn
+}
+
+// SetHTTPClientSignFunc stores the given httpclient signing function and returns the wrapped
+// context. See HTTPClientSignFunc() for further information on the signing function value.
+func SetHTTPClientSignFunc(ctx context.Context, fn func(*http.Request) error) context.Context {
+	return context.WithValue(ctx, httpClientSignFnKey, fn)
+}
+
 // HTTPSignatureVerifier returns an http signature verifier for the current ActivityPub
 // request chain. This verifier can be called to authenticate the current request.
 func HTTPSignatureVerifier(ctx context.Context) httpsig.VerifierWithOptions {

internal/httpclient/client.go

@@ -32,7 +32,6 @@
 	"time"
 
 	"codeberg.org/gruf/go-bytesize"
-	"codeberg.org/gruf/go-byteutil"
 	"codeberg.org/gruf/go-cache/v3"
 	errorsv2 "codeberg.org/gruf/go-errors/v2"
 	"codeberg.org/gruf/go-iotools"
@@ -163,7 +162,7 @@ func New(cfg Config) *Client {
 	}
 
 	// Set underlying HTTP client roundtripper.
-	c.client.Transport = &http.Transport{
+	c.client.Transport = &signingtransport{http.Transport{
 		Proxy:                 http.ProxyFromEnvironment,
 		ForceAttemptHTTP2:     true,
 		DialContext:           d.DialContext,
@@ -175,7 +174,7 @@ func New(cfg Config) *Client {
 		ReadBufferSize:        cfg.ReadBufferSize,
 		WriteBufferSize:       cfg.WriteBufferSize,
 		DisableCompression:    cfg.DisableCompression,
-	}
+	}}
 
 	// Initiate outgoing bad hosts lookup cache.
 	c.badHosts = cache.NewTTL[string, struct{}](0, 1000, 0)
@@ -239,23 +238,6 @@ func (c *Client) DoSigned(r *http.Request, sign SignFunc) (rsp *http.Response, e
 	for i := 0; i < maxRetries; i++ {
 		var backoff time.Duration
 
-		// Reset signing header fields
-		now := time.Now().UTC()
-		r.Header.Set("Date", now.Format("Mon, 02 Jan 2006 15:04:05")+" GMT")
-		r.Header.Del("Signature")
-		r.Header.Del("Digest")
-
-		// Rewind body reader and content-length if set.
-		if rc, ok := r.Body.(*byteutil.ReadNopCloser); ok {
-			rc.Rewind() // set len AFTER rewind
-			r.ContentLength = int64(rc.Len())
-		}
-
-		// Sign the outgoing request.
-		if err := sign(r); err != nil {
-			return nil, err
-		}
-
 		l.Info("performing request")
 
 		// Perform the request.
@@ -276,6 +258,9 @@ func (c *Client) DoSigned(r *http.Request, sign SignFunc) (rsp *http.Response, e
 			// Search for a provided "Retry-After" header value.
 			if after := rsp.Header.Get("Retry-After"); after != "" {
 
+				// Get current time.
+				now := time.Now()
+
 				if u, _ := strconv.ParseUint(after, 10, 32); u != 0 {
 					// An integer number of backoff seconds was provided.
 					backoff = time.Duration(u) * time.Second

internal/httpclient/sign.go

@@ -17,12 +17,45 @@
 
 package httpclient
 
-import "net/http"
+import (
+	"net/http"
+	"time"
+
+	"codeberg.org/gruf/go-byteutil"
+	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
+)
 
 // SignFunc is a function signature that provides request signing.
 type SignFunc func(r *http.Request) error
 
-type SigningClient interface {
-	Do(r *http.Request) (*http.Response, error)
-	DoSigned(r *http.Request, sign SignFunc) (*http.Response, error)
+// signingtransport wraps an http.Transport{}
+// (RoundTripper implementer) to check request
+// context for a signing function and using for
+// all subsequent trips through RoundTrip().
+type signingtransport struct {
+	http.Transport // underlying transport
+}
+
+func (t *signingtransport) RoundTrip(r *http.Request) (*http.Response, error) {
+	if sign := gtscontext.HTTPClientSignFunc(r.Context()); sign != nil {
+		// Reset signing header fields
+		now := time.Now().UTC()
+		r.Header.Set("Date", now.Format("Mon, 02 Jan 2006 15:04:05")+" GMT")
+		r.Header.Del("Signature")
+		r.Header.Del("Digest")
+
+		// Rewind body reader and content-length if set.
+		if rc, ok := r.Body.(*byteutil.ReadNopCloser); ok {
+			rc.Rewind() // set len AFTER rewind
+			r.ContentLength = int64(rc.Len())
+		}
+
+		// Sign the outgoing request.
+		if err := sign(r); err != nil {
+			return nil, err
+		}
+	}
+
+	// Pass to underlying transport.
+	return t.Transport.RoundTrip(r)
 }

internal/transport/controller.go

@@ -37,7 +37,6 @@
 	"github.com/superseriousbusiness/gotosocial/internal/config"
 	"github.com/superseriousbusiness/gotosocial/internal/db"
 	"github.com/superseriousbusiness/gotosocial/internal/federation/federatingdb"
-	"github.com/superseriousbusiness/gotosocial/internal/httpclient"
 	"github.com/superseriousbusiness/gotosocial/internal/state"
 )
 
@@ -54,14 +53,14 @@ type controller struct {
 	state     *state.State
 	fedDB     federatingdb.DB
 	clock     pub.Clock
-	client    httpclient.SigningClient
+	client    pub.HttpClient
 	trspCache cache.TTLCache[string, *transport]
 	userAgent string
 	senders   int // no. concurrent batch delivery routines.
 }
 
 // NewController returns an implementation of the Controller interface for creating new transports
-func NewController(state *state.State, federatingDB federatingdb.DB, clock pub.Clock, client httpclient.SigningClient) Controller {
+func NewController(state *state.State, federatingDB federatingdb.DB, clock pub.Clock, client pub.HttpClient) Controller {
 	var (
 		host             = config.GetHost()
 		proto            = config.GetProtocol()

internal/transport/transport.go

@@ -93,30 +93,61 @@ func (t *transport) GET(r *http.Request) (*http.Response, error) {
 	if r.Method != http.MethodGet {
 		return nil, errors.New("must be GET request")
 	}
-	ctx := r.Context() // extract, set pubkey ID.
+
+	// Prepare HTTP GET signing func with opts.
+	sign := t.signGET(httpsig.SignatureOption{
+		ExcludeQueryStringFromPathPseudoHeader: false,
+	})
+
+	ctx := r.Context() // update with signing details.
 	ctx = gtscontext.SetOutgoingPublicKeyID(ctx, t.pubKeyID)
+	ctx = gtscontext.SetHTTPClientSignFunc(ctx, sign)
 	r = r.WithContext(ctx) // replace request ctx.
+
+	// Set our predefined controller user-agent.
 	r.Header.Set("User-Agent", t.controller.userAgent)
 
-	resp, err := t.controller.client.DoSigned(r, t.signGET(httpsig.SignatureOption{ExcludeQueryStringFromPathPseudoHeader: false}))
+	// Pass to underlying HTTP client.
+	resp, err := t.controller.client.Do(r)
 	if err != nil || resp.StatusCode != http.StatusUnauthorized {
 		return resp, err
 	}
 
-	// try again without the path included in the HTTP signature for better compatibility
+	// Ignore this response.
 	_ = resp.Body.Close()
-	return t.controller.client.DoSigned(r, t.signGET(httpsig.SignatureOption{ExcludeQueryStringFromPathPseudoHeader: true}))
+
+	// Try again without the path included in
+	// the HTTP signature for better compatibility.
+	sign = t.signGET(httpsig.SignatureOption{
+		ExcludeQueryStringFromPathPseudoHeader: true,
+	})
+
+	ctx = r.Context() // update with signing details.
+	ctx = gtscontext.SetHTTPClientSignFunc(ctx, sign)
+	r = r.WithContext(ctx) // replace request ctx.
+
+	// Pass to underlying HTTP client.
+	return t.controller.client.Do(r)
 }
 
 func (t *transport) POST(r *http.Request, body []byte) (*http.Response, error) {
 	if r.Method != http.MethodPost {
 		return nil, errors.New("must be POST request")
 	}
-	ctx := r.Context() // extract, set pubkey ID.
+
+	// Prepare POST signer.
+	sign := t.signPOST(body)
+
+	ctx := r.Context() // update with signing details.
 	ctx = gtscontext.SetOutgoingPublicKeyID(ctx, t.pubKeyID)
+	ctx = gtscontext.SetHTTPClientSignFunc(ctx, sign)
 	r = r.WithContext(ctx) // replace request ctx.
+
+	// Set our predefined controller user-agent.
 	r.Header.Set("User-Agent", t.controller.userAgent)
-	return t.controller.client.DoSigned(r, t.signPOST(body))
+
+	// Pass to underlying HTTP client.
+	return t.controller.client.Do(r)
 }
 
 // signGET will safely sign an HTTP GET request.

testrig/transportcontroller.go

@@ -26,6 +26,7 @@
 	"strings"
 	"sync"
 
+	"github.com/superseriousbusiness/activity/pub"
 	"github.com/superseriousbusiness/activity/streams"
 	"github.com/superseriousbusiness/activity/streams/vocab"
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
@@ -51,7 +52,7 @@
 // Unlike the other test interfaces provided in this package, you'll probably want to call this function
 // PER TEST rather than per suite, so that the do function can be set on a test by test (or even more granular)
 // basis.
-func NewTestTransportController(state *state.State, client httpclient.SigningClient) transport.Controller {
+func NewTestTransportController(state *state.State, client pub.HttpClient) transport.Controller {
 	return transport.NewController(state, NewTestFederatingDB(state), &federation.Clock{}, client)
 }