diff options
author | 2022-05-23 11:46:50 +0200 | |
---|---|---|
committer | 2022-05-23 11:46:50 +0200 | |
commit | 469da93678b3f738f65372d13dcd1ea7de390063 (patch) | |
tree | 33d6b5b9facd3cf242235fbfb5f9275051864923 /internal/federation/federatingprotocol.go | |
parent | [docs] Add Caddy instructions to the documentation (#594) (diff) | |
download | gotosocial-469da93678b3f738f65372d13dcd1ea7de390063.tar.xz |
[security] Check all involved IRIs during block checking (#593)
* tidy up context keys, add otherInvolvedIRIs
* add ReplyToable interface
* skip block check if we own the requesting domain
* add block check for other involved IRIs
* use cacheable status fetch
* remove unused ContextActivity
* remove unused ContextActivity
* add helper for unique URIs
* check through CCs and clean slice
* add GetAccountIDForStatusURI
* add GetAccountIDForAccountURI
* check blocks on involved account
* add statuses to tests
* add some blocked tests
* go fmt
* extract Tos as well as CCs
* test PostInboxRequestBodyHook
* add some more testActivities
* deduplicate involvedAccountIDs
* go fmt
* use cacheable db functions, remove new functions
Diffstat (limited to 'internal/federation/federatingprotocol.go')
-rw-r--r-- | internal/federation/federatingprotocol.go | 141 |
1 files changed, 114 insertions, 27 deletions
diff --git a/internal/federation/federatingprotocol.go b/internal/federation/federatingprotocol.go index e1ca3e7e5..a41d1ae80 100644 --- a/internal/federation/federatingprotocol.go +++ b/internal/federation/federatingprotocol.go @@ -33,6 +33,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/uris" + "github.com/superseriousbusiness/gotosocial/internal/util" ) /* @@ -62,19 +63,60 @@ import ( // 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) { - l := logrus.WithFields(logrus.Fields{ - "func": "PostInboxRequestBodyHook", - "useragent": r.UserAgent(), - "url": r.URL.String(), - }) + // extract any other IRIs involved in this activity + otherInvolvedIRIs := []*url.URL{} + + // check if the Activity itself has an 'inReplyTo' + if replyToable, ok := activity.(ap.ReplyToable); ok { + if inReplyToURI := ap.ExtractInReplyToURI(replyToable); inReplyToURI != nil { + otherInvolvedIRIs = append(otherInvolvedIRIs, inReplyToURI) + } + } - if activity == nil { - err := errors.New("nil activity in PostInboxRequestBodyHook") - l.Debug(err) - return nil, err + // 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 + 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...) + } } - // set the activity on the context for use later on - return context.WithValue(ctx, ap.ContextActivity, activity), nil + + // 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...) + } + if toURIs, err := ap.ExtractTos(addressable); err == nil { + otherInvolvedIRIs = append(otherInvolvedIRIs, toURIs...) + } + } + } + + // 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 + cleaned := []*url.URL{} + for _, u := range deduped { + if !pub.IsPublic(u.String()) { + cleaned = append(cleaned, u) + } + } + + withOtherInvolvedIRIs := context.WithValue(ctx, ap.ContextOtherInvolvedIRIs, cleaned) + return withOtherInvolvedIRIs, nil } // AuthenticatePostInbox delegates the authentication of a POST to an @@ -185,40 +227,85 @@ func (f *federator) Blocked(ctx context.Context, actorIRIs []*url.URL) (bool, er }) l.Debugf("entering BLOCKED function with IRI list: %+v", actorIRIs) + // check domain blocks first for the given actor IRIs + blocked, err := f.db.AreURIsBlocked(ctx, actorIRIs) + if err != nil { + return false, fmt.Errorf("error checking domain blocks of actorIRIs: %s", err) + } + if blocked { + return blocked, nil + } + + // check domain blocks for any other involved IRIs + otherInvolvedIRIsI := ctx.Value(ap.ContextOtherInvolvedIRIs) + otherInvolvedIRIs, ok := otherInvolvedIRIsI.([]*url.URL) + if !ok { + l.Errorf("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 { l.Errorf("receiving account not set on request context") return false, errors.New("receiving account not set on request context, so couldn't determine blocks") } - - blocked, err := f.db.AreURIsBlocked(ctx, actorIRIs) + requestingAccountI := ctx.Value(ap.ContextRequestingAccount) + requestingAccount, ok := requestingAccountI.(*gtsmodel.Account) + if !ok { + l.Errorf("requesting account not set on request context") + return false, errors.New("requesting account not set on request context, so couldn't determine blocks") + } + // the receiver shouldn't block the sender + blocked, err = f.db.IsBlocked(ctx, receivingAccount.ID, requestingAccount.ID, false) if err != nil { - return false, fmt.Errorf("error checking domain blocks: %s", err) + return false, fmt.Errorf("error checking user-level blocks: %s", err) } if blocked { return blocked, nil } - for _, uri := range actorIRIs { - requestingAccount, err := f.db.GetAccountByURI(ctx, uri.String()) + // 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 + } + + if involvedAccountID != "" { + involvedAccountIDs = append(involvedAccountIDs, involvedAccountID) + } + } + 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, false) if err != nil { - if err == db.ErrNoEntries { - // we don't have an entry for this account so it's not blocked - // TODO: allow a different default to be set for this behavior - l.Tracef("no entry for account with URI %s so it can't be blocked", uri) - continue - } - return false, fmt.Errorf("error getting account with uri %s: %s", uri.String(), err) + return false, fmt.Errorf("error checking user-level otherInvolvedIRI blocks: %s", err) + } + if blocked { + return blocked, nil } - blocked, err = f.db.IsBlocked(ctx, receivingAccount.ID, requestingAccount.ID, false) + // whoever is receiving this request shouldn't block the involved account + blocked, err = f.db.IsBlocked(ctx, receivingAccount.ID, involvedAccountID, false) if err != nil { - return false, fmt.Errorf("error checking account block: %s", err) + return false, fmt.Errorf("error checking user-level otherInvolvedIRI blocks: %s", err) } if blocked { - l.Tracef("local account %s blocks account with uri %s", receivingAccount.Username, uri) - return true, nil + return blocked, nil } } |