From 1ad96e2e2df9dc1f5fa7d31522babd6a64ca517f Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Fri, 5 Mar 2021 10:40:27 +0000 Subject: [PATCH] Tweak AS registration check and AS component HTTP clients (#1785) * Tweak AS registration check * Check appservice usernames using correct function * Update sytest-whitelist * Use gomatrixserverlib.Client since that allows us to disable TLS validation using the config * Add appservice-specific client and ability to control TLS validation for appservices only * Set timeout on appservice client * Review comments * Remove dead code * Enforce LoginTypeApplicationService after all * Check correct auth type field --- appservice/appservice.go | 11 +++------ appservice/query/query.go | 25 +++---------------- appservice/workers/transaction_scheduler.go | 16 ++++-------- clientapi/routing/register.go | 27 +++++++++++++++------ cmd/generate-config/main.go | 1 + dendrite-config.yaml | 5 ++++ setup/base.go | 16 ++++++++++++ setup/config/config_appservice.go | 4 +++ sytest-whitelist | 6 +++++ 9 files changed, 64 insertions(+), 47 deletions(-) diff --git a/appservice/appservice.go b/appservice/appservice.go index d783c7eb7..f608e8e76 100644 --- a/appservice/appservice.go +++ b/appservice/appservice.go @@ -16,9 +16,7 @@ package appservice import ( "context" - "net/http" "sync" - "time" "github.com/gorilla/mux" appserviceAPI "github.com/matrix-org/dendrite/appservice/api" @@ -48,6 +46,7 @@ func NewInternalAPI( userAPI userapi.UserInternalAPI, rsAPI roomserverAPI.RoomserverInternalAPI, ) appserviceAPI.AppServiceQueryAPI { + client := base.CreateAppserviceClient() consumer, _ := kafka.SetupConsumerProducer(&base.Cfg.Global.Kafka) // Create a connection to the appservice postgres DB @@ -79,10 +78,8 @@ func NewInternalAPI( // Create appserivce query API with an HTTP client that will be used for all // outbound and inbound requests (inbound only for the internal API) appserviceQueryAPI := &query.AppServiceQueryAPI{ - HTTPClient: &http.Client{ - Timeout: time.Second * 30, - }, - Cfg: base.Cfg, + HTTPClient: client, + Cfg: base.Cfg, } // Only consume if we actually have ASes to track, else we'll just chew cycles needlessly. @@ -98,7 +95,7 @@ func NewInternalAPI( } // Create application service transaction workers - if err := workers.SetupTransactionWorkers(appserviceDB, workerStates); err != nil { + if err := workers.SetupTransactionWorkers(client, appserviceDB, workerStates); err != nil { logrus.WithError(err).Panicf("failed to start app service transaction workers") } return appserviceQueryAPI diff --git a/appservice/query/query.go b/appservice/query/query.go index 7e5ac4753..b4c335287 100644 --- a/appservice/query/query.go +++ b/appservice/query/query.go @@ -20,10 +20,10 @@ import ( "context" "net/http" "net/url" - "time" "github.com/matrix-org/dendrite/appservice/api" "github.com/matrix-org/dendrite/setup/config" + "github.com/matrix-org/gomatrixserverlib" opentracing "github.com/opentracing/opentracing-go" log "github.com/sirupsen/logrus" ) @@ -33,7 +33,7 @@ const userIDExistsPath = "/users/" // AppServiceQueryAPI is an implementation of api.AppServiceQueryAPI type AppServiceQueryAPI struct { - HTTPClient *http.Client + HTTPClient *gomatrixserverlib.Client Cfg *config.Dendrite } @@ -47,11 +47,6 @@ func (a *AppServiceQueryAPI) RoomAliasExists( span, ctx := opentracing.StartSpanFromContext(ctx, "ApplicationServiceRoomAlias") defer span.Finish() - // Create an HTTP client if one does not already exist - if a.HTTPClient == nil { - a.HTTPClient = makeHTTPClient() - } - // Determine which application service should handle this request for _, appservice := range a.Cfg.Derived.ApplicationServices { if appservice.URL != "" && appservice.IsInterestedInRoomAlias(request.Alias) { @@ -68,7 +63,7 @@ func (a *AppServiceQueryAPI) RoomAliasExists( } req = req.WithContext(ctx) - resp, err := a.HTTPClient.Do(req) + resp, err := a.HTTPClient.DoHTTPRequest(ctx, req) if resp != nil { defer func() { err = resp.Body.Close() @@ -115,11 +110,6 @@ func (a *AppServiceQueryAPI) UserIDExists( span, ctx := opentracing.StartSpanFromContext(ctx, "ApplicationServiceUserID") defer span.Finish() - // Create an HTTP client if one does not already exist - if a.HTTPClient == nil { - a.HTTPClient = makeHTTPClient() - } - // Determine which application service should handle this request for _, appservice := range a.Cfg.Derived.ApplicationServices { if appservice.URL != "" && appservice.IsInterestedInUserID(request.UserID) { @@ -134,7 +124,7 @@ func (a *AppServiceQueryAPI) UserIDExists( if err != nil { return err } - resp, err := a.HTTPClient.Do(req.WithContext(ctx)) + resp, err := a.HTTPClient.DoHTTPRequest(ctx, req) if resp != nil { defer func() { err = resp.Body.Close() @@ -169,10 +159,3 @@ func (a *AppServiceQueryAPI) UserIDExists( response.UserIDExists = false return nil } - -// makeHTTPClient creates an HTTP client with certain options that will be used for all query requests to application services -func makeHTTPClient() *http.Client { - return &http.Client{ - Timeout: time.Second * 30, - } -} diff --git a/appservice/workers/transaction_scheduler.go b/appservice/workers/transaction_scheduler.go index 45748c214..47d447c2c 100644 --- a/appservice/workers/transaction_scheduler.go +++ b/appservice/workers/transaction_scheduler.go @@ -34,8 +34,6 @@ import ( var ( // Maximum size of events sent in each transaction. transactionBatchSize = 50 - // Timeout for sending a single transaction to an application service. - transactionTimeout = time.Second * 60 ) // SetupTransactionWorkers spawns a separate goroutine for each application @@ -44,6 +42,7 @@ var ( // size), then send that off to the AS's /transactions/{txnID} endpoint. It also // handles exponentially backing off in case the AS isn't currently available. func SetupTransactionWorkers( + client *gomatrixserverlib.Client, appserviceDB storage.Database, workerStates []types.ApplicationServiceWorkerState, ) error { @@ -51,7 +50,7 @@ func SetupTransactionWorkers( for _, workerState := range workerStates { // Don't create a worker if this AS doesn't want to receive events if workerState.AppService.URL != "" { - go worker(appserviceDB, workerState) + go worker(client, appserviceDB, workerState) } } return nil @@ -59,17 +58,12 @@ func SetupTransactionWorkers( // worker is a goroutine that sends any queued events to the application service // it is given. -func worker(db storage.Database, ws types.ApplicationServiceWorkerState) { +func worker(client *gomatrixserverlib.Client, db storage.Database, ws types.ApplicationServiceWorkerState) { log.WithFields(log.Fields{ "appservice": ws.AppService.ID, }).Info("Starting application service") ctx := context.Background() - // Create a HTTP client for sending requests to app services - client := &http.Client{ - Timeout: transactionTimeout, - } - // Initial check for any leftover events to send from last time eventCount, err := db.CountEventsWithAppServiceID(ctx, ws.AppService.ID) if err != nil { @@ -206,7 +200,7 @@ func createTransaction( // send sends events to an application service. Returns an error if an OK was not // received back from the application service or the request timed out. func send( - client *http.Client, + client *gomatrixserverlib.Client, appservice config.ApplicationService, txnID int, transaction []byte, @@ -219,7 +213,7 @@ func send( return err } req.Header.Set("Content-Type", "application/json") - resp, err := client.Do(req) + resp, err := client.DoHTTPRequest(context.TODO(), req) if err != nil { return err } diff --git a/clientapi/routing/register.go b/clientapi/routing/register.go index 614e19d50..7d5ddbea9 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -496,11 +496,20 @@ func Register( r.Username = strconv.FormatInt(id, 10) } + // Is this an appservice registration? It will be if the access + // token is supplied + accessToken, accessTokenErr := auth.ExtractAccessToken(req) + // Squash username to all lowercase letters r.Username = strings.ToLower(r.Username) - - if resErr = validateUsername(r.Username); resErr != nil { - return *resErr + if r.Type == authtypes.LoginTypeApplicationService && accessTokenErr == nil { + if resErr = validateApplicationServiceUsername(r.Username); resErr != nil { + return *resErr + } + } else { + if resErr = validateUsername(r.Username); resErr != nil { + return *resErr + } } if resErr = validatePassword(r.Password); resErr != nil { return *resErr @@ -513,7 +522,7 @@ func Register( "session_id": r.Auth.Session, }).Info("Processing registration request") - return handleRegistrationFlow(req, r, sessionID, cfg, userAPI) + return handleRegistrationFlow(req, r, sessionID, cfg, userAPI, accessToken, accessTokenErr) } func handleGuestRegistration( @@ -579,6 +588,8 @@ func handleRegistrationFlow( sessionID string, cfg *config.ClientAPI, userAPI userapi.UserInternalAPI, + accessToken string, + accessTokenErr error, ) util.JSONResponse { // TODO: Shared secret registration (create new user scripts) // TODO: Enable registration config flag @@ -588,12 +599,12 @@ func handleRegistrationFlow( // TODO: Handle mapping registrationRequest parameters into session parameters // TODO: email / msisdn auth types. - accessToken, accessTokenErr := auth.ExtractAccessToken(req) // Appservices are special and are not affected by disabled - // registration or user exclusivity. - if r.Auth.Type == authtypes.LoginTypeApplicationService || - (r.Auth.Type == "" && accessTokenErr == nil) { + // registration or user exclusivity. We'll go onto the appservice + // registration flow if a valid access token was provided or if + // the login type specifically requests it. + if r.Type == authtypes.LoginTypeApplicationService && accessTokenErr == nil { return handleApplicationServiceRegistration( accessToken, accessTokenErr, req, r, cfg, userAPI, ) diff --git a/cmd/generate-config/main.go b/cmd/generate-config/main.go index fa0da10c5..9ef0f0b41 100644 --- a/cmd/generate-config/main.go +++ b/cmd/generate-config/main.go @@ -61,6 +61,7 @@ func main() { } if *defaultsForCI { + cfg.AppServiceAPI.DisableTLSValidation = true cfg.ClientAPI.RateLimiting.Enabled = false cfg.FederationSender.DisableTLSValidation = true cfg.MSCs.MSCs = []string{"msc2836", "msc2946", "msc2444", "msc2753"} diff --git a/dendrite-config.yaml b/dendrite-config.yaml index a3d1065d4..bf604c9d6 100644 --- a/dendrite-config.yaml +++ b/dendrite-config.yaml @@ -125,6 +125,11 @@ app_service_api: max_idle_conns: 2 conn_max_lifetime: -1 + # Disable the validation of TLS certificates of appservices. This is + # not recommended in production since it may allow appservice traffic + # to be sent to an unverified endpoint. + disable_tls_validation: false + # Appservice configuration files to load into this homeserver. config_files: [] diff --git a/setup/base.go b/setup/base.go index e9aa2a45e..f8a45409f 100644 --- a/setup/base.go +++ b/setup/base.go @@ -290,6 +290,22 @@ func (b *BaseDendrite) CreateClient() *gomatrixserverlib.Client { return client } +// CreateAppserviceClient creates a new client for application services. +// It has a specific timeout and obeys TLS validation from the appservice +// config rather than the federation config. +func (b *BaseDendrite) CreateAppserviceClient() *gomatrixserverlib.Client { + opts := []gomatrixserverlib.ClientOption{ + gomatrixserverlib.WithSkipVerify(b.Cfg.AppServiceAPI.DisableTLSValidation), + gomatrixserverlib.WithTimeout(time.Second * 60), + } + if b.Cfg.Global.DNSCache.Enabled { + opts = append(opts, gomatrixserverlib.WithDNSCache(b.DNSCache)) + } + client := gomatrixserverlib.NewClient(opts...) + client.SetUserAgent(fmt.Sprintf("Dendrite/%s", internal.VersionString())) + return client +} + // CreateFederationClient creates a new federation client. Should only be called // once per component. func (b *BaseDendrite) CreateFederationClient() *gomatrixserverlib.FederationClient { diff --git a/setup/config/config_appservice.go b/setup/config/config_appservice.go index a042691db..a6f77abfe 100644 --- a/setup/config/config_appservice.go +++ b/setup/config/config_appservice.go @@ -33,6 +33,10 @@ type AppServiceAPI struct { Database DatabaseOptions `yaml:"database"` + // DisableTLSValidation disables the validation of X.509 TLS certs + // on appservice endpoints. This is not recommended in production! + DisableTLSValidation bool `yaml:"disable_tls_validation"` + ConfigFiles []string `yaml:"config_files"` } diff --git a/sytest-whitelist b/sytest-whitelist index 1e4442a09..7ccbaad14 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -510,3 +510,9 @@ Can pass a JSON filter as a query parameter Local room members can get room messages Remote room members can get room messages Guest users can send messages to guest_access rooms if joined +AS can create a user +AS can create a user with an underscore +AS can create a user with inhibit_login +AS can set avatar for ghosted users +AS can set displayname for ghosted users +Ghost user must register before joining room