mirrors/gotosocial
1
0
Fork 0
mirror of https://github.com/superseriousbusiness/gotosocial.git synced 2025-11-18 14:17:30 -06:00
Code Issues Projects Releases Packages Wiki Activity Actions

[bugfix] add stricter checks during all stages of dereferencing remote AS objects (#2639)

Browse source 
* add stricter checks during all stages of dereferencing remote AS objects

* a comment
This commit is contained in:
kim 2024-02-14 11:13:38 +00:00 • committed by tobi
commit b9013a8ab3
15 changed files with 351 additions and 167 deletions
Download patch file Download diff file Expand all files Collapse all files

47
internal/federation/dereferencing/account.go
View file

@ -586,6 +586,16 @@ func (d *Dereferencer) enrichAccount(
}
if account.Username == "" {
// Assume the host from the
// ActivityPub representation.
id := ap.GetJSONLDId(apubAcc)
if id == nil {
return nil, nil, gtserror.New("no id property found on person, or id was not an iri")
}
// Get IRI host value.
accHost := id.Host
// No username was provided, so no webfinger was attempted earlier.
//
// Now we have a username we can attempt again, to ensure up-to-date
@ -596,42 +606,37 @@ func (d *Dereferencer) enrichAccount(
// https://example.org/@someone@somewhere.else and we've been redirected
// from example.org to somewhere.else: we want to take somewhere.else
// as the accountDomain then, not the example.org we were redirected from.
// Assume the host from the returned
// ActivityPub representation.
id := ap.GetJSONLDId(apubAcc)
if id == nil {
return nil, nil, gtserror.New("no id property found on person, or id was not an iri")
}
// Get IRI host value.
accHost := id.Host
latestAcc.Domain, _, err = d.fingerRemoteAccount(ctx,
tsport,
latestAcc.Username,
accHost,
)
if err != nil {
// We still couldn't webfinger the account, so we're not certain
// what the accountDomain actually is. Still, we can make a solid
// guess that it's the Host of the ActivityPub URI of the account.
// If we're wrong, we can just try again in a couple days.
log.Errorf(ctx, "error webfingering[2] remote account %s@%s: %v", latestAcc.Username, accHost, err)
latestAcc.Domain = accHost
// Webfingering account still failed, so we're not certain
// what the accountDomain actually is. Exit here for safety.
return nil, nil, gtserror.Newf(
"error webfingering remote account %s@%s: %w",
latestAcc.Username, accHost, err,
)
}
}
if latestAcc.Domain == "" {
// Ensure we have a domain set by this point,
// otherwise it gets stored as a local user!
//
// TODO: there is probably a more granular way
// way of checking this in each of the above parts,
// and honestly it could do with a smol refactor.
return nil, nil, gtserror.Newf("empty domain for %s", uri)
}
// Ensure the final parsed account URI / URL matches
// the input URI we fetched (or received) it as.
if expect := uri.String(); latestAcc.URI != expect &&
latestAcc.URL != expect {
return nil, nil, gtserror.Newf(
"dereferenced account uri %s does not match %s",
latestAcc.URI, expect,
)
}
/*
BY THIS POINT we have more or less a fullly-formed
representation of the target account, derived from

23
internal/federation/dereferencing/account_test.go
View file

@ -19,6 +19,7 @@ package dereferencing_test
import (
"context"
"fmt"
"testing"
"time"
@ -207,6 +208,28 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountWithUnknownUserURI() {
suite.Nil(fetchedAccount)
}
func (suite *AccountTestSuite) TestDereferenceRemoteAccountWithNonMatchingURI() {
fetchingAccount := suite.testAccounts["local_account_1"]
const (
remoteURI = "https://turnip.farm/users/turniplover6969"
remoteAltURI = "https://turnip.farm/users/turniphater420"
)
// Create a copy of this remote account at alternative URI.
remotePerson := suite.client.TestRemotePeople[remoteURI]
suite.client.TestRemotePeople[remoteAltURI] = remotePerson
// Attempt to fetch account at alternative URI, it should fail!
fetchedAccount, _, err := suite.dereferencer.GetAccountByURI(
context.Background(),
fetchingAccount.Username,
testrig.URLMustParse(remoteAltURI),
)
suite.Equal(err.Error(), fmt.Sprintf("enrichAccount: dereferenced account uri %s does not match %s", remoteURI, remoteAltURI))
suite.Nil(fetchedAccount)
}
func TestAccountTestSuite(t *testing.T) {
suite.Run(t, new(AccountTestSuite))
}

4
internal/federation/dereferencing/dereferencer_test.go
View file

@ -35,6 +35,7 @@ type DereferencerStandardTestSuite struct {
db db.DB
storage *storage.Driver
state state.State
client *testrig.MockHTTPClient
testRemoteStatuses map[string]vocab.ActivityStreamsNote
testRemotePeople map[string]vocab.ActivityStreamsPerson
@ -72,11 +73,12 @@ func (suite *DereferencerStandardTestSuite) SetupTest() {
converter,
)
suite.client = testrig.NewMockHTTPClient(nil, "../../../testrig/media")
suite.storage = testrig.NewInMemoryStorage()
suite.state.DB = suite.db
suite.state.Storage = suite.storage
media := testrig.NewTestMediaManager(&suite.state)
suite.dereferencer = dereferencing.NewDereferencer(&suite.state, converter, testrig.NewTestTransportController(&suite.state, testrig.NewMockHTTPClient(nil, "../../../testrig/media")), media)
suite.dereferencer = dereferencing.NewDereferencer(&suite.state, converter, testrig.NewTestTransportController(&suite.state, suite.client), media)
testrig.StandardDBSetup(suite.db, nil)
}

11
internal/federation/dereferencing/finger.go
View file

@ -21,9 +21,9 @@ import (
"context"
"encoding/json"
"net/url"
"strings"
apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/log"
"github.com/superseriousbusiness/gotosocial/internal/transport"
@ -74,10 +74,12 @@ func (d *Dereferencer) fingerRemoteAccount(
return "", nil, err
}
_, accountDomain, err := util.ExtractWebfingerParts(resp.Subject)
accUsername, accDomain, err := util.ExtractWebfingerParts(resp.Subject)
if err != nil {
err = gtserror.Newf("error extracting subject parts for %s: %w", target, err)
return "", nil, err
} else if accUsername != username {
return "", nil, gtserror.Newf("response username does not match input for %s: %w", target, err)
}
// Look through links for the first
@ -92,8 +94,7 @@ func (d *Dereferencer) fingerRemoteAccount(
continue
}
if !strings.EqualFold(link.Type, "application/activity+json") &&
!strings.EqualFold(link.Type, "application/ld+json; profile=\"https://www.w3.org/ns/activitystreams\"") {
if !apiutil.ASContentType(link.Type) {
// Not an AP type, ignore.
continue
}
@ -121,7 +122,7 @@ func (d *Dereferencer) fingerRemoteAccount(
}
// All looks good, return happily!
return accountDomain, uri, nil
return accDomain, uri, nil
}
return "", nil, gtserror.Newf("no suitable self, AP-type link found in webfinger response for %s", target)

36
internal/federation/dereferencing/status.go
View file

@ -396,7 +396,7 @@ func (d *Dereferencer) enrichStatus(
}
// Ensure we have the author account of the status dereferenced (+ up-to-date). If this is a new status
// (i.e. status.AccountID == "") then any error here is irrecoverable. AccountID must ALWAYS be set.
// (i.e. status.AccountID == "") then any error here is irrecoverable. status.AccountID must ALWAYS be set.
if _, _, err := d.getAccountByURI(ctx, requestUser, attributedTo); err != nil && status.AccountID == "" {
return nil, nil, gtserror.Newf("failed to dereference status author %s: %w", uri, err)
}
@ -408,13 +408,33 @@ func (d *Dereferencer) enrichStatus(
return nil, nil, gtserror.Newf("error converting statusable to gts model for status %s: %w", uri, err)
}
// Check if we've previously
// stored this status in the DB.
// If we have, it'll be ID'd.
var isNew = (status.ID == "")
if isNew {
// No ID, we haven't stored this status before.
// Generate new status ID from the status publication time.
// Ensure final status isn't attempting
// to claim being authored by local user.
if latestStatus.Account.IsLocal() {
return nil, nil, gtserror.Newf(
"dereferenced status %s claiming to be local",
latestStatus.URI,
)
}
// Ensure the final parsed status URI / URL matches
// the input URI we fetched (or received) it as.
if expect := uri.String(); latestStatus.URI != expect &&
latestStatus.URL != expect {
return nil, nil, gtserror.Newf(
"dereferenced status uri %s does not match %s",
latestStatus.URI, expect,
)
}
var isNew bool
// Based on the original provided
// status model, determine whether
// this is a new insert / update.
if isNew = (status.ID == ""); isNew {
// Generate new status ID from the provided creation date.
latestStatus.ID, err = id.NewULIDFromTime(latestStatus.CreatedAt)
if err != nil {
log.Errorf(ctx, "invalid created at date (falling back to 'now'): %v", err)

23
internal/federation/dereferencing/status_test.go
View file

@ -19,6 +19,7 @@ package dereferencing_test
import (
"context"
"fmt"
"testing"
"github.com/stretchr/testify/suite"
@ -218,6 +219,28 @@ func (suite *StatusTestSuite) TestDereferenceStatusWithImageAndNoContent() {
suite.NoError(err)
}
func (suite *StatusTestSuite) TestDereferenceStatusWithNonMatchingURI() {
fetchingAccount := suite.testAccounts["local_account_1"]
const (
remoteURI = "https://turnip.farm/users/turniplover6969/statuses/70c53e54-3146-42d5-a630-83c8b6c7c042"
remoteAltURI = "https://turnip.farm/users/turniphater420/statuses/70c53e54-3146-42d5-a630-83c8b6c7c042"
)
// Create a copy of this remote account at alternative URI.
remoteStatus := suite.client.TestRemoteStatuses[remoteURI]
suite.client.TestRemoteStatuses[remoteAltURI] = remoteStatus
// Attempt to fetch account at alternative URI, it should fail!
fetchedStatus, _, err := suite.dereferencer.GetStatusByURI(
context.Background(),
fetchingAccount.Username,
testrig.URLMustParse(remoteAltURI),
)
suite.Equal(err.Error(), fmt.Sprintf("enrichStatus: dereferenced status uri %s does not match %s", remoteURI, remoteAltURI))
suite.Nil(fetchedStatus)
}
func TestStatusTestSuite(t *testing.T) {
suite.Run(t, new(StatusTestSuite))
}

56
internal/federation/federatingactor.go
View file

@ -23,70 +23,18 @@ import (
"fmt"
"net/http"
"net/url"
"strings"
errorsv2 "codeberg.org/gruf/go-errors/v2"
"codeberg.org/gruf/go-kv"
"github.com/superseriousbusiness/activity/pub"
"github.com/superseriousbusiness/activity/streams/vocab"
"github.com/superseriousbusiness/gotosocial/internal/ap"
apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"
"github.com/superseriousbusiness/gotosocial/internal/db"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/log"
)
// IsASMediaType will return whether the given content-type string
// matches one of the 2 possible ActivityStreams incoming content types:
// - application/activity+json
// - application/ld+json;profile=https://w3.org/ns/activitystreams
//
// Where for the above we are leniant with whitespace, quotes, and charset.
func IsASMediaType(ct string) bool {
var (
// First content-type part,
// contains the application/...
p1 string = ct //nolint:revive
// Second content-type part,
// contains AS IRI or charset
// if provided.
p2 string
)
// Split content-type by semi-colon.
sep := strings.IndexByte(ct, ';')
if sep >= 0 {
p1 = ct[:sep]
// Trim all start/end
// space of second part.
p2 = ct[sep+1:]
p2 = strings.Trim(p2, " ")
}
// Trim any ending space from the
// main content-type part of string.
p1 = strings.TrimRight(p1, " ")
switch p1 {
case "application/activity+json":
// Accept with or without charset.
// This should be case insensitive.
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type#charset
return p2 == "" || strings.EqualFold(p2, "charset=utf-8")
case "application/ld+json":
// Drop any quotes around the URI str.
p2 = strings.ReplaceAll(p2, "\"", "")
// End part must be a ref to the main AS namespace IRI.
return p2 == "profile=https://www.w3.org/ns/activitystreams"
default:
return false
}
}
// federatingActor wraps the pub.FederatingActor
// with some custom GoToSocial-specific logic.
type federatingActor struct {
@ -124,7 +72,7 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr
// Ensure valid ActivityPub Content-Type.
// https://www.w3.org/TR/activitypub/#server-to-server-interactions
if ct := r.Header.Get("Content-Type"); !IsASMediaType(ct) {
if ct := r.Header.Get("Content-Type"); !apiutil.ASContentType(ct) {
const ct1 = "application/activity+json"
const ct2 = "application/ld+json;profile=https://w3.org/ns/activitystreams"
err := fmt.Errorf("Content-Type %s not acceptable, this endpoint accepts: [%q %q]", ct, ct1, ct2)

68
internal/federation/federatingactor_test.go
View file

@ -154,71 +154,3 @@ func (suite *FederatingActorTestSuite) TestSendRemoteFollower() {
func TestFederatingActorTestSuite(t *testing.T) {
suite.Run(t, new(FederatingActorTestSuite))
}
func TestIsASMediaType(t *testing.T) {
for _, test := range []struct {
Input string
Expect bool
}{
{
Input: "application/activity+json",
Expect: true,
},
{
Input: "application/activity+json; charset=utf-8",
Expect: true,
},
{
Input: "application/activity+json;charset=utf-8",
Expect: true,
},
{
Input: "application/activity+json ;charset=utf-8",
Expect: true,
},
{
Input: "application/activity+json ; charset=utf-8",
Expect: true,
},
{
Input: "application/ld+json;profile=https://www.w3.org/ns/activitystreams",
Expect: true,
},
{
Input: "application/ld+json;profile=\"https://www.w3.org/ns/activitystreams\"",
Expect: true,
},
{
Input: "application/ld+json ;profile=https://www.w3.org/ns/activitystreams",
Expect: true,
},
{
Input: "application/ld+json ;profile=\"https://www.w3.org/ns/activitystreams\"",
Expect: true,
},
{
Input: "application/ld+json ; profile=https://www.w3.org/ns/activitystreams",
Expect: true,
},
{
Input: "application/ld+json ; profile=\"https://www.w3.org/ns/activitystreams\"",
Expect: true,
},
{
Input: "application/ld+json; profile=https://www.w3.org/ns/activitystreams",
Expect: true,
},
{
Input: "application/ld+json; profile=\"https://www.w3.org/ns/activitystreams\"",
Expect: true,
},
{
Input: "application/ld+json",
Expect: false,
},
} {
if federation.IsASMediaType(test.Input) != test.Expect {
t.Errorf("did not get expected result %v for input: %s", test.Expect, test.Input)
}
}
}