SuperSeriousBusiness
/
GoToSocial
mirror of https://github.com/superseriousbusiness/gotosocial
1
1
Fork 0
Code Issues Projects Releases Wiki Activity

[chore/security] refactor AuthenticateFederatedRequest() to handle account deref + suspension checks (#2371) 

* refactor AuthenticateFederatedRequest() to handle account suspension + fetching of owner

* small fixups

* small changes

* revert to 'IsEitherBlocked' instead of just 'IsBlocked" :grimace:

* update code comment to indicate that AuthenticateFederatedRequest() will handle account + instance dereferencing
This commit is contained in:
kim 2023-11-21 10:35:30 +00:00 committed by GitHub
parent 1ba3e14b36
commit 42d8011ff4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 205 additions and 198 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

224
internal/federation/authenticate.go
View File

@ -40,12 +40,7 @@ import (
)
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.
}
errUnsigned = errors.New("http request wasn't signed or http signature was invalid")
)
// PubKeyAuth models authorization information for a remote
@ -67,21 +62,18 @@ type PubKeyAuth struct {
// OwnerURI is the ActivityPub id of the owner of
// the public key used to sign the request we're
// now authenticating. This will always be set
// even if Owner isn't, so that callers can use
// this URI to go fetch the Owner from remote.
// now authenticating. This will always be set.
OwnerURI *url.URL
// Owner is the account corresponding to OwnerURI.
//
// Owner will only be defined if the account who
// owns the public key was already cached in the
// database when we received the request we're now
// authenticating (ie., we've seen it before).
//
// If it's not defined, callers should use OwnerURI
// to go and dereference it.
// Owner is the account corresponding to
// OwnerURI. This will always be set UNLESS
// the PubKeyAuth.Handshaking field is set..
Owner *gtsmodel.Account
// Handshaking indicates that uncached owner
// account was NOT dereferenced due to an ongoing
// handshake with another instance.
Handshaking bool
}
// AuthenticateFederatedRequest authenticates any kind of incoming federated
@ -103,12 +95,12 @@ type PubKeyAuth struct {
// know that this is the user making the dereferencing request, and they can decide
// to allow or deny the request depending on their settings.
//
// This function will handle dereferencing and storage of any new remote accounts
// and / or instances. The returned PubKeyAuth{}.Owner account will ONLY ever be
// nil in the case that there is an ongoing handshake involving this account.
//
// Note that it is also valid to pass in an empty string here, in which case the
// keys of the instance account will be used.
//
// Also note that this function *does not* dereference the remote account that
// the signature key is associated with. Other functions should use the returned
// URL to dereference the remote account, if required.
func (f *Federator) AuthenticateFederatedRequest(ctx context.Context, requestedUsername string) (*PubKeyAuth, gtserror.WithCode) {
// Thanks to the signature check middleware,
// we should already have an http signature
@ -142,7 +134,7 @@ func (f *Federator) AuthenticateFederatedRequest(ctx context.Context, requestedU
var (
pubKeyIDStr = pubKeyID.String()
local = (pubKeyID.Host == config.GetHost())
isLocal = (pubKeyID.Host == config.GetHost())
pubKeyAuth *PubKeyAuth
errWithCode gtserror.WithCode
)
@ -154,7 +146,7 @@ func (f *Federator) AuthenticateFederatedRequest(ctx context.Context, requestedU
{"pubKeyID", pubKeyIDStr},
}...)
if local {
if isLocal {
l.Trace("public key is local, no dereference needed")
pubKeyAuth, errWithCode = f.derefPubKeyDBOnly(ctx, pubKeyIDStr)
} else {
@ -166,7 +158,7 @@ func (f *Federator) AuthenticateFederatedRequest(ctx context.Context, requestedU
return nil, errWithCode
}
if local && pubKeyAuth == nil {
if isLocal && pubKeyAuth == nil {
// We signed this request, apparently, but
// local lookup didn't find anything. This
// is an almost impossible error condition!
@ -175,37 +167,61 @@ func (f *Federator) AuthenticateFederatedRequest(ctx context.Context, requestedU
return nil, gtserror.NewErrorInternalError(err)
}
// Try to authenticate using permitted algorithms in
// order of most -> least common, checking each defined
// pubKey for this Actor. Return OK as soon as one passes.
for _, pubKey := range [2]*rsa.PublicKey{
pubKeyAuth.FetchedPubKey,
pubKeyAuth.CachedPubKey,
} {
if pubKey == nil {
continue
// Attempt to verify auth with both fetched and cached keys.
if !verifyAuth(&l, verifier, pubKeyAuth.CachedPubKey) &&
!verifyAuth(&l, verifier, pubKeyAuth.FetchedPubKey) {
const format = "authentication NOT PASSED for public key %s; tried algorithms %+v; signature value was '%s'"
text := fmt.Sprintf(format, pubKeyIDStr, signingAlgorithms, signature)
return nil, gtserror.NewErrorUnauthorized(errors.New(text), text)
}
if pubKeyAuth.Owner == nil {
// Ensure we have instance stored in
// database for the account at URI.
err := f.fetchAccountInstance(ctx,
requestedUsername,
pubKeyAuth.OwnerURI,
)
if err != nil {
return nil, gtserror.NewErrorInternalError(err)
}
for _, algo := range signingAlgorithms {
l.Tracef("trying %s", algo)
// If we're currently handshaking with another instance, return
// without derefing the owner, the only possible time we do this.
// This prevents deadlocks when GTS instances mutually deref.
if f.Handshaking(requestedUsername, pubKeyAuth.OwnerURI) {
log.Warnf(ctx, "network race during %s handshake", pubKeyAuth.OwnerURI)
pubKeyAuth.Handshaking = true
return pubKeyAuth, nil
}
err := verifier.Verify(pubKey, algo)
if err == nil {
l.Tracef("authentication PASSED with %s", algo)
return pubKeyAuth, nil
// Dereference the account located at owner URI.
pubKeyAuth.Owner, _, err = f.GetAccountByURI(ctx,
requestedUsername,
pubKeyAuth.OwnerURI,
)
if err != nil {
if gtserror.StatusCode(err) == http.StatusGone {
// This can happen here instead of the pubkey 'gone'
// checks due to: the server sending account deletion
// notifications out, we start processing, the request above
// succeeds, and *then* the profile is removed and starts
// returning 410 Gone, at which point _this_ request fails.
return nil, gtserror.NewErrorGone(err)
}
l.Tracef("authentication NOT PASSED with %s: %q", algo, err)
err := gtserror.Newf("error dereferencing account %s: %w", pubKeyAuth.OwnerURI, err)
return nil, gtserror.NewErrorInternalError(err)
}
}
// At this point no algorithms passed.
err := gtserror.Newf(
"authentication NOT PASSED for public key %s; tried algorithms %+v; signature value was '%s'",
pubKeyIDStr, signature, signingAlgorithms,
)
if !pubKeyAuth.Owner.SuspendedAt.IsZero() {
const text = "requesting account suspended"
return nil, gtserror.NewErrorForbidden(errors.New(text))
}
return nil, gtserror.NewErrorUnauthorized(err, err.Error())
return pubKeyAuth, nil
}
// derefPubKeyDBOnly tries to dereference the given
@ -218,22 +234,27 @@ func (f *Federator) AuthenticateFederatedRequest(ctx context.Context, requestedU
func (f *Federator) derefPubKeyDBOnly(
ctx context.Context,
pubKeyIDStr string,
) (*PubKeyAuth, gtserror.WithCode) {
) (
*PubKeyAuth,
gtserror.WithCode,
) {
// Look for pubkey ID owner in the database.
owner, err := f.db.GetAccountByPubkeyID(ctx, pubKeyIDStr)
if err != nil {
if errors.Is(err, db.ErrNoEntries) {
// We don't have this
// account stored (yet).
return nil, nil
}
if err != nil && !errors.Is(err, db.ErrNoEntries) {
err = gtserror.Newf("db error getting account with pubKeyID %s: %w", pubKeyIDStr, err)
return nil, gtserror.NewErrorInternalError(err)
}
if owner == nil {
// We don't have this
// account stored (yet).
return nil, nil
}
// Parse owner account URI as URL obj.
ownerURI, err := url.Parse(owner.URI)
if err != nil {
err = gtserror.Newf("error parsing account uri with pubKeyID %s: %w", pubKeyIDStr, err)
err := gtserror.Newf("error parsing account uri with pubKeyID %s: %w", pubKeyIDStr, err)
return nil, gtserror.NewErrorInternalError(err)
}
@ -253,7 +274,10 @@ func (f *Federator) derefPubKey(
requestedUsername string,
pubKeyIDStr string,
pubKeyID *url.URL,
) (*PubKeyAuth, gtserror.WithCode) {
) (
*PubKeyAuth,
gtserror.WithCode,
) {
l := log.
WithContext(ctx).
WithFields(kv.Fields{
@ -316,7 +340,7 @@ func (f *Federator) derefPubKey(
// 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)
err := gtserror.Newf("error parsing public key (%s): %w", pubKeyID, err)
return nil, gtserror.NewErrorUnauthorized(err)
}
@ -337,16 +361,12 @@ func (f *Federator) derefPubKey(
// had an owner stored for it locally. Since
// we now successfully refreshed the pub key,
// we should update the account to reflect that.
ownerAcct := pubKeyAuth.Owner
ownerAcct.PublicKey = pubKeyAuth.FetchedPubKey
ownerAcct.PublicKeyExpiresAt = time.Time{}
l.Info("obtained a new public key to replace expired key, caching now; " +
"authorization for this request will be attempted with both old and new keys")
owner := pubKeyAuth.Owner
owner.PublicKey = pubKeyAuth.FetchedPubKey
owner.PublicKeyExpiresAt = time.Time{}
if err := f.db.UpdateAccount(
ctx,
ownerAcct,
owner,
"public_key",
"public_key_expires_at",
); err != nil {
@ -354,6 +374,8 @@ func (f *Federator) derefPubKey(
return nil, gtserror.NewErrorInternalError(err)
}
l.Info("obtained new public key to replace expired; attempting auth with old / new")
// Return both new and cached (now
// expired) keys, authentication
// will be attempted with both.
@ -406,6 +428,47 @@ func (f *Federator) callForPubKey(
return nil, gtserror.NewErrorInternalError(err)
}
// fetchAccountInstance ensures that an instance model exists in
// the database for the given account URI, deref'ing if necessary.
func (f *Federator) fetchAccountInstance(
ctx context.Context,
requestedUsername string,
accountURI *url.URL,
) error {
// Look for an existing entry for instance in database.
instance, err := f.db.GetInstance(ctx, accountURI.Host)
if err != nil && !errors.Is(err, db.ErrNoEntries) {
return gtserror.Newf("error getting instance from database: %w", err)
}
if instance != nil {
// already fetched.
return nil
}
// We don't have an entry for this
// instance yet; go dereference it.
instance, err = f.GetRemoteInstance(
gtscontext.SetFastFail(ctx),
requestedUsername,
&url.URL{
Scheme: accountURI.Scheme,
Host: accountURI.Host,
},
)
if err != nil {
return gtserror.Newf("error dereferencing instance %s: %w", accountURI.Host, err)
}
// Insert new instance into the datbase.
err = f.db.PutInstance(ctx, instance)
if err != nil && !errors.Is(err, db.ErrAlreadyExists) {
return gtserror.Newf("error inserting instance %s into database: %w", accountURI.Host, err)
}
return nil
}
// 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
@ -439,3 +502,32 @@ func parsePubKeyBytes(
return pubKey, pubKeyOwnerID, nil
}
var 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.
}
// verifyAuth verifies auth using generated verifier, according to pubkey and our supported signing algorithms.
func verifyAuth(l *log.Entry, verifier httpsig.Verifier, pubKey *rsa.PublicKey) bool {
if pubKey == nil {
return false
}
// Loop through all supported algorithms.
for _, algo := range signingAlgorithms {
// Verify according to pubkey and algo.
err := verifier.Verify(pubKey, algo)
if err != nil {
l.Tracef("authentication NOT PASSED with %s: %v", algo, err)
continue
}
l.Tracef("authenticated PASSED with %s", algo)
return true
}
return false
}

5
internal/federation/dereferencing/handshake.go
View File

@ -38,8 +38,11 @@ func (d *Dereferencer) Handshaking(username string, remoteAccountID *url.URL) bo
return false
}
// Calculate remote account ID str once.
remoteIDStr := remoteAccountID.String()
for _, id := range remoteIDs {
if id.String() == remoteAccountID.String() {
if id.String() == remoteIDStr {
// We are currently handshaking
// with the remote account.
return true

67
internal/federation/federatingprotocol.go
View File

@ -232,72 +232,17 @@ func (f *Federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr
}
}
pubKeyOwnerURI := pubKeyAuth.OwnerURI
// 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, pubKeyOwnerURI.Host); err != nil {
if !errors.Is(err, db.ErrNoEntries) {
// There's been an actual error.
err = gtserror.Newf("error getting instance %s: %w", pubKeyOwnerURI.Host, err)
return ctx, false, err
}
// We don't have an entry for this
// instance yet; go dereference it.
instance, err := f.GetRemoteInstance(
gtscontext.SetFastFail(ctx),
username,
&url.URL{
Scheme: pubKeyOwnerURI.Scheme,
Host: pubKeyOwnerURI.Host,
},
)
if err != nil {
err = gtserror.Newf("error dereferencing instance %s: %w", pubKeyOwnerURI.Host, err)
return nil, false, err
}
if err := f.db.PutInstance(ctx, instance); err != nil && !errors.Is(err, db.ErrAlreadyExists) {
err = gtserror.Newf("error inserting instance entry for %s: %w", pubKeyOwnerURI.Host, err)
return nil, false, err
}
}
// 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,
pubKeyOwnerURI,
)
if err != nil {
if gtserror.StatusCode(err) == http.StatusGone {
// This is the same case as the http.StatusGone check above.
// It can happen here and not there because there's a race
// where the sending server starts sending account deletion
// notifications out, we start processing, the request above
// succeeds, and *then* the profile is removed and starts
// returning 410 Gone, at which point _this_ request fails.
w.WriteHeader(http.StatusAccepted)
return ctx, false, nil
}
err = gtserror.Newf("couldn't get requesting account %s: %w", pubKeyOwnerURI, err)
return nil, false, err
}
if !requestingAccount.SuspendedAt.IsZero() {
// Account was marked as suspended by a
// local admin action. Stop request early.
w.WriteHeader(http.StatusForbidden)
if pubKeyAuth.Handshaking {
// There is a mutal handshake occurring between us and
// the owner URI. Return 202 and leave as we can't do
// much else until the handshake procedure has finished.
w.WriteHeader(http.StatusAccepted)
return ctx, false, nil
}
// We have everything we need now, set the requesting
// and receiving accounts on the context for later use.
ctx = gtscontext.SetRequestingAccount(ctx, requestingAccount)
ctx = gtscontext.SetRequestingAccount(ctx, pubKeyAuth.Owner)
ctx = gtscontext.SetReceivingAccount(ctx, receivingAccount)
return ctx, true, nil
}

4
internal/processing/admin/domainkeysexpire.go
View File

@ -71,9 +71,7 @@ func (p *Processor) domainKeysExpireSideEffects(ctx context.Context, domain stri
// the public key and update the account.
if err := p.rangeDomainAccounts(ctx, domain, func(account *gtsmodel.Account) {
account.PublicKeyExpiresAt = expiresAt
if err := p.state.DB.UpdateAccount(
ctx,
if err := p.state.DB.UpdateAccount(ctx,
account,
"public_key_expires_at",
); err != nil {

33
internal/processing/fedi/common.go
View File

@ -22,7 +22,6 @@ import (
"errors"
"github.com/superseriousbusiness/gotosocial/internal/db"
"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
)
@ -45,8 +44,6 @@ func (p *Processor) authenticate(ctx context.Context, requestedUser string) (
return nil, nil, gtserror.NewErrorNotFound(err)
}
var requester *gtsmodel.Account
// Ensure request signed, and use signature URI to
// get requesting account, dereferencing if necessary.
pubKeyAuth, errWithCode := p.federator.AuthenticateFederatedRequest(ctx, requestedUser)
@ -54,33 +51,23 @@ func (p *Processor) authenticate(ctx context.Context, requestedUser string) (
return nil, nil, errWithCode
}
if requester = pubKeyAuth.Owner; requester == nil {
requester, _, err = p.federator.GetAccountByURI(
gtscontext.SetFastFail(ctx),
requestedUser,
pubKeyAuth.OwnerURI,
)
if err != nil {
err = gtserror.Newf("error getting account %s: %w", pubKeyAuth.OwnerURI, err)
return nil, nil, gtserror.NewErrorUnauthorized(err)
}
if pubKeyAuth.Handshaking {
// This should happen very rarely, we are in the middle of handshaking.
err := gtserror.Newf("network race handshaking %s", pubKeyAuth.OwnerURI)
return nil, nil, gtserror.NewErrorInternalError(err)
}
if !requester.SuspendedAt.IsZero() {
// Account was marked as suspended by a
// local admin action. Stop request early.
const text = "requesting account is suspended"
return nil, nil, gtserror.NewErrorForbidden(errors.New(text))
}
// Get requester from auth.
requester := pubKeyAuth.Owner
// Ensure no block exists between requester + requested.
// Check that block does not exist between receiver and requester.
blocked, err := p.state.DB.IsEitherBlocked(ctx, receiver.ID, requester.ID)
if err != nil {
err = gtserror.Newf("db error getting checking block: %w", err)
err := gtserror.Newf("error checking block: %w", err)
return nil, nil, gtserror.NewErrorInternalError(err)
} else if blocked {
err = gtserror.Newf("block exists between accounts %s and %s", requester.ID, receiver.ID)
return nil, nil, gtserror.NewErrorForbidden(err)
const text = "block exists between accounts"
return nil, nil, gtserror.NewErrorForbidden(errors.New(text))
}
return requester, receiver, nil

1
internal/processing/fedi/status.go
View File

@ -35,7 +35,6 @@ import (
// StatusGet handles the getting of a fedi/activitypub representation of a local status.
// It performs appropriate authentication before returning a JSON serializable interface.
func (p *Processor) StatusGet(ctx context.Context, requestedUser string, statusID string) (interface{}, gtserror.WithCode) {
// Authenticate using http signature.
// Authenticate the incoming request, getting related user accounts.
requester, receiver, errWithCode := p.authenticate(ctx, requestedUser)
if errWithCode != nil {

69
internal/processing/fedi/user.go
View File

@ -26,7 +26,6 @@ import (
"github.com/superseriousbusiness/activity/streams/vocab"
"github.com/superseriousbusiness/gotosocial/internal/ap"
"github.com/superseriousbusiness/gotosocial/internal/db"
"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/uris"
)
@ -35,7 +34,7 @@ import (
// performing authentication before returning a JSON serializable interface to the caller.
func (p *Processor) UserGet(ctx context.Context, requestedUsername string, requestURL *url.URL) (interface{}, gtserror.WithCode) {
// (Try to) get the requested local account from the db.
requestedAccount, err := p.state.DB.GetAccountByUsernameDomain(ctx, requestedUsername, "")
receiver, err := p.state.DB.GetAccountByUsernameDomain(ctx, requestedUsername, "")
if err != nil {
if errors.Is(err, db.ErrNoEntries) {
// Account just not found w/ this username.
@ -54,8 +53,9 @@ func (p *Processor) UserGet(ctx context.Context, requestedUsername string, reque
// the bare minimum user profile needed for the pubkey.
//
// TODO: https://github.com/superseriousbusiness/gotosocial/issues/1186
minimalPerson, err := p.converter.AccountToASMinimal(ctx, requestedAccount)
minimalPerson, err := p.converter.AccountToASMinimal(ctx, receiver)
if err != nil {
err := gtserror.Newf("error converting to minimal account: %w", err)
return nil, gtserror.NewErrorInternalError(err)
}
@ -72,56 +72,39 @@ func (p *Processor) UserGet(ctx context.Context, requestedUsername string, reque
}
// Auth passed, generate the proper AP representation.
person, err := p.converter.AccountToAS(ctx, requestedAccount)
person, err := p.converter.AccountToAS(ctx, receiver)
if err != nil {
err := gtserror.Newf("error converting account: %w", err)
return nil, gtserror.NewErrorInternalError(err)
}
// If we are currently handshaking with the remote account
// making the request, then don't be coy: just serve the AP
// representation of the target account.
//
// This handshake check ensures that we don't get stuck in
// a loop with another GtS instance, where each instance is
// trying repeatedly to dereference the other account that's
// making the request before it will reveal its own account.
//
// Instead, we end up in an 'I'll show you mine if you show me
// yours' situation, where we sort of agree to reveal each
// other's profiles at the same time.
if p.federator.Handshaking(requestedUsername, pubKeyAuth.OwnerURI) {
if pubKeyAuth.Handshaking {
// If we are currently handshaking with the remote account
// making the request, then don't be coy: just serve the AP
// representation of the target account.
//
// This handshake check ensures that we don't get stuck in
// a loop with another GtS instance, where each instance is
// trying repeatedly to dereference the other account that's
// making the request before it will reveal its own account.
//
// Instead, we end up in an 'I'll show you mine if you show me
// yours' situation, where we sort of agree to reveal each
// other's profiles at the same time.
return data(person)
}
// We're not currently handshaking with the requestingAccountURI,
// so fetch its details and ensure it's up to date + not blocked.
requestingAccount, _, err := p.federator.GetAccountByURI(
// On a hot path so fail quickly.
gtscontext.SetFastFail(ctx),
requestedUsername,
pubKeyAuth.OwnerURI,
)
if err != nil {
err := gtserror.Newf("error getting account %s: %w", pubKeyAuth.OwnerURI, err)
return nil, gtserror.NewErrorUnauthorized(err)
}
// Get requester from auth.
requester := pubKeyAuth.Owner
if !requestingAccount.SuspendedAt.IsZero() {
// Account was marked as suspended by a
// local admin action. Stop request early.
err = fmt.Errorf("account %s marked as suspended", requestingAccount.ID)
return nil, gtserror.NewErrorForbidden(err)
}
blocked, err := p.state.DB.IsBlocked(ctx, requestedAccount.ID, requestingAccount.ID)
// Check that block does not exist between receiver and requester.
blocked, err := p.state.DB.IsBlocked(ctx, receiver.ID, requester.ID)
if err != nil {
err := gtserror.Newf("error checking block from account %s to account %s: %w", requestedAccount.ID, requestingAccount.ID, err)
err := gtserror.Newf("error checking block: %w", err)
return nil, gtserror.NewErrorInternalError(err)
}
if blocked {
err := fmt.Errorf("account %s blocks account %s", requestedAccount.ID, requestingAccount.ID)
return nil, gtserror.NewErrorForbidden(err)
} else if blocked {
const text = "block exists between accounts"
return nil, gtserror.NewErrorForbidden(errors.New(text))
}
return data(person)