From dca7a81d806dce8f25aa801e4122919df5a5e06f Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Tue, 13 Jun 2023 16:47:56 +0200 Subject: [PATCH] [chore] Refactor AP authentication, other small bits of tidying up (#1874) --- internal/ap/contextkey.go | 36 - internal/ap/extract.go | 63 +- internal/api/activitypub/emoji/emojiget.go | 2 +- .../api/activitypub/publickey/publickeyget.go | 2 +- internal/api/activitypub/users/featured.go | 2 +- internal/api/activitypub/users/followers.go | 2 +- internal/api/activitypub/users/following.go | 2 +- internal/api/activitypub/users/inboxpost.go | 2 +- .../api/activitypub/users/inboxpost_test.go | 25 + internal/api/activitypub/users/outboxget.go | 2 +- internal/api/activitypub/users/repliesget.go | 2 +- internal/api/activitypub/users/statusget.go | 2 +- internal/api/activitypub/users/user_test.go | 2 + internal/api/activitypub/users/userget.go | 2 +- internal/api/util/signaturectx.go | 40 -- internal/federation/authenticate.go | 491 +++++++------- internal/federation/federatingactor.go | 36 +- internal/federation/federatingactor_test.go | 6 +- internal/federation/federatingdb/accept.go | 9 +- internal/federation/federatingdb/announce.go | 9 +- internal/federation/federatingdb/create.go | 9 +- internal/federation/federatingdb/delete.go | 9 +- .../federatingdb/federatingdb_test.go | 6 +- internal/federation/federatingdb/reject.go | 9 +- internal/federation/federatingdb/undo.go | 9 +- internal/federation/federatingdb/update.go | 22 +- internal/federation/federatingdb/util.go | 40 +- internal/federation/federatingprotocol.go | 420 ++++++++---- .../federation/federatingprotocol_test.go | 635 ++++++++++-------- internal/federation/federator_test.go | 47 +- internal/gtscontext/context.go | 103 ++- internal/gtscontext/log_hooks.go | 2 +- internal/middleware/signaturecheck.go | 117 ++-- internal/transport/transport.go | 4 +- internal/typeutils/astointernal.go | 7 +- internal/util/unique.go | 54 +- internal/web/profile.go | 21 +- internal/web/thread.go | 21 +- 38 files changed, 1279 insertions(+), 993 deletions(-) delete mode 100644 internal/ap/contextkey.go delete mode 100644 internal/api/util/signaturectx.go 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 . - -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 } - if pkey.GetW3IDSecurityV1Owner() == nil || pkey.GetW3IDSecurityV1Owner().Get() == nil || pkey.GetW3IDSecurityV1Owner().Get().String() != forOwner.String() { + pubKeyOwnerProp := pkey.GetW3IDSecurityV1Owner() + if pubKeyOwnerProp == nil { continue } - if pkey.GetW3IDSecurityV1PublicKeyPem() == nil { + pubKeyOwner := pubKeyOwnerProp.GetIRI() + if pubKeyOwner == nil { continue } - pkeyPem := pkey.GetW3IDSecurityV1PublicKeyPem().Get() + pubKeyPemProp := pkey.GetW3IDSecurityV1PublicKeyPem() + if pubKeyPemProp == nil { + continue + } + + 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 1dd9b045f..4e55268c6 100644 --- a/internal/api/activitypub/users/user_test.go +++ b/internal/api/activitypub/users/user_test.go @@ -55,6 +55,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 @@ -71,6 +72,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 . - -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 - } - - pker, ok := t.(publicKeyer) - if !ok { - return nil, fmt.Errorf("ActivityStreams type cannot be converted to one known to have publicKey property: %T", t) - } - - pkp := pker.GetW3IDSecurityV1PublicKey() - if pkp == nil { - return nil, errors.New("publicKey property is not provided") - } - - 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 - } - - if pkpFound == nil { - return nil, fmt.Errorf("cannot find publicKey with id: %s", keyID) - } - - return pkpFound, nil -} - -// 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. +// 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. +// '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. +// 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. +// 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. +// 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) + // 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 } - 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) + // 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 } - // 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) + // 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 } - 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 - } - - // now figure out who actually signed it - requestingPublicKeyID, err := url.Parse(verifier.KeyId()) - 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 - } + // At this point we know the request was signed, + // so now we need to validate the signature. var ( - requestingLocalAccount *gtsmodel.Account - requestingRemoteAccount *gtsmodel.Account - requestingHost = requestingPublicKeyID.Host + pubKeyIDStr = pubKeyID.String() + requestingAccountURI *url.URL + pubKey interface{} + errWithCode gtserror.WithCode ) - 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) + l := log. + WithContext(ctx). + WithFields(kv.Fields{ + {"requestedUsername", requestedUsername}, + {"pubKeyID", pubKeyIDStr}, + }...) - 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 - } - - publicKey = requestingLocalAccount.PublicKey - - 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 - } + if pubKeyID.Host == config.GetHost() { + l.Trace("public key is ours, no dereference needed") + requestingAccountURI, pubKey, errWithCode = f.derefDBOnly(ctx, pubKeyIDStr) } 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 { - errWithCode := gtserror.NewErrorGone(fmt.Errorf("account with public key %s is gone", requestingPublicKeyID)) - log.Debug(ctx, errWithCode) - return 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 - } - - // 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 - } - - // 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 - } - - // 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 - } - - // 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 - } - - 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 - } - - // 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() + l.Trace("public key is not ours, checking if we need to dereference") + requestingAccountURI, pubKey, errWithCode = f.deref(ctx, requestedUsername, pubKeyIDStr, pubKeyID) } - // 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) + if errWithCode != nil { return nil, errWithCode } - // do the actual authentication here! - algos := []httpsig.Algorithm{ - httpsig.RSA_SHA256, - httpsig.RSA_SHA512, - httpsig.ED25519, + // Ensure public key now defined. + if pubKey == nil { + err := gtserror.New("public key was nil") + return nil, gtserror.NewErrorInternalError(err) } - for _, algo := range algos { - log.Tracef(ctx, "trying algo: %s", algo) - err := verifier.Verify(publicKey, algo) + // 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 { - log.Tracef(ctx, "authentication for %s PASSED with algorithm %s", pkOwnerURI, algo) - return pkOwnerURI, nil + l.Tracef("authentication PASSED with %s", algo) + return requestingAccountURI, nil } - log.Tracef(ctx, "authentication for %s NOT PASSED with algorithm %s: %s", pkOwnerURI, algo, err) + + l.Tracef("authentication NOT PASSED with %s: %q", algo, 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 + // 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) + } + + reqAcctURI, err := url.Parse(reqAcct.URI) + if err != nil { + err = gtserror.Newf("error parsing account uri with pubKeyID %s: %w", pubKeyIDStr, err) + return nil, nil, gtserror.NewErrorInternalError(err) + } + + return reqAcctURI, reqAcct.PublicKey, nil +} + +// 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 + } + + l.Trace("public key not cached, trying dereference") + + // 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) + } + + if gone { + err := gtserror.Newf("account with public key %s is gone", pubKeyIDStr) + return nil, nil, gtserror.NewErrorGone(err) + } + + // Make an http call to get the pubkey. + pubKeyBytes, errWithCode := f.callForPubKey(ctx, requestedUsername, pubKeyID) + if errWithCode != nil { + return nil, 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) + } + + return pubKeyOwner, pubKey, nil +} + +// 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) + } + + // 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 + } + + 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) + } + + err := gtserror.Newf("account with public key %s is gone", pubKeyID) + return nil, gtserror.NewErrorGone(err) + } + + // 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 + } + + t, err := streams.ToType(ctx, m) + if err != nil { + return nil, nil, err + } + + 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 + } + + 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 + } + + 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 32495a2f8..76413d8cb 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" @@ -98,7 +98,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. +// 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. -// -// 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 Activity itself + // Check for TOs and CCs on the Activity. 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, toURIs...) + } + + if ccURIs, err := ap.ExtractCCs(addressable); err == nil { + 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) - if err != nil { - return false, fmt.Errorf("error checking domain blocks of otherInvolvedIRIs: %s", err) - } - if blocked { - 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 + // 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 user-level blocks: %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 } - // 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 + // 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 { + err := gtserror.Newf("error checking domain block of otherIRIs: %w", err) + return false, err + } + + if blocked { + err := newErrOtherIRIBlocked(receivingAccount.URI, true, otherIRIs) + l.Trace(err.Error()) + return false, err + } + + // 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) - } - if blocked { - return blocked, nil + err = gtserror.Newf("db error checking block between receiver and other account: %w", err) + 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 + l.Trace("receiving account blocks one or more otherIRIs") + err := newErrOtherIRIBlocked(receivingAccount.URI, false, otherIRIs) + return false, err + } + + // 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") - } - - 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") - } - - 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) - - involvedIRIsI := newContext.Value(ap.ContextOtherInvolvedIRIs) - involvedIRIs, ok := involvedIRIsI.([]*url.URL) - if !ok { - suite.FailNow("couldn't get involved IRIs from context") - } - - 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) 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) - - // 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)) - - 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) - - verifier, err := httpsig.NewVerifier(request) - suite.NoError(err) - - 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) - - // we can pass this recorder as a writer and read it back after - recorder := httptest.NewRecorder() - - // 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) - - // 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) -} - -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"] - - httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") - tc := testrig.NewTestTransportController(&suite.state, httpClient) - - // 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)) - - 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) - - verifier, err := httpsig.NewVerifier(request) - suite.NoError(err) - - 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) - - // we can pass this recorder as a writer and read it back after - recorder := httptest.NewRecorder() - - // 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) -} - -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 { +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()) } - // 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"] + b, err := json.Marshal(raw) + if err != nil { + suite.FailNow(err.Error()) + } + suite.NoError(err) + 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) - httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media") - tc := testrig.NewTestTransportController(&suite.state, httpClient) + newContext, err := suite.federator.PostInboxRequestBodyHook(ctx, request, 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)) + return newContext +} - request := httptest.NewRequest(http.MethodPost, "http://localhost:8080/users/the_mighty_zork/inbox", nil) - // we need these headers for the request to be validated +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()) + } + + b, err := json.Marshal(raw) + if err != nil { + suite.FailNow(err.Error()) + } + + 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 - _, authed, err := federator.AuthenticatePostInbox(ctxWithSignature, recorder, request) - suite.NoError(err) + res := recorder.Result() + defer res.Body.Close() + + b, err = io.ReadAll(res.Body) + if err != nil { + suite.FailNow(err.Error()) + } + + return newContext, authed, b, res.StatusCode +} + +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()) + } + + 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) +} + +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()) + } + + 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) +} + +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()) + } + + 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) +} + +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()) + } + + 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) 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()) + } + + 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) +} + +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) +} + +func (suite *FederatingProtocolTestSuite) TestAuthenticatePostGoneWithTombstone() { + var ( + activity = suite.testActivities["delete_https://somewhere.mysterious/users/rest_in_piss#main-key"] + receivingAccount = suite.testAccounts["local_account_1"] + ) + + ctx, authed, resp, code := suite.authenticatePostInbox( + context.Background(), + receivingAccount, + activity, + ) + + // Tombstone exists for this account, should simply return accepted. + suite.Nil(gtscontext.RequestingAccount(ctx)) suite.False(authed) - suite.Equal(http.StatusAccepted, recorder.Code) + suite.Equal([]byte{}, resp) + suite.Equal(http.StatusAccepted, code) +} - // 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") +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"] + ) + + // 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()) + } + + ctx, authed, resp, code := suite.authenticatePostInbox( + context.Background(), + receivingAccount, + activity, + ) + + 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)) +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) +} - 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) TestBlockedNoProblem() { + var ( + receivingAccount = suite.testAccounts["local_account_1"] + requestingAccount = suite.testAccounts["remote_account_1"] + otherIRIs = []*url.URL{} + actorIRIs = []*url.URL{ + testrig.URLMustParse(requestingAccount.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, + ) - 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)) +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), + } + ) - 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{ + // 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)) +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), + } + ) - 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{ + // 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) + blocked, err := suite.blocked( + context.Background(), + receivingAccount, + requestingAccount, + otherIRIs, + actorIRIs, + ) + + 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) } -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)) +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), + } + ) - sendingAccount := suite.testAccounts["remote_account_1"] - inboxAccount := suite.testAccounts["local_account_1"] - repliedStatus := suite.testStatuses["local_account_2_status_1"] + blocked, err := suite.blocked( + context.Background(), + receivingAccount, + requestingAccount, + otherIRIs, + actorIRIs, + ) - 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) - - // local account 2 (replied status account) blocks sending account already so we don't need to add a block here - - blocked, err := federator.Blocked(ctxWithOtherInvolvedIRIs, actorIRIs) - suite.NoError(err) - suite.True(blocked) + 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 abb462ee6..df1747dc5 100644 --- a/internal/federation/federator_test.go +++ b/internal/federation/federator_test.go @@ -20,49 +20,64 @@ 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/testrig" ) 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) - testrig.StandardDBSetup(suite.db, suite.testAccounts) + suite.storage = testrig.NewInMemoryStorage() + suite.state.Storage = suite.storage + suite.typeconverter = testrig.NewTestTypeConverter(suite.state.DB) + + 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) + // 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 + } + + // 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.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) - 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) + 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 }