diff options
Diffstat (limited to 'internal')
38 files changed, 1240 insertions, 956 deletions
diff --git a/internal/ap/contextkey.go b/internal/ap/contextkey.go deleted file mode 100644 index af9b62d0e..000000000 --- a/internal/ap/contextkey.go +++ /dev/null @@ -1,36 +0,0 @@ -// GoToSocial -// Copyright (C) GoToSocial Authors admin@gotosocial.org -// SPDX-License-Identifier: AGPL-3.0-or-later -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program.  If not, see <http://www.gnu.org/licenses/>. - -package ap - -// ContextKey is a type used specifically for settings values on contexts within go-fed AP request chains -type ContextKey string - -const ( -	// ContextReceivingAccount can be used the set and retrieve the account being interacted with / receiving an activity in their inbox. -	ContextReceivingAccount ContextKey = "receivingAccount" -	// ContextRequestingAccount can be used to set and retrieve the account of an incoming federation request. -	// This will often be the actor of the instance that's posting the request. -	ContextRequestingAccount ContextKey = "requestingAccount" -	// ContextOtherInvolvedIRIs can be used to set and retrieve a slice of all IRIs that are 'involved' in an Activity without being -	// the receivingAccount or the requestingAccount. In other words, people or notes who are CC'ed or Replied To by an Activity. -	ContextOtherInvolvedIRIs ContextKey = "otherInvolvedIRIs" -	// ContextRequestingPublicKeyVerifier can be used to set and retrieve the public key verifier of an incoming federation request. -	ContextRequestingPublicKeyVerifier ContextKey = "requestingPublicKeyVerifier" -	// ContextRequestingPublicKeySignature can be used to set and retrieve the value of the signature header of an incoming federation request. -	ContextRequestingPublicKeySignature ContextKey = "requestingPublicKeySignature" -) diff --git a/internal/ap/extract.go b/internal/ap/extract.go index ce7c03901..ee6a513f6 100644 --- a/internal/ap/extract.go +++ b/internal/ap/extract.go @@ -32,6 +32,7 @@ import (  	"time"  	"github.com/superseriousbusiness/activity/pub" +	"github.com/superseriousbusiness/gotosocial/internal/gtserror"  	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"  	"github.com/superseriousbusiness/gotosocial/internal/util"  ) @@ -343,42 +344,59 @@ func ExtractURL(i WithURL) (*url.URL, error) {  	return nil, errors.New("could not extract url")  } -// ExtractPublicKeyForOwner extracts the public key from an interface, as long as it belongs to the specified owner. -// It will return the public key itself, the id/URL of the public key, or an error if something goes wrong. -func ExtractPublicKeyForOwner(i WithPublicKey, forOwner *url.URL) (*rsa.PublicKey, *url.URL, error) { -	publicKeyProp := i.GetW3IDSecurityV1PublicKey() -	if publicKeyProp == nil { -		return nil, nil, errors.New("public key property was nil") +// ExtractPublicKey extracts the public key, public key ID, and public +// key owner ID from an interface, or an error if something goes wrong. +func ExtractPublicKey(i WithPublicKey) ( +	*rsa.PublicKey, // pubkey +	*url.URL, // pubkey ID +	*url.URL, // pubkey owner +	error, +) { +	pubKeyProp := i.GetW3IDSecurityV1PublicKey() +	if pubKeyProp == nil { +		return nil, nil, nil, gtserror.New("public key property was nil")  	} -	for iter := publicKeyProp.Begin(); iter != publicKeyProp.End(); iter = iter.Next() { +	for iter := pubKeyProp.Begin(); iter != pubKeyProp.End(); iter = iter.Next() { +		if !iter.IsW3IDSecurityV1PublicKey() { +			continue +		} +  		pkey := iter.Get()  		if pkey == nil {  			continue  		} -		pkeyID, err := pub.GetId(pkey) -		if err != nil || pkeyID == nil { +		pubKeyID, err := pub.GetId(pkey) +		if err != nil { +			continue +		} + +		pubKeyOwnerProp := pkey.GetW3IDSecurityV1Owner() +		if pubKeyOwnerProp == nil {  			continue  		} -		if pkey.GetW3IDSecurityV1Owner() == nil || pkey.GetW3IDSecurityV1Owner().Get() == nil || pkey.GetW3IDSecurityV1Owner().Get().String() != forOwner.String() { +		pubKeyOwner := pubKeyOwnerProp.GetIRI() +		if pubKeyOwner == nil {  			continue  		} -		if pkey.GetW3IDSecurityV1PublicKeyPem() == nil { +		pubKeyPemProp := pkey.GetW3IDSecurityV1PublicKeyPem() +		if pubKeyPemProp == nil {  			continue  		} -		pkeyPem := pkey.GetW3IDSecurityV1PublicKeyPem().Get() +		pkeyPem := pubKeyPemProp.Get()  		if pkeyPem == "" {  			continue  		}  		block, _ := pem.Decode([]byte(pkeyPem))  		if block == nil { -			return nil, nil, errors.New("could not decode publicKeyPem: no PEM data") +			continue  		} +  		var p crypto.PublicKey  		switch block.Type {  		case "PUBLIC KEY": @@ -386,19 +404,26 @@ func ExtractPublicKeyForOwner(i WithPublicKey, forOwner *url.URL) (*rsa.PublicKe  		case "RSA PUBLIC KEY":  			p, err = x509.ParsePKCS1PublicKey(block.Bytes)  		default: -			return nil, nil, fmt.Errorf("could not parse public key: unknown block type: %q", block.Type) +			err = fmt.Errorf("unknown block type: %q", block.Type)  		}  		if err != nil { -			return nil, nil, fmt.Errorf("could not parse public key from block bytes: %s", err) +			err = gtserror.Newf("could not parse public key from block bytes: %w", err) +			return nil, nil, nil, err  		} +  		if p == nil { -			return nil, nil, errors.New("returned public key was empty") +			return nil, nil, nil, gtserror.New("returned public key was empty")  		} -		if publicKey, ok := p.(*rsa.PublicKey); ok { -			return publicKey, pkeyID, nil + +		pubKey, ok := p.(*rsa.PublicKey) +		if !ok { +			continue  		} + +		return pubKey, pubKeyID, pubKeyOwner, nil  	} -	return nil, nil, errors.New("couldn't find public key") + +	return nil, nil, nil, gtserror.New("couldn't find public key")  }  // ExtractContent returns a string representation of the interface's Content property, diff --git a/internal/api/activitypub/emoji/emojiget.go b/internal/api/activitypub/emoji/emojiget.go index 8ea84f565..8602b6052 100644 --- a/internal/api/activitypub/emoji/emojiget.go +++ b/internal/api/activitypub/emoji/emojiget.go @@ -42,7 +42,7 @@ func (m *Module) EmojiGetHandler(c *gin.Context) {  		return  	} -	resp, errWithCode := m.processor.Fedi().EmojiGet(apiutil.TransferSignatureContext(c), requestedEmojiID) +	resp, errWithCode := m.processor.Fedi().EmojiGet(c.Request.Context(), requestedEmojiID)  	if errWithCode != nil {  		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)  		return diff --git a/internal/api/activitypub/publickey/publickeyget.go b/internal/api/activitypub/publickey/publickeyget.go index 77e969469..5ccb86328 100644 --- a/internal/api/activitypub/publickey/publickeyget.go +++ b/internal/api/activitypub/publickey/publickeyget.go @@ -54,7 +54,7 @@ func (m *Module) PublicKeyGETHandler(c *gin.Context) {  		return  	} -	resp, errWithCode := m.processor.Fedi().UserGet(apiutil.TransferSignatureContext(c), requestedUsername, c.Request.URL) +	resp, errWithCode := m.processor.Fedi().UserGet(c.Request.Context(), requestedUsername, c.Request.URL)  	if errWithCode != nil {  		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)  		return diff --git a/internal/api/activitypub/users/featured.go b/internal/api/activitypub/users/featured.go index 9ccaed069..de6ff14ae 100644 --- a/internal/api/activitypub/users/featured.go +++ b/internal/api/activitypub/users/featured.go @@ -80,7 +80,7 @@ func (m *Module) FeaturedCollectionGETHandler(c *gin.Context) {  		return  	} -	resp, errWithCode := m.processor.Fedi().FeaturedCollectionGet(apiutil.TransferSignatureContext(c), requestedUsername) +	resp, errWithCode := m.processor.Fedi().FeaturedCollectionGet(c.Request.Context(), requestedUsername)  	if errWithCode != nil {  		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)  		return diff --git a/internal/api/activitypub/users/followers.go b/internal/api/activitypub/users/followers.go index ddfa3e9d0..0825651a8 100644 --- a/internal/api/activitypub/users/followers.go +++ b/internal/api/activitypub/users/followers.go @@ -51,7 +51,7 @@ func (m *Module) FollowersGETHandler(c *gin.Context) {  		return  	} -	resp, errWithCode := m.processor.Fedi().FollowersGet(apiutil.TransferSignatureContext(c), requestedUsername) +	resp, errWithCode := m.processor.Fedi().FollowersGet(c.Request.Context(), requestedUsername)  	if errWithCode != nil {  		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)  		return diff --git a/internal/api/activitypub/users/following.go b/internal/api/activitypub/users/following.go index 79722cea1..bc6e96ca1 100644 --- a/internal/api/activitypub/users/following.go +++ b/internal/api/activitypub/users/following.go @@ -51,7 +51,7 @@ func (m *Module) FollowingGETHandler(c *gin.Context) {  		return  	} -	resp, errWithCode := m.processor.Fedi().FollowingGet(apiutil.TransferSignatureContext(c), requestedUsername) +	resp, errWithCode := m.processor.Fedi().FollowingGet(c.Request.Context(), requestedUsername)  	if errWithCode != nil {  		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)  		return diff --git a/internal/api/activitypub/users/inboxpost.go b/internal/api/activitypub/users/inboxpost.go index 4f535f534..c2d3d79c4 100644 --- a/internal/api/activitypub/users/inboxpost.go +++ b/internal/api/activitypub/users/inboxpost.go @@ -30,7 +30,7 @@ import (  // InboxPOSTHandler deals with incoming POST requests to an actor's inbox.  // Eg., POST to https://example.org/users/whatever/inbox.  func (m *Module) InboxPOSTHandler(c *gin.Context) { -	_, err := m.processor.Fedi().InboxPost(apiutil.TransferSignatureContext(c), c.Writer, c.Request) +	_, err := m.processor.Fedi().InboxPost(c.Request.Context(), c.Writer, c.Request)  	if err != nil {  		errWithCode := new(gtserror.WithCode) diff --git a/internal/api/activitypub/users/inboxpost_test.go b/internal/api/activitypub/users/inboxpost_test.go index 82e86fb9c..c5027f342 100644 --- a/internal/api/activitypub/users/inboxpost_test.go +++ b/internal/api/activitypub/users/inboxpost_test.go @@ -517,6 +517,31 @@ func (suite *InboxPostTestSuite) TestPostFromBlockedAccount() {  	)  } +func (suite *InboxPostTestSuite) TestPostFromBlockedAccountToOtherAccount() { +	var ( +		requestingAccount = suite.testAccounts["remote_account_1"] +		targetAccount     = suite.testAccounts["local_account_1"] +		activity          = suite.testActivities["reply_to_turtle_for_turtle"] +		statusURI         = "http://fossbros-anonymous.io/users/foss_satan/statuses/2f1195a6-5cb0-4475-adf5-92ab9a0147fe" +	) + +	// Post an reply to turtle to ZORK from remote account. +	// Turtle blocks the remote account but is only tangentially +	// related to this POST request. The response will indicate +	// accepted but the post won't actually be processed. +	suite.inboxPost( +		activity.Activity, +		requestingAccount, +		targetAccount, +		http.StatusAccepted, +		`{"status":"Accepted"}`, +		suite.signatureCheck, +	) + +	_, err := suite.state.DB.GetStatusByURI(context.Background(), statusURI) +	suite.ErrorIs(err, db.ErrNoEntries) +} +  func (suite *InboxPostTestSuite) TestPostUnauthorized() {  	var (  		requestingAccount = suite.testAccounts["remote_account_1"] diff --git a/internal/api/activitypub/users/outboxget.go b/internal/api/activitypub/users/outboxget.go index 7abc3921f..9e3ec2d15 100644 --- a/internal/api/activitypub/users/outboxget.go +++ b/internal/api/activitypub/users/outboxget.go @@ -129,7 +129,7 @@ func (m *Module) OutboxGETHandler(c *gin.Context) {  		maxID = maxIDString  	} -	resp, errWithCode := m.processor.Fedi().OutboxGet(apiutil.TransferSignatureContext(c), requestedUsername, page, maxID, minID) +	resp, errWithCode := m.processor.Fedi().OutboxGet(c.Request.Context(), requestedUsername, page, maxID, minID)  	if errWithCode != nil {  		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)  		return diff --git a/internal/api/activitypub/users/repliesget.go b/internal/api/activitypub/users/repliesget.go index bece312b8..70764a73d 100644 --- a/internal/api/activitypub/users/repliesget.go +++ b/internal/api/activitypub/users/repliesget.go @@ -149,7 +149,7 @@ func (m *Module) StatusRepliesGETHandler(c *gin.Context) {  		minID = minIDString  	} -	resp, errWithCode := m.processor.Fedi().StatusRepliesGet(apiutil.TransferSignatureContext(c), requestedUsername, requestedStatusID, page, onlyOtherAccounts, c.Query("only_other_accounts") != "", minID) +	resp, errWithCode := m.processor.Fedi().StatusRepliesGet(c.Request.Context(), requestedUsername, requestedStatusID, page, onlyOtherAccounts, c.Query("only_other_accounts") != "", minID)  	if errWithCode != nil {  		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)  		return diff --git a/internal/api/activitypub/users/statusget.go b/internal/api/activitypub/users/statusget.go index b9e526080..4a107c5a1 100644 --- a/internal/api/activitypub/users/statusget.go +++ b/internal/api/activitypub/users/statusget.go @@ -58,7 +58,7 @@ func (m *Module) StatusGETHandler(c *gin.Context) {  		return  	} -	resp, errWithCode := m.processor.Fedi().StatusGet(apiutil.TransferSignatureContext(c), requestedUsername, requestedStatusID) +	resp, errWithCode := m.processor.Fedi().StatusGet(c.Request.Context(), requestedUsername, requestedStatusID)  	if errWithCode != nil {  		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)  		return diff --git a/internal/api/activitypub/users/user_test.go b/internal/api/activitypub/users/user_test.go index 8e30eecf3..d0487777b 100644 --- a/internal/api/activitypub/users/user_test.go +++ b/internal/api/activitypub/users/user_test.go @@ -56,6 +56,7 @@ type UserStandardTestSuite struct {  	testAttachments  map[string]*gtsmodel.MediaAttachment  	testStatuses     map[string]*gtsmodel.Status  	testBlocks       map[string]*gtsmodel.Block +	testActivities   map[string]testrig.ActivityWithSignature  	// module being tested  	userModule *users.Module @@ -72,6 +73,7 @@ func (suite *UserStandardTestSuite) SetupSuite() {  	suite.testAttachments = testrig.NewTestAttachments()  	suite.testStatuses = testrig.NewTestStatuses()  	suite.testBlocks = testrig.NewTestBlocks() +	suite.testActivities = testrig.NewTestActivities(suite.testAccounts)  }  func (suite *UserStandardTestSuite) SetupTest() { diff --git a/internal/api/activitypub/users/userget.go b/internal/api/activitypub/users/userget.go index 7dc7f0822..536da9e81 100644 --- a/internal/api/activitypub/users/userget.go +++ b/internal/api/activitypub/users/userget.go @@ -58,7 +58,7 @@ func (m *Module) UsersGETHandler(c *gin.Context) {  		return  	} -	resp, errWithCode := m.processor.Fedi().UserGet(apiutil.TransferSignatureContext(c), requestedUsername, c.Request.URL) +	resp, errWithCode := m.processor.Fedi().UserGet(c.Request.Context(), requestedUsername, c.Request.URL)  	if errWithCode != nil {  		apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)  		return diff --git a/internal/api/util/signaturectx.go b/internal/api/util/signaturectx.go deleted file mode 100644 index 38abfdeb1..000000000 --- a/internal/api/util/signaturectx.go +++ /dev/null @@ -1,40 +0,0 @@ -// GoToSocial -// Copyright (C) GoToSocial Authors admin@gotosocial.org -// SPDX-License-Identifier: AGPL-3.0-or-later -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program.  If not, see <http://www.gnu.org/licenses/>. - -package util - -import ( -	"context" - -	"github.com/gin-gonic/gin" -	"github.com/superseriousbusiness/gotosocial/internal/ap" -) - -// TransferSignatureContext transfers a signature verifier and signature from a gin context to a go context. -func TransferSignatureContext(c *gin.Context) context.Context { -	ctx := c.Request.Context() - -	if verifier, signed := c.Get(string(ap.ContextRequestingPublicKeyVerifier)); signed { -		ctx = context.WithValue(ctx, ap.ContextRequestingPublicKeyVerifier, verifier) -	} - -	if signature, signed := c.Get(string(ap.ContextRequestingPublicKeySignature)); signed { -		ctx = context.WithValue(ctx, ap.ContextRequestingPublicKeySignature, signature) -	} - -	return ctx -} diff --git a/internal/federation/authenticate.go b/internal/federation/authenticate.go index 5fe4873d4..80998680e 100644 --- a/internal/federation/authenticate.go +++ b/internal/federation/authenticate.go @@ -19,298 +19,297 @@ package federation  import (  	"context" -	"crypto/x509" +	"crypto/rsa"  	"encoding/json" -	"encoding/pem"  	"errors"  	"fmt"  	"net/http"  	"net/url" -	"strings" +	"codeberg.org/gruf/go-kv"  	"github.com/go-fed/httpsig" -	"github.com/superseriousbusiness/activity/pub"  	"github.com/superseriousbusiness/activity/streams" -	"github.com/superseriousbusiness/activity/streams/vocab"  	"github.com/superseriousbusiness/gotosocial/internal/ap"  	"github.com/superseriousbusiness/gotosocial/internal/config"  	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"  	"github.com/superseriousbusiness/gotosocial/internal/gtserror" -	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"  	"github.com/superseriousbusiness/gotosocial/internal/log"  ) -/* -publicKeyer is BORROWED DIRECTLY FROM https://github.com/go-fed/apcore/blob/master/ap/util.go -Thank you @cj@mastodon.technology ! <3 -*/ -type publicKeyer interface { -	GetW3IDSecurityV1PublicKey() vocab.W3IDSecurityV1PublicKeyProperty -} - -/* -getPublicKeyFromResponse is adapted from https://github.com/go-fed/apcore/blob/master/ap/util.go -Thank you @cj@mastodon.technology ! <3 -*/ -func getPublicKeyFromResponse(c context.Context, b []byte, keyID *url.URL) (vocab.W3IDSecurityV1PublicKey, error) { -	m := make(map[string]interface{}) -	if err := json.Unmarshal(b, &m); err != nil { -		return nil, err +var ( +	errUnsigned       = errors.New("http request wasn't signed or http signature was invalid") +	signingAlgorithms = []httpsig.Algorithm{ +		httpsig.RSA_SHA256, // Prefer common RSA_SHA256. +		httpsig.RSA_SHA512, // Fall back to less common RSA_SHA512. +		httpsig.ED25519,    // Try ED25519 as a long shot.  	} +) -	t, err := streams.ToType(c, m) -	if err != nil { -		return nil, err +// AuthenticateFederatedRequest authenticates any kind of incoming federated +// request from a remote server. This includes things like GET requests for +// dereferencing our users or statuses etc, and POST requests for delivering +// new Activities. The function returns the URL of the owner of the public key +// used in the requesting http signature. +// +// 'Authenticate' in this case is defined as making sure that the http request +// is actually signed by whoever claims to have signed it, by fetching the public +// key from the signature and checking it against the remote public key. +// +// The provided username will be used to generate a transport for making remote +// requests/derefencing the public key ID of the request signature. Ideally you +// should pass in the username of the user *being requested*, so that the remote +// server can decide how to handle the request based on who's making it. Ie., if +// the request on this server is for https://example.org/users/some_username then +// you should pass in the username 'some_username'. The remote server will then +// know that this is the user making the dereferencing request, and they can decide +// to allow or deny the request depending on their settings. +// +// Note that it is also valid to pass in an empty string here, in which case the +// keys of the instance account will be used. +// +// Also note that this function *does not* dereference the remote account that +// the signature key is associated with. Other functions should use the returned +// URL to dereference the remote account, if required. +func (f *federator) AuthenticateFederatedRequest(ctx context.Context, requestedUsername string) (*url.URL, gtserror.WithCode) { +	// Thanks to the signature check middleware, +	// we should already have an http signature +	// verifier set on the context. If we don't, +	// this is an unsigned request. +	verifier := gtscontext.HTTPSignatureVerifier(ctx) +	if verifier == nil { +		err := gtserror.Newf("%w", errUnsigned) +		errWithCode := gtserror.NewErrorUnauthorized(err, errUnsigned.Error(), "(verifier)") +		return nil, errWithCode  	} -	pker, ok := t.(publicKeyer) -	if !ok { -		return nil, fmt.Errorf("ActivityStreams type cannot be converted to one known to have publicKey property: %T", t) +	// We should have the signature itself set too. +	signature := gtscontext.HTTPSignature(ctx) +	if signature == "" { +		err := gtserror.Newf("%w", errUnsigned) +		errWithCode := gtserror.NewErrorUnauthorized(err, errUnsigned.Error(), "(signature)") +		return nil, errWithCode  	} -	pkp := pker.GetW3IDSecurityV1PublicKey() -	if pkp == nil { -		return nil, errors.New("publicKey property is not provided") +	// And finally the public key ID URI. +	pubKeyID := gtscontext.HTTPSignaturePubKeyID(ctx) +	if pubKeyID == nil { +		err := gtserror.Newf("%w", errUnsigned) +		errWithCode := gtserror.NewErrorUnauthorized(err, errUnsigned.Error(), "(pubKeyID)") +		return nil, errWithCode  	} -	var pkpFound vocab.W3IDSecurityV1PublicKey -	for pkpIter := pkp.Begin(); pkpIter != pkp.End(); pkpIter = pkpIter.Next() { -		if !pkpIter.IsW3IDSecurityV1PublicKey() { -			continue -		} -		pkValue := pkpIter.Get() -		var pkID *url.URL -		pkID, err = pub.GetId(pkValue) -		if err != nil { -			return nil, err -		} -		if pkID.String() != keyID.String() { -			continue -		} -		pkpFound = pkValue -		break -	} +	// At this point we know the request was signed, +	// so now we need to validate the signature. -	if pkpFound == nil { -		return nil, fmt.Errorf("cannot find publicKey with id: %s", keyID) -	} +	var ( +		pubKeyIDStr          = pubKeyID.String() +		requestingAccountURI *url.URL +		pubKey               interface{} +		errWithCode          gtserror.WithCode +	) -	return pkpFound, nil -} +	l := log. +		WithContext(ctx). +		WithFields(kv.Fields{ +			{"requestedUsername", requestedUsername}, +			{"pubKeyID", pubKeyIDStr}, +		}...) -// AuthenticateFederatedRequest authenticates any kind of incoming federated request from a remote server. This includes things like -// GET requests for dereferencing our users or statuses etc, and POST requests for delivering new Activities. The function returns -// the URL of the owner of the public key used in the requesting http signature. -// -// Authenticate in this case is defined as making sure that the http request is actually signed by whoever claims -// to have signed it, by fetching the public key from the signature and checking it against the remote public key. -// -// The provided username will be used to generate a transport for making remote requests/derefencing the public key ID of the request signature. -// Ideally you should pass in the username of the user *being requested*, so that the remote server can decide how to handle the request based on who's making it. -// Ie., if the request on this server is for https://example.org/users/some_username then you should pass in the username 'some_username'. -// The remote server will then know that this is the user making the dereferencing request, and they can decide to allow or deny the request depending on their settings. -// -// Note that it is also valid to pass in an empty string here, in which case the keys of the instance account will be used. -// -// Also note that this function *does not* dereference the remote account that the signature key is associated with. -// Other functions should use the returned URL to dereference the remote account, if required. -func (f *federator) AuthenticateFederatedRequest(ctx context.Context, requestedUsername string) (*url.URL, gtserror.WithCode) { -	var publicKey interface{} -	var pkOwnerURI *url.URL -	var err error - -	// thanks to signaturecheck.go in the security package, we should already have a signature verifier set on the context -	vi := ctx.Value(ap.ContextRequestingPublicKeyVerifier) -	if vi == nil { -		err := errors.New("http request wasn't signed or http signature was invalid") -		errWithCode := gtserror.NewErrorUnauthorized(err, err.Error()) -		log.Debug(ctx, errWithCode) -		return nil, errWithCode +	if pubKeyID.Host == config.GetHost() { +		l.Trace("public key is ours, no dereference needed") +		requestingAccountURI, pubKey, errWithCode = f.derefDBOnly(ctx, pubKeyIDStr) +	} else { +		l.Trace("public key is not ours, checking if we need to dereference") +		requestingAccountURI, pubKey, errWithCode = f.deref(ctx, requestedUsername, pubKeyIDStr, pubKeyID)  	} -	verifier, ok := vi.(httpsig.Verifier) -	if !ok { -		err := errors.New("http request wasn't signed or http signature was invalid") -		errWithCode := gtserror.NewErrorUnauthorized(err, err.Error()) -		log.Debug(ctx, errWithCode) +	if errWithCode != nil {  		return nil, errWithCode  	} -	// we should have the signature itself set too -	si := ctx.Value(ap.ContextRequestingPublicKeySignature) -	if si == nil { -		err := errors.New("http request wasn't signed or http signature was invalid") -		errWithCode := gtserror.NewErrorUnauthorized(err, err.Error()) -		log.Debug(ctx, errWithCode) -		return nil, errWithCode +	// Ensure public key now defined. +	if pubKey == nil { +		err := gtserror.New("public key was nil") +		return nil, gtserror.NewErrorInternalError(err)  	} -	signature, ok := si.(string) -	if !ok { -		err := errors.New("http request wasn't signed or http signature was invalid") -		errWithCode := gtserror.NewErrorUnauthorized(err, err.Error()) -		log.Debug(ctx, errWithCode) -		return nil, errWithCode +	// Try to authenticate using permitted algorithms in +	// order of most -> least common. Return OK as soon +	// as one passes. +	for _, algo := range signingAlgorithms { +		l.Tracef("trying %s", algo) + +		err := verifier.Verify(pubKey, algo) +		if err == nil { +			l.Tracef("authentication PASSED with %s", algo) +			return requestingAccountURI, nil +		} + +		l.Tracef("authentication NOT PASSED with %s: %q", algo, err) +	} + +	// At this point no algorithms passed. +	err := gtserror.Newf( +		"authentication NOT PASSED for public key %s; tried algorithms %+v; signature value was '%s'", +		pubKeyIDStr, signature, signingAlgorithms, +	) + +	return nil, gtserror.NewErrorUnauthorized(err, err.Error()) +} + +// derefDBOnly tries to dereference the given public +// key using only entries already in the database. +func (f *federator) derefDBOnly( +	ctx context.Context, +	pubKeyIDStr string, +) (*url.URL, interface{}, gtserror.WithCode) { +	reqAcct, err := f.db.GetAccountByPubkeyID(ctx, pubKeyIDStr) +	if err != nil { +		err = gtserror.Newf("db error getting account with pubKeyID %s: %w", pubKeyIDStr, err) +		return nil, nil, gtserror.NewErrorInternalError(err)  	} -	// now figure out who actually signed it -	requestingPublicKeyID, err := url.Parse(verifier.KeyId()) +	reqAcctURI, err := url.Parse(reqAcct.URI)  	if err != nil { -		errWithCode := gtserror.NewErrorBadRequest(err, fmt.Sprintf("couldn't parse public key URL %s", verifier.KeyId())) -		log.Debug(ctx, errWithCode) -		return nil, errWithCode +		err = gtserror.Newf("error parsing account uri with pubKeyID %s: %w", pubKeyIDStr, err) +		return nil, nil, gtserror.NewErrorInternalError(err)  	} -	var ( -		requestingLocalAccount  *gtsmodel.Account -		requestingRemoteAccount *gtsmodel.Account -		requestingHost          = requestingPublicKeyID.Host -	) +	return reqAcctURI, reqAcct.PublicKey, nil +} -	if host := config.GetHost(); strings.EqualFold(requestingHost, host) { -		// LOCAL ACCOUNT REQUEST -		// the request is coming from INSIDE THE HOUSE so skip the remote dereferencing -		log.Tracef(ctx, "proceeding without dereference for local public key %s", requestingPublicKeyID) +// deref tries to dereference the given public key by first +// checking in the database, and then (if no entries found) +// calling the remote pub key URI and extracting the key. +func (f *federator) deref( +	ctx context.Context, +	requestedUsername string, +	pubKeyIDStr string, +	pubKeyID *url.URL, +) (*url.URL, interface{}, gtserror.WithCode) { +	l := log. +		WithContext(ctx). +		WithFields(kv.Fields{ +			{"requestedUsername", requestedUsername}, +			{"pubKeyID", pubKeyIDStr}, +		}...) + +	// Try a database only deref first. We may already +	// have the requesting account cached locally. +	reqAcctURI, pubKey, errWithCode := f.derefDBOnly(ctx, pubKeyIDStr) +	if errWithCode == nil { +		l.Trace("public key cached, no dereference needed") +		return reqAcctURI, pubKey, nil +	} -		requestingLocalAccount, err = f.db.GetAccountByPubkeyID(ctx, requestingPublicKeyID.String()) -		if err != nil { -			errWithCode := gtserror.NewErrorInternalError(fmt.Errorf("couldn't get account with public key uri %s from the database: %s", requestingPublicKeyID.String(), err)) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode -		} +	l.Trace("public key not cached, trying dereference") -		publicKey = requestingLocalAccount.PublicKey +	// If we've tried to get this account before and we +	// now have a tombstone for it (ie., it's been deleted +	// from remote), don't try to dereference it again. +	gone, err := f.CheckGone(ctx, pubKeyID) +	if err != nil { +		err := gtserror.Newf("error checking for tombstone for %s: %w", pubKeyIDStr, err) +		return nil, nil, gtserror.NewErrorInternalError(err) +	} -		pkOwnerURI, err = url.Parse(requestingLocalAccount.URI) -		if err != nil { -			errWithCode := gtserror.NewErrorBadRequest(err, fmt.Sprintf("couldn't parse public key owner URL %s", requestingLocalAccount.URI)) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode -		} -	} else if requestingRemoteAccount, err = f.db.GetAccountByPubkeyID(ctx, requestingPublicKeyID.String()); err == nil { -		// REMOTE ACCOUNT REQUEST WITH KEY CACHED LOCALLY -		// this is a remote account and we already have the public key for it so use that -		log.Tracef(ctx, "proceeding without dereference for cached public key %s", requestingPublicKeyID) -		publicKey = requestingRemoteAccount.PublicKey -		pkOwnerURI, err = url.Parse(requestingRemoteAccount.URI) -		if err != nil { -			errWithCode := gtserror.NewErrorBadRequest(err, fmt.Sprintf("couldn't parse public key owner URL %s", requestingRemoteAccount.URI)) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode -		} -	} else { -		// REMOTE ACCOUNT REQUEST WITHOUT KEY CACHED LOCALLY -		// the request is remote and we don't have the public key yet, -		// so we need to authenticate the request properly by dereferencing the remote key -		gone, err := f.CheckGone(ctx, requestingPublicKeyID) -		if err != nil { -			errWithCode := gtserror.NewErrorInternalError(fmt.Errorf("error checking for tombstone for %s: %s", requestingPublicKeyID, err)) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode -		} +	if gone { +		err := gtserror.Newf("account with public key %s is gone", pubKeyIDStr) +		return nil, nil, gtserror.NewErrorGone(err) +	} -		if gone { -			errWithCode := gtserror.NewErrorGone(fmt.Errorf("account with public key %s is gone", requestingPublicKeyID)) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode -		} +	// Make an http call to get the pubkey. +	pubKeyBytes, errWithCode := f.callForPubKey(ctx, requestedUsername, pubKeyID) +	if errWithCode != nil { +		return nil, nil, errWithCode +	} -		log.Tracef(ctx, "proceeding with dereference for uncached public key %s", requestingPublicKeyID) -		trans, err := f.transportController.NewTransportForUsername(gtscontext.SetFastFail(ctx), requestedUsername) -		if err != nil { -			errWithCode := gtserror.NewErrorInternalError(fmt.Errorf("error creating transport for %s: %s", requestedUsername, err)) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode -		} +	// Extract the key and the owner from the response. +	pubKey, pubKeyOwner, err := parsePubKeyBytes(ctx, pubKeyBytes, pubKeyID) +	if err != nil { +		err := fmt.Errorf("error parsing public key %s: %w", pubKeyID, err) +		return nil, nil, gtserror.NewErrorUnauthorized(err) +	} -		// The actual http call to the remote server is made right here in the Dereference function. -		b, err := trans.Dereference(ctx, requestingPublicKeyID) -		if err != nil { -			if gtserror.StatusCode(err) == http.StatusGone { -				// if we get a 410 error it means the account that owns this public key has been deleted; -				// we should add a tombstone to our database so that we can avoid trying to deref it in future -				if err := f.HandleGone(ctx, requestingPublicKeyID); err != nil { -					errWithCode := gtserror.NewErrorInternalError(fmt.Errorf("error marking account with public key %s as gone: %s", requestingPublicKeyID, err)) -					log.Debug(ctx, errWithCode) -					return nil, errWithCode -				} -				errWithCode := gtserror.NewErrorGone(fmt.Errorf("account with public key %s is gone", requestingPublicKeyID)) -				log.Debug(ctx, errWithCode) -				return nil, errWithCode -			} - -			errWithCode := gtserror.NewErrorUnauthorized(fmt.Errorf("error dereferencing public key %s: %s", requestingPublicKeyID, err)) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode -		} +	return pubKeyOwner, pubKey, nil +} -		// if the key isn't in the response, we can't authenticate the request -		requestingPublicKey, err := getPublicKeyFromResponse(ctx, b, requestingPublicKeyID) -		if err != nil { -			errWithCode := gtserror.NewErrorUnauthorized(fmt.Errorf("error parsing public key %s: %s", requestingPublicKeyID, err)) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode -		} +// callForPubKey handles the nitty gritty of actually +// making a request for the given pubKeyID with a +// transport created on behalf of requestedUsername. +func (f *federator) callForPubKey( +	ctx context.Context, +	requestedUsername string, +	pubKeyID *url.URL, +) ([]byte, gtserror.WithCode) { +	// Use a transport to dereference the remote. +	trans, err := f.transportController.NewTransportForUsername( +		// We're on a hot path: don't retry if req fails. +		gtscontext.SetFastFail(ctx), +		requestedUsername, +	) +	if err != nil { +		err = gtserror.Newf("error creating transport for %s: %w", requestedUsername, err) +		return nil, gtserror.NewErrorInternalError(err) +	} -		// we should be able to get the actual key embedded in the vocab.W3IDSecurityV1PublicKey -		pkPemProp := requestingPublicKey.GetW3IDSecurityV1PublicKeyPem() -		if pkPemProp == nil || !pkPemProp.IsXMLSchemaString() { -			errWithCode := gtserror.NewErrorUnauthorized(errors.New("publicKeyPem property is not provided or it is not embedded as a value")) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode -		} +	// The actual http call to the remote server is +	// made right here by the Dereference function. +	pubKeyBytes, err := trans.Dereference(ctx, pubKeyID) +	if err == nil { +		// No problem. +		return pubKeyBytes, nil +	} -		// and decode the PEM so that we can parse it as a golang public key -		pubKeyPem := pkPemProp.Get() -		block, _ := pem.Decode([]byte(pubKeyPem)) -		if block == nil || block.Type != "PUBLIC KEY" { -			errWithCode := gtserror.NewErrorUnauthorized(errors.New("could not decode publicKeyPem to PUBLIC KEY pem block type")) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode +	if gtserror.StatusCode(err) == http.StatusGone { +		// 410 indicates remote public key no longer exists +		// (account deleted, moved, etc). Add a tombstone +		// to our database so that we can avoid trying to +		// dereference it in future. +		if err := f.HandleGone(ctx, pubKeyID); err != nil { +			err := gtserror.Newf("error marking public key %s as gone: %w", pubKeyID, err) +			return nil, gtserror.NewErrorInternalError(err)  		} -		publicKey, err = x509.ParsePKIXPublicKey(block.Bytes) -		if err != nil { -			errWithCode := gtserror.NewErrorUnauthorized(fmt.Errorf("could not parse public key %s from block bytes: %s", requestingPublicKeyID, err)) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode -		} +		err := gtserror.Newf("account with public key %s is gone", pubKeyID) +		return nil, gtserror.NewErrorGone(err) +	} -		// all good! we just need the URI of the key owner to return -		pkOwnerProp := requestingPublicKey.GetW3IDSecurityV1Owner() -		if pkOwnerProp == nil || !pkOwnerProp.IsIRI() { -			errWithCode := gtserror.NewErrorUnauthorized(errors.New("publicKeyOwner property is not provided or it is not embedded as a value")) -			log.Debug(ctx, errWithCode) -			return nil, errWithCode -		} -		pkOwnerURI = pkOwnerProp.GetIRI() +	// Fall back to generic error. +	err = gtserror.Newf("error dereferencing public key %s: %w", pubKeyID, err) +	return nil, gtserror.NewErrorInternalError(err) +} + +// parsePubKeyBytes extracts an rsa public key from the +// given pubKeyBytes by trying to parse the pubKeyBytes +// as an ActivityPub type. It will return the public key +// itself, and the URI of the public key owner. +func parsePubKeyBytes( +	ctx context.Context, +	pubKeyBytes []byte, +	pubKeyID *url.URL, +) (*rsa.PublicKey, *url.URL, error) { +	m := make(map[string]interface{}) +	if err := json.Unmarshal(pubKeyBytes, &m); err != nil { +		return nil, nil, err  	} -	// after all that, public key should be defined -	if publicKey == nil { -		errWithCode := gtserror.NewErrorInternalError(errors.New("returned public key was empty")) -		log.Debug(ctx, errWithCode) -		return nil, errWithCode +	t, err := streams.ToType(ctx, m) +	if err != nil { +		return nil, nil, err  	} -	// do the actual authentication here! -	algos := []httpsig.Algorithm{ -		httpsig.RSA_SHA256, -		httpsig.RSA_SHA512, -		httpsig.ED25519, +	withPublicKey, ok := t.(ap.WithPublicKey) +	if !ok { +		err = gtserror.Newf("resource at %s with type %T could not be converted to ap.WithPublicKey", pubKeyID, t) +		return nil, nil, err  	} -	for _, algo := range algos { -		log.Tracef(ctx, "trying algo: %s", algo) -		err := verifier.Verify(publicKey, algo) -		if err == nil { -			log.Tracef(ctx, "authentication for %s PASSED with algorithm %s", pkOwnerURI, algo) -			return pkOwnerURI, nil -		} -		log.Tracef(ctx, "authentication for %s NOT PASSED with algorithm %s: %s", pkOwnerURI, algo, err) +	pubKey, _, pubKeyOwnerID, err := ap.ExtractPublicKey(withPublicKey) +	if err != nil { +		err = gtserror.Newf("resource at %s with type %T did not contain recognizable public key", pubKeyID, t) +		return nil, nil, err  	} -	errWithCode := gtserror.NewErrorUnauthorized(fmt.Errorf("authentication not passed for public key owner %s; signature value was '%s'", pkOwnerURI, signature)) -	log.Debug(ctx, errWithCode) -	return nil, errWithCode +	return pubKey, pubKeyOwnerID, nil  } diff --git a/internal/federation/federatingactor.go b/internal/federation/federatingactor.go index 25282235a..708114484 100644 --- a/internal/federation/federatingactor.go +++ b/internal/federation/federatingactor.go @@ -84,15 +84,14 @@ func IsASMediaType(ct string) bool {  	}  } -// federatingActor wraps the pub.FederatingActor interface +// federatingActor wraps the pub.FederatingActor  // with some custom GoToSocial-specific logic.  type federatingActor struct {  	sideEffectActor pub.DelegateActor  	wrapped         pub.FederatingActor  } -// newFederatingProtocol returns a new federatingActor, which -// implements the pub.FederatingActor interface. +// newFederatingActor returns a federatingActor.  func newFederatingActor(c pub.CommonBehavior, s2s pub.FederatingProtocol, db pub.Database, clock pub.Clock) pub.FederatingActor {  	sideEffectActor := pub.NewSideEffectActor(c, s2s, nil, db, clock)  	sideEffectActor.Serialize = ap.Serialize // hook in our own custom Serialize function @@ -133,8 +132,11 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr  	ctx, authenticated, err := f.sideEffectActor.AuthenticatePostInbox(ctx, w, r)  	if err != nil {  		return false, gtserror.NewErrorInternalError(err) -	} else if !authenticated { -		return false, gtserror.NewErrorUnauthorized(errors.New("unauthorized")) +	} + +	if !authenticated { +		err = errors.New("not authenticated") +		return false, gtserror.NewErrorUnauthorized(err)  	}  	/* @@ -148,20 +150,38 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr  		return false, errWithCode  	} -	// Set additional context data. +	// Set additional context data. Primarily this means +	// looking at the Activity and seeing which IRIs are +	// involved in it tangentially.  	ctx, err = f.sideEffectActor.PostInboxRequestBodyHook(ctx, r, activity)  	if err != nil {  		return false, gtserror.NewErrorInternalError(err)  	} -	// Check authorization of the activity. +	// Check authorization of the activity; this will include blocks.  	authorized, err := f.sideEffectActor.AuthorizePostInbox(ctx, w, activity)  	if err != nil { +		if errors.As(err, new(errOtherIRIBlocked)) { +			// There's no direct block between requester(s) and +			// receiver. However, one or more of the other IRIs +			// involved in the request (account replied to, note +			// boosted, etc) is blocked either at domain level or +			// by the receiver. We don't need to return 403 here, +			// instead, just return 202 accepted but don't do any +			// further processing of the activity. +			return true, nil +		} + +		// Real error has occurred.  		return false, gtserror.NewErrorInternalError(err)  	}  	if !authorized { -		return false, gtserror.NewErrorForbidden(errors.New("blocked")) +		// Block exists either from this instance against +		// one or more directly involved actors, or between +		// receiving account and one of those actors. +		err = errors.New("blocked") +		return false, gtserror.NewErrorForbidden(err)  	}  	// Copy existing URL + add request host and scheme. diff --git a/internal/federation/federatingactor_test.go b/internal/federation/federatingactor_test.go index a8f71af4c..cdc265d82 100644 --- a/internal/federation/federatingactor_test.go +++ b/internal/federation/federatingactor_test.go @@ -58,7 +58,7 @@ func (suite *FederatingActorTestSuite) TestSendNoRemoteFollowers() {  	tc := testrig.NewTestTransportController(&suite.state, httpClient)  	// setup module being tested -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) +	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.typeconverter, testrig.NewTestMediaManager(&suite.state))  	activity, err := federator.FederatingActor().Send(ctx, testrig.URLMustParse(testAccount.OutboxURI), testActivity)  	suite.NoError(err) @@ -73,7 +73,7 @@ func (suite *FederatingActorTestSuite) TestSendRemoteFollower() {  	testAccount := suite.testAccounts["local_account_1"]  	testRemoteAccount := suite.testAccounts["remote_account_1"] -	err := suite.db.Put(ctx, >smodel.Follow{ +	err := suite.state.DB.Put(ctx, >smodel.Follow{  		ID:              "01G1TRWV4AYCDBX5HRWT2EVBCV",  		CreatedAt:       testrig.TimeMustParse("2022-06-02T12:22:21+02:00"),  		UpdatedAt:       testrig.TimeMustParse("2022-06-02T12:22:21+02:00"), @@ -103,7 +103,7 @@ func (suite *FederatingActorTestSuite) TestSendRemoteFollower() {  	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media")  	tc := testrig.NewTestTransportController(&suite.state, httpClient)  	// setup module being tested -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) +	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.typeconverter, testrig.NewTestMediaManager(&suite.state))  	activity, err := federator.FederatingActor().Send(ctx, testrig.URLMustParse(testAccount.OutboxURI), testActivity)  	suite.NoError(err) diff --git a/internal/federation/federatingdb/accept.go b/internal/federation/federatingdb/accept.go index 1c39bc134..7d3e16d4e 100644 --- a/internal/federation/federatingdb/accept.go +++ b/internal/federation/federatingdb/accept.go @@ -41,12 +41,9 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA  		l.Debug("entering Accept")  	} -	receivingAccount, _ := extractFromCtx(ctx) -	if receivingAccount == nil { -		// If the receiving account  wasn't set on the context, that means this request didn't pass -		// through the API, but came from inside GtS as the result of another activity on this instance. That being so, -		// we can safely just ignore this activity, since we know we've already processed it elsewhere. -		return nil +	receivingAccount, _, internal := extractFromCtx(ctx) +	if internal { +		return nil // Already processed.  	}  	acceptObject := accept.GetActivityStreamsObject() diff --git a/internal/federation/federatingdb/announce.go b/internal/federation/federatingdb/announce.go index 88b150cf0..ab230cdfb 100644 --- a/internal/federation/federatingdb/announce.go +++ b/internal/federation/federatingdb/announce.go @@ -39,12 +39,9 @@ func (f *federatingDB) Announce(ctx context.Context, announce vocab.ActivityStre  		l.Debug("entering Announce")  	} -	receivingAccount, _ := extractFromCtx(ctx) -	if receivingAccount == nil { -		// If the receiving account wasn't set on the context, that means this request didn't pass -		// through the API, but came from inside GtS as the result of another activity on this instance. That being so, -		// we can safely just ignore this activity, since we know we've already processed it elsewhere. -		return nil +	receivingAccount, _, internal := extractFromCtx(ctx) +	if internal { +		return nil // Already processed.  	}  	boost, isNew, err := f.typeConverter.ASAnnounceToStatus(ctx, announce) diff --git a/internal/federation/federatingdb/create.go b/internal/federation/federatingdb/create.go index f68279d37..3650f8c3d 100644 --- a/internal/federation/federatingdb/create.go +++ b/internal/federation/federatingdb/create.go @@ -57,12 +57,9 @@ func (f *federatingDB) Create(ctx context.Context, asType vocab.Type) error {  		l.Trace("entering Create")  	} -	receivingAccount, requestingAccount := extractFromCtx(ctx) -	if receivingAccount == nil { -		// If the receiving account wasn't set on the context, that means this request didn't pass -		// through the API, but came from inside GtS as the result of another activity on this instance. That being so, -		// we can safely just ignore this activity, since we know we've already processed it elsewhere. -		return nil +	receivingAccount, requestingAccount, internal := extractFromCtx(ctx) +	if internal { +		return nil // Already processed.  	}  	switch asType.GetTypeName() { diff --git a/internal/federation/federatingdb/delete.go b/internal/federation/federatingdb/delete.go index 7a8b51329..95f9be354 100644 --- a/internal/federation/federatingdb/delete.go +++ b/internal/federation/federatingdb/delete.go @@ -40,12 +40,9 @@ func (f *federatingDB) Delete(ctx context.Context, id *url.URL) error {  		}...)  	l.Debug("entering Delete") -	receivingAccount, requestingAccount := extractFromCtx(ctx) -	if receivingAccount == nil { -		// If the receiving account wasn't set on the context, that means this request didn't pass -		// through the API, but came from inside GtS as the result of another activity on this instance. That being so, -		// we can safely just ignore this activity, since we know we've already processed it elsewhere. -		return nil +	receivingAccount, requestingAccount, internal := extractFromCtx(ctx) +	if internal { +		return nil // Already processed.  	}  	// in a delete we only get the URI, we can't know if we have a status or a profile or something else, diff --git a/internal/federation/federatingdb/federatingdb_test.go b/internal/federation/federatingdb/federatingdb_test.go index 6a6cb9262..6a8754519 100644 --- a/internal/federation/federatingdb/federatingdb_test.go +++ b/internal/federation/federatingdb/federatingdb_test.go @@ -21,9 +21,9 @@ import (  	"context"  	"github.com/stretchr/testify/suite" -	"github.com/superseriousbusiness/gotosocial/internal/ap"  	"github.com/superseriousbusiness/gotosocial/internal/db"  	"github.com/superseriousbusiness/gotosocial/internal/federation/federatingdb" +	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"  	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"  	"github.com/superseriousbusiness/gotosocial/internal/messages"  	"github.com/superseriousbusiness/gotosocial/internal/state" @@ -107,7 +107,7 @@ func (suite *FederatingDBTestSuite) TearDownTest() {  func createTestContext(receivingAccount *gtsmodel.Account, requestingAccount *gtsmodel.Account) context.Context {  	ctx := context.Background() -	ctx = context.WithValue(ctx, ap.ContextReceivingAccount, receivingAccount) -	ctx = context.WithValue(ctx, ap.ContextRequestingAccount, requestingAccount) +	ctx = gtscontext.SetReceivingAccount(ctx, receivingAccount) +	ctx = gtscontext.SetRequestingAccount(ctx, requestingAccount)  	return ctx  } diff --git a/internal/federation/federatingdb/reject.go b/internal/federation/federatingdb/reject.go index 17f0f84d0..10882db83 100644 --- a/internal/federation/federatingdb/reject.go +++ b/internal/federation/federatingdb/reject.go @@ -40,12 +40,9 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR  		l.Debug("entering Reject")  	} -	receivingAccount, _ := extractFromCtx(ctx) -	if receivingAccount == nil { -		// If the receiving account or federator channel wasn't set on the context, that means this request didn't pass -		// through the API, but came from inside GtS as the result of another activity on this instance. That being so, -		// we can safely just ignore this activity, since we know we've already processed it elsewhere. -		return nil +	receivingAccount, _, internal := extractFromCtx(ctx) +	if internal { +		return nil // Already processed.  	}  	rejectObject := reject.GetActivityStreamsObject() diff --git a/internal/federation/federatingdb/undo.go b/internal/federation/federatingdb/undo.go index c17bd2e90..704ca5502 100644 --- a/internal/federation/federatingdb/undo.go +++ b/internal/federation/federatingdb/undo.go @@ -43,12 +43,9 @@ func (f *federatingDB) Undo(ctx context.Context, undo vocab.ActivityStreamsUndo)  		l.Debug("entering Undo")  	} -	receivingAccount, _ := extractFromCtx(ctx) -	if receivingAccount == nil { -		// If the receiving account wasn't set on the context, that means this request didn't pass -		// through the API, but came from inside GtS as the result of another activity on this instance. That being so, -		// we can safely just ignore this activity, since we know we've already processed it elsewhere. -		return nil +	receivingAccount, _, internal := extractFromCtx(ctx) +	if internal { +		return nil // Already processed.  	}  	undoObject := undo.GetActivityStreamsObject() diff --git a/internal/federation/federatingdb/update.go b/internal/federation/federatingdb/update.go index 5c8785c08..aad386085 100644 --- a/internal/federation/federatingdb/update.go +++ b/internal/federation/federatingdb/update.go @@ -52,28 +52,14 @@ func (f *federatingDB) Update(ctx context.Context, asType vocab.Type) error {  		l.Debug("entering Update")  	} -	receivingAccount, _ := extractFromCtx(ctx) -	if receivingAccount == nil { -		// If the receiving account wasn't set on the context, that means -		// this request didn't pass through the API, but came from inside -		// GtS as the result of another activity on this instance. As such, -		// we must have already processed it in order to reach this stage. -		return nil -	} - -	requestingAcctI := ctx.Value(ap.ContextRequestingAccount) -	if requestingAcctI == nil { -		return errors.New("Update: requesting account wasn't set on context") -	} - -	requestingAcct, ok := requestingAcctI.(*gtsmodel.Account) -	if !ok { -		return errors.New("Update: requesting account was set on context but couldn't be parsed") +	receivingAccount, requestingAccount, internal := extractFromCtx(ctx) +	if internal { +		return nil // Already processed.  	}  	switch asType.GetTypeName() {  	case ap.ActorApplication, ap.ActorGroup, ap.ActorOrganization, ap.ActorPerson, ap.ActorService: -		return f.updateAccountable(ctx, receivingAccount, requestingAcct, asType) +		return f.updateAccountable(ctx, receivingAccount, requestingAccount, asType)  	}  	return nil diff --git a/internal/federation/federatingdb/util.go b/internal/federation/federatingdb/util.go index 8e9f67c59..d46451e21 100644 --- a/internal/federation/federatingdb/util.go +++ b/internal/federation/federatingdb/util.go @@ -30,6 +30,7 @@ import (  	"github.com/superseriousbusiness/gotosocial/internal/ap"  	"github.com/superseriousbusiness/gotosocial/internal/config"  	"github.com/superseriousbusiness/gotosocial/internal/db" +	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"  	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"  	"github.com/superseriousbusiness/gotosocial/internal/id"  	"github.com/superseriousbusiness/gotosocial/internal/log" @@ -296,30 +297,23 @@ func (f *federatingDB) collectIRIs(ctx context.Context, iris []*url.URL) (vocab.  	return collection, nil  } -// extractFromCtx extracts some useful values from a context passed into the federatingDB via the API: -//   - The target account that owns the inbox or URI being interacted with. -//   - The requesting account that posted to the inbox. -//   - A channel that messages for the processor can be placed into. +// extractFromCtx extracts some useful values from a context passed into the federatingDB:  // -// If a value is not present, nil will be returned for it. It's up to the caller to check this and respond appropriately. -func extractFromCtx(ctx context.Context) (receivingAccount, requestingAccount *gtsmodel.Account) { -	receivingAccountI := ctx.Value(ap.ContextReceivingAccount) -	if receivingAccountI != nil { -		var ok bool -		receivingAccount, ok = receivingAccountI.(*gtsmodel.Account) -		if !ok { -			log.Panicf(ctx, "context entry with key %s could not be asserted to *gtsmodel.Account", ap.ContextReceivingAccount) -		} -	} +//   - The account that owns the inbox or URI being interacted with. +//   - The account that POSTed a request to the inbox. +//   - Whether this is an internal request (one originating not from +//     the API but from inside the instance). +// +// If the request is internal, the caller can assume that the activity has +// already been processed elsewhere, and should return with no further action. +func extractFromCtx(ctx context.Context) (receivingAccount *gtsmodel.Account, requestingAccount *gtsmodel.Account, internal bool) { +	receivingAccount = gtscontext.ReceivingAccount(ctx) +	requestingAccount = gtscontext.RequestingAccount(ctx) -	requestingAcctI := ctx.Value(ap.ContextRequestingAccount) -	if requestingAcctI != nil { -		var ok bool -		requestingAccount, ok = requestingAcctI.(*gtsmodel.Account) -		if !ok { -			log.Panicf(ctx, "context entry with key %s could not be asserted to *gtsmodel.Account", ap.ContextRequestingAccount) -		} -	} +	// If the receiving account wasn't set on the context, that +	// means this request didn't pass through the API, but +	// came from inside GtS as the result of a local activity. +	internal = receivingAccount == nil  	return  } @@ -329,9 +323,11 @@ func marshalItem(item vocab.Type) (string, error) {  	if err != nil {  		return "", err  	} +  	b, err := json.Marshal(m)  	if err != nil {  		return "", err  	} +  	return string(b), nil  } diff --git a/internal/federation/federatingprotocol.go b/internal/federation/federatingprotocol.go index d7e598edc..18feb2429 100644 --- a/internal/federation/federatingprotocol.go +++ b/internal/federation/federatingprotocol.go @@ -20,23 +20,56 @@ package federation  import (  	"context"  	"errors" -	"fmt"  	"net/http"  	"net/url" +	"strings" +	"codeberg.org/gruf/go-kv"  	"github.com/superseriousbusiness/activity/pub"  	"github.com/superseriousbusiness/activity/streams"  	"github.com/superseriousbusiness/activity/streams/vocab"  	"github.com/superseriousbusiness/gotosocial/internal/ap" +	"github.com/superseriousbusiness/gotosocial/internal/config"  	"github.com/superseriousbusiness/gotosocial/internal/db"  	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"  	"github.com/superseriousbusiness/gotosocial/internal/gtserror" -	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"  	"github.com/superseriousbusiness/gotosocial/internal/log"  	"github.com/superseriousbusiness/gotosocial/internal/uris"  	"github.com/superseriousbusiness/gotosocial/internal/util"  ) +type errOtherIRIBlocked struct { +	account     string +	domainBlock bool +	iriStrs     []string +} + +func (e errOtherIRIBlocked) Error() string { +	iriStrsNice := "[" + strings.Join(e.iriStrs, ", ") + "]" +	if e.domainBlock { +		return "domain block exists for one or more of " + iriStrsNice +	} +	return "block exists between " + e.account + " and one or more of " + iriStrsNice +} + +func newErrOtherIRIBlocked( +	account string, +	domainBlock bool, +	otherIRIs []*url.URL, +) error { +	e := errOtherIRIBlocked{ +		account:     account, +		domainBlock: domainBlock, +		iriStrs:     make([]string, 0, len(otherIRIs)), +	} + +	for _, iri := range otherIRIs { +		e.iriStrs = append(e.iriStrs, iri.String()) +	} + +	return e +} +  /*  	GO FED FEDERATING PROTOCOL INTERFACE  	FederatingProtocol contains behaviors an application needs to satisfy for the @@ -47,77 +80,104 @@ import (  	application.  */ -// PostInboxRequestBodyHook callback after parsing the request body for a federated request -// to the Actor's inbox. -// -// Can be used to set contextual information based on the Activity -// received. +// PostInboxRequestBodyHook callback after parsing the request body for a +// federated request to the Actor's inbox.  // -// Only called if the Federated Protocol is enabled. +// Can be used to set contextual information based on the Activity received.  //  // Warning: Neither authentication nor authorization has taken place at  // this time. Doing anything beyond setting contextual information is  // strongly discouraged.  // -// If an error is returned, it is passed back to the caller of -// PostInbox. In this case, the DelegateActor implementation must not -// write a response to the ResponseWriter as is expected that the caller -// to PostInbox will do so when handling the error. +// If an error is returned, it is passed back to the caller of PostInbox. +// In this case, the DelegateActor implementation must not write a response +// to the ResponseWriter as is expected that the caller to PostInbox will +// do so when handling the error.  func (f *federator) PostInboxRequestBodyHook(ctx context.Context, r *http.Request, activity pub.Activity) (context.Context, error) { -	// extract any other IRIs involved in this activity -	otherInvolvedIRIs := []*url.URL{} +	// Extract any other IRIs involved in this activity. +	otherIRIs := []*url.URL{} -	// check if the Activity itself has an 'inReplyTo' +	// Get the ID of the Activity itslf. +	activityID, err := pub.GetId(activity) +	if err == nil { +		otherIRIs = append(otherIRIs, activityID) +	} + +	// Check if the Activity has an 'inReplyTo'.  	if replyToable, ok := activity.(ap.ReplyToable); ok {  		if inReplyToURI := ap.ExtractInReplyToURI(replyToable); inReplyToURI != nil { -			otherInvolvedIRIs = append(otherInvolvedIRIs, inReplyToURI) +			otherIRIs = append(otherIRIs, inReplyToURI)  		}  	} -	// now check if the Object of the Activity (usually a Note or something) has an 'inReplyTo' -	if object := activity.GetActivityStreamsObject(); object != nil { -		if replyToable, ok := object.(ap.ReplyToable); ok { -			if inReplyToURI := ap.ExtractInReplyToURI(replyToable); inReplyToURI != nil { -				otherInvolvedIRIs = append(otherInvolvedIRIs, inReplyToURI) -			} +	// Check for TOs and CCs on the Activity. +	if addressable, ok := activity.(ap.Addressable); ok { +		if toURIs, err := ap.ExtractTos(addressable); err == nil { +			otherIRIs = append(otherIRIs, toURIs...)  		} -	} -	// check for Tos and CCs on Activity itself -	if addressable, ok := activity.(ap.Addressable); ok {  		if ccURIs, err := ap.ExtractCCs(addressable); err == nil { -			otherInvolvedIRIs = append(otherInvolvedIRIs, ccURIs...) -		} -		if toURIs, err := ap.ExtractTos(addressable); err == nil { -			otherInvolvedIRIs = append(otherInvolvedIRIs, toURIs...) +			otherIRIs = append(otherIRIs, ccURIs...)  		}  	} -	// and on the Object itself -	if object := activity.GetActivityStreamsObject(); object != nil { -		if addressable, ok := object.(ap.Addressable); ok { -			if ccURIs, err := ap.ExtractCCs(addressable); err == nil { -				otherInvolvedIRIs = append(otherInvolvedIRIs, ccURIs...) +	// Now perform the same checks, but for the Object(s) of the Activity. +	objectProp := activity.GetActivityStreamsObject() +	for iter := objectProp.Begin(); iter != objectProp.End(); iter = iter.Next() { +		if iter.IsIRI() { +			otherIRIs = append(otherIRIs, iter.GetIRI()) +			continue +		} + +		t := iter.GetType() +		if t == nil { +			continue +		} + +		objectID, err := pub.GetId(t) +		if err == nil { +			otherIRIs = append(otherIRIs, objectID) +		} + +		if replyToable, ok := t.(ap.ReplyToable); ok { +			if inReplyToURI := ap.ExtractInReplyToURI(replyToable); inReplyToURI != nil { +				otherIRIs = append(otherIRIs, inReplyToURI)  			} +		} + +		if addressable, ok := t.(ap.Addressable); ok {  			if toURIs, err := ap.ExtractTos(addressable); err == nil { -				otherInvolvedIRIs = append(otherInvolvedIRIs, toURIs...) +				otherIRIs = append(otherIRIs, toURIs...) +			} + +			if ccURIs, err := ap.ExtractCCs(addressable); err == nil { +				otherIRIs = append(otherIRIs, ccURIs...)  			}  		}  	} -	// remove any duplicate entries in the slice we put together -	deduped := util.UniqueURIs(otherInvolvedIRIs) +	// Clean any instances of the public URI, since +	// we don't care about that in this context. +	otherIRIs = func(iris []*url.URL) []*url.URL { +		np := make([]*url.URL, 0, len(iris)) -	// clean any instances of the public URI since we don't care about that in this context -	cleaned := []*url.URL{} -	for _, u := range deduped { -		if !pub.IsPublic(u.String()) { -			cleaned = append(cleaned, u) +		for _, i := range iris { +			if !pub.IsPublic(i.String()) { +				np = append(np, i) +			}  		} -	} -	withOtherInvolvedIRIs := context.WithValue(ctx, ap.ContextOtherInvolvedIRIs, cleaned) -	return withOtherInvolvedIRIs, nil +		return np +	}(otherIRIs) + +	// OtherIRIs will likely contain some +	// duplicate entries now, so remove them. +	otherIRIs = util.UniqueURIs(otherIRIs) + +	// Finished, set other IRIs on the context +	// so they can be checked for blocks later. +	ctx = gtscontext.SetOtherIRIs(ctx, otherIRIs) +	return ctx, nil  }  // AuthenticatePostInbox delegates the authentication of a POST to an @@ -143,23 +203,23 @@ func (f *federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr  	// account by parsing username from `/users/{username}/inbox`.  	username, err := uris.ParseInboxPath(r.URL)  	if err != nil { -		err = fmt.Errorf("AuthenticatePostInbox: could not parse %s as inbox path: %w", r.URL.String(), err) +		err = gtserror.Newf("could not parse %s as inbox path: %w", r.URL.String(), err)  		return nil, false, err  	}  	if username == "" { -		err = errors.New("AuthenticatePostInbox: inbox username was empty") +		err = gtserror.New("inbox username was empty")  		return nil, false, err  	}  	receivingAccount, err := f.db.GetAccountByUsernameDomain(ctx, username, "")  	if err != nil { -		err = fmt.Errorf("AuthenticatePostInbox: could not fetch receiving account %s: %w", username, err) +		err = gtserror.Newf("could not fetch receiving account %s: %w", username, err)  		return nil, false, err  	} -	// Check who's delivering by inspecting the http signature. -	publicKeyOwnerURI, errWithCode := f.AuthenticateFederatedRequest(ctx, receivingAccount.Username) +	// Check who's trying to deliver to us by inspecting the http signature. +	pubKeyOwner, errWithCode := f.AuthenticateFederatedRequest(ctx, receivingAccount.Username)  	if errWithCode != nil {  		switch errWithCode.Code() {  		case http.StatusUnauthorized, http.StatusForbidden, http.StatusBadRequest: @@ -184,25 +244,30 @@ func (f *federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr  	// Authentication has passed, check if we need to create a  	// new instance entry for the Host of the requesting account. -	if _, err := f.db.GetInstance(ctx, publicKeyOwnerURI.Host); err != nil { +	if _, err := f.db.GetInstance(ctx, pubKeyOwner.Host); err != nil {  		if !errors.Is(err, db.ErrNoEntries) {  			// There's been an actual error. -			err = fmt.Errorf("AuthenticatePostInbox: error getting instance %s: %w", publicKeyOwnerURI.Host, err) +			err = gtserror.Newf("error getting instance %s: %w", pubKeyOwner.Host, err)  			return ctx, false, err  		} -		// we don't have an entry for this instance yet so dereference it -		instance, err := f.GetRemoteInstance(gtscontext.SetFastFail(ctx), username, &url.URL{ -			Scheme: publicKeyOwnerURI.Scheme, -			Host:   publicKeyOwnerURI.Host, -		}) +		// We don't have an entry for this +		// instance yet; go dereference it. +		instance, err := f.GetRemoteInstance( +			gtscontext.SetFastFail(ctx), +			username, +			&url.URL{ +				Scheme: pubKeyOwner.Scheme, +				Host:   pubKeyOwner.Host, +			}, +		)  		if err != nil { -			err = fmt.Errorf("AuthenticatePostInbox: error dereferencing instance %s: %w", publicKeyOwnerURI.Host, err) +			err = gtserror.Newf("error dereferencing instance %s: %w", pubKeyOwner.Host, err)  			return nil, false, err  		} -		if err := f.db.Put(ctx, instance); err != nil { -			err = fmt.Errorf("AuthenticatePostInbox: error inserting instance entry for %s: %w", publicKeyOwnerURI.Host, err) +		if err := f.db.Put(ctx, instance); err != nil && !errors.Is(err, db.ErrAlreadyExists) { +			err = gtserror.Newf("error inserting instance entry for %s: %w", pubKeyOwner.Host, err)  			return nil, false, err  		}  	} @@ -210,7 +275,11 @@ func (f *federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr  	// We know the public key owner URI now, so we can  	// dereference the remote account (or just get it  	// from the db if we already have it). -	requestingAccount, _, err := f.GetAccountByURI(gtscontext.SetFastFail(ctx), username, publicKeyOwnerURI) +	requestingAccount, _, err := f.GetAccountByURI( +		gtscontext.SetFastFail(ctx), +		username, +		pubKeyOwner, +	)  	if err != nil {  		if gtserror.StatusCode(err) == http.StatusGone {  			// This is the same case as the http.StatusGone check above. @@ -222,113 +291,196 @@ func (f *federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr  			w.WriteHeader(http.StatusAccepted)  			return ctx, false, nil  		} -		err = fmt.Errorf("AuthenticatePostInbox: couldn't get requesting account %s: %w", publicKeyOwnerURI, err) + +		err = gtserror.Newf("couldn't get requesting account %s: %w", pubKeyOwner, err)  		return nil, false, err  	}  	// We have everything we need now, set the requesting  	// and receiving accounts on the context for later use. -	withRequesting := context.WithValue(ctx, ap.ContextRequestingAccount, requestingAccount) -	withReceiving := context.WithValue(withRequesting, ap.ContextReceivingAccount, receivingAccount) -	return withReceiving, true, nil +	ctx = gtscontext.SetRequestingAccount(ctx, requestingAccount) +	ctx = gtscontext.SetReceivingAccount(ctx, receivingAccount) +	return ctx, true, nil  }  // Blocked should determine whether to permit a set of actors given by  // their ids are able to interact with this particular end user due to  // being blocked or other application-specific logic. -// -// If an error is returned, it is passed back to the caller of -// PostInbox. -// -// If no error is returned, but authentication or authorization fails, -// then blocked must be true and error nil. An http.StatusForbidden -// will be written in the wresponse. -// -// Finally, if the authentication and authorization succeeds, then -// blocked must be false and error nil. The request will continue -// to be processed.  func (f *federator) Blocked(ctx context.Context, actorIRIs []*url.URL) (bool, error) { -	log.Tracef(ctx, "entering BLOCKED function with IRI list: %+v", actorIRIs) +	// Fetch relevant items from request context. +	// These should have been set further up the flow. +	receivingAccount := gtscontext.ReceivingAccount(ctx) +	if receivingAccount == nil { +		err := gtserror.New("couldn't determine blocks (receiving account not set on request context)") +		return false, err +	} -	// check domain blocks first for the given actor IRIs +	requestingAccount := gtscontext.RequestingAccount(ctx) +	if requestingAccount == nil { +		err := gtserror.New("couldn't determine blocks (requesting account not set on request context)") +		return false, err +	} + +	otherIRIs := gtscontext.OtherIRIs(ctx) +	if otherIRIs == nil { +		err := gtserror.New("couldn't determine blocks (otherIRIs not set on request context)") +		return false, err +	} + +	l := log. +		WithContext(ctx). +		WithFields(kv.Fields{ +			{"actorIRIs", actorIRIs}, +			{"receivingAccount", receivingAccount.URI}, +			{"requestingAccount", requestingAccount.URI}, +			{"otherIRIs", otherIRIs}, +		}...) +	l.Trace("checking blocks") + +	// Start broad by checking domain-level blocks first for +	// the given actor IRIs; if any of them are domain blocked +	// then we can save some work.  	blocked, err := f.db.AreURIsBlocked(ctx, actorIRIs)  	if err != nil { -		return false, fmt.Errorf("error checking domain blocks of actorIRIs: %s", err) +		err = gtserror.Newf("error checking domain blocks of actorIRIs: %w", err) +		return false, err  	} +  	if blocked { +		l.Trace("one or more actorIRIs are domain blocked")  		return blocked, nil  	} -	// check domain blocks for any other involved IRIs -	otherInvolvedIRIsI := ctx.Value(ap.ContextOtherInvolvedIRIs) -	otherInvolvedIRIs, ok := otherInvolvedIRIsI.([]*url.URL) -	if !ok { -		log.Error(ctx, "other involved IRIs not set on request context") -		return false, errors.New("other involved IRIs not set on request context, so couldn't determine blocks") -	} -	blocked, err = f.db.AreURIsBlocked(ctx, otherInvolvedIRIs) +	// Now user level blocks. Receiver should not block requester. +	blocked, err = f.db.IsBlocked(ctx, receivingAccount.ID, requestingAccount.ID)  	if err != nil { -		return false, fmt.Errorf("error checking domain blocks of otherInvolvedIRIs: %s", err) +		err = gtserror.Newf("db error checking block between receiver and requester: %w", err) +		return false, err  	} +  	if blocked { +		l.Trace("receiving account blocks requesting account")  		return blocked, nil  	} -	// now check for user-level block from receiving against requesting account -	receivingAccountI := ctx.Value(ap.ContextReceivingAccount) -	receivingAccount, ok := receivingAccountI.(*gtsmodel.Account) -	if !ok { -		log.Error(ctx, "receiving account not set on request context") -		return false, errors.New("receiving account not set on request context, so couldn't determine blocks") -	} -	requestingAccountI := ctx.Value(ap.ContextRequestingAccount) -	requestingAccount, ok := requestingAccountI.(*gtsmodel.Account) -	if !ok { -		log.Error(ctx, "requesting account not set on request context") -		return false, errors.New("requesting account not set on request context, so couldn't determine blocks") -	} -	// the receiver shouldn't block the sender -	blocked, err = f.db.IsBlocked(ctx, receivingAccount.ID, requestingAccount.ID) +	// We've established that no blocks exist between directly +	// involved actors, but what about IRIs of other actors and +	// objects which are tangentially involved in the activity +	// (ie., replied to, boosted)? +	// +	// If one or more of these other IRIs is domain blocked, or +	// blocked by the receiving account, this shouldn't return +	// blocked=true to send a 403, since that would be rather +	// silly behavior. Instead, we should indicate to the caller +	// that we should stop processing the activity and just write +	// 202 Accepted instead. +	// +	// For this, we can use the errOtherIRIBlocked type, which +	// will be checked for + +	// Check high-level domain blocks first. +	blocked, err = f.db.AreURIsBlocked(ctx, otherIRIs)  	if err != nil { -		return false, fmt.Errorf("error checking user-level blocks: %s", err) +		err := gtserror.Newf("error checking domain block of otherIRIs: %w", err) +		return false, err  	} +  	if blocked { -		return blocked, nil +		err := newErrOtherIRIBlocked(receivingAccount.URI, true, otherIRIs) +		l.Trace(err.Error()) +		return false, err  	} -	// get account IDs for other involved accounts -	var involvedAccountIDs []string -	for _, iri := range otherInvolvedIRIs { -		var involvedAccountID string -		if involvedStatus, err := f.db.GetStatusByURI(ctx, iri.String()); err == nil { -			involvedAccountID = involvedStatus.AccountID -		} else if involvedAccount, err := f.db.GetAccountByURI(ctx, iri.String()); err == nil { -			involvedAccountID = involvedAccount.ID +	// For each other IRI, check whether the IRI points to an +	// account or a status, and try to get (an) accountID(s) +	// from it to do further checks on. +	// +	// We use a map for this instead of a slice in order to +	// deduplicate entries and avoid doing the same check twice. +	// The map value is the host of the otherIRI. +	accountIDs := make(map[string]string, len(otherIRIs)) +	for _, iri := range otherIRIs { +		// Assemble iri string just once. +		iriStr := iri.String() + +		account, err := f.db.GetAccountByURI( +			// We're on a hot path, fetch bare minimum. +			gtscontext.SetBarebones(ctx), +			iriStr, +		) +		if err != nil && !errors.Is(err, db.ErrNoEntries) { +			// Real db error. +			err = gtserror.Newf("db error trying to get %s as account: %w", iriStr, err) +			return false, err +		} else if err == nil { +			// IRI is for an account. +			accountIDs[account.ID] = iri.Host +			continue  		} -		if involvedAccountID != "" { -			involvedAccountIDs = append(involvedAccountIDs, involvedAccountID) +		status, err := f.db.GetStatusByURI( +			// We're on a hot path, fetch bare minimum. +			gtscontext.SetBarebones(ctx), +			iriStr, +		) +		if err != nil && !errors.Is(err, db.ErrNoEntries) { +			// Real db error. +			err = gtserror.Newf("db error trying to get %s as status: %w", iriStr, err) +			return false, err +		} else if err == nil { +			// IRI is for a status. +			accountIDs[status.AccountID] = iri.Host +			continue  		}  	} -	deduped := util.UniqueStrings(involvedAccountIDs) -	for _, involvedAccountID := range deduped { -		// the involved account shouldn't block whoever is making this request -		blocked, err = f.db.IsBlocked(ctx, involvedAccountID, requestingAccount.ID) +	// Get our own host value just once outside the loop. +	ourHost := config.GetHost() + +	for accountID, iriHost := range accountIDs { +		// Receiver shouldn't block other IRI owner. +		// +		// This check protects against cases where someone on our +		// instance is receiving a boost from someone they don't +		// block, but the boost target is the status of an account +		// they DO have blocked, or the boosted status mentions an +		// account they have blocked. In this case, it's v. unlikely +		// they care to see the boost in their timeline, so there's +		// no point in us processing it. +		blocked, err = f.db.IsBlocked(ctx, receivingAccount.ID, accountID)  		if err != nil { -			return false, fmt.Errorf("error checking user-level otherInvolvedIRI blocks: %s", err) +			err = gtserror.Newf("db error checking block between receiver and other account: %w", err) +			return false, err  		} +  		if blocked { -			return blocked, nil +			l.Trace("receiving account blocks one or more otherIRIs") +			err := newErrOtherIRIBlocked(receivingAccount.URI, false, otherIRIs) +			return false, err  		} -		// whoever is receiving this request shouldn't block the involved account -		blocked, err = f.db.IsBlocked(ctx, receivingAccount.ID, involvedAccountID) -		if err != nil { -			return false, fmt.Errorf("error checking user-level otherInvolvedIRI blocks: %s", err) -		} -		if blocked { -			return blocked, nil +		// If other account is from our instance (indicated by the +		// host of the URI stored in the map), ensure they don't block +		// the requester. +		// +		// This check protects against cases where one of our users +		// might be mentioned by the requesting account, and therefore +		// appear in otherIRIs, but the activity itself has been sent +		// to a different account on our instance. In other words, two +		// accounts are gossiping about + trying to tag a third account +		// who has one or the other of them blocked. +		if iriHost == ourHost { +			blocked, err = f.db.IsBlocked(ctx, accountID, requestingAccount.ID) +			if err != nil { +				err = gtserror.Newf("db error checking block between other account and requester: %w", err) +				return false, err +			} + +			if blocked { +				l.Trace("one or more otherIRIs belonging to us blocks requesting account") +				err := newErrOtherIRIBlocked(requestingAccount.URI, false, otherIRIs) +				return false, err +			}  		}  	} diff --git a/internal/federation/federatingprotocol_test.go b/internal/federation/federatingprotocol_test.go index 3350099b0..8da6859dd 100644 --- a/internal/federation/federatingprotocol_test.go +++ b/internal/federation/federatingprotocol_test.go @@ -18,7 +18,10 @@  package federation_test  import ( +	"bytes"  	"context" +	"encoding/json" +	"io"  	"net/http"  	"net/http/httptest"  	"net/url" @@ -27,7 +30,7 @@ import (  	"github.com/go-fed/httpsig"  	"github.com/stretchr/testify/suite"  	"github.com/superseriousbusiness/gotosocial/internal/ap" -	"github.com/superseriousbusiness/gotosocial/internal/federation" +	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"  	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"  	"github.com/superseriousbusiness/gotosocial/testrig"  ) @@ -36,342 +39,402 @@ type FederatingProtocolTestSuite struct {  	FederatorStandardTestSuite  } -func (suite *FederatingProtocolTestSuite) TestPostInboxRequestBodyHook1() { -	// the activity we're gonna use -	activity := suite.testActivities["dm_for_zork"] - -	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") -	tc := testrig.NewTestTransportController(&suite.state, httpClient) -	// setup module being tested -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) - -	// setup request -	ctx := context.Background() -	request := httptest.NewRequest(http.MethodPost, "http://localhost:8080/users/the_mighty_zork/inbox", nil) // the endpoint we're hitting -	request.Header.Set("Signature", activity.SignatureHeader) - -	// trigger the function being tested, and return the new context it creates -	newContext, err := federator.PostInboxRequestBodyHook(ctx, request, activity.Activity) -	suite.NoError(err) -	suite.NotNil(newContext) - -	involvedIRIsI := newContext.Value(ap.ContextOtherInvolvedIRIs) -	involvedIRIs, ok := involvedIRIsI.([]*url.URL) -	if !ok { -		suite.FailNow("couldn't get involved IRIs from context") +func (suite *FederatingProtocolTestSuite) postInboxRequestBodyHook( +	ctx context.Context, +	receivingAccount *gtsmodel.Account, +	activity testrig.ActivityWithSignature, +) context.Context { +	raw, err := ap.Serialize(activity.Activity) +	if err != nil { +		suite.FailNow(err.Error())  	} -	suite.Len(involvedIRIs, 1) -	suite.Contains(involvedIRIs, testrig.URLMustParse("http://localhost:8080/users/the_mighty_zork")) -} - -func (suite *FederatingProtocolTestSuite) TestPostInboxRequestBodyHook2() { -	// the activity we're gonna use -	activity := suite.testActivities["reply_to_turtle_for_zork"] - -	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") -	tc := testrig.NewTestTransportController(&suite.state, httpClient) - -	// setup module being tested -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) - -	// setup request -	ctx := context.Background() -	request := httptest.NewRequest(http.MethodPost, "http://localhost:8080/users/the_mighty_zork/inbox", nil) // the endpoint we're hitting -	request.Header.Set("Signature", activity.SignatureHeader) - -	// trigger the function being tested, and return the new context it creates -	newContext, err := federator.PostInboxRequestBodyHook(ctx, request, activity.Activity) -	suite.NoError(err) -	suite.NotNil(newContext) - -	involvedIRIsI := newContext.Value(ap.ContextOtherInvolvedIRIs) -	involvedIRIs, ok := involvedIRIsI.([]*url.URL) -	if !ok { -		suite.FailNow("couldn't get involved IRIs from context") +	b, err := json.Marshal(raw) +	if err != nil { +		suite.FailNow(err.Error())  	} - -	suite.Len(involvedIRIs, 2) -	suite.Contains(involvedIRIs, testrig.URLMustParse("http://localhost:8080/users/1happyturtle")) -	suite.Contains(involvedIRIs, testrig.URLMustParse("http://fossbros-anonymous.io/users/foss_satan/followers")) -} - -func (suite *FederatingProtocolTestSuite) TestPostInboxRequestBodyHook3() { -	// the activity we're gonna use -	activity := suite.testActivities["reply_to_turtle_for_turtle"] - -	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") -	tc := testrig.NewTestTransportController(&suite.state, httpClient) - -	// setup module being tested -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) - -	// setup request -	ctx := context.Background() -	request := httptest.NewRequest(http.MethodPost, "http://localhost:8080/users/1happyturtle/inbox", nil) // the endpoint we're hitting -	request.Header.Set("Signature", activity.SignatureHeader) - -	// trigger the function being tested, and return the new context it creates -	newContext, err := federator.PostInboxRequestBodyHook(ctx, request, activity.Activity)  	suite.NoError(err) -	suite.NotNil(newContext) +	request := httptest.NewRequest(http.MethodPost, receivingAccount.InboxURI, bytes.NewBuffer(b)) +	request.Header.Set("Signature", activity.SignatureHeader) +	request.Header.Set("Date", activity.DateHeader) +	request.Header.Set("Digest", activity.DigestHeader) -	involvedIRIsI := newContext.Value(ap.ContextOtherInvolvedIRIs) -	involvedIRIs, ok := involvedIRIsI.([]*url.URL) -	if !ok { -		suite.FailNow("couldn't get involved IRIs from context") +	newContext, err := suite.federator.PostInboxRequestBodyHook(ctx, request, activity.Activity) +	if err != nil { +		suite.FailNow(err.Error())  	} -	suite.Len(involvedIRIs, 2) -	suite.Contains(involvedIRIs, testrig.URLMustParse("http://localhost:8080/users/1happyturtle")) -	suite.Contains(involvedIRIs, testrig.URLMustParse("http://fossbros-anonymous.io/users/foss_satan/followers")) +	return newContext  } -func (suite *FederatingProtocolTestSuite) TestAuthenticatePostInbox() { -	// the activity we're gonna use -	activity := suite.testActivities["dm_for_zork"] -	sendingAccount := suite.testAccounts["remote_account_1"] -	inboxAccount := suite.testAccounts["local_account_1"] - -	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") -	tc := testrig.NewTestTransportController(&suite.state, httpClient) +func (suite *FederatingProtocolTestSuite) authenticatePostInbox( +	ctx context.Context, +	receivingAccount *gtsmodel.Account, +	activity testrig.ActivityWithSignature, +) (context.Context, bool, []byte, int) { +	raw, err := ap.Serialize(activity.Activity) +	if err != nil { +		suite.FailNow(err.Error()) +	} -	// now setup module being tested, with the mock transport controller -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) +	b, err := json.Marshal(raw) +	if err != nil { +		suite.FailNow(err.Error()) +	} -	request := httptest.NewRequest(http.MethodPost, "http://localhost:8080/users/the_mighty_zork/inbox", nil) -	// we need these headers for the request to be validated +	request := httptest.NewRequest(http.MethodPost, receivingAccount.InboxURI, bytes.NewBuffer(b))  	request.Header.Set("Signature", activity.SignatureHeader)  	request.Header.Set("Date", activity.DateHeader)  	request.Header.Set("Digest", activity.DigestHeader)  	verifier, err := httpsig.NewVerifier(request) -	suite.NoError(err) +	if err != nil { +		suite.FailNow(err.Error()) +	} -	ctx := context.Background() -	// by the time AuthenticatePostInbox is called, PostInboxRequestBodyHook should have already been called, -	// which should have set the account and username onto the request. We can replicate that behavior here: -	ctxWithAccount := context.WithValue(ctx, ap.ContextReceivingAccount, inboxAccount) -	ctxWithVerifier := context.WithValue(ctxWithAccount, ap.ContextRequestingPublicKeyVerifier, verifier) -	ctxWithSignature := context.WithValue(ctxWithVerifier, ap.ContextRequestingPublicKeySignature, activity.SignatureHeader) +	ctx = gtscontext.SetReceivingAccount(ctx, receivingAccount) +	ctx = gtscontext.SetHTTPSignatureVerifier(ctx, verifier) +	ctx = gtscontext.SetHTTPSignature(ctx, activity.SignatureHeader) +	ctx = gtscontext.SetHTTPSignaturePubKeyID(ctx, testrig.URLMustParse(verifier.KeyId())) -	// we can pass this recorder as a writer and read it back after  	recorder := httptest.NewRecorder() +	newContext, authed, err := suite.federator.AuthenticatePostInbox(ctx, recorder, request) +	if err != nil { +		suite.FailNow(err.Error()) +	} -	// trigger the function being tested, and return the new context it creates -	newContext, authed, err := federator.AuthenticatePostInbox(ctxWithSignature, recorder, request) -	suite.NoError(err) -	suite.True(authed) +	res := recorder.Result() +	defer res.Body.Close() -	// since we know this account already it should be set on the context -	requestingAccountI := newContext.Value(ap.ContextRequestingAccount) -	suite.NotNil(requestingAccountI) -	requestingAccount, ok := requestingAccountI.(*gtsmodel.Account) -	suite.True(ok) -	suite.Equal(sendingAccount.Username, requestingAccount.Username) +	b, err = io.ReadAll(res.Body) +	if err != nil { +		suite.FailNow(err.Error()) +	} + +	return newContext, authed, b, res.StatusCode  } -func (suite *FederatingProtocolTestSuite) TestAuthenticatePostGone() { -	// the activity we're gonna use -	activity := suite.testActivities["delete_https://somewhere.mysterious/users/rest_in_piss#main-key"] -	inboxAccount := suite.testAccounts["local_account_1"] +func (suite *FederatingProtocolTestSuite) TestPostInboxRequestBodyHookDM() { +	var ( +		receivingAccount = suite.testAccounts["local_account_1"] +		activity         = suite.testActivities["dm_for_zork"] +	) + +	ctx := suite.postInboxRequestBodyHook( +		context.Background(), +		receivingAccount, +		activity, +	) + +	otherIRIs := gtscontext.OtherIRIs(ctx) +	otherIRIStrs := make([]string, 0, len(otherIRIs)) +	for _, i := range otherIRIs { +		otherIRIStrs = append(otherIRIStrs, i.String()) +	} -	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") -	tc := testrig.NewTestTransportController(&suite.state, httpClient) +	suite.Equal([]string{ +		"http://fossbros-anonymous.io/users/foss_satan/statuses/5424b153-4553-4f30-9358-7b92f7cd42f6/activity", +		"http://localhost:8080/users/the_mighty_zork", +		"http://fossbros-anonymous.io/users/foss_satan/statuses/5424b153-4553-4f30-9358-7b92f7cd42f6", +	}, otherIRIStrs) +} -	// now setup module being tested, with the mock transport controller -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) +func (suite *FederatingProtocolTestSuite) TestPostInboxRequestBodyHookReply() { +	var ( +		receivingAccount = suite.testAccounts["local_account_1"] +		activity         = suite.testActivities["reply_to_turtle_for_zork"] +	) + +	ctx := suite.postInboxRequestBodyHook( +		context.Background(), +		receivingAccount, +		activity, +	) + +	otherIRIs := gtscontext.OtherIRIs(ctx) +	otherIRIStrs := make([]string, 0, len(otherIRIs)) +	for _, i := range otherIRIs { +		otherIRIStrs = append(otherIRIStrs, i.String()) +	} -	request := httptest.NewRequest(http.MethodPost, "http://localhost:8080/users/the_mighty_zork/inbox", nil) -	// we need these headers for the request to be validated -	request.Header.Set("Signature", activity.SignatureHeader) -	request.Header.Set("Date", activity.DateHeader) -	request.Header.Set("Digest", activity.DigestHeader) +	suite.Equal([]string{ +		"http://fossbros-anonymous.io/users/foss_satan/statuses/2f1195a6-5cb0-4475-adf5-92ab9a0147fe", +		"http://fossbros-anonymous.io/users/foss_satan/followers", +		"http://localhost:8080/users/1happyturtle", +	}, otherIRIStrs) +} -	verifier, err := httpsig.NewVerifier(request) -	suite.NoError(err) +func (suite *FederatingProtocolTestSuite) TestPostInboxRequestBodyHookReplyToReply() { +	var ( +		receivingAccount = suite.testAccounts["local_account_2"] +		activity         = suite.testActivities["reply_to_turtle_for_turtle"] +	) + +	ctx := suite.postInboxRequestBodyHook( +		context.Background(), +		receivingAccount, +		activity, +	) + +	otherIRIs := gtscontext.OtherIRIs(ctx) +	otherIRIStrs := make([]string, 0, len(otherIRIs)) +	for _, i := range otherIRIs { +		otherIRIStrs = append(otherIRIStrs, i.String()) +	} -	ctx := context.Background() -	// by the time AuthenticatePostInbox is called, PostInboxRequestBodyHook should have already been called, -	// which should have set the account and username onto the request. We can replicate that behavior here: -	ctxWithAccount := context.WithValue(ctx, ap.ContextReceivingAccount, inboxAccount) -	ctxWithVerifier := context.WithValue(ctxWithAccount, ap.ContextRequestingPublicKeyVerifier, verifier) -	ctxWithSignature := context.WithValue(ctxWithVerifier, ap.ContextRequestingPublicKeySignature, activity.SignatureHeader) +	suite.Equal([]string{ +		"http://fossbros-anonymous.io/users/foss_satan/statuses/2f1195a6-5cb0-4475-adf5-92ab9a0147fe", +		"http://fossbros-anonymous.io/users/foss_satan/followers", +		"http://localhost:8080/users/1happyturtle", +	}, otherIRIStrs) +} -	// we can pass this recorder as a writer and read it back after -	recorder := httptest.NewRecorder() +func (suite *FederatingProtocolTestSuite) TestPostInboxRequestBodyHookAnnounceForwardedToTurtle() { +	var ( +		receivingAccount = suite.testAccounts["local_account_2"] +		activity         = suite.testActivities["announce_forwarded_1_turtle"] +	) + +	ctx := suite.postInboxRequestBodyHook( +		context.Background(), +		receivingAccount, +		activity, +	) + +	otherIRIs := gtscontext.OtherIRIs(ctx) +	otherIRIStrs := make([]string, 0, len(otherIRIs)) +	for _, i := range otherIRIs { +		otherIRIStrs = append(otherIRIStrs, i.String()) +	} -	// trigger the function being tested, and return the new context it creates -	_, authed, err := federator.AuthenticatePostInbox(ctxWithSignature, recorder, request) -	suite.NoError(err) -	suite.False(authed) -	suite.Equal(http.StatusAccepted, recorder.Code) +	suite.Equal([]string{ +		"http://fossbros-anonymous.io/users/foss_satan/first_announce", +		"http://example.org/users/Some_User", +		"http://example.org/users/Some_User/statuses/afaba698-5740-4e32-a702-af61aa543bc1", +	}, otherIRIStrs)  } -func (suite *FederatingProtocolTestSuite) TestAuthenticatePostGoneNoTombstoneYet() { -	// delete the relevant tombstone -	if err := suite.db.DeleteTombstone(context.Background(), suite.testTombstones["https://somewhere.mysterious/users/rest_in_piss#main-key"].ID); err != nil { -		suite.FailNow(err.Error()) +func (suite *FederatingProtocolTestSuite) TestPostInboxRequestBodyHookAnnounceForwardedToZork() { +	var ( +		receivingAccount = suite.testAccounts["local_account_1"] +		activity         = suite.testActivities["announce_forwarded_2_zork"] +	) + +	ctx := suite.postInboxRequestBodyHook( +		context.Background(), +		receivingAccount, +		activity, +	) + +	otherIRIs := gtscontext.OtherIRIs(ctx) +	otherIRIStrs := make([]string, 0, len(otherIRIs)) +	for _, i := range otherIRIs { +		otherIRIStrs = append(otherIRIStrs, i.String())  	} -	// the activity we're gonna use -	activity := suite.testActivities["delete_https://somewhere.mysterious/users/rest_in_piss#main-key"] -	inboxAccount := suite.testAccounts["local_account_1"] +	suite.Equal([]string{ +		"http://fossbros-anonymous.io/users/foss_satan/second_announce", +		"http://example.org/users/Some_User", +		"http://example.org/users/Some_User/statuses/afaba698-5740-4e32-a702-af61aa543bc1", +	}, otherIRIStrs) +} -	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") -	tc := testrig.NewTestTransportController(&suite.state, httpClient) +func (suite *FederatingProtocolTestSuite) TestAuthenticatePostInbox() { +	var ( +		activity         = suite.testActivities["dm_for_zork"] +		receivingAccount = suite.testAccounts["local_account_1"] +	) + +	ctx, authed, resp, code := suite.authenticatePostInbox( +		context.Background(), +		receivingAccount, +		activity, +	) + +	suite.NotNil(gtscontext.RequestingAccount(ctx)) +	suite.True(authed) +	suite.Equal([]byte{}, resp) +	suite.Equal(http.StatusOK, code) +} -	// now setup module being tested, with the mock transport controller -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) +func (suite *FederatingProtocolTestSuite) TestAuthenticatePostGoneWithTombstone() { +	var ( +		activity         = suite.testActivities["delete_https://somewhere.mysterious/users/rest_in_piss#main-key"] +		receivingAccount = suite.testAccounts["local_account_1"] +	) -	request := httptest.NewRequest(http.MethodPost, "http://localhost:8080/users/the_mighty_zork/inbox", nil) -	// we need these headers for the request to be validated -	request.Header.Set("Signature", activity.SignatureHeader) -	request.Header.Set("Date", activity.DateHeader) -	request.Header.Set("Digest", activity.DigestHeader) +	ctx, authed, resp, code := suite.authenticatePostInbox( +		context.Background(), +		receivingAccount, +		activity, +	) -	verifier, err := httpsig.NewVerifier(request) -	suite.NoError(err) +	// Tombstone exists for this account, should simply return accepted. +	suite.Nil(gtscontext.RequestingAccount(ctx)) +	suite.False(authed) +	suite.Equal([]byte{}, resp) +	suite.Equal(http.StatusAccepted, code) +} -	ctx := context.Background() -	// by the time AuthenticatePostInbox is called, PostInboxRequestBodyHook should have already been called, -	// which should have set the account and username onto the request. We can replicate that behavior here: -	ctxWithAccount := context.WithValue(ctx, ap.ContextReceivingAccount, inboxAccount) -	ctxWithVerifier := context.WithValue(ctxWithAccount, ap.ContextRequestingPublicKeyVerifier, verifier) -	ctxWithSignature := context.WithValue(ctxWithVerifier, ap.ContextRequestingPublicKeySignature, activity.SignatureHeader) +func (suite *FederatingProtocolTestSuite) TestAuthenticatePostGoneNoTombstone() { +	var ( +		activity         = suite.testActivities["delete_https://somewhere.mysterious/users/rest_in_piss#main-key"] +		receivingAccount = suite.testAccounts["local_account_1"] +		testTombstone    = suite.testTombstones["https://somewhere.mysterious/users/rest_in_piss#main-key"] +	) -	// we can pass this recorder as a writer and read it back after -	recorder := httptest.NewRecorder() +	// Delete the tombstone; it'll have to be created again. +	if err := suite.state.DB.DeleteTombstone(context.Background(), testTombstone.ID); err != nil { +		suite.FailNow(err.Error()) +	} -	// trigger the function being tested, and return the new context it creates -	_, authed, err := federator.AuthenticatePostInbox(ctxWithSignature, recorder, request) -	suite.NoError(err) -	suite.False(authed) -	suite.Equal(http.StatusAccepted, recorder.Code) +	ctx, authed, resp, code := suite.authenticatePostInbox( +		context.Background(), +		receivingAccount, +		activity, +	) -	// there should be a tombstone in the db now for this account -	exists, err := suite.db.TombstoneExistsWithURI(ctx, "https://somewhere.mysterious/users/rest_in_piss#main-key") +	suite.Nil(gtscontext.RequestingAccount(ctx)) +	suite.False(authed) +	suite.Equal([]byte{}, resp) +	suite.Equal(http.StatusAccepted, code) + +	// Tombstone should be back, baby! +	exists, err := suite.state.DB.TombstoneExistsWithURI( +		context.Background(), +		"https://somewhere.mysterious/users/rest_in_piss#main-key", +	)  	suite.NoError(err)  	suite.True(exists)  } -func (suite *FederatingProtocolTestSuite) TestBlocked1() { -	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") -	tc := testrig.NewTestTransportController(&suite.state, httpClient) -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) - -	sendingAccount := suite.testAccounts["remote_account_1"] -	inboxAccount := suite.testAccounts["local_account_1"] -	otherInvolvedIRIs := []*url.URL{} -	actorIRIs := []*url.URL{ -		testrig.URLMustParse(sendingAccount.URI), -	} +func (suite *FederatingProtocolTestSuite) blocked( +	ctx context.Context, +	receivingAccount *gtsmodel.Account, +	requestingAccount *gtsmodel.Account, +	otherIRIs []*url.URL, +	actorIRIs []*url.URL, +) (bool, error) { +	ctx = gtscontext.SetReceivingAccount(ctx, receivingAccount) +	ctx = gtscontext.SetRequestingAccount(ctx, requestingAccount) +	ctx = gtscontext.SetOtherIRIs(ctx, otherIRIs) +	return suite.federator.Blocked(ctx, actorIRIs) +} -	ctx := context.Background() -	ctxWithReceivingAccount := context.WithValue(ctx, ap.ContextReceivingAccount, inboxAccount) -	ctxWithRequestingAccount := context.WithValue(ctxWithReceivingAccount, ap.ContextRequestingAccount, sendingAccount) -	ctxWithOtherInvolvedIRIs := context.WithValue(ctxWithRequestingAccount, ap.ContextOtherInvolvedIRIs, otherInvolvedIRIs) +func (suite *FederatingProtocolTestSuite) TestBlockedNoProblem() { +	var ( +		receivingAccount  = suite.testAccounts["local_account_1"] +		requestingAccount = suite.testAccounts["remote_account_1"] +		otherIRIs         = []*url.URL{} +		actorIRIs         = []*url.URL{ +			testrig.URLMustParse(requestingAccount.URI), +		} +	) + +	blocked, err := suite.blocked( +		context.Background(), +		receivingAccount, +		requestingAccount, +		otherIRIs, +		actorIRIs, +	) -	blocked, err := federator.Blocked(ctxWithOtherInvolvedIRIs, actorIRIs)  	suite.NoError(err)  	suite.False(blocked)  } -func (suite *FederatingProtocolTestSuite) TestBlocked2() { -	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") -	tc := testrig.NewTestTransportController(&suite.state, httpClient) -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) - -	sendingAccount := suite.testAccounts["remote_account_1"] -	inboxAccount := suite.testAccounts["local_account_1"] -	otherInvolvedIRIs := []*url.URL{} -	actorIRIs := []*url.URL{ -		testrig.URLMustParse(sendingAccount.URI), -	} - -	ctx := context.Background() -	ctxWithReceivingAccount := context.WithValue(ctx, ap.ContextReceivingAccount, inboxAccount) -	ctxWithRequestingAccount := context.WithValue(ctxWithReceivingAccount, ap.ContextRequestingAccount, sendingAccount) -	ctxWithOtherInvolvedIRIs := context.WithValue(ctxWithRequestingAccount, ap.ContextOtherInvolvedIRIs, otherInvolvedIRIs) - -	// insert a block from inboxAccount targeting sendingAccount -	if err := suite.db.PutBlock(context.Background(), >smodel.Block{ +func (suite *FederatingProtocolTestSuite) TestBlockedReceiverBlocksRequester() { +	var ( +		receivingAccount  = suite.testAccounts["local_account_1"] +		requestingAccount = suite.testAccounts["remote_account_1"] +		otherIRIs         = []*url.URL{} +		actorIRIs         = []*url.URL{ +			testrig.URLMustParse(requestingAccount.URI), +		} +	) + +	// Insert a block from receivingAccount targeting requestingAccount. +	if err := suite.state.DB.PutBlock(context.Background(), >smodel.Block{  		ID:              "01G3KBEMJD4VQ2D615MPV7KTRD",  		URI:             "whatever", -		AccountID:       inboxAccount.ID, -		TargetAccountID: sendingAccount.ID, +		AccountID:       receivingAccount.ID, +		TargetAccountID: requestingAccount.ID,  	}); err != nil {  		suite.Fail(err.Error())  	} -	// request should be blocked now -	blocked, err := federator.Blocked(ctxWithOtherInvolvedIRIs, actorIRIs) +	blocked, err := suite.blocked( +		context.Background(), +		receivingAccount, +		requestingAccount, +		otherIRIs, +		actorIRIs, +	) +  	suite.NoError(err)  	suite.True(blocked)  } -func (suite *FederatingProtocolTestSuite) TestBlocked3() { -	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") -	tc := testrig.NewTestTransportController(&suite.state, httpClient) -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) - -	sendingAccount := suite.testAccounts["remote_account_1"] -	inboxAccount := suite.testAccounts["local_account_1"] -	ccedAccount := suite.testAccounts["remote_account_2"] - -	otherInvolvedIRIs := []*url.URL{ -		testrig.URLMustParse(ccedAccount.URI), -	} -	actorIRIs := []*url.URL{ -		testrig.URLMustParse(sendingAccount.URI), -	} - -	ctx := context.Background() -	ctxWithReceivingAccount := context.WithValue(ctx, ap.ContextReceivingAccount, inboxAccount) -	ctxWithRequestingAccount := context.WithValue(ctxWithReceivingAccount, ap.ContextRequestingAccount, sendingAccount) -	ctxWithOtherInvolvedIRIs := context.WithValue(ctxWithRequestingAccount, ap.ContextOtherInvolvedIRIs, otherInvolvedIRIs) - -	// insert a block from inboxAccount targeting CCed account -	if err := suite.db.PutBlock(context.Background(), >smodel.Block{ +func (suite *FederatingProtocolTestSuite) TestBlockedCCd() { +	var ( +		receivingAccount  = suite.testAccounts["local_account_1"] +		requestingAccount = suite.testAccounts["remote_account_1"] +		ccedAccount       = suite.testAccounts["remote_account_2"] +		otherIRIs         = []*url.URL{ +			testrig.URLMustParse(ccedAccount.URI), +		} +		actorIRIs = []*url.URL{ +			testrig.URLMustParse(requestingAccount.URI), +		} +	) + +	// Insert a block from receivingAccount targeting ccedAccount. +	if err := suite.state.DB.PutBlock(context.Background(), >smodel.Block{  		ID:              "01G3KBEMJD4VQ2D615MPV7KTRD",  		URI:             "whatever", -		AccountID:       inboxAccount.ID, +		AccountID:       receivingAccount.ID,  		TargetAccountID: ccedAccount.ID,  	}); err != nil {  		suite.Fail(err.Error())  	} -	blocked, err := federator.Blocked(ctxWithOtherInvolvedIRIs, actorIRIs) -	suite.NoError(err) -	suite.True(blocked) -} - -func (suite *FederatingProtocolTestSuite) TestBlocked4() { -	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") -	tc := testrig.NewTestTransportController(&suite.state, httpClient) -	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.tc, testrig.NewTestMediaManager(&suite.state)) - -	sendingAccount := suite.testAccounts["remote_account_1"] -	inboxAccount := suite.testAccounts["local_account_1"] -	repliedStatus := suite.testStatuses["local_account_2_status_1"] - -	otherInvolvedIRIs := []*url.URL{ -		testrig.URLMustParse(repliedStatus.URI), // this status is involved because the hypothetical activity is a reply to this status -	} -	actorIRIs := []*url.URL{ -		testrig.URLMustParse(sendingAccount.URI), -	} - -	ctx := context.Background() -	ctxWithReceivingAccount := context.WithValue(ctx, ap.ContextReceivingAccount, inboxAccount) -	ctxWithRequestingAccount := context.WithValue(ctxWithReceivingAccount, ap.ContextRequestingAccount, sendingAccount) -	ctxWithOtherInvolvedIRIs := context.WithValue(ctxWithRequestingAccount, ap.ContextOtherInvolvedIRIs, otherInvolvedIRIs) +	blocked, err := suite.blocked( +		context.Background(), +		receivingAccount, +		requestingAccount, +		otherIRIs, +		actorIRIs, +	) -	// local account 2 (replied status account) blocks sending account already so we don't need to add a block here +	suite.EqualError(err, "block exists between http://localhost:8080/users/the_mighty_zork and one or more of [http://example.org/users/Some_User]") +	suite.False(blocked) +} -	blocked, err := federator.Blocked(ctxWithOtherInvolvedIRIs, actorIRIs) -	suite.NoError(err) -	suite.True(blocked) +func (suite *FederatingProtocolTestSuite) TestBlockedRepliedStatus() { +	var ( +		receivingAccount  = suite.testAccounts["local_account_1"] +		requestingAccount = suite.testAccounts["remote_account_1"] +		repliedStatus     = suite.testStatuses["local_account_2_status_1"] +		otherIRIs         = []*url.URL{ +			// This status is involved because the +			// hypothetical activity replies to it. +			testrig.URLMustParse(repliedStatus.URI), +		} +		actorIRIs = []*url.URL{ +			testrig.URLMustParse(requestingAccount.URI), +		} +	) + +	blocked, err := suite.blocked( +		context.Background(), +		receivingAccount, +		requestingAccount, +		otherIRIs, +		actorIRIs, +	) + +	suite.EqualError(err, "block exists between http://fossbros-anonymous.io/users/foss_satan and one or more of [http://localhost:8080/users/1happyturtle/statuses/01F8MHBQCBTDKN6X5VHGMMN4MA]") +	suite.False(blocked)  }  func TestFederatingProtocolTestSuite(t *testing.T) { diff --git a/internal/federation/federator_test.go b/internal/federation/federator_test.go index fdcfda19c..a80d590a4 100644 --- a/internal/federation/federator_test.go +++ b/internal/federation/federator_test.go @@ -20,10 +20,11 @@ package federation_test  import (  	"github.com/stretchr/testify/suite" -	"github.com/superseriousbusiness/gotosocial/internal/db" +	"github.com/superseriousbusiness/gotosocial/internal/federation"  	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"  	"github.com/superseriousbusiness/gotosocial/internal/state"  	"github.com/superseriousbusiness/gotosocial/internal/storage" +	"github.com/superseriousbusiness/gotosocial/internal/transport"  	"github.com/superseriousbusiness/gotosocial/internal/typeutils"  	"github.com/superseriousbusiness/gotosocial/internal/visibility"  	"github.com/superseriousbusiness/gotosocial/testrig" @@ -31,46 +32,58 @@ import (  type FederatorStandardTestSuite struct {  	suite.Suite -	db             db.DB -	storage        *storage.Driver -	state          state.State -	tc             typeutils.TypeConverter +	storage             *storage.Driver +	state               state.State +	typeconverter       typeutils.TypeConverter +	transportController transport.Controller +	httpClient          *testrig.MockHTTPClient +	federator           federation.Federator +  	testAccounts   map[string]*gtsmodel.Account  	testStatuses   map[string]*gtsmodel.Status  	testActivities map[string]testrig.ActivityWithSignature  	testTombstones map[string]*gtsmodel.Tombstone  } -// SetupSuite sets some variables on the suite that we can use as consts (more or less) throughout  func (suite *FederatorStandardTestSuite) SetupSuite() { -	// setup standard items -	testrig.StartWorkers(&suite.state) -	suite.storage = testrig.NewInMemoryStorage() -	suite.state.Storage = suite.storage  	suite.testAccounts = testrig.NewTestAccounts()  	suite.testStatuses = testrig.NewTestStatuses() +	suite.testActivities = testrig.NewTestActivities(suite.testAccounts)  	suite.testTombstones = testrig.NewTestTombstones()  }  func (suite *FederatorStandardTestSuite) SetupTest() { +	suite.state.Caches.Init() +	testrig.StartWorkers(&suite.state) +  	testrig.InitTestConfig()  	testrig.InitTestLog() -	suite.state.Caches.Init() -	suite.db = testrig.NewTestDB(&suite.state) -	suite.tc = testrig.NewTestTypeConverter(suite.db) -	suite.state.DB = suite.db + +	suite.state.DB = testrig.NewTestDB(&suite.state) +	suite.testActivities = testrig.NewTestActivities(suite.testAccounts) +	suite.storage = testrig.NewInMemoryStorage() +	suite.state.Storage = suite.storage +	suite.typeconverter = testrig.NewTestTypeConverter(suite.state.DB)  	testrig.StartTimelines(  		&suite.state,  		visibility.NewFilter(&suite.state), -		suite.tc, +		suite.typeconverter,  	) -	suite.testActivities = testrig.NewTestActivities(suite.testAccounts) -	testrig.StandardDBSetup(suite.db, suite.testAccounts) +	suite.httpClient = testrig.NewMockHTTPClient(nil, "../../testrig/media") +	suite.httpClient.TestRemotePeople = testrig.NewTestFediPeople() +	suite.httpClient.TestRemoteStatuses = testrig.NewTestFediStatuses() + +	suite.transportController = testrig.NewTestTransportController(&suite.state, suite.httpClient) +	suite.federator = testrig.NewTestFederator(&suite.state, suite.transportController, testrig.NewTestMediaManager(&suite.state)) + +	testrig.StandardDBSetup(suite.state.DB, nil) +	testrig.StandardStorageSetup(suite.storage, "../../testrig/media")  } -// TearDownTest drops tables to make sure there's no data in the db  func (suite *FederatorStandardTestSuite) TearDownTest() { -	testrig.StandardDBTeardown(suite.db) +	testrig.StandardDBTeardown(suite.state.DB) +	testrig.StandardStorageTeardown(suite.storage) +	testrig.StopWorkers(&suite.state)  } diff --git a/internal/gtscontext/context.go b/internal/gtscontext/context.go index d52bf2801..c8cd42208 100644 --- a/internal/gtscontext/context.go +++ b/internal/gtscontext/context.go @@ -19,6 +19,10 @@ package gtscontext  import (  	"context" +	"net/url" + +	"github.com/go-fed/httpsig" +	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"  )  // package private context key type. @@ -29,8 +33,14 @@ const (  	_ ctxkey = iota  	barebonesKey  	fastFailKey -	pubKeyIDKey +	outgoingPubKeyIDKey  	requestIDKey +	receivingAccountKey +	requestingAccountKey +	otherIRIsKey +	httpSigVerifierKey +	httpSigKey +	httpSigPubKeyIDKey  )  // RequestID returns the request ID associated with context. This value will usually @@ -48,18 +58,97 @@ func SetRequestID(ctx context.Context, id string) context.Context {  	return context.WithValue(ctx, requestIDKey, id)  } -// PublicKeyID returns the public key ID (URI) associated with context. This +// OutgoingPublicKeyID returns the public key ID (URI) associated with context. This  // value is useful for logging situations in which a given public key URI is  // relevant, e.g. for outgoing requests being signed by the given key. -func PublicKeyID(ctx context.Context) string { -	id, _ := ctx.Value(pubKeyIDKey).(string) +func OutgoingPublicKeyID(ctx context.Context) string { +	id, _ := ctx.Value(outgoingPubKeyIDKey).(string)  	return id  } -// SetPublicKeyID stores the given public key ID value and returns the wrapped +// SetOutgoingPublicKeyID stores the given public key ID value and returns the wrapped  // context. See PublicKeyID() for further information on the public key ID value. -func SetPublicKeyID(ctx context.Context, id string) context.Context { -	return context.WithValue(ctx, pubKeyIDKey, id) +func SetOutgoingPublicKeyID(ctx context.Context, id string) context.Context { +	return context.WithValue(ctx, outgoingPubKeyIDKey, id) +} + +// ReceivingAccount returns the local account who owns the resource being +// interacted with (inbox, uri, etc) in the current ActivityPub request chain. +func ReceivingAccount(ctx context.Context) *gtsmodel.Account { +	acct, _ := ctx.Value(receivingAccountKey).(*gtsmodel.Account) +	return acct +} + +// SetReceivingAccount stores the given receiving account value and returns the wrapped +// context. See ReceivingAccount() for further information on the receiving account value. +func SetReceivingAccount(ctx context.Context, acct *gtsmodel.Account) context.Context { +	return context.WithValue(ctx, receivingAccountKey, acct) +} + +// RequestingAccount returns the remote account interacting with a local +// resource (inbox, uri, etc) in the current ActivityPub request chain. +func RequestingAccount(ctx context.Context) *gtsmodel.Account { +	acct, _ := ctx.Value(requestingAccountKey).(*gtsmodel.Account) +	return acct +} + +// SetRequestingAccount stores the given requesting account value and returns the wrapped +// context. See RequestingAccount() for further information on the requesting account value. +func SetRequestingAccount(ctx context.Context, acct *gtsmodel.Account) context.Context { +	return context.WithValue(ctx, requestingAccountKey, acct) +} + +// OtherIRIs returns other IRIs which are involved in the current ActivityPub request +// chain. This usually means: other accounts who are mentioned, CC'd, TO'd, or boosted +// by the current inbox POST request. +func OtherIRIs(ctx context.Context) []*url.URL { +	iris, _ := ctx.Value(otherIRIsKey).([]*url.URL) +	return iris +} + +// SetOtherIRIs stores the given IRIs slice and returns the wrapped context. +// See OtherIRIs() for further information on the IRIs slice value. +func SetOtherIRIs(ctx context.Context, iris []*url.URL) context.Context { +	return context.WithValue(ctx, otherIRIsKey, iris) +} + +// 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.Verifier { +	verifier, _ := ctx.Value(httpSigVerifierKey).(httpsig.Verifier) +	return verifier +} + +// SetHTTPSignatureVerifier stores the given http signature verifier and returns the +// wrapped context. See HTTPSignatureVerifier() for further information on the verifier value. +func SetHTTPSignatureVerifier(ctx context.Context, verifier httpsig.Verifier) context.Context { +	return context.WithValue(ctx, httpSigVerifierKey, verifier) +} + +// HTTPSignature returns the http signature string +// value for the current ActivityPub request chain. +func HTTPSignature(ctx context.Context) string { +	signature, _ := ctx.Value(httpSigKey).(string) +	return signature +} + +// SetHTTPSignature stores the given http signature string and returns the wrapped +// context. See HTTPSignature() for further information on the verifier value. +func SetHTTPSignature(ctx context.Context, signature string) context.Context { +	return context.WithValue(ctx, httpSigKey, signature) +} + +// HTTPSignaturePubKeyID returns the public key id of the http signature +// for the current ActivityPub request chain. +func HTTPSignaturePubKeyID(ctx context.Context) *url.URL { +	pubKeyID, _ := ctx.Value(httpSigPubKeyIDKey).(*url.URL) +	return pubKeyID +} + +// SetHTTPSignaturePubKeyID stores the given http signature public key id and returns +// the wrapped context. See HTTPSignaturePubKeyID() for further information on the value. +func SetHTTPSignaturePubKeyID(ctx context.Context, pubKeyID *url.URL) context.Context { +	return context.WithValue(ctx, httpSigPubKeyIDKey, pubKeyID)  }  // IsFastFail returns whether the "fastfail" context key has been set. This diff --git a/internal/gtscontext/log_hooks.go b/internal/gtscontext/log_hooks.go index 94300cbb7..494ce1f63 100644 --- a/internal/gtscontext/log_hooks.go +++ b/internal/gtscontext/log_hooks.go @@ -36,7 +36,7 @@ func init() {  	})  	// Public Key ID middleware hook.  	log.Hook(func(ctx context.Context, kvs []kv.Field) []kv.Field { -		if id := PublicKeyID(ctx); id != "" { +		if id := OutgoingPublicKeyID(ctx); id != "" {  			return append(kvs, kv.Field{K: "pubKeyID", V: id})  		}  		return kvs diff --git a/internal/middleware/signaturecheck.go b/internal/middleware/signaturecheck.go index 0fba21205..df2ac0300 100644 --- a/internal/middleware/signaturecheck.go +++ b/internal/middleware/signaturecheck.go @@ -19,95 +19,102 @@ package middleware  import (  	"context" -	"fmt"  	"net/http"  	"net/url" -	"github.com/superseriousbusiness/gotosocial/internal/ap"  	"github.com/superseriousbusiness/gotosocial/internal/db" +	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"  	"github.com/superseriousbusiness/gotosocial/internal/log"  	"github.com/gin-gonic/gin"  	"github.com/go-fed/httpsig"  ) -var ( -	// this mimics an untyped error returned by httpsig when no signature is present; -	// define it here so that we can use it to decide what to log without hitting -	// performance too hard -	noSignatureError    = fmt.Sprintf("neither %q nor %q have signature parameters", httpsig.Signature, httpsig.Authorization) -	signatureHeader     = string(httpsig.Signature) -	authorizationHeader = string(httpsig.Authorization) +const ( +	sigHeader  = string(httpsig.Signature) +	authHeader = string(httpsig.Authorization) +	// untyped error returned by httpsig when no signature is present +	noSigError = "neither \"" + sigHeader + "\" nor \"" + authHeader + "\" have signature parameters"  )  // SignatureCheck returns a gin middleware for checking http signatures.  // -// The middleware first checks whether an incoming http request has been http-signed with a well-formed signature. +// The middleware first checks whether an incoming http request has been +// http-signed with a well-formed signature. If so, it will check if the +// domain that signed the request is permitted to access the server, using +// the provided uriBlocked function. If the domain is blocked, the middleware +// will abort the request chain with http code 403 forbidden. If it is not +// blocked, the handler will set the key verifier and the signature in the +// context for use down the line.  // -// If so, it will check if the domain that signed the request is permitted to access the server, using the provided isURIBlocked function. -// -// If it is permitted, the handler will set the key verifier and the signature in the gin context for use down the line. -// -// If the domain is blocked, the middleware will abort the request chain instead with http code 403 forbidden. -// -// In case of an error, the request will be aborted with http code 500 internal server error. -func SignatureCheck(isURIBlocked func(context.Context, *url.URL) (bool, db.Error)) func(*gin.Context) { +// In case of an error, the request will be aborted with http code 500. +func SignatureCheck(uriBlocked func(context.Context, *url.URL) (bool, db.Error)) func(*gin.Context) {  	return func(c *gin.Context) { -		// Acquire ctx from gin request.  		ctx := c.Request.Context() -		// create the verifier from the request, this will error if the request wasn't signed +		// Create the signature verifier from the request; +		// this will error if the request wasn't signed.  		verifier, err := httpsig.NewVerifier(c.Request)  		if err != nil { -			// Something went wrong, so we need to return regardless, but only actually -			// *abort* the request with 401 if a signature was present but malformed -			if err.Error() != noSignatureError { +			// Only actually *abort* the request with 401 +			// if a signature was present but malformed. +			// Otherwise proceed with an unsigned request; +			// it's up to other functions to reject this. +			if err.Error() != noSigError {  				log.Debugf(ctx, "http signature was present but invalid: %s", err)  				c.AbortWithStatus(http.StatusUnauthorized)  			} +  			return  		} -		// The request was signed! -		// The key ID should be given in the signature so that we know where to fetch it from the remote server. -		// This will be something like https://example.org/users/whatever_requesting_user#main-key -		requestingPublicKeyIDString := verifier.KeyId() -		requestingPublicKeyID, err := url.Parse(requestingPublicKeyIDString) -		if err != nil { -			log.Debugf(ctx, "http signature requesting public key id %s could not be parsed as a url: %s", requestingPublicKeyIDString, err) -			c.AbortWithStatus(http.StatusUnauthorized) -			return -		} else if requestingPublicKeyID == nil { -			// Key can sometimes be nil, according to url parse function: -			// 'Trying to parse a hostname and path without a scheme is invalid but may not necessarily return an error, due to parsing ambiguities' -			log.Debugf(ctx, "http signature requesting public key id %s was nil after parsing as a url", requestingPublicKeyIDString) +		// The request was signed! The key ID should be given +		// in the signature so that we know where to fetch it +		// from the remote server. This will be something like: +		// https://example.org/users/some_remote_user#main-key +		pubKeyIDStr := verifier.KeyId() + +		// Key can sometimes be nil, according to url parse +		// func: 'Trying to parse a hostname and path without +		// a scheme is invalid but may not necessarily return +		// an error, due to parsing ambiguities'. Catch this. +		pubKeyID, err := url.Parse(pubKeyIDStr) +		if err != nil || pubKeyID == nil { +			log.Warnf(ctx, "pubkey id %s could not be parsed as a url", pubKeyIDStr)  			c.AbortWithStatus(http.StatusUnauthorized)  			return  		} -		// we managed to parse the url! -		// if the domain is blocked we want to bail as early as possible -		if blocked, err := isURIBlocked(c.Request.Context(), requestingPublicKeyID); err != nil { -			log.Errorf(ctx, "could not tell if domain %s was blocked or not: %s", requestingPublicKeyID.Host, err) +		// If the domain is blocked we want to bail as fast as +		// possible without the request proceeding further. +		blocked, err := uriBlocked(ctx, pubKeyID) +		if err != nil { +			log.Errorf(ctx, "error checking block for domain %s: %s", pubKeyID.Host, err)  			c.AbortWithStatus(http.StatusInternalServerError)  			return -		} else if blocked { -			log.Infof(ctx, "domain %s is blocked", requestingPublicKeyID.Host) +		} + +		if blocked { +			log.Infof(ctx, "domain %s is blocked", pubKeyID.Host)  			c.AbortWithStatus(http.StatusForbidden)  			return  		} -		// assume signature was set on Signature header (most common behavior), -		// but fall back to Authorization header if necessary -		var signature string -		if s := c.GetHeader(signatureHeader); s != "" { -			signature = s -		} else { -			signature = c.GetHeader(authorizationHeader) +		// Assume signature was set on Signature header, +		// but fall back to Authorization header if necessary. +		signature := c.GetHeader(sigHeader) +		if signature == "" { +			signature = c.GetHeader(authHeader)  		} -		// set the verifier and signature on the context here to save some work further down the line -		c.Set(string(ap.ContextRequestingPublicKeyVerifier), verifier) -		c.Set(string(ap.ContextRequestingPublicKeySignature), signature) +		// Set relevant values on the request context +		// to save some work further down the line. +		ctx = gtscontext.SetHTTPSignatureVerifier(ctx, verifier) +		ctx = gtscontext.SetHTTPSignature(ctx, signature) +		ctx = gtscontext.SetHTTPSignaturePubKeyID(ctx, pubKeyID) + +		// Replace request with a shallow +		// copy with the new context. +		c.Request = c.Request.WithContext(ctx)  	}  } diff --git a/internal/transport/transport.go b/internal/transport/transport.go index 0123b3ea8..fc85e5141 100644 --- a/internal/transport/transport.go +++ b/internal/transport/transport.go @@ -87,7 +87,7 @@ func (t *transport) GET(r *http.Request) (*http.Response, error) {  		return nil, errors.New("must be GET request")  	}  	ctx := r.Context() // extract, set pubkey ID. -	ctx = gtscontext.SetPublicKeyID(ctx, t.pubKeyID) +	ctx = gtscontext.SetOutgoingPublicKeyID(ctx, t.pubKeyID)  	r = r.WithContext(ctx) // replace request ctx.  	r.Header.Set("User-Agent", t.controller.userAgent)  	return t.controller.client.DoSigned(r, t.signGET()) @@ -99,7 +99,7 @@ func (t *transport) POST(r *http.Request, body []byte) (*http.Response, error) {  		return nil, errors.New("must be POST request")  	}  	ctx := r.Context() // extract, set pubkey ID. -	ctx = gtscontext.SetPublicKeyID(ctx, t.pubKeyID) +	ctx = gtscontext.SetOutgoingPublicKeyID(ctx, t.pubKeyID)  	r = r.WithContext(ctx) // replace request ctx.  	r.Header.Set("User-Agent", t.controller.userAgent)  	return t.controller.client.DoSigned(r, t.signPOST(body)) diff --git a/internal/typeutils/astointernal.go b/internal/typeutils/astointernal.go index 7c0b60ad5..aeb6a5917 100644 --- a/internal/typeutils/astointernal.go +++ b/internal/typeutils/astointernal.go @@ -196,10 +196,15 @@ func (c *converter) ASRepresentationToAccount(ctx context.Context, accountable a  	// TODO: alsoKnownAs  	// publicKey -	pkey, pkeyURL, err := ap.ExtractPublicKeyForOwner(accountable, uri) +	pkey, pkeyURL, pkeyOwnerID, err := ap.ExtractPublicKey(accountable)  	if err != nil {  		return nil, fmt.Errorf("couldn't get public key for person %s: %s", uri.String(), err)  	} + +	if pkeyOwnerID.String() != acct.URI { +		return nil, fmt.Errorf("public key %s was owned by %s and not by %s", pkeyURL, pkeyOwnerID, acct.URI) +	} +  	acct.PublicKey = pkey  	acct.PublicKeyURI = pkeyURL.String() diff --git a/internal/util/unique.go b/internal/util/unique.go index 253bb621b..f0ded1446 100644 --- a/internal/util/unique.go +++ b/internal/util/unique.go @@ -19,28 +19,46 @@ package util  import "net/url" -// UniqueStrings returns a deduplicated version of a given string slice. -func UniqueStrings(s []string) []string { -	keys := make(map[string]bool, len(s)) -	list := []string{} -	for _, entry := range s { -		if _, value := keys[entry]; !value { -			keys[entry] = true -			list = append(list, entry) +// UniqueStrings returns a deduplicated version of the given +// slice of strings, without changing the order of the entries. +func UniqueStrings(strings []string) []string { +	var ( +		l      = len(strings) +		keys   = make(map[string]any, l) // Use map to dedupe items. +		unique = make([]string, 0, l)    // Return slice. +	) + +	for _, str := range strings { +		// Check if already set as a key in the map; +		// if not, add to return slice + mark key as set. +		if _, set := keys[str]; !set { +			keys[str] = nil // Value doesn't matter. +			unique = append(unique, str)  		}  	} -	return list + +	return unique  } -// UniqueURIs returns a deduplicated version of a given *url.URL slice. -func UniqueURIs(s []*url.URL) []*url.URL { -	keys := make(map[string]bool, len(s)) -	list := []*url.URL{} -	for _, entry := range s { -		if _, value := keys[entry.String()]; !value { -			keys[entry.String()] = true -			list = append(list, entry) +// UniqueURIs returns a deduplicated version of the given +// slice of URIs, without changing the order of the entries. +func UniqueURIs(uris []*url.URL) []*url.URL { +	var ( +		l      = len(uris) +		keys   = make(map[string]any, l) // Use map to dedupe items. +		unique = make([]*url.URL, 0, l)  // Return slice. +	) + +	for _, uri := range uris { +		uriStr := uri.String() + +		// Check if already set as a key in the map; +		// if not, add to return slice + mark key as set. +		if _, set := keys[uriStr]; !set { +			keys[uriStr] = nil // Value doesn't matter. +			unique = append(unique, uri)  		}  	} -	return list + +	return unique  } diff --git a/internal/web/profile.go b/internal/web/profile.go index 56f8e0a56..2ffc7411e 100644 --- a/internal/web/profile.go +++ b/internal/web/profile.go @@ -26,7 +26,6 @@ import (  	"strings"  	"github.com/gin-gonic/gin" -	"github.com/superseriousbusiness/gotosocial/internal/ap"  	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"  	apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"  	"github.com/superseriousbusiness/gotosocial/internal/config" @@ -75,7 +74,7 @@ func (m *Module) profileGETHandler(c *gin.Context) {  	// should render the account's AP representation instead  	accept := apiutil.NegotiateFormat(c, string(apiutil.TextHTML), string(apiutil.AppActivityJSON), string(apiutil.AppActivityLDJSON))  	if accept == string(apiutil.AppActivityJSON) || accept == string(apiutil.AppActivityLDJSON) { -		m.returnAPProfile(ctx, c, username, accept) +		m.returnAPProfile(c, username, accept)  		return  	} @@ -145,27 +144,17 @@ func (m *Module) profileGETHandler(c *gin.Context) {  	})  } -func (m *Module) returnAPProfile(ctx context.Context, c *gin.Context, username string, accept string) { -	verifier, signed := c.Get(string(ap.ContextRequestingPublicKeyVerifier)) -	if signed { -		ctx = context.WithValue(ctx, ap.ContextRequestingPublicKeyVerifier, verifier) -	} - -	signature, signed := c.Get(string(ap.ContextRequestingPublicKeySignature)) -	if signed { -		ctx = context.WithValue(ctx, ap.ContextRequestingPublicKeySignature, signature) -	} - -	user, errWithCode := m.processor.Fedi().UserGet(ctx, username, c.Request.URL) +func (m *Module) returnAPProfile(c *gin.Context, username string, accept string) { +	user, errWithCode := m.processor.Fedi().UserGet(c.Request.Context(), username, c.Request.URL)  	if errWithCode != nil { -		apiutil.WebErrorHandler(c, errWithCode, m.processor.InstanceGetV1) //nolint:contextcheck +		apiutil.WebErrorHandler(c, errWithCode, m.processor.InstanceGetV1)  		return  	}  	b, mErr := json.Marshal(user)  	if mErr != nil {  		err := fmt.Errorf("could not marshal json: %s", mErr) -		apiutil.WebErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGetV1) //nolint:contextcheck +		apiutil.WebErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGetV1)  		return  	} diff --git a/internal/web/thread.go b/internal/web/thread.go index 8d4e99bef..41f81eb14 100644 --- a/internal/web/thread.go +++ b/internal/web/thread.go @@ -26,7 +26,6 @@ import (  	"strings"  	"github.com/gin-gonic/gin" -	"github.com/superseriousbusiness/gotosocial/internal/ap"  	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"  	apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"  	"github.com/superseriousbusiness/gotosocial/internal/config" @@ -92,7 +91,7 @@ func (m *Module) threadGETHandler(c *gin.Context) {  	// should render the status's AP representation instead  	accept := apiutil.NegotiateFormat(c, string(apiutil.TextHTML), string(apiutil.AppActivityJSON), string(apiutil.AppActivityLDJSON))  	if accept == string(apiutil.AppActivityJSON) || accept == string(apiutil.AppActivityLDJSON) { -		m.returnAPStatus(ctx, c, username, statusID, accept) +		m.returnAPStatus(c, username, statusID, accept)  		return  	} @@ -120,27 +119,17 @@ func (m *Module) threadGETHandler(c *gin.Context) {  	})  } -func (m *Module) returnAPStatus(ctx context.Context, c *gin.Context, username string, statusID string, accept string) { -	verifier, signed := c.Get(string(ap.ContextRequestingPublicKeyVerifier)) -	if signed { -		ctx = context.WithValue(ctx, ap.ContextRequestingPublicKeyVerifier, verifier) -	} - -	signature, signed := c.Get(string(ap.ContextRequestingPublicKeySignature)) -	if signed { -		ctx = context.WithValue(ctx, ap.ContextRequestingPublicKeySignature, signature) -	} - -	status, errWithCode := m.processor.Fedi().StatusGet(ctx, username, statusID) +func (m *Module) returnAPStatus(c *gin.Context, username string, statusID string, accept string) { +	status, errWithCode := m.processor.Fedi().StatusGet(c.Request.Context(), username, statusID)  	if errWithCode != nil { -		apiutil.WebErrorHandler(c, errWithCode, m.processor.InstanceGetV1) //nolint:contextcheck +		apiutil.WebErrorHandler(c, errWithCode, m.processor.InstanceGetV1)  		return  	}  	b, mErr := json.Marshal(status)  	if mErr != nil {  		err := fmt.Errorf("could not marshal json: %s", mErr) -		apiutil.WebErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGetV1) //nolint:contextcheck +		apiutil.WebErrorHandler(c, gtserror.NewErrorInternalError(err), m.processor.InstanceGetV1)  		return  	}  | 
