From 223025fc27ef636206027b360201877848d426a4 Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Sun, 15 May 2022 10:16:43 +0100 Subject: [security] transport.Controller{} and transport.Transport{} security and performance improvements (#564) * cache transports in controller by privkey-generated pubkey, add retry logic to transport requests Signed-off-by: kim * update code comments, defer mutex unlocks Signed-off-by: kim * add count to 'performing request' log message Signed-off-by: kim * reduce repeated conversions of same url.URL object Signed-off-by: kim * move worker.Worker to concurrency subpackage, add WorkQueue type, limit transport http client use by WorkQueue Signed-off-by: kim * fix security advisories regarding max outgoing conns, max rsp body size - implemented by a new httpclient.Client{} that wraps an underlying client with a queue to limit connections, and limit reader wrapping a response body with a configured maximum size - update pub.HttpClient args passed around to be this new httpclient.Client{} Signed-off-by: kim * add httpclient tests, move ip validation to separate package + change mechanism Signed-off-by: kim * fix merge conflicts Signed-off-by: kim * use singular mutex in transport rather than separate signer mus Signed-off-by: kim * improved useragent string Signed-off-by: kim * add note regarding missing test Signed-off-by: kim * remove useragent field from transport (instead store in controller) Signed-off-by: kim * shutup linter Signed-off-by: kim * reset other signing headers on each loop iteration Signed-off-by: kim * respect request ctx during retry-backoff sleep period Signed-off-by: kim * use external pkg with docs explaining performance "hack" Signed-off-by: kim * use http package constants instead of string method literals Signed-off-by: kim * add license file headers Signed-off-by: kim * update code comment to match new func names Signed-off-by: kim * updates to user-agent string Signed-off-by: kim * update signed testrig models to fit with new transport logic (instead uses separate signer now) Signed-off-by: kim * fuck you linter Signed-off-by: kim --- internal/federation/dereferencing/dereferencer_test.go | 4 ++-- internal/federation/federatingactor_test.go | 6 +++--- internal/federation/federatingdb/db.go | 6 +++--- internal/federation/federatingdb/federatingdb_test.go | 6 +++--- internal/federation/federatingprotocol_test.go | 6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-) (limited to 'internal/federation') diff --git a/internal/federation/dereferencing/dereferencer_test.go b/internal/federation/dereferencing/dereferencer_test.go index 441019866..339490e5d 100644 --- a/internal/federation/dereferencing/dereferencer_test.go +++ b/internal/federation/dereferencing/dereferencer_test.go @@ -29,12 +29,12 @@ import ( "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/activity/streams" "github.com/superseriousbusiness/activity/streams/vocab" + "github.com/superseriousbusiness/gotosocial/internal/concurrency" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/messages" "github.com/superseriousbusiness/gotosocial/internal/transport" - "github.com/superseriousbusiness/gotosocial/internal/worker" "github.com/superseriousbusiness/gotosocial/testrig" ) @@ -150,7 +150,7 @@ func (suite *DereferencerStandardTestSuite) mockTransportController() transport. return response, nil } - fedWorker := worker.New[messages.FromFederator](-1, -1) + fedWorker := concurrency.NewWorkerPool[messages.FromFederator](-1, -1) mockClient := testrig.NewMockHTTPClient(do) return testrig.NewTestTransportController(mockClient, suite.db, fedWorker) } diff --git a/internal/federation/federatingactor_test.go b/internal/federation/federatingactor_test.go index 4039783a4..fdf907030 100644 --- a/internal/federation/federatingactor_test.go +++ b/internal/federation/federatingactor_test.go @@ -28,10 +28,10 @@ import ( "time" "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/concurrency" "github.com/superseriousbusiness/gotosocial/internal/federation" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/messages" - "github.com/superseriousbusiness/gotosocial/internal/worker" "github.com/superseriousbusiness/gotosocial/testrig" ) @@ -57,7 +57,7 @@ func (suite *FederatingActorTestSuite) TestSendNoRemoteFollowers() { ) testActivity := testrig.WrapAPNoteInCreate(testrig.URLMustParse("http://localhost:8080/whatever_some_create"), testrig.URLMustParse(testAccount.URI), time.Now(), testNote) - fedWorker := worker.New[messages.FromFederator](-1, -1) + fedWorker := concurrency.NewWorkerPool[messages.FromFederator](-1, -1) // setup transport controller with a no-op client so we don't make external calls sentMessages := []*url.URL{} @@ -112,7 +112,7 @@ func (suite *FederatingActorTestSuite) TestSendRemoteFollower() { ) testActivity := testrig.WrapAPNoteInCreate(testrig.URLMustParse("http://localhost:8080/whatever_some_create"), testrig.URLMustParse(testAccount.URI), time.Now(), testNote) - fedWorker := worker.New[messages.FromFederator](-1, -1) + fedWorker := concurrency.NewWorkerPool[messages.FromFederator](-1, -1) // setup transport controller with a no-op client so we don't make external calls sentMessages := []*url.URL{} diff --git a/internal/federation/federatingdb/db.go b/internal/federation/federatingdb/db.go index 60f09b909..cbe65e922 100644 --- a/internal/federation/federatingdb/db.go +++ b/internal/federation/federatingdb/db.go @@ -24,10 +24,10 @@ import ( "codeberg.org/gruf/go-mutexes" "github.com/superseriousbusiness/activity/pub" "github.com/superseriousbusiness/activity/streams/vocab" + "github.com/superseriousbusiness/gotosocial/internal/concurrency" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/messages" "github.com/superseriousbusiness/gotosocial/internal/typeutils" - "github.com/superseriousbusiness/gotosocial/internal/worker" ) // DB wraps the pub.Database interface with a couple of custom functions for GoToSocial. @@ -44,12 +44,12 @@ type DB interface { type federatingDB struct { locks mutexes.MutexMap db db.DB - fedWorker *worker.Worker[messages.FromFederator] + fedWorker *concurrency.WorkerPool[messages.FromFederator] typeConverter typeutils.TypeConverter } // New returns a DB interface using the given database and config -func New(db db.DB, fedWorker *worker.Worker[messages.FromFederator]) DB { +func New(db db.DB, fedWorker *concurrency.WorkerPool[messages.FromFederator]) DB { fdb := federatingDB{ locks: mutexes.NewMap(-1, -1), // use defaults db: db, diff --git a/internal/federation/federatingdb/federatingdb_test.go b/internal/federation/federatingdb/federatingdb_test.go index d53294c1c..8e6c1802d 100644 --- a/internal/federation/federatingdb/federatingdb_test.go +++ b/internal/federation/federatingdb/federatingdb_test.go @@ -23,12 +23,12 @@ import ( "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/ap" + "github.com/superseriousbusiness/gotosocial/internal/concurrency" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/federation/federatingdb" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/messages" "github.com/superseriousbusiness/gotosocial/internal/typeutils" - "github.com/superseriousbusiness/gotosocial/internal/worker" "github.com/superseriousbusiness/gotosocial/testrig" ) @@ -36,7 +36,7 @@ type FederatingDBTestSuite struct { suite.Suite db db.DB tc typeutils.TypeConverter - fedWorker *worker.Worker[messages.FromFederator] + fedWorker *concurrency.WorkerPool[messages.FromFederator] fromFederator chan messages.FromFederator federatingDB federatingdb.DB @@ -65,7 +65,7 @@ func (suite *FederatingDBTestSuite) SetupSuite() { func (suite *FederatingDBTestSuite) SetupTest() { testrig.InitTestLog() testrig.InitTestConfig() - suite.fedWorker = worker.New[messages.FromFederator](-1, -1) + suite.fedWorker = concurrency.NewWorkerPool[messages.FromFederator](-1, -1) suite.fromFederator = make(chan messages.FromFederator, 10) suite.fedWorker.SetProcessor(func(ctx context.Context, msg messages.FromFederator) error { suite.fromFederator <- msg diff --git a/internal/federation/federatingprotocol_test.go b/internal/federation/federatingprotocol_test.go index 09817cff3..b4769a70f 100644 --- a/internal/federation/federatingprotocol_test.go +++ b/internal/federation/federatingprotocol_test.go @@ -28,10 +28,10 @@ import ( "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/activity/pub" "github.com/superseriousbusiness/gotosocial/internal/ap" + "github.com/superseriousbusiness/gotosocial/internal/concurrency" "github.com/superseriousbusiness/gotosocial/internal/federation" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/messages" - "github.com/superseriousbusiness/gotosocial/internal/worker" "github.com/superseriousbusiness/gotosocial/testrig" ) @@ -44,7 +44,7 @@ func (suite *FederatingProtocolTestSuite) TestPostInboxRequestBodyHook() { // the activity we're gonna use activity := suite.testActivities["dm_for_zork"] - fedWorker := worker.New[messages.FromFederator](-1, -1) + fedWorker := concurrency.NewWorkerPool[messages.FromFederator](-1, -1) // setup transport controller with a no-op client so we don't make external calls tc := testrig.NewTestTransportController(testrig.NewMockHTTPClient(func(req *http.Request) (*http.Response, error) { @@ -78,7 +78,7 @@ func (suite *FederatingProtocolTestSuite) TestAuthenticatePostInbox() { sendingAccount := suite.testAccounts["remote_account_1"] inboxAccount := suite.testAccounts["local_account_1"] - fedWorker := worker.New[messages.FromFederator](-1, -1) + fedWorker := concurrency.NewWorkerPool[messages.FromFederator](-1, -1) tc := testrig.NewTestTransportController(testrig.NewMockHTTPClient(nil), suite.db, fedWorker) // now setup module being tested, with the mock transport controller -- cgit v1.2.3