From 92186c8c6f1c374146f085b46a440b69a1d97aa8 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Thu, 16 Sep 2021 11:35:09 +0200 Subject: [PATCH] federated authentication better logging + tidying (#232) * change trace logging in authenticator * messing about * lil changes * go fmt * error fix * Fix broken test --- internal/api/s2s/user/common.go | 59 +++++++++++++++++++++++++ internal/api/s2s/user/followers.go | 24 ++++------ internal/api/s2s/user/following.go | 24 ++++------ internal/api/s2s/user/inboxpost.go | 9 +--- internal/api/s2s/user/publickeyget.go | 24 ++++------ internal/api/s2s/user/repliesget.go | 24 ++++------ internal/api/s2s/user/statusget.go | 24 ++++------ internal/api/s2s/user/user.go | 7 --- internal/api/s2s/user/userget.go | 24 ++++------ internal/api/security/signaturecheck.go | 11 ++--- internal/federation/authenticate.go | 22 ++++++--- internal/federation/federator_test.go | 3 +- internal/util/uri.go | 2 + 13 files changed, 134 insertions(+), 123 deletions(-) create mode 100644 internal/api/s2s/user/common.go diff --git a/internal/api/s2s/user/common.go b/internal/api/s2s/user/common.go new file mode 100644 index 000000000..8a3846901 --- /dev/null +++ b/internal/api/s2s/user/common.go @@ -0,0 +1,59 @@ +/* + GoToSocial + Copyright (C) 2021 GoToSocial Authors admin@gotosocial.org + + 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 user + +import ( + "context" + "fmt" + + "github.com/gin-gonic/gin" + "github.com/superseriousbusiness/gotosocial/internal/util" +) + +// ActivityPubAcceptHeaders represents the Accept headers mentioned here: +// https://www.w3.org/TR/activitypub/#retrieving-objects +var ActivityPubAcceptHeaders = []string{ + `application/activity+json`, + `application/ld+json; profile="https://www.w3.org/ns/activitystreams"`, +} + +// populateContext transfers the signature verifier and signature from the gin context to the request context +func populateContext(c *gin.Context) context.Context { + ctx := c.Request.Context() + + verifier, signed := c.Get(string(util.APRequestingPublicKeyVerifier)) + if signed { + ctx = context.WithValue(ctx, util.APRequestingPublicKeyVerifier, verifier) + } + + signature, signed := c.Get(string(util.APRequestingPublicKeySignature)) + if signed { + ctx = context.WithValue(ctx, util.APRequestingPublicKeySignature, signature) + } + + return ctx +} + +func negotiateFormat(c *gin.Context) (string, error) { + format := c.NegotiateFormat(ActivityPubAcceptHeaders...) + if format == "" { + return "", fmt.Errorf("no format can be offered for Accept headers %s", c.Request.Header.Get("Accept")) + } + return format, nil +} diff --git a/internal/api/s2s/user/followers.go b/internal/api/s2s/user/followers.go index f9aec7ffb..c85c24c95 100644 --- a/internal/api/s2s/user/followers.go +++ b/internal/api/s2s/user/followers.go @@ -19,14 +19,12 @@ package user import ( - "context" "encoding/json" "fmt" "net/http" "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" - "github.com/superseriousbusiness/gotosocial/internal/util" ) // FollowersGETHandler returns a collection of URIs for followers of the target user, formatted so that other AP servers can understand it. @@ -42,25 +40,19 @@ func (m *Module) FollowersGETHandler(c *gin.Context) { return } - // make sure this actually an AP request - format := c.NegotiateFormat(ActivityPubAcceptHeaders...) - if format == "" { - c.JSON(http.StatusNotAcceptable, gin.H{"error": "could not negotiate format with given Accept header(s)"}) + format, err := negotiateFormat(c) + if err != nil { + c.JSON(http.StatusNotAcceptable, gin.H{"error": fmt.Sprintf("could not negotiate format with given Accept header(s): %s", err)}) return } l.Tracef("negotiated format: %s", format) - // transfer the signature verifier from the gin context to the request context - ctx := c.Request.Context() - verifier, signed := c.Get(string(util.APRequestingPublicKeyVerifier)) - if signed { - ctx = context.WithValue(ctx, util.APRequestingPublicKeyVerifier, verifier) - } + ctx := populateContext(c) - followers, err := m.processor.GetFediFollowers(ctx, requestedUsername, c.Request.URL) // GetFediUser handles auth as well - if err != nil { - l.Info(err.Error()) - c.JSON(err.Code(), gin.H{"error": err.Safe()}) + followers, errWithCode := m.processor.GetFediFollowers(ctx, requestedUsername, c.Request.URL) + if errWithCode != nil { + l.Info(errWithCode.Error()) + c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()}) return } diff --git a/internal/api/s2s/user/following.go b/internal/api/s2s/user/following.go index 4c010dbe0..7c5b1bbf7 100644 --- a/internal/api/s2s/user/following.go +++ b/internal/api/s2s/user/following.go @@ -19,14 +19,12 @@ package user import ( - "context" "encoding/json" "fmt" "net/http" "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" - "github.com/superseriousbusiness/gotosocial/internal/util" ) // FollowingGETHandler returns a collection of URIs for accounts that the target user follows, formatted so that other AP servers can understand it. @@ -42,25 +40,19 @@ func (m *Module) FollowingGETHandler(c *gin.Context) { return } - // make sure this actually an AP request - format := c.NegotiateFormat(ActivityPubAcceptHeaders...) - if format == "" { - c.JSON(http.StatusNotAcceptable, gin.H{"error": "could not negotiate format with given Accept header(s)"}) + format, err := negotiateFormat(c) + if err != nil { + c.JSON(http.StatusNotAcceptable, gin.H{"error": fmt.Sprintf("could not negotiate format with given Accept header(s): %s", err)}) return } l.Tracef("negotiated format: %s", format) - // transfer the signature verifier from the gin context to the request context - ctx := c.Request.Context() - verifier, signed := c.Get(string(util.APRequestingPublicKeyVerifier)) - if signed { - ctx = context.WithValue(ctx, util.APRequestingPublicKeyVerifier, verifier) - } + ctx := populateContext(c) - following, err := m.processor.GetFediFollowing(ctx, requestedUsername, c.Request.URL) // handles auth as well - if err != nil { - l.Info(err.Error()) - c.JSON(err.Code(), gin.H{"error": err.Safe()}) + following, errWithCode := m.processor.GetFediFollowing(ctx, requestedUsername, c.Request.URL) + if errWithCode != nil { + l.Info(errWithCode.Error()) + c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()}) return } diff --git a/internal/api/s2s/user/inboxpost.go b/internal/api/s2s/user/inboxpost.go index 98442af13..8e44454d7 100644 --- a/internal/api/s2s/user/inboxpost.go +++ b/internal/api/s2s/user/inboxpost.go @@ -19,13 +19,11 @@ package user import ( - "context" "net/http" "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" "github.com/superseriousbusiness/gotosocial/internal/gtserror" - "github.com/superseriousbusiness/gotosocial/internal/util" ) // InboxPOSTHandler deals with incoming POST requests to an actor's inbox. @@ -42,12 +40,7 @@ func (m *Module) InboxPOSTHandler(c *gin.Context) { return } - // transfer the signature verifier from the gin context to the request context - ctx := c.Request.Context() - verifier, signed := c.Get(string(util.APRequestingPublicKeyVerifier)) - if signed { - ctx = context.WithValue(ctx, util.APRequestingPublicKeyVerifier, verifier) - } + ctx := populateContext(c) posted, err := m.processor.InboxPost(ctx, c.Writer, c.Request) if err != nil { diff --git a/internal/api/s2s/user/publickeyget.go b/internal/api/s2s/user/publickeyget.go index 2cbe564df..f5fe79441 100644 --- a/internal/api/s2s/user/publickeyget.go +++ b/internal/api/s2s/user/publickeyget.go @@ -1,14 +1,12 @@ package user import ( - "context" "encoding/json" "fmt" "net/http" "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" - "github.com/superseriousbusiness/gotosocial/internal/util" ) // PublicKeyGETHandler should be served at eg https://example.org/users/:username/main-key. @@ -28,25 +26,19 @@ func (m *Module) PublicKeyGETHandler(c *gin.Context) { return } - // make sure this actually an AP request - format := c.NegotiateFormat(ActivityPubAcceptHeaders...) - if format == "" { - c.JSON(http.StatusNotAcceptable, gin.H{"error": "could not negotiate format with given Accept header(s)"}) + format, err := negotiateFormat(c) + if err != nil { + c.JSON(http.StatusNotAcceptable, gin.H{"error": fmt.Sprintf("could not negotiate format with given Accept header(s): %s", err)}) return } l.Tracef("negotiated format: %s", format) - // transfer the signature verifier from the gin context to the request context - ctx := c.Request.Context() - verifier, signed := c.Get(string(util.APRequestingPublicKeyVerifier)) - if signed { - ctx = context.WithValue(ctx, util.APRequestingPublicKeyVerifier, verifier) - } + ctx := populateContext(c) - user, err := m.processor.GetFediUser(ctx, requestedUsername, c.Request.URL) // GetFediUser handles auth as well - if err != nil { - l.Info(err.Error()) - c.JSON(err.Code(), gin.H{"error": err.Safe()}) + user, errWithCode := m.processor.GetFediUser(ctx, requestedUsername, c.Request.URL) + if errWithCode != nil { + l.Info(errWithCode.Error()) + c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()}) return } diff --git a/internal/api/s2s/user/repliesget.go b/internal/api/s2s/user/repliesget.go index 951cc428c..9031b7841 100644 --- a/internal/api/s2s/user/repliesget.go +++ b/internal/api/s2s/user/repliesget.go @@ -1,7 +1,6 @@ package user import ( - "context" "encoding/json" "fmt" "net/http" @@ -9,7 +8,6 @@ "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" - "github.com/superseriousbusiness/gotosocial/internal/util" ) // StatusRepliesGETHandler swagger:operation GET /users/{username}/statuses/{status}/replies s2sRepliesGet @@ -116,25 +114,19 @@ func (m *Module) StatusRepliesGETHandler(c *gin.Context) { minID = minIDString } - // make sure this actually an AP request - format := c.NegotiateFormat(ActivityPubAcceptHeaders...) - if format == "" { - c.JSON(http.StatusNotAcceptable, gin.H{"error": "could not negotiate format with given Accept header(s)"}) + format, err := negotiateFormat(c) + if err != nil { + c.JSON(http.StatusNotAcceptable, gin.H{"error": fmt.Sprintf("could not negotiate format with given Accept header(s): %s", err)}) return } l.Tracef("negotiated format: %s", format) - // transfer the signature verifier from the gin context to the request context - ctx := c.Request.Context() - verifier, signed := c.Get(string(util.APRequestingPublicKeyVerifier)) - if signed { - ctx = context.WithValue(ctx, util.APRequestingPublicKeyVerifier, verifier) - } + ctx := populateContext(c) - replies, err := m.processor.GetFediStatusReplies(ctx, requestedUsername, requestedStatusID, page, onlyOtherAccounts, minID, c.Request.URL) - if err != nil { - l.Info(err.Error()) - c.JSON(err.Code(), gin.H{"error": err.Safe()}) + replies, errWithCode := m.processor.GetFediStatusReplies(ctx, requestedUsername, requestedStatusID, page, onlyOtherAccounts, minID, c.Request.URL) + if errWithCode != nil { + l.Info(errWithCode.Error()) + c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()}) return } diff --git a/internal/api/s2s/user/statusget.go b/internal/api/s2s/user/statusget.go index 04241d075..20b9064eb 100644 --- a/internal/api/s2s/user/statusget.go +++ b/internal/api/s2s/user/statusget.go @@ -1,14 +1,12 @@ package user import ( - "context" "encoding/json" "fmt" "net/http" "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" - "github.com/superseriousbusiness/gotosocial/internal/util" ) // StatusGETHandler serves the target status as an activitystreams NOTE so that other AP servers can parse it. @@ -30,25 +28,19 @@ func (m *Module) StatusGETHandler(c *gin.Context) { return } - // make sure this actually an AP request - format := c.NegotiateFormat(ActivityPubAcceptHeaders...) - if format == "" { - c.JSON(http.StatusNotAcceptable, gin.H{"error": "could not negotiate format with given Accept header(s)"}) + format, err := negotiateFormat(c) + if err != nil { + c.JSON(http.StatusNotAcceptable, gin.H{"error": fmt.Sprintf("could not negotiate format with given Accept header(s): %s", err)}) return } l.Tracef("negotiated format: %s", format) - // transfer the signature verifier from the gin context to the request context - ctx := c.Request.Context() - verifier, signed := c.Get(string(util.APRequestingPublicKeyVerifier)) - if signed { - ctx = context.WithValue(ctx, util.APRequestingPublicKeyVerifier, verifier) - } + ctx := populateContext(c) - status, err := m.processor.GetFediStatus(ctx, requestedUsername, requestedStatusID, c.Request.URL) // handles auth as well - if err != nil { - l.Info(err.Error()) - c.JSON(err.Code(), gin.H{"error": err.Safe()}) + status, errWithCode := m.processor.GetFediStatus(ctx, requestedUsername, requestedStatusID, c.Request.URL) + if errWithCode != nil { + l.Info(errWithCode.Error()) + c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()}) return } diff --git a/internal/api/s2s/user/user.go b/internal/api/s2s/user/user.go index b5ff9a699..cd57ab14e 100644 --- a/internal/api/s2s/user/user.go +++ b/internal/api/s2s/user/user.go @@ -61,13 +61,6 @@ UsersStatusRepliesPath = UsersStatusPath + "/replies" ) -// ActivityPubAcceptHeaders represents the Accept headers mentioned here: -// https://www.w3.org/TR/activitypub/#retrieving-objects -var ActivityPubAcceptHeaders = []string{ - `application/activity+json`, - `application/ld+json; profile="https://www.w3.org/ns/activitystreams"`, -} - // Module implements the FederationAPIModule interface type Module struct { config *config.Config diff --git a/internal/api/s2s/user/userget.go b/internal/api/s2s/user/userget.go index f2f846f97..abd52da22 100644 --- a/internal/api/s2s/user/userget.go +++ b/internal/api/s2s/user/userget.go @@ -19,14 +19,12 @@ package user import ( - "context" "encoding/json" "fmt" "net/http" "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" - "github.com/superseriousbusiness/gotosocial/internal/util" ) // UsersGETHandler should be served at https://example.org/users/:username. @@ -50,25 +48,19 @@ func (m *Module) UsersGETHandler(c *gin.Context) { return } - // make sure this actually an AP request - format := c.NegotiateFormat(ActivityPubAcceptHeaders...) - if format == "" { - c.JSON(http.StatusNotAcceptable, gin.H{"error": "could not negotiate format with given Accept header(s)"}) + format, err := negotiateFormat(c) + if err != nil { + c.JSON(http.StatusNotAcceptable, gin.H{"error": fmt.Sprintf("could not negotiate format with given Accept header(s): %s", err)}) return } l.Tracef("negotiated format: %s", format) - // transfer the signature verifier from the gin context to the request context - ctx := c.Request.Context() - verifier, signed := c.Get(string(util.APRequestingPublicKeyVerifier)) - if signed { - ctx = context.WithValue(ctx, util.APRequestingPublicKeyVerifier, verifier) - } + ctx := populateContext(c) - user, err := m.processor.GetFediUser(ctx, requestedUsername, c.Request.URL) // GetFediUser handles auth as well - if err != nil { - l.Info(err.Error()) - c.JSON(err.Code(), gin.H{"error": err.Safe()}) + user, errWithCode := m.processor.GetFediUser(ctx, requestedUsername, c.Request.URL) // GetFediUser handles auth as well + if errWithCode != nil { + l.Info(errWithCode.Error()) + c.JSON(errWithCode.Code(), gin.H{"error": errWithCode.Safe()}) return } diff --git a/internal/api/security/signaturecheck.go b/internal/api/security/signaturecheck.go index 71e539e96..be8c87c0d 100644 --- a/internal/api/security/signaturecheck.go +++ b/internal/api/security/signaturecheck.go @@ -11,13 +11,10 @@ // SignatureCheck checks whether an incoming http request has been signed. If so, it will check if the domain // that signed the request is permitted to access the server. If it is permitted, the handler will set the key -// verifier in the gin context for use down the line. +// verifier and the signature in the gin context for use down the line. func (m *Module) SignatureCheck(c *gin.Context) { l := m.log.WithField("func", "DomainBlockChecker") - // set this extra field for signature validation - c.Request.Header.Set("host", m.config.Host) - // create the verifier from the request // if the request is signed, it will have a signature header verifier, err := httpsig.NewVerifier(c.Request) @@ -43,8 +40,12 @@ func (m *Module) SignatureCheck(c *gin.Context) { return } - // set the verifier on the context here to save some work further down the line + // set the verifier and signature on the context here to save some work further down the line c.Set(string(util.APRequestingPublicKeyVerifier), verifier) + signature := c.GetHeader("Signature") + if signature != "" { + c.Set(string(util.APRequestingPublicKeySignature), signature) + } } } } diff --git a/internal/federation/authenticate.go b/internal/federation/authenticate.go index 81ac84544..1e359aa28 100644 --- a/internal/federation/authenticate.go +++ b/internal/federation/authenticate.go @@ -102,10 +102,6 @@ func getPublicKeyFromResponse(c context.Context, b []byte, keyID *url.URL) (voca // 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. // -// To avoid making unnecessary http calls towards blocked domains, this function *does* bail early if an instance-level domain block exists -// for the request from the incoming domain. However, it does not check whether individual blocks exist between the requesting user or domain -// and the requested user: this should be done elsewhere. -// // 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'. @@ -135,6 +131,19 @@ func (f *federator) AuthenticateFederatedRequest(ctx context.Context, requestedU return nil, false, nil // couldn't extract the verifier } + // we should have the signature itself set too + si := ctx.Value(util.APRequestingPublicKeySignature) + if vi == nil { + l.Debug("request wasn't signed") + return nil, false, nil // request wasn't signed + } + + signature, ok := si.(string) + if !ok { + l.Debug("couldn't extract signature") + return nil, false, nil // couldn't extract the signature + } + requestingPublicKeyID, err := url.Parse(verifier.KeyId()) if err != nil { l.Debug("couldn't parse public key URL") @@ -227,13 +236,14 @@ func (f *federator) AuthenticateFederatedRequest(ctx context.Context, requestedU for _, algo := range algos { l.Tracef("trying algo: %s", algo) - if err := verifier.Verify(publicKey, algo); err == nil { + err := verifier.Verify(publicKey, algo) + if err == nil { l.Tracef("authentication for %s PASSED with algorithm %s", pkOwnerURI, algo) return pkOwnerURI, true, nil } l.Tracef("authentication for %s NOT PASSED with algorithm %s: %s", pkOwnerURI, algo, err) } - l.Infof("authentication not passed for %s", pkOwnerURI) + l.Infof("authentication not passed for public key owner %s; signature value was '%s'", pkOwnerURI, signature) return nil, false, nil } diff --git a/internal/federation/federator_test.go b/internal/federation/federator_test.go index 7a76089d7..5a7056e6b 100644 --- a/internal/federation/federator_test.go +++ b/internal/federation/federator_test.go @@ -128,12 +128,13 @@ func (suite *ProtocolTestSuite) TestAuthenticatePostInbox() { ctxWithAccount := context.WithValue(ctx, util.APAccount, inboxAccount) ctxWithActivity := context.WithValue(ctxWithAccount, util.APActivity, activity) ctxWithVerifier := context.WithValue(ctxWithActivity, util.APRequestingPublicKeyVerifier, verifier) + ctxWithSignature := context.WithValue(ctxWithVerifier, util.APRequestingPublicKeySignature, 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(ctxWithVerifier, recorder, request) + newContext, authed, err := federator.AuthenticatePostInbox(ctxWithSignature, recorder, request) assert.NoError(suite.T(), err) assert.True(suite.T(), authed) diff --git a/internal/util/uri.go b/internal/util/uri.go index 91f523a4d..734becf13 100644 --- a/internal/util/uri.go +++ b/internal/util/uri.go @@ -72,6 +72,8 @@ APRequestingActorIRI APContextKey = "requestingActorIRI" // APRequestingPublicKeyVerifier can be used to set and retrieve the public key verifier of an incoming federation request. APRequestingPublicKeyVerifier APContextKey = "requestingPublicKeyVerifier" + // APRequestingPublicKeySignature can be used to set and retrieve the value of the signature header of an incoming federation request. + APRequestingPublicKeySignature APContextKey = "requestingPublicKeySignature" // APFromFederatorChanKey can be used to pass a pointer to the fromFederator channel into the federator for use in callbacks. APFromFederatorChanKey APContextKey = "fromFederatorChan" )