federation: protect against SSRF attacks (#11795)

Adds an extra check to ensure the `keyId` and `actorId` included in signed requests and actor records point back to the originating host.

This check prevents server-side request forgery (SSRF) attacks where a carefully crafted request could be used to trick a federation server into making requests to arbitrary hosts and ports.

Further refactors can make these checks more robust, but would better fit after other existing refactor PRs are merged.

Related: https://codeberg.org/forgejo/forgejo/issues/11779

### Tests for Go changes

- I added test coverage for Go changes...
  - [x] in their respective `*_test.go` for unit tests.
  - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
  - [x] `make pr-go` before pushing

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11795
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>

Author: elle

modules/activitypub/client.go

@@ -11,13 +11,16 @@ import (
 	"crypto/rsa"
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"strings"
 	"time"
 
 	user_model "forgejo.org/models/user"
+	"forgejo.org/modules/hostmatcher"
 	"forgejo.org/modules/log"
 	"forgejo.org/modules/proxy"
 	"forgejo.org/modules/setting"
@@ -92,9 +95,60 @@ func NewClientFactoryWithTimeout(timeout time.Duration) (c *ClientFactory, err e
 	return c, err
 }
 
+// SetHostMatcher sets the HTTP dialer that ensures the `to` host matches the set federation host.
+//
+// This prevents specially crafted key IDs or other IRIs from triggering a SSRF
+// against hosts that do not match the host of the originating request.
+//
+// If no host is set, an error will be returned unless `setting.Federation.InsecureAllowInvalidHosts` is set to `true`.
+func (cf *ClientFactory) setHostMatcher(hosts []*url.URL) error {
+	if cf == nil {
+		return errors.New("nil client factory")
+	}
+
+	hostsNil := len(hosts) == 0
+	for _, host := range hosts {
+		hostsNil = hostsNil || host == nil
+	}
+
+	if hostsNil && !setting.Federation.InsecureAllowInvalidHosts {
+		return errors.New("nil client host(s)")
+	}
+
+	var hostMatchAllow, hostMatchBlock string
+	if setting.Federation.InsecureAllowInvalidHosts {
+		hostMatchAllow = fmt.Sprintf("%s, %s", hostmatcher.MatchBuiltinPrivate, hostmatcher.MatchBuiltinLoopback)
+		for _, host := range hosts {
+			if host != nil {
+				hostMatchAllow = fmt.Sprintf("%s, %s", hostMatchAllow, host.Host)
+			}
+		}
+	} else {
+		for i, host := range hosts {
+			if i == 0 {
+				hostMatchAllow = host.Host
+			} else {
+				hostMatchAllow = fmt.Sprintf("%s, %s", hostMatchAllow, host.Host)
+			}
+		}
+		hostMatchBlock = fmt.Sprintf("%s, %s", hostmatcher.MatchBuiltinPrivate, hostmatcher.MatchBuiltinLoopback)
+	}
+
+	allowMatcher := hostmatcher.ParseHostMatchList("", hostMatchAllow)
+	blockMatcher := hostmatcher.ParseHostMatchList("", hostMatchBlock)
+	dialCtx := hostmatcher.NewDialContext("activitypub", allowMatcher, blockMatcher, nil)
+
+	cf.client.Transport = &http.Transport{
+		Proxy:       proxy.Proxy(),
+		DialContext: dialCtx,
+	}
+
+	return nil
+}
+
 type APClientFactory interface {
-	WithKeys(ctx context.Context, user *user_model.User, pubID string) (APClient, error)
-	WithKeysDirect(ctx context.Context, privateKey, pubID string) (APClient, error)
+	WithKeys(ctx context.Context, user *user_model.User, pubID string, hosts []*url.URL) (APClient, error)
+	WithKeysDirect(ctx context.Context, privateKey, pubID string, hosts []*url.URL) (APClient, error)
 }
 
 // Client struct
@@ -109,13 +163,17 @@ type Client struct {
 }
 
 // NewRequest function
-func (cf *ClientFactory) WithKeysDirect(ctx context.Context, privateKey, pubID string) (APClient, error) {
+func (cf *ClientFactory) WithKeysDirect(ctx context.Context, privateKey, pubID string, hosts []*url.URL) (APClient, error) {
 	privPem, _ := pem.Decode([]byte(privateKey))
 	privParsed, err := x509.ParsePKCS1PrivateKey(privPem.Bytes)
 	if err != nil {
 		return nil, err
 	}
 
+	if err = cf.setHostMatcher(hosts); err != nil {
+		return nil, fmt.Errorf("client: invalid host for HostMatcher: %w", err)
+	}
+
 	c := Client{
 		client:      cf.client,
 		algs:        cf.algs,
@@ -128,15 +186,15 @@ func (cf *ClientFactory) WithKeysDirect(ctx context.Context, privateKey, pubID s
 	return &c, nil
 }
 
-func (cf *ClientFactory) WithKeys(ctx context.Context, user *user_model.User, pubID string) (APClient, error) {
+func (cf *ClientFactory) WithKeys(ctx context.Context, user *user_model.User, pubID string, hosts []*url.URL) (APClient, error) {
 	priv, err := GetPrivateKey(ctx, user)
 	if err != nil {
 		return nil, err
 	}
-	return cf.WithKeysDirect(ctx, priv, pubID)
+	return cf.WithKeysDirect(ctx, priv, pubID, hosts)
 }
 
-// NewRequest function
+// NewRequest function creates a new signed request to an external federation host.
 func (c *Client) newRequest(method string, b []byte, to string) (req *http.Request, err error) {
 	buf := bytes.NewBuffer(b)
 	req, err = http.NewRequest(method, to, buf)

modules/activitypub/client_test.go

@@ -9,6 +9,7 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 	"time"
 
@@ -18,6 +19,7 @@ import (
 	"forgejo.org/modules/activitypub"
 	"forgejo.org/modules/log"
 	"forgejo.org/modules/setting"
+	"forgejo.org/modules/test"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -63,20 +65,46 @@ Set up a user called "me" for all tests
 */
 
 func TestClientCtx(t *testing.T) {
+	defer test.MockVariableValue(&setting.Federation.InsecureAllowInvalidHosts, true)()
 	require.NoError(t, unittest.PrepareTestDatabase())
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
 	pubID := "myGpgId"
 	cf, err := activitypub.NewClientFactory()
 	log.Debug("ClientFactory: %v\nError: %v", cf, err)
 	require.NoError(t, err)
 
-	c, err := cf.WithKeys(db.DefaultContext, user, pubID)
+	c, err := cf.WithKeys(db.DefaultContext, user, pubID, nil)
 
 	log.Debug("Client: %v\nError: %v", c, err)
 	require.NoError(t, err)
 	_ = activitypub.NewContext(db.DefaultContext, cf)
 }
 
+func TestClientNilHostsCtx(t *testing.T) {
+	defer test.MockVariableValue(&setting.Federation.InsecureAllowInvalidHosts, false)()
+	require.NoError(t, unittest.PrepareTestDatabase())
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+	pubID := "myGpgId"
+	cf, err := activitypub.NewClientFactory()
+	log.Debug("ClientFactory: %v\nError: %v", cf, err)
+	require.NoError(t, err)
+
+	_, err = cf.WithKeys(db.DefaultContext, user, pubID, nil)
+	require.Error(t, err)
+
+	_, err = cf.WithKeys(db.DefaultContext, user, pubID, nil)
+	require.Error(t, err)
+
+	_, err = cf.WithKeys(db.DefaultContext, user, pubID, []*url.URL{nil})
+	require.Error(t, err)
+
+	testURL, err := url.Parse("https://example.dev")
+	require.NoError(t, err)
+
+	_, err = cf.WithKeys(db.DefaultContext, user, pubID, []*url.URL{testURL, nil})
+	require.Error(t, err)
+}
+
 /* TODO: bring this test to work or delete
 func TestActivityPubSignedGet(t *testing.T) {
 	require.NoError(t, unittest.PrepareTestDatabase())
@@ -109,13 +137,10 @@ func TestActivityPubSignedGet(t *testing.T) {
 */
 
 func TestActivityPubSignedPost(t *testing.T) {
+	defer test.MockVariableValue(&setting.Federation.InsecureAllowInvalidHosts, true)()
 	require.NoError(t, unittest.PrepareTestDatabase())
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
 	pubID := "https://example.com/pubID"
-	cf, err := activitypub.NewClientFactory()
-	require.NoError(t, err)
-	c, err := cf.WithKeys(db.DefaultContext, user, pubID)
-	require.NoError(t, err)
 
 	expected := "BODY"
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -129,6 +154,13 @@ func TestActivityPubSignedPost(t *testing.T) {
 	}))
 	defer srv.Close()
 
+	cf, err := activitypub.NewClientFactory()
+	require.NoError(t, err)
+	srvURL, err := url.Parse(srv.URL)
+	require.NoError(t, err)
+	c, err := cf.WithKeys(db.DefaultContext, user, pubID, []*url.URL{srvURL})
+	require.NoError(t, err)
+
 	r, err := c.Post([]byte(expected), srv.URL)
 	require.NoError(t, err)
 	defer r.Body.Close()

modules/setting/federation.go

@@ -12,23 +12,25 @@ import (
 // Federation settings
 var (
 	Federation = struct {
-		Enabled             bool
-		ShareUserStatistics bool
-		MaxSize             int64
-		SignatureAlgorithms []string
-		DigestAlgorithm     string
-		GetHeaders          []string
-		PostHeaders         []string
-		SignatureEnforced   bool
+		Enabled                   bool
+		ShareUserStatistics       bool
+		MaxSize                   int64
+		SignatureAlgorithms       []string
+		DigestAlgorithm           string
+		GetHeaders                []string
+		PostHeaders               []string
+		SignatureEnforced         bool
+		InsecureAllowInvalidHosts bool
 	}{
-		Enabled:             false,
-		ShareUserStatistics: true,
-		MaxSize:             4,
-		SignatureAlgorithms: []string{"rsa-sha256", "rsa-sha512", "ed25519"},
-		DigestAlgorithm:     "SHA-256",
-		GetHeaders:          []string{"(request-target)", "Date", "Host"},
-		PostHeaders:         []string{"(request-target)", "Date", "Host", "Digest"},
-		SignatureEnforced:   true,
+		Enabled:                   false,
+		ShareUserStatistics:       true,
+		MaxSize:                   4,
+		SignatureAlgorithms:       []string{"rsa-sha256", "rsa-sha512", "ed25519"},
+		DigestAlgorithm:           "SHA-256",
+		GetHeaders:                []string{"(request-target)", "Date", "Host"},
+		PostHeaders:               []string{"(request-target)", "Date", "Host", "Digest"},
+		SignatureEnforced:         true,
+		InsecureAllowInvalidHosts: false,
 	}
 )
 

services/federation/delivery_queue.go

@@ -6,6 +6,7 @@ package federation
 import (
 	"fmt"
 	"io"
+	"net/url"
 
 	"forgejo.org/models/user"
 	"forgejo.org/modules/activitypub"
@@ -54,7 +55,13 @@ func deliverToInbox(item deliveryQueueItem) error {
 	if err != nil {
 		return err
 	}
-	apclient, err := clientFactory.WithKeys(ctx, item.Doer, item.Doer.APActorID()+"#main-key")
+
+	inboxURL, err := url.Parse(item.InboxURL)
+	if err != nil {
+		return fmt.Errorf("invalid delivery item inbox URL: %w", err)
+	}
+
+	apclient, err := clientFactory.WithKeys(ctx, item.Doer, item.Doer.APActorID()+"#main-key", []*url.URL{inboxURL})
 	if err != nil {
 		return err
 	}

services/federation/federation_service.go

@@ -129,7 +129,12 @@ func createFederationHostFromAP(ctx context.Context, actorID fm.ActorID) (*forge
 		return nil, err
 	}
 
-	client, err := clientFactory.WithKeys(ctx, actionsUser, actionsUser.KeyID())
+	uri, err := url.Parse(actorID.AsWellKnownNodeInfoURI())
+	if err != nil {
+		return nil, fmt.Errorf("invalid actor URI: %w", err)
+	}
+
+	client, err := clientFactory.WithKeys(ctx, actionsUser, actionsUser.KeyID(), []*url.URL{uri})
 	if err != nil {
 		return nil, err
 	}
@@ -175,7 +180,8 @@ func fetchUserFromAP(ctx context.Context, personID fm.PersonID, federationHost *
 		return nil, nil, err
 	}
 
-	apClient, err := clientFactory.WithKeys(ctx, actionsUser, actionsUser.KeyID())
+	hostURL := federationHost.AsURL()
+	apClient, err := clientFactory.WithKeys(ctx, actionsUser, actionsUser.KeyID(), []*url.URL{&hostURL})
 	if err != nil {
 		return nil, nil, err
 	}

services/federation/repository_inbox_like.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"net/url"
 	"time"
 
 	"forgejo.org/models/forgefed"
@@ -112,9 +113,15 @@ func SendLikeActivities(ctx context.Context, doer user.User, repoID int64) error
 	}
 
 	likeActivityList := make([]fm.ForgeLike, 0)
+	var hosts []*url.URL
 	for _, followingRepo := range followingRepos {
 		log.Trace("Found following repo: %#v", followingRepo)
 		target := followingRepo.URI
+		hostURL, err := url.Parse(target)
+		if err != nil {
+			return fmt.Errorf("invalid repository URL: %w", err)
+		}
+		hosts = append(hosts, hostURL)
 		likeActivity, err := fm.NewForgeLike(doer.APActorID(), target, time.Now())
 		if err != nil {
 			return err
@@ -126,7 +133,7 @@ func SendLikeActivities(ctx context.Context, doer user.User, repoID int64) error
 	if err != nil {
 		return err
 	}
-	apclient, err := apclientFactory.WithKeys(ctx, &doer, doer.APActorID()+"#main-key")
+	apclient, err := apclientFactory.WithKeys(ctx, &doer, doer.APActorID()+"#main-key", hosts)
 	if err != nil {
 		return err
 	}

services/federation/signature_service.go

@@ -11,6 +11,7 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"strconv"
 
 	"forgejo.org/models/forgefed"
 	"forgejo.org/models/user"
@@ -57,8 +58,25 @@ func FindOrCreateActorKey(ctx context.Context, keyID string) (pubKey any, err er
 		return pubKey, nil
 	}
 
+	port := uint16(443)
+	if keyURL.Port() != "" {
+		port64, err := strconv.ParseUint(keyURL.Port(), 10, 16)
+		if err != nil {
+			return nil, err
+		}
+		port = uint16(port64)
+	}
+
+	hostURL := *keyURL
+	// if the `Actor` is not an `Application`, check for a `FederationHost` record associated with the key ID
+	// for user `Actor`s, we should already have a `FederationHost` record in the database, so we ensure that the user `Actor` matches the respective `FederationHost` URL
+	// otherwise, we just match against the supplied key ID URL
+	if federationHost, err := forgefed.FindFederationHostByFqdnAndPort(ctx, keyURL.Hostname(), port); err == nil {
+		hostURL = federationHost.AsURL()
+	}
+
 	// Fetch missing key
-	pubKey, pubKeyBytes, actor, err := fetchKeyFromAp(ctx, *keyURL)
+	pubKey, pubKeyBytes, actor, err := fetchKeyFromAp(ctx, *keyURL, hostURL)
 	if err != nil {
 		return nil, err
 	}
@@ -136,7 +154,7 @@ func updateFederationHostKey(ctx context.Context, federationHost *forgefed.Feder
 	return nil
 }
 
-func fetchKeyFromAp(ctx context.Context, keyURL url.URL) (pubKey any, pubKeyBytes []byte, apPerson *ap.Actor, err error) {
+func fetchKeyFromAp(ctx context.Context, keyURL, hostURL url.URL) (pubKey any, pubKeyBytes []byte, apPerson *ap.Actor, err error) {
 	log.Trace("keyURL %v", keyURL)
 	actionsUser := user.NewAPServerActor()
 
@@ -145,7 +163,7 @@ func fetchKeyFromAp(ctx context.Context, keyURL url.URL) (pubKey any, pubKeyByte
 		return nil, nil, nil, err
 	}
 
-	apClient, err := clientFactory.WithKeys(ctx, actionsUser, actionsUser.KeyID())
+	apClient, err := clientFactory.WithKeys(ctx, actionsUser, actionsUser.KeyID(), []*url.URL{&hostURL})
 	if err != nil {
 		return nil, nil, nil, err
 	}