[bugfix] Ensure activities sender always = activities actor (#2608)

internal/federation/authenticate.go

@@ -214,6 +214,17 @@ func (f *Federator) AuthenticateFederatedRequest(ctx context.Context, requestedU
 			err := gtserror.Newf("error dereferencing account %s: %w", pubKeyAuth.OwnerURI, err)
 			return nil, gtserror.NewErrorInternalError(err)
 		}
+
+		// Catch a possible (but very rare) race condition where
+		// we've fetched a key, then fetched the Actor who owns the
+		// key, but the Key of the Actor has changed in the meantime.
+		if !pubKeyAuth.Owner.PublicKey.Equal(pubKeyAuth.FetchedPubKey) {
+			err := gtserror.Newf(
+				"key mismatch: fetched key %s does not match pubkey of fetched Actor %s",
+				pubKeyID, pubKeyAuth.Owner.URI,
+			)
+			return nil, gtserror.NewErrorUnauthorized(err)
+		}
 	}
 
 	if !pubKeyAuth.Owner.SuspendedAt.IsZero() {

internal/federation/federatingdb/accept.go

@@ -41,7 +41,7 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA
 		l.Debug("entering Accept")
 	}
 
-	receivingAccount, _, internal := extractFromCtx(ctx)
+	receivingAccount, requestingAccount, internal := extractFromCtx(ctx)
 	if internal {
 		return nil // Already processed.
 	}
@@ -63,9 +63,16 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA
 					return fmt.Errorf("ACCEPT: error converting asfollow to gtsfollow: %s", err)
 				}
 
-				// make sure the addressee of the original follow is the same as whatever inbox this landed in
+				// Make sure the creator of the original follow
+				// is the same as whatever inbox this landed in.
 				if gtsFollow.AccountID != receivingAccount.ID {
-					return errors.New("ACCEPT: follow object account and inbox account were not the same")
+					return errors.New("ACCEPT: follow account and inbox account were not the same")
+				}
+
+				// Make sure the target of the original follow
+				// is the same as the account making the request.
+				if gtsFollow.TargetAccountID != requestingAccount.ID {
+					return errors.New("ACCEPT: follow target account and requesting account were not the same")
 				}
 
 				follow, err := f.state.DB.AcceptFollowRequest(ctx, gtsFollow.AccountID, gtsFollow.TargetAccountID)
@@ -103,9 +110,16 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA
 				return fmt.Errorf("ACCEPT: couldn't get follow request with id %s from the database: %s", iriStr, err)
 			}
 
-			// make sure the addressee of the original follow is the same as whatever inbox this landed in
+			// Make sure the creator of the original follow
+			// is the same as whatever inbox this landed in.
 			if followReq.AccountID != receivingAccount.ID {
-				return errors.New("ACCEPT: follow object account and inbox account were not the same")
+				return errors.New("ACCEPT: follow account and inbox account were not the same")
+			}
+
+			// Make sure the target of the original follow
+			// is the same as the account making the request.
+			if followReq.TargetAccountID != requestingAccount.ID {
+				return errors.New("ACCEPT: follow target account and requesting account were not the same")
 			}
 
 			follow, err := f.state.DB.AcceptFollowRequest(ctx, followReq.AccountID, followReq.TargetAccountID)

internal/federation/federatingdb/announce.go

@@ -19,6 +19,8 @@
 
 import (
 	"context"
+	"net/url"
+	"slices"
 
 	"codeberg.org/gruf/go-logger/v2/level"
 	"github.com/superseriousbusiness/activity/streams/vocab"
@@ -39,11 +41,25 @@ func (f *federatingDB) Announce(ctx context.Context, announce vocab.ActivityStre
 		l.Debug("entering Announce")
 	}
 
-	receivingAccount, _, internal := extractFromCtx(ctx)
+	receivingAccount, requestingAccount, internal := extractFromCtx(ctx)
 	if internal {
 		return nil // Already processed.
 	}
 
+	// Ensure requestingAccount is among
+	// the Actors doing the Announce.
+	//
+	// We don't support Announce forwards.
+	actorIRIs := ap.GetActorIRIs(announce)
+	if !slices.ContainsFunc(actorIRIs, func(actorIRI *url.URL) bool {
+		return actorIRI.String() == requestingAccount.URI
+	}) {
+		return gtserror.Newf(
+			"requestingAccount %s was not among Announce Actors",
+			requestingAccount.URI,
+		)
+	}
+
 	boost, isNew, err := f.converter.ASAnnounceToStatus(ctx, announce)
 	if err != nil {
 		return gtserror.Newf("error converting announce to boost: %w", err)

internal/federation/federatingdb/create.go

@@ -24,6 +24,7 @@
 	"strings"
 
 	"codeberg.org/gruf/go-logger/v2/level"
+	"github.com/miekg/dns"
 	"github.com/superseriousbusiness/activity/streams/vocab"
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
 	"github.com/superseriousbusiness/gotosocial/internal/config"
@@ -103,6 +104,20 @@ func (f *federatingDB) activityBlock(ctx context.Context, asType vocab.Type, rec
 		return fmt.Errorf("activityBlock: could not convert Block to gts model block")
 	}
 
+	if block.AccountID != requestingAccount.ID {
+		return fmt.Errorf(
+			"activityBlock: requestingAccount %s is not Block actor account %s",
+			requestingAccount.URI, block.Account.URI,
+		)
+	}
+
+	if block.TargetAccountID != receiving.ID {
+		return fmt.Errorf(
+			"activityBlock: inbox account %s is not Block object account %s",
+			receiving.URI, block.TargetAccount.URI,
+		)
+	}
+
 	block.ID = id.NewULID()
 
 	if err := f.state.DB.PutBlock(ctx, block); err != nil {
@@ -420,6 +435,20 @@ func (f *federatingDB) activityFollow(ctx context.Context, asType vocab.Type, re
 		return fmt.Errorf("activityFollow: could not convert Follow to follow request: %s", err)
 	}
 
+	if followRequest.AccountID != requestingAccount.ID {
+		return fmt.Errorf(
+			"activityFollow: requestingAccount %s is not Follow actor account %s",
+			requestingAccount.URI, followRequest.Account.URI,
+		)
+	}
+
+	if followRequest.TargetAccountID != receivingAccount.ID {
+		return fmt.Errorf(
+			"activityFollow: inbox account %s is not Follow object account %s",
+			receivingAccount.URI, followRequest.TargetAccount.URI,
+		)
+	}
+
 	followRequest.ID = id.NewULID()
 
 	if err := f.state.DB.PutFollowRequest(ctx, followRequest); err != nil {
@@ -451,6 +480,13 @@ func (f *federatingDB) activityLike(ctx context.Context, asType vocab.Type, rece
 		return fmt.Errorf("activityLike: could not convert Like to fave: %w", err)
 	}
 
+	if fave.AccountID != requestingAccount.ID {
+		return fmt.Errorf(
+			"activityLike: requestingAccount %s is not Like actor account %s",
+			requestingAccount.URI, fave.Account.URI,
+		)
+	}
+
 	fave.ID = id.NewULID()
 
 	if err := f.state.DB.PutStatusFave(ctx, fave); err != nil {
@@ -488,6 +524,26 @@ func (f *federatingDB) activityFlag(ctx context.Context, asType vocab.Type, rece
 		return fmt.Errorf("activityFlag: could not convert Flag to report: %w", err)
 	}
 
+	// Requesting account must have at
+	// least two domains from the right
+	// in common with reporting account.
+	if dns.CompareDomainName(
+		requestingAccount.Domain,
+		report.Account.Domain,
+	) < 2 {
+		return fmt.Errorf(
+			"activityFlag: requesting account %s does not share a domain with Flag Actor account %s",
+			requestingAccount.URI, report.Account.URI,
+		)
+	}
+
+	if report.TargetAccountID != receivingAccount.ID {
+		return fmt.Errorf(
+			"activityFlag: inbox account %s is not Flag object account %s",
+			receivingAccount.URI, report.TargetAccount.URI,
+		)
+	}
+
 	report.ID = id.NewULID()
 
 	if err := f.state.DB.PutReport(ctx, report); err != nil {

internal/federation/federatingdb/reject.go

@@ -40,7 +40,7 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR
 		l.Debug("entering Reject")
 	}
 
-	receivingAccount, _, internal := extractFromCtx(ctx)
+	receivingAccount, requestingAccount, internal := extractFromCtx(ctx)
 	if internal {
 		return nil // Already processed.
 	}
@@ -62,9 +62,16 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR
 					return fmt.Errorf("Reject: couldn't get follow request with id %s from the database: %s", rejectedObjectIRI.String(), err)
 				}
 
-				// make sure the addressee of the original follow is the same as whatever inbox this landed in
+				// Make sure the creator of the original follow
+				// is the same as whatever inbox this landed in.
 				if followReq.AccountID != receivingAccount.ID {
-					return errors.New("Reject: follow object account and inbox account were not the same")
+					return errors.New("Reject: follow account and inbox account were not the same")
+				}
+
+				// Make sure the target of the original follow
+				// is the same as the account making the request.
+				if followReq.TargetAccountID != requestingAccount.ID {
+					return errors.New("Reject: follow target account and requesting account were not the same")
 				}
 
 				return f.state.DB.RejectFollowRequest(ctx, followReq.AccountID, followReq.TargetAccountID)
@@ -90,9 +97,16 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR
 				return fmt.Errorf("Reject: error converting asfollow to gtsfollow: %s", err)
 			}
 
-			// make sure the addressee of the original follow is the same as whatever inbox this landed in
+			// Make sure the creator of the original follow
+			// is the same as whatever inbox this landed in.
 			if gtsFollow.AccountID != receivingAccount.ID {
-				return errors.New("Reject: follow object account and inbox account were not the same")
+				return errors.New("Reject: follow account and inbox account were not the same")
+			}
+
+			// Make sure the target of the original follow
+			// is the same as the account making the request.
+			if gtsFollow.TargetAccountID != requestingAccount.ID {
+				return errors.New("Reject: follow target account and requesting account were not the same")
 			}
 
 			return f.state.DB.RejectFollowRequest(ctx, gtsFollow.AccountID, gtsFollow.TargetAccountID)

internal/federation/federatingdb/undo.go

@@ -44,7 +44,7 @@ func (f *federatingDB) Undo(ctx context.Context, undo vocab.ActivityStreamsUndo)
 		l.Debug("entering Undo")
 	}
 
-	receivingAccount, _, internal := extractFromCtx(ctx)
+	receivingAccount, requestingAccount, internal := extractFromCtx(ctx)
 	if internal {
 		return nil // Already processed.
 	}
@@ -61,18 +61,18 @@ func (f *federatingDB) Undo(ctx context.Context, undo vocab.ActivityStreamsUndo)
 
 		switch objType.GetTypeName() {
 		case ap.ActivityFollow:
-			if err := f.undoFollow(ctx, receivingAccount, undo, objType); err != nil {
+			if err := f.undoFollow(ctx, receivingAccount, requestingAccount, undo, objType); err != nil {
 				errs.Appendf("error undoing follow: %w", err)
 			}
 		case ap.ActivityLike:
-			if err := f.undoLike(ctx, receivingAccount, undo, objType); err != nil {
+			if err := f.undoLike(ctx, receivingAccount, requestingAccount, undo, objType); err != nil {
 				errs.Appendf("error undoing like: %w", err)
 			}
 		case ap.ActivityAnnounce:
 			// TODO: actually handle this !
 			log.Warn(ctx, "skipped undo announce")
 		case ap.ActivityBlock:
-			if err := f.undoBlock(ctx, receivingAccount, undo, objType); err != nil {
+			if err := f.undoBlock(ctx, receivingAccount, requestingAccount, undo, objType); err != nil {
 				errs.Appendf("error undoing block: %w", err)
 			}
 		}
@@ -84,6 +84,7 @@ func (f *federatingDB) Undo(ctx context.Context, undo vocab.ActivityStreamsUndo)
 func (f *federatingDB) undoFollow(
 	ctx context.Context,
 	receivingAccount *gtsmodel.Account,
+	requestingAccount *gtsmodel.Account,
 	undo vocab.ActivityStreamsUndo,
 	t vocab.Type,
 ) error {
@@ -109,6 +110,12 @@ func (f *federatingDB) undoFollow(
 		return nil
 	}
 
+	// Ensure requester is follow origin.
+	if follow.AccountID != requestingAccount.ID {
+		// Ignore this Activity.
+		return nil
+	}
+
 	// Delete any existing follow with this URI.
 	if err := f.state.DB.DeleteFollowByURI(ctx, follow.URI); err != nil && !errors.Is(err, db.ErrNoEntries) {
 		return fmt.Errorf("undoFollow: db error removing follow: %w", err)
@@ -126,6 +133,7 @@ func (f *federatingDB) undoFollow(
 func (f *federatingDB) undoLike(
 	ctx context.Context,
 	receivingAccount *gtsmodel.Account,
+	requestingAccount *gtsmodel.Account,
 	undo vocab.ActivityStreamsUndo,
 	t vocab.Type,
 ) error {
@@ -151,6 +159,12 @@ func (f *federatingDB) undoLike(
 		return nil
 	}
 
+	// Ensure requester is fave origin.
+	if fave.AccountID != requestingAccount.ID {
+		// Ignore this Activity.
+		return nil
+	}
+
 	// Ignore URI on Likes, since we often get multiple Likes
 	// with the same target and account ID, but differing URIs.
 	// Instead, we'll select using account and target status.
@@ -179,6 +193,7 @@ func (f *federatingDB) undoLike(
 func (f *federatingDB) undoBlock(
 	ctx context.Context,
 	receivingAccount *gtsmodel.Account,
+	requestingAccount *gtsmodel.Account,
 	undo vocab.ActivityStreamsUndo,
 	t vocab.Type,
 ) error {
@@ -204,6 +219,12 @@ func (f *federatingDB) undoBlock(
 		return nil
 	}
 
+	// Ensure requester is block origin.
+	if block.AccountID != requestingAccount.ID {
+		// Ignore this Activity.
+		return nil
+	}
+
 	// Delete any existing BLOCK
 	if err := f.state.DB.DeleteBlockByURI(ctx, block.URI); err != nil && !errors.Is(err, db.ErrNoEntries) {
 		return fmt.Errorf("undoBlock: db error removing block: %w", err)