From 9045b8e89fbe3fa1c441a59029365a98318180d8 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Mon, 20 Apr 2020 17:42:34 +0100 Subject: [PATCH] Perspective key fetching, some federation room join fixes (#975) * Update gomatrixserverlib * Test matrix.org as perspective key server * Base64 decode better * Optional strict validity checking in gmsl * Update gomatrixserverlib * Attempt to find missing auth events over federation (this shouldn't happen but I am guessing there is a synapse bug involved where we don't get all of the auth events) * Update gomatrixserverlib, debug logging * Remove debugging output * More verbose debugging * Print outliers * Increase timeouts for testing, observe contexts before trying to join over more servers * Don't block on roomserver (experimental) * Don't block on roomserver * Update gomatrixserverlib * Update gomatrixserverlib * Configurable perspective key fetchers * Output number of configured keys for perspective * Example perspective config included * Undo debug stack trace * Undo debug stack trace * Restore original HTTP listener in monolith * Fix lint * Review comments * Set default HTTP server timeout to 5 minutes now, block again when joining * Don't use HTTP address for HTTPS whoops * Update gomatrixserverlib * Update gomatrixserverlib * Update gomatrixserverlib * Actually add perspectives * Actually add perspectives * Update gomatrixserverlib --- clientapi/routing/joinroom.go | 69 ++++++++++++++++++++-- cmd/dendrite-client-api-server/main.go | 2 +- cmd/dendrite-demo-libp2p/main.go | 2 +- cmd/dendrite-federation-api-server/main.go | 2 +- cmd/dendrite-monolith-server/main.go | 28 ++++++--- common/basecomponent/base.go | 19 +++--- common/config/config.go | 18 ++++++ common/keydb/keyring.go | 52 ++++++++++++++-- dendrite-config.yaml | 8 +++ federationapi/routing/invite.go | 7 ++- federationapi/routing/join.go | 7 ++- federationapi/routing/leave.go | 7 ++- go.mod | 2 +- go.sum | 5 +- sytest-whitelist | 1 + 15 files changed, 189 insertions(+), 40 deletions(-) diff --git a/clientapi/routing/joinroom.go b/clientapi/routing/joinroom.go index 0f1a9ba4..f72bb916 100644 --- a/clientapi/routing/joinroom.go +++ b/clientapi/routing/joinroom.go @@ -32,6 +32,7 @@ import ( "github.com/matrix-org/gomatrix" "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/util" + "github.com/sirupsen/logrus" ) // JoinRoomByIDOrAlias implements the "/join/{roomIDOrAlias}" API. @@ -290,7 +291,15 @@ func (r joinRoomReq) joinRoomUsingServers( // There was a problem talking to one of the servers. util.GetLogger(r.req.Context()).WithError(lastErr).WithField("server", server).Warn("Failed to join room using server") // Try the next server. - continue + if r.req.Context().Err() != nil { + // The request context has expired so don't bother trying any + // more servers - they will immediately fail due to the expired + // context. + break + } else { + // The request context hasn't expired yet so try the next server. + continue + } } return *response } @@ -365,16 +374,22 @@ func (r joinRoomReq) joinRoomUsingServer(roomID string, server gomatrixserverlib return nil, fmt.Errorf("r.federation.SendJoin: %w", err) } - if err = respSendJoin.Check(r.req.Context(), r.keyRing, event); err != nil { - return nil, fmt.Errorf("respSendJoin: %w", err) + if err = r.checkSendJoinResponse(event, server, respMakeJoin, respSendJoin); err != nil { + return nil, err } + util.GetLogger(r.req.Context()).WithFields(logrus.Fields{ + "room_id": roomID, + "num_auth_events": len(respSendJoin.AuthEvents), + "num_state_events": len(respSendJoin.StateEvents), + }).Info("Room join signature and auth verification passed") + if err = r.producer.SendEventWithState( r.req.Context(), gomatrixserverlib.RespState(respSendJoin.RespState), event.Headered(respMakeJoin.RoomVersion), ); err != nil { - return nil, fmt.Errorf("r.producer.SendEventWithState: %w", err) + util.GetLogger(r.req.Context()).WithError(err).Error("r.producer.SendEventWithState") } return &util.JSONResponse{ @@ -385,3 +400,49 @@ func (r joinRoomReq) joinRoomUsingServer(roomID string, server gomatrixserverlib }{roomID}, }, nil } + +// checkSendJoinResponse checks that all of the signatures are correct +// and that the join is allowed by the supplied state. +func (r joinRoomReq) checkSendJoinResponse( + event gomatrixserverlib.Event, + server gomatrixserverlib.ServerName, + respMakeJoin gomatrixserverlib.RespMakeJoin, + respSendJoin gomatrixserverlib.RespSendJoin, +) error { + // A list of events that we have retried, if they were not included in + // the auth events supplied in the send_join. + retries := map[string]bool{} + +retryCheck: + // TODO: Can we expand Check here to return a list of missing auth + // events rather than failing one at a time? + if err := respSendJoin.Check(r.req.Context(), r.keyRing, event); err != nil { + switch e := err.(type) { + case gomatrixserverlib.MissingAuthEventError: + // Check that we haven't already retried for this event, prevents + // us from ending up in endless loops + if !retries[e.AuthEventID] { + // Ask the server that we're talking to right now for the event + tx, txerr := r.federation.GetEvent(r.req.Context(), server, e.AuthEventID) + if txerr != nil { + return fmt.Errorf("r.federation.GetEvent: %w", txerr) + } + // For each event returned, add it to the auth events. + for _, pdu := range tx.PDUs { + ev, everr := gomatrixserverlib.NewEventFromUntrustedJSON(pdu, respMakeJoin.RoomVersion) + if everr != nil { + return fmt.Errorf("gomatrixserverlib.NewEventFromUntrustedJSON: %w", everr) + } + respSendJoin.AuthEvents = append(respSendJoin.AuthEvents, ev) + } + // Mark the event as retried and then give the check another go. + retries[e.AuthEventID] = true + goto retryCheck + } + return fmt.Errorf("respSendJoin (after retries): %w", e) + default: + return fmt.Errorf("respSendJoin: %w", err) + } + } + return nil +} diff --git a/cmd/dendrite-client-api-server/main.go b/cmd/dendrite-client-api-server/main.go index a7e241b1..815a978a 100644 --- a/cmd/dendrite-client-api-server/main.go +++ b/cmd/dendrite-client-api-server/main.go @@ -33,7 +33,7 @@ func main() { deviceDB := base.CreateDeviceDB() keyDB := base.CreateKeyDB() federation := base.CreateFederationClient() - keyRing := keydb.CreateKeyRing(federation.Client, keyDB) + keyRing := keydb.CreateKeyRing(federation.Client, keyDB, cfg.Matrix.KeyPerspectives) asQuery := base.CreateHTTPAppServiceAPIs() alias, input, query := base.CreateHTTPRoomserverAPIs() diff --git a/cmd/dendrite-demo-libp2p/main.go b/cmd/dendrite-demo-libp2p/main.go index df3b48ad..f280c748 100644 --- a/cmd/dendrite-demo-libp2p/main.go +++ b/cmd/dendrite-demo-libp2p/main.go @@ -146,7 +146,7 @@ func main() { deviceDB := base.Base.CreateDeviceDB() keyDB := createKeyDB(base) federation := createFederationClient(base) - keyRing := keydb.CreateKeyRing(federation.Client, keyDB) + keyRing := keydb.CreateKeyRing(federation.Client, keyDB, cfg.Matrix.KeyPerspectives) alias, input, query := roomserver.SetupRoomServerComponent(&base.Base) eduInputAPI := eduserver.SetupEDUServerComponent(&base.Base, cache.New()) diff --git a/cmd/dendrite-federation-api-server/main.go b/cmd/dendrite-federation-api-server/main.go index d18926a6..dd06cd3f 100644 --- a/cmd/dendrite-federation-api-server/main.go +++ b/cmd/dendrite-federation-api-server/main.go @@ -33,7 +33,7 @@ func main() { keyDB := base.CreateKeyDB() federation := base.CreateFederationClient() federationSender := base.CreateHTTPFederationSenderAPIs() - keyRing := keydb.CreateKeyRing(federation.Client, keyDB) + keyRing := keydb.CreateKeyRing(federation.Client, keyDB, cfg.Matrix.KeyPerspectives) alias, input, query := base.CreateHTTPRoomserverAPIs() asQuery := base.CreateHTTPAppServiceAPIs() diff --git a/cmd/dendrite-monolith-server/main.go b/cmd/dendrite-monolith-server/main.go index d47d2e1b..6b0d83ae 100644 --- a/cmd/dendrite-monolith-server/main.go +++ b/cmd/dendrite-monolith-server/main.go @@ -55,7 +55,7 @@ func main() { deviceDB := base.CreateDeviceDB() keyDB := base.CreateKeyDB() federation := base.CreateFederationClient() - keyRing := keydb.CreateKeyRing(federation.Client, keyDB) + keyRing := keydb.CreateKeyRing(federation.Client, keyDB, cfg.Matrix.KeyPerspectives) alias, input, query := roomserver.SetupRoomServerComponent(base) eduInputAPI := eduserver.SetupEDUServerComponent(base, cache.New()) @@ -90,16 +90,26 @@ func main() { // Expose the matrix APIs directly rather than putting them under a /api path. go func() { - logrus.Info("Listening on ", *httpBindAddr) - logrus.Fatal(http.ListenAndServe(*httpBindAddr, nil)) + serv := http.Server{ + Addr: *httpBindAddr, + WriteTimeout: basecomponent.HTTPServerTimeout, + } + + logrus.Info("Listening on ", serv.Addr) + logrus.Fatal(serv.ListenAndServe()) }() // Handle HTTPS if certificate and key are provided - go func() { - if *certFile != "" && *keyFile != "" { - logrus.Info("Listening on ", *httpsBindAddr) - logrus.Fatal(http.ListenAndServeTLS(*httpsBindAddr, *certFile, *keyFile, nil)) - } - }() + if *certFile != "" && *keyFile != "" { + go func() { + serv := http.Server{ + Addr: *httpsBindAddr, + WriteTimeout: basecomponent.HTTPServerTimeout, + } + + logrus.Info("Listening on ", serv.Addr) + logrus.Fatal(serv.ListenAndServeTLS(*certFile, *keyFile)) + }() + } // We want to block forever to let the HTTP and HTTPS handler serve the APIs select {} diff --git a/common/basecomponent/base.go b/common/basecomponent/base.go index 5c6f6477..78894289 100644 --- a/common/basecomponent/base.go +++ b/common/basecomponent/base.go @@ -60,6 +60,9 @@ type BaseDendrite struct { KafkaProducer sarama.SyncProducer } +const HTTPServerTimeout = time.Minute * 5 +const HTTPClientTimeout = time.Second * 30 + // NewBaseDendrite creates a new instance to be used by a component. // The componentName is used for logging purposes, and should be a friendly name // of the compontent running, e.g. "SyncAPI" @@ -80,14 +83,12 @@ func NewBaseDendrite(cfg *config.Dendrite, componentName string) *BaseDendrite { kafkaConsumer, kafkaProducer = setupKafka(cfg) } - const defaultHTTPTimeout = 30 * time.Second - return &BaseDendrite{ componentName: componentName, tracerCloser: closer, Cfg: cfg, APIMux: mux.NewRouter().UseEncodedPath(), - httpClient: &http.Client{Timeout: defaultHTTPTimeout}, + httpClient: &http.Client{Timeout: HTTPClientTimeout}, KafkaConsumer: kafkaConsumer, KafkaProducer: kafkaProducer, } @@ -209,16 +210,20 @@ func (b *BaseDendrite) SetupAndServeHTTP(bindaddr string, listenaddr string) { addr = listenaddr } + serv := http.Server{ + Addr: addr, + WriteTimeout: HTTPServerTimeout, + } + common.SetupHTTPAPI(http.DefaultServeMux, common.WrapHandlerInCORS(b.APIMux), b.Cfg) - logrus.Infof("Starting %s server on %s", b.componentName, addr) - - err := http.ListenAndServe(addr, nil) + logrus.Infof("Starting %s server on %s", b.componentName, serv.Addr) + err := serv.ListenAndServe() if err != nil { logrus.WithError(err).Fatal("failed to serve http") } - logrus.Infof("Stopped %s server on %s", b.componentName, addr) + logrus.Infof("Stopped %s server on %s", b.componentName, serv.Addr) } // setupKafka creates kafka consumer/producer pair from the config. diff --git a/common/config/config.go b/common/config/config.go index a1a84425..6b61fda7 100644 --- a/common/config/config.go +++ b/common/config/config.go @@ -99,6 +99,9 @@ type Dendrite struct { // If set disables new users from registering (except via shared // secrets) RegistrationDisabled bool `yaml:"registration_disabled"` + // Perspective keyservers, to use as a backup when direct key fetch + // requests don't succeed + KeyPerspectives KeyPerspectives `yaml:"key_perspectives"` } `yaml:"matrix"` // The configuration specific to the media repostitory. @@ -285,6 +288,21 @@ type Dendrite struct { } `yaml:"-"` } +// KeyPerspectives are used to configure perspective key servers for +// retrieving server keys. +type KeyPerspectives []struct { + // The server name of the perspective key server + ServerName gomatrixserverlib.ServerName `yaml:"server_name"` + // Server keys for the perspective user, used to verify the + // keys have been signed by the perspective server + Keys []struct { + // The key ID, e.g. ed25519:auto + KeyID gomatrixserverlib.KeyID `yaml:"key_id"` + // The public key in base64 unpadded format + PublicKey string `yaml:"public_key"` + } `yaml:"keys"` +} + // A Path on the filesystem. type Path string diff --git a/common/keydb/keyring.go b/common/keydb/keyring.go index 1b20f781..e9cc7903 100644 --- a/common/keydb/keyring.go +++ b/common/keydb/keyring.go @@ -14,19 +14,61 @@ package keydb -import "github.com/matrix-org/gomatrixserverlib" +import ( + "encoding/base64" + + "github.com/matrix-org/dendrite/common/config" + "github.com/matrix-org/gomatrixserverlib" + "github.com/sirupsen/logrus" + "golang.org/x/crypto/ed25519" +) // CreateKeyRing creates and configures a KeyRing object. // // It creates the necessary key fetchers and collects them into a KeyRing // backed by the given KeyDatabase. func CreateKeyRing(client gomatrixserverlib.Client, - keyDB gomatrixserverlib.KeyDatabase) gomatrixserverlib.KeyRing { - return gomatrixserverlib.KeyRing{ + keyDB gomatrixserverlib.KeyDatabase, + cfg config.KeyPerspectives) gomatrixserverlib.KeyRing { + + fetchers := gomatrixserverlib.KeyRing{ KeyFetchers: []gomatrixserverlib.KeyFetcher{ - // TODO: Use perspective key fetchers for production. - &gomatrixserverlib.DirectKeyFetcher{Client: client}, + &gomatrixserverlib.DirectKeyFetcher{ + Client: client, + }, }, KeyDatabase: keyDB, } + + logrus.Info("Enabled direct key fetcher") + + var b64e = base64.StdEncoding.WithPadding(base64.NoPadding) + for _, ps := range cfg { + perspective := &gomatrixserverlib.PerspectiveKeyFetcher{ + PerspectiveServerName: ps.ServerName, + PerspectiveServerKeys: map[gomatrixserverlib.KeyID]ed25519.PublicKey{}, + Client: client, + } + + for _, key := range ps.Keys { + rawkey, err := b64e.DecodeString(key.PublicKey) + if err != nil { + logrus.WithError(err).WithFields(logrus.Fields{ + "server_name": ps.ServerName, + "public_key": key.PublicKey, + }).Warn("Couldn't parse perspective key") + continue + } + perspective.PerspectiveServerKeys[key.KeyID] = rawkey + } + + fetchers.KeyFetchers = append(fetchers.KeyFetchers, perspective) + + logrus.WithFields(logrus.Fields{ + "server_name": ps.ServerName, + "num_public_keys": len(ps.Keys), + }).Info("Enabled perspective key fetcher") + } + + return fetchers } diff --git a/dendrite-config.yaml b/dendrite-config.yaml index 86a208d7..bed78a5a 100644 --- a/dendrite-config.yaml +++ b/dendrite-config.yaml @@ -17,6 +17,14 @@ matrix: trusted_third_party_id_servers: - vector.im - matrix.org + # Perspective key servers which are used when direct key requests fail + #key_perspectives: + # - server_name: matrix.org + # keys: + # - key_id: ed25519:auto + # public_key: Noi6WqcDj0QmPxCNQqgezwTlBKrfqehY1u2FyWP9uYw + # - key_id: ed25519:a_RXGa + # public_key: l8Hft5qXKn1vfHrg3p4+W8gELQVo8N13JkluMfmn2sQ # The media repository config media: diff --git a/federationapi/routing/invite.go b/federationapi/routing/invite.go index 6c3e12e2..4b367e00 100644 --- a/federationapi/routing/invite.go +++ b/federationapi/routing/invite.go @@ -63,9 +63,10 @@ func Invite( // Check that the event is signed by the server sending the request. redacted := event.Redact() verifyRequests := []gomatrixserverlib.VerifyJSONRequest{{ - ServerName: event.Origin(), - Message: redacted.JSON(), - AtTS: event.OriginServerTS(), + ServerName: event.Origin(), + Message: redacted.JSON(), + AtTS: event.OriginServerTS(), + StrictValidityChecking: true, }} verifyResults, err := keys.VerifyJSONs(httpReq.Context(), verifyRequests) if err != nil { diff --git a/federationapi/routing/join.go b/federationapi/routing/join.go index 0a7b2300..e0678595 100644 --- a/federationapi/routing/join.go +++ b/federationapi/routing/join.go @@ -196,9 +196,10 @@ func SendJoin( // Check that the event is signed by the server sending the request. redacted := event.Redact() verifyRequests := []gomatrixserverlib.VerifyJSONRequest{{ - ServerName: event.Origin(), - Message: redacted.JSON(), - AtTS: event.OriginServerTS(), + ServerName: event.Origin(), + Message: redacted.JSON(), + AtTS: event.OriginServerTS(), + StrictValidityChecking: true, }} verifyResults, err := keys.VerifyJSONs(httpReq.Context(), verifyRequests) if err != nil { diff --git a/federationapi/routing/leave.go b/federationapi/routing/leave.go index e0a14263..6fc3b12e 100644 --- a/federationapi/routing/leave.go +++ b/federationapi/routing/leave.go @@ -145,9 +145,10 @@ func SendLeave( // Check that the event is signed by the server sending the request. redacted := event.Redact() verifyRequests := []gomatrixserverlib.VerifyJSONRequest{{ - ServerName: event.Origin(), - Message: redacted.JSON(), - AtTS: event.OriginServerTS(), + ServerName: event.Origin(), + Message: redacted.JSON(), + AtTS: event.OriginServerTS(), + StrictValidityChecking: true, }} verifyResults, err := keys.VerifyJSONs(httpReq.Context(), verifyRequests) if err != nil { diff --git a/go.mod b/go.mod index 401492ae..1a3dddc2 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( github.com/matrix-org/go-http-js-libp2p v0.0.0-20200318135427-31631a9ef51f github.com/matrix-org/go-sqlite3-js v0.0.0-20200325174927-327088cdef10 github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26 - github.com/matrix-org/gomatrixserverlib v0.0.0-20200416165239-837ed63a0046 + github.com/matrix-org/gomatrixserverlib v0.0.0-20200420162430-9662fc15e7e2 github.com/matrix-org/naffka v0.0.0-20200127221512-0716baaabaf1 github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7 github.com/mattn/go-sqlite3 v2.0.3+incompatible diff --git a/go.sum b/go.sum index 3bf623a9..627372be 100644 --- a/go.sum +++ b/go.sum @@ -364,8 +364,8 @@ github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26 h1:Hr3zjRsq2bh github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26/go.mod h1:3fxX6gUjWyI/2Bt7J1OLhpCzOfO/bB3AiX0cJtEKud0= github.com/matrix-org/gomatrixserverlib v0.0.0-20200124100636-0c2ec91d1df5 h1:kmRjpmFOenVpOaV/DRlo9p6z/IbOKlUC+hhKsAAh8Qg= github.com/matrix-org/gomatrixserverlib v0.0.0-20200124100636-0c2ec91d1df5/go.mod h1:FsKa2pWE/bpQql9H7U4boOPXFoJX/QcqaZZ6ijLkaZI= -github.com/matrix-org/gomatrixserverlib v0.0.0-20200416165239-837ed63a0046 h1:R7iYuS8hhXdrqs5OSNF3Y3chaIFWU9KLpnzjjsdJkMU= -github.com/matrix-org/gomatrixserverlib v0.0.0-20200416165239-837ed63a0046/go.mod h1:FsKa2pWE/bpQql9H7U4boOPXFoJX/QcqaZZ6ijLkaZI= +github.com/matrix-org/gomatrixserverlib v0.0.0-20200420162430-9662fc15e7e2 h1:XTg0bCZ+fQlWZHi1CinNecYfg2SFhyRNnGxKxXbLubg= +github.com/matrix-org/gomatrixserverlib v0.0.0-20200420162430-9662fc15e7e2/go.mod h1:FsKa2pWE/bpQql9H7U4boOPXFoJX/QcqaZZ6ijLkaZI= github.com/matrix-org/naffka v0.0.0-20200127221512-0716baaabaf1 h1:osLoFdOy+ChQqVUn2PeTDETFftVkl4w9t/OW18g3lnk= github.com/matrix-org/naffka v0.0.0-20200127221512-0716baaabaf1/go.mod h1:cXoYQIENbdWIQHt1SyCo6Bl3C3raHwJ0wgVrXHSqf+A= github.com/matrix-org/util v0.0.0-20171127121716-2e2df66af2f5 h1:W7l5CP4V7wPyPb4tYE11dbmeAOwtFQBTW0rf4OonOS8= @@ -721,4 +721,5 @@ gopkg.in/yaml.v2 v2.2.5 h1:ymVxjfMaHvXD8RqPRmzHHsB3VvucivSkIAvJFDI5O3c= gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099 h1:XJP7lxbSxWLOMNdBE4B/STaqVy6L73o0knwj2vIlxnw= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/sytest-whitelist b/sytest-whitelist index d47bf1f6..7bd2a63c 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -252,3 +252,4 @@ Outbound federation can send invites via v2 API User can invite local user to room with version 3 User can invite local user to room with version 4 A pair of servers can establish a join in a v2 room +Can logout all devices