From c39fc9873d49f3bc1f0e9bd02d7c5b8bce52b594 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 9 Sep 2023 23:27:57 -0600 Subject: [PATCH 01/51] Add support for MSC3916 --- api/_apimeta/auth.go | 4 + api/_auth_cache/auth_cache.go | 2 +- api/_routers/97-require-server-auth.go | 30 +++++ api/_routers/98-use-rcontext.go | 32 +++--- api/custom/federation.go | 3 + api/routes.go | 18 ++- api/unstable/msc3916_download.go | 37 ++++++ api/unstable/msc3916_thumbnail.go | 32 ++++++ matrix/requests_signing.go | 150 +++++++++++++++++++++++++ matrix/xmatrix.go | 65 +++++++++++ util/http.go | 77 +++++++++++++ util/readers/multipart_reader.go | 57 ++++++++++ 12 files changed, 490 insertions(+), 17 deletions(-) create mode 100644 api/_routers/97-require-server-auth.go create mode 100644 api/unstable/msc3916_download.go create mode 100644 api/unstable/msc3916_thumbnail.go create mode 100644 matrix/requests_signing.go create mode 100644 matrix/xmatrix.go create mode 100644 util/readers/multipart_reader.go diff --git a/api/_apimeta/auth.go b/api/_apimeta/auth.go index 5bd5dfd6..d16df56f 100644 --- a/api/_apimeta/auth.go +++ b/api/_apimeta/auth.go @@ -16,6 +16,10 @@ type UserInfo struct { IsShared bool } +type ServerInfo struct { + ServerName string +} + func GetRequestUserAdminStatus(r *http.Request, rctx rcontext.RequestContext, user UserInfo) (bool, bool) { isGlobalAdmin := util.IsGlobalAdmin(user.UserId) || user.IsShared isLocalAdmin, err := matrix.IsUserAdmin(rctx, r.Host, user.AccessToken, r.RemoteAddr) diff --git a/api/_auth_cache/auth_cache.go b/api/_auth_cache/auth_cache.go index da1f2d5c..8ac47758 100644 --- a/api/_auth_cache/auth_cache.go +++ b/api/_auth_cache/auth_cache.go @@ -12,7 +12,7 @@ import ( "github.com/t2bot/matrix-media-repo/matrix" ) -var tokenCache = cache.New(0*time.Second, 30*time.Second) +var tokenCache = cache.New(cache.NoExpiration, 30*time.Second) var rwLock = &sync.RWMutex{} var regexCache = make(map[string]*regexp.Regexp) diff --git a/api/_routers/97-require-server-auth.go b/api/_routers/97-require-server-auth.go new file mode 100644 index 00000000..b7ca3bac --- /dev/null +++ b/api/_routers/97-require-server-auth.go @@ -0,0 +1,30 @@ +package _routers + +import ( + "net/http" + + "github.com/turt2live/matrix-media-repo/api/_apimeta" + "github.com/turt2live/matrix-media-repo/api/_responses" + "github.com/turt2live/matrix-media-repo/common" + "github.com/turt2live/matrix-media-repo/common/rcontext" + "github.com/turt2live/matrix-media-repo/matrix" +) + +type GeneratorWithServerFn = func(r *http.Request, ctx rcontext.RequestContext, server _apimeta.ServerInfo) interface{} + +func RequireServerAuth(generator GeneratorWithServerFn) GeneratorFn { + return func(r *http.Request, ctx rcontext.RequestContext) interface{} { + serverName, err := matrix.ValidateXMatrixAuth(r, true) + if err != nil { + ctx.Log.Debug("Error with X-Matrix auth: ", err) + return &_responses.ErrorResponse{ + Code: common.ErrCodeForbidden, + Message: "no auth provided (required)", + InternalCode: common.ErrCodeMissingToken, + } + } + return generator(r, ctx, _apimeta.ServerInfo{ + ServerName: serverName, + }) + } +} diff --git a/api/_routers/98-use-rcontext.go b/api/_routers/98-use-rcontext.go index 35e36369..420df864 100644 --- a/api/_routers/98-use-rcontext.go +++ b/api/_routers/98-use-rcontext.go @@ -101,20 +101,24 @@ func (c *RContextRouter) ServeHTTP(w http.ResponseWriter, r *http.Request) { beforeParseDownload: log.Infof("Replying with result: %T %+v", res, res) if downloadRes, isDownload := res.(*_responses.DownloadResponse); isDownload { - ranges, err := http_range.ParseRange(r.Header.Get("Range"), downloadRes.SizeBytes, rctx.Config.Downloads.DefaultRangeChunkSizeBytes) - if errors.Is(err, http_range.ErrInvalid) { - proposedStatusCode = http.StatusRequestedRangeNotSatisfiable - res = _responses.BadRequest("invalid range header") - goto beforeParseDownload // reprocess `res` - } else if errors.Is(err, http_range.ErrNoOverlap) { - proposedStatusCode = http.StatusRequestedRangeNotSatisfiable - res = _responses.BadRequest("out of range") - goto beforeParseDownload // reprocess `res` - } - if len(ranges) > 1 { - proposedStatusCode = http.StatusRequestedRangeNotSatisfiable - res = _responses.BadRequest("only 1 range is supported") - goto beforeParseDownload // reprocess `res` + var ranges []http_range.Range + var err error + if downloadRes.SizeBytes > 0 { + ranges, err = http_range.ParseRange(r.Header.Get("Range"), downloadRes.SizeBytes, rctx.Config.Downloads.DefaultRangeChunkSizeBytes) + if errors.Is(err, http_range.ErrInvalid) { + proposedStatusCode = http.StatusRequestedRangeNotSatisfiable + res = _responses.BadRequest("invalid range header") + goto beforeParseDownload // reprocess `res` + } else if errors.Is(err, http_range.ErrNoOverlap) { + proposedStatusCode = http.StatusRequestedRangeNotSatisfiable + res = _responses.BadRequest("out of range") + goto beforeParseDownload // reprocess `res` + } + if len(ranges) > 1 { + proposedStatusCode = http.StatusRequestedRangeNotSatisfiable + res = _responses.BadRequest("only 1 range is supported") + goto beforeParseDownload // reprocess `res` + } } contentType = downloadRes.ContentType diff --git a/api/custom/federation.go b/api/custom/federation.go index 48a0fb92..ceee40a4 100644 --- a/api/custom/federation.go +++ b/api/custom/federation.go @@ -34,6 +34,9 @@ func GetFederationInfo(r *http.Request, rctx rcontext.RequestContext, user _apim versionUrl := url + "/_matrix/federation/v1/version" versionResponse, err := matrix.FederatedGet(versionUrl, hostname, rctx) + if versionResponse != nil { + defer versionResponse.Body.Close() + } if err != nil { rctx.Log.Error(err) sentry.CaptureException(err) diff --git a/api/routes.go b/api/routes.go index 49f51ebc..d30feea9 100644 --- a/api/routes.go +++ b/api/routes.go @@ -18,6 +18,7 @@ import ( const PrefixMedia = "/_matrix/media" const PrefixClient = "/_matrix/client" +const PrefixFederation = "/_matrix/federation" func buildRoutes() http.Handler { counter := &_routers.RequestCounter{} @@ -36,13 +37,25 @@ func buildRoutes() http.Handler { register([]string{"GET", "HEAD"}, PrefixMedia, "download/:server/:mediaId/:filename", mxSpecV3Transition, router, downloadRoute) register([]string{"GET", "HEAD"}, PrefixMedia, "download/:server/:mediaId", mxSpecV3Transition, router, downloadRoute) register([]string{"GET"}, PrefixMedia, "thumbnail/:server/:mediaId", mxSpecV3Transition, router, makeRoute(_routers.OptionalAccessToken(r0.ThumbnailMedia), "thumbnail", counter)) - register([]string{"GET"}, PrefixMedia, "preview_url", mxSpecV3TransitionCS, router, makeRoute(_routers.RequireAccessToken(r0.PreviewUrl), "url_preview", counter)) + previewUrlRoute := makeRoute(_routers.RequireAccessToken(r0.PreviewUrl), "url_preview", counter) + register([]string{"GET"}, PrefixMedia, "preview_url", mxSpecV3TransitionCS, router, previewUrlRoute) register([]string{"GET"}, PrefixMedia, "identicon/*seed", mxR0, router, makeRoute(_routers.OptionalAccessToken(r0.Identicon), "identicon", counter)) - register([]string{"GET"}, PrefixMedia, "config", mxSpecV3TransitionCS, router, makeRoute(_routers.RequireAccessToken(r0.PublicConfig), "config", counter)) + configRoute := makeRoute(_routers.RequireAccessToken(r0.PublicConfig), "config", counter) + register([]string{"GET"}, PrefixMedia, "config", mxSpecV3TransitionCS, router, configRoute) register([]string{"POST"}, PrefixClient, "logout", mxSpecV3TransitionCS, router, makeRoute(_routers.RequireAccessToken(r0.Logout), "logout", counter)) register([]string{"POST"}, PrefixClient, "logout/all", mxSpecV3TransitionCS, router, makeRoute(_routers.RequireAccessToken(r0.LogoutAll), "logout_all", counter)) register([]string{"POST"}, PrefixMedia, "create", mxV1, router, makeRoute(_routers.RequireAccessToken(v1.CreateMedia), "create", counter)) + // MSC3916 - Authentication & endpoint API separation + register([]string{"GET"}, PrefixClient, "media/preview_url", msc3916, router, previewUrlRoute) + register([]string{"GET"}, PrefixClient, "media/config", msc3916, router, configRoute) + authedDownloadRoute := makeRoute(_routers.RequireAccessToken(unstable.ClientDownloadMedia), "download", counter) + register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId/:filename", msc3916, router, authedDownloadRoute) + register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId", msc3916, router, authedDownloadRoute) + register([]string{"GET"}, PrefixClient, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireAccessToken(r0.ThumbnailMedia), "thumbnail", counter)) + register([]string{"GET"}, PrefixFederation, "media/download/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) + register([]string{"GET"}, PrefixFederation, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) + // Custom features register([]string{"GET"}, PrefixMedia, "local_copy/:server/:mediaId", mxUnstable, router, makeRoute(_routers.RequireAccessToken(unstable.LocalCopy), "local_copy", counter)) register([]string{"GET"}, PrefixMedia, "info/:server/:mediaId", mxUnstable, router, makeRoute(_routers.RequireAccessToken(unstable.MediaInfo), "info", counter)) @@ -129,6 +142,7 @@ var ( //mxAllSpec matrixVersions = []string{"r0", "v1", "v3", "unstable", "unstable/io.t2bot.media" /* and MSC routes */} mxUnstable matrixVersions = []string{"unstable", "unstable/io.t2bot.media"} msc4034 matrixVersions = []string{"unstable/org.matrix.msc4034"} + msc3916 matrixVersions = []string{"unstable/org.matrix.msc3916"} mxSpecV3Transition matrixVersions = []string{"r0", "v1", "v3"} mxSpecV3TransitionCS matrixVersions = []string{"r0", "v3"} mxR0 matrixVersions = []string{"r0"} diff --git a/api/unstable/msc3916_download.go b/api/unstable/msc3916_download.go new file mode 100644 index 00000000..64d9d5b1 --- /dev/null +++ b/api/unstable/msc3916_download.go @@ -0,0 +1,37 @@ +package unstable + +import ( + "bytes" + "net/http" + + "github.com/turt2live/matrix-media-repo/api/_apimeta" + "github.com/turt2live/matrix-media-repo/api/_responses" + "github.com/turt2live/matrix-media-repo/api/r0" + "github.com/turt2live/matrix-media-repo/common/rcontext" + "github.com/turt2live/matrix-media-repo/util/readers" +) + +func ClientDownloadMedia(r *http.Request, rctx rcontext.RequestContext, user _apimeta.UserInfo) interface{} { + r.URL.Query().Set("allow_remote", "true") + return r0.DownloadMedia(r, rctx, user) +} + +func FederationDownloadMedia(r *http.Request, rctx rcontext.RequestContext, server _apimeta.ServerInfo) interface{} { + r.URL.Query().Set("allow_remote", "false") + + res := r0.DownloadMedia(r, rctx, _apimeta.UserInfo{}) + if dl, ok := res.(*_responses.DownloadResponse); ok { + return &_responses.DownloadResponse{ + ContentType: "multipart/mixed", + Filename: "", + SizeBytes: 0, + Data: readers.NewMultipartReader( + &readers.MultipartPart{ContentType: "application/json", Reader: readers.MakeCloser(bytes.NewReader([]byte("{}")))}, + &readers.MultipartPart{ContentType: dl.ContentType, FileName: dl.Filename, Reader: dl.Data}, + ), + TargetDisposition: "attachment", + } + } else { + return res + } +} diff --git a/api/unstable/msc3916_thumbnail.go b/api/unstable/msc3916_thumbnail.go new file mode 100644 index 00000000..81b77d20 --- /dev/null +++ b/api/unstable/msc3916_thumbnail.go @@ -0,0 +1,32 @@ +package unstable + +import ( + "bytes" + "net/http" + + "github.com/turt2live/matrix-media-repo/api/_apimeta" + "github.com/turt2live/matrix-media-repo/api/_responses" + "github.com/turt2live/matrix-media-repo/api/r0" + "github.com/turt2live/matrix-media-repo/common/rcontext" + "github.com/turt2live/matrix-media-repo/util/readers" +) + +func FederationThumbnailMedia(r *http.Request, rctx rcontext.RequestContext, server _apimeta.ServerInfo) interface{} { + r.URL.Query().Set("allow_remote", "false") + + res := r0.ThumbnailMedia(r, rctx, _apimeta.UserInfo{}) + if dl, ok := res.(*_responses.DownloadResponse); ok { + return &_responses.DownloadResponse{ + ContentType: "multipart/mixed", + Filename: "", + SizeBytes: 0, + Data: readers.NewMultipartReader( + &readers.MultipartPart{ContentType: "application/json", Reader: readers.MakeCloser(bytes.NewReader([]byte("{}")))}, + &readers.MultipartPart{ContentType: dl.ContentType, FileName: dl.Filename, Reader: dl.Data}, + ), + TargetDisposition: "attachment", + } + } else { + return res + } +} diff --git a/matrix/requests_signing.go b/matrix/requests_signing.go new file mode 100644 index 00000000..15e37997 --- /dev/null +++ b/matrix/requests_signing.go @@ -0,0 +1,150 @@ +package matrix + +import ( + "crypto/ed25519" + "encoding/json" + "errors" + "fmt" + "sync" + "time" + + "github.com/patrickmn/go-cache" + "github.com/sirupsen/logrus" + "github.com/t2bot/go-typed-singleflight" + "github.com/turt2live/matrix-media-repo/common/rcontext" + "github.com/turt2live/matrix-media-repo/database" + "github.com/turt2live/matrix-media-repo/util" +) + +type signingKey struct { + Key string `json:"key"` +} + +type serverKeyResult struct { + ServerName string `json:"server_name"` + ValidUntilTs int64 `json:"valid_until_ts"` + VerifyKeys map[string]signingKey `json:"verify_keys"` // unpadded base64 + OldVerifyKeys map[string]signingKey `json:"old_verify_keys"` // unpadded base64 + Signatures map[string]map[string]string `json:"signatures"` // unpadded base64; > +} + +type ServerSigningKeys map[string]ed25519.PublicKey + +var signingKeySf = new(typedsf.Group[*ServerSigningKeys]) +var signingKeyCache = cache.New(cache.NoExpiration, 30*time.Second) +var signingKeyRWLock = new(sync.RWMutex) + +func querySigningKeyCache(serverName string) *ServerSigningKeys { + if val, ok := signingKeyCache.Get(serverName); ok { + return val.(*ServerSigningKeys) + } + return nil +} + +func QuerySigningKeys(serverName string) (*ServerSigningKeys, error) { + signingKeyRWLock.RLock() + keys := querySigningKeyCache(serverName) + signingKeyRWLock.RUnlock() + if keys != nil { + return keys, nil + } + + keys, err, _ := signingKeySf.Do(serverName, func() (*ServerSigningKeys, error) { + ctx := rcontext.Initial().LogWithFields(logrus.Fields{ + "keysForServer": serverName, + }) + + signingKeyRWLock.Lock() + defer signingKeyRWLock.Unlock() + + // check cache once more, just in case the locks overlapped + cachedKeys := querySigningKeyCache(serverName) + if keys != nil { + return cachedKeys, nil + } + + // now we can try to get the keys from the source + url, hostname, err := GetServerApiUrl(serverName) + if err != nil { + return nil, err + } + + keysUrl := url + "/_matrix/key/v2/server" + keysResponse, err := FederatedGet(keysUrl, hostname, ctx) + if keysResponse != nil { + defer keysResponse.Body.Close() + } + if err != nil { + return nil, err + } + + decoder := json.NewDecoder(keysResponse.Body) + raw := database.AnonymousJson{} + if err = decoder.Decode(&raw); err != nil { + return nil, err + } + keyInfo := new(serverKeyResult) + if err = raw.ApplyTo(keyInfo); err != nil { + return nil, err + } + + // Check validity before we go much further + if keyInfo.ServerName != serverName { + return nil, fmt.Errorf("got keys for '%s' but expected '%s'", keyInfo.ServerName, serverName) + } + if keyInfo.ValidUntilTs <= util.NowMillis() { + return nil, errors.New("returned server keys are expired") + } + cacheUntil := time.Until(time.UnixMilli(keyInfo.ValidUntilTs)) / 2 + if cacheUntil <= (6 * time.Second) { + return nil, errors.New("returned server keys would expire too quickly") + } + + // Convert to something useful + serverKeys := make(ServerSigningKeys) + for keyId, keyObj := range keyInfo.VerifyKeys { + b, err := util.DecodeUnpaddedBase64String(keyObj.Key) + if err != nil { + return nil, errors.Join(fmt.Errorf("bad base64 for key ID '%s' for '%s'", keyId, serverName), err) + } + + serverKeys[keyId] = b + } + + // Check signatures + if len(keyInfo.Signatures) == 0 || len(keyInfo.Signatures[serverName]) == 0 { + return nil, fmt.Errorf("missing signatures from '%s'", serverName) + } + delete(raw, "signatures") + canonical, err := util.EncodeCanonicalJson(raw) + if err != nil { + return nil, err + } + for domain, sig := range keyInfo.Signatures { + if domain != serverName { + return nil, fmt.Errorf("unexpected signature from '%s' (expected '%s')", domain, serverName) + } + + for keyId, b64 := range sig { + signatureBytes, err := util.DecodeUnpaddedBase64String(b64) + if err != nil { + return nil, errors.Join(fmt.Errorf("bad base64 signature for key ID '%s' for '%s'", keyId, serverName), err) + } + + key, ok := serverKeys[keyId] + if !ok { + return nil, fmt.Errorf("unknown key ID '%s' for signature from '%s'", keyId, serverName) + } + + if !ed25519.Verify(key, canonical, signatureBytes) { + return nil, fmt.Errorf("invalid signature '%s' from key ID '%s' for '%s'", b64, keyId, serverName) + } + } + } + + // Cache & return (unlock was deferred) + signingKeyCache.Set(serverName, &serverKeys, cacheUntil) + return &serverKeys, nil + }) + return keys, err +} diff --git a/matrix/xmatrix.go b/matrix/xmatrix.go new file mode 100644 index 00000000..72482f13 --- /dev/null +++ b/matrix/xmatrix.go @@ -0,0 +1,65 @@ +package matrix + +import ( + "crypto/ed25519" + "errors" + "fmt" + "github.com/turt2live/matrix-media-repo/util" + "net/http" +) + +var ErrNoXMatrixAuth = errors.New("no X-Matrix auth headers") + +func ValidateXMatrixAuth(request *http.Request, expectNoContent bool) (string, error) { + if !expectNoContent { + panic("development error: X-Matrix auth validation can only be done with an empty body for now") + } + + auths, err := util.GetXMatrixAuth(request) + if err != nil { + return "", err + } + + if len(auths) == 0 { + return "", ErrNoXMatrixAuth + } + + obj := map[string]interface{}{ + "method": request.Method, + "uri": request.RequestURI, + "origin": auths[0].Origin, + "destination": auths[0].Destination, + "content": "{}", + } + canonical, err := util.EncodeCanonicalJson(obj) + if err != nil { + return "", err + } + + keys, err := QuerySigningKeys(auths[0].Origin) + if err != nil { + return "", err + } + + for _, h := range auths { + if h.Origin != obj["origin"] { + return "", errors.New("auth is from multiple servers") + } + if h.Destination != obj["destination"] { + return "", errors.New("auth is for multiple servers") + } + if h.Destination != "" && !util.IsServerOurs(h.Destination) { + return "", errors.New("unknown destination") + } + + if key, ok := (*keys)[h.KeyId]; ok { + if !ed25519.Verify(key, canonical, h.Signature) { + return "", fmt.Errorf("failed signatures on '%s'", h.KeyId) + } + } else { + return "", fmt.Errorf("unknown key '%s'", h.KeyId) + } + } + + return auths[0].Origin, nil +} diff --git a/util/http.go b/util/http.go index 2188feb3..ac5144a4 100644 --- a/util/http.go +++ b/util/http.go @@ -1,11 +1,19 @@ package util import ( + "fmt" "net/http" "net/url" "strings" ) +type XMatrixAuth struct { + Origin string + Destination string + KeyId string + Signature []byte +} + func GetAccessTokenFromRequest(request *http.Request) string { token := request.Header.Get("Authorization") @@ -40,3 +48,72 @@ func GetLogSafeUrl(r *http.Request) string { copyUrl.RawQuery = GetLogSafeQueryString(r) return copyUrl.String() } + +func GetXMatrixAuth(request *http.Request) ([]XMatrixAuth, error) { + headers := request.Header.Values("Authorization") + auths := make([]XMatrixAuth, 0) + for _, h := range headers { + if !strings.HasPrefix(h, "X-Matrix ") { + continue + } + + paramCsv := h[len("X-Matrix "):] + params := make(map[string]string) + isKey := true + keyName := "" + keyValue := "" + escape := false + for _, c := range paramCsv { + if c == ',' && isKey { + params[strings.TrimSpace(strings.ToLower(keyName))] = keyValue + keyName = "" + keyValue = "" + continue + } + if c == '=' { + isKey = false + continue + } + + if isKey { + keyName = fmt.Sprintf("%s%s", keyName, string(c)) + } else { + if c == '\\' && !escape { + escape = true + continue + } + if c == '"' && !escape { + escape = false + if len(keyValue) > 0 { + isKey = true + } + continue + } + if escape { + escape = false + } + keyValue = fmt.Sprintf("%s%s", keyValue, string(c)) + } + } + if len(keyName) > 0 && isKey { + params[strings.TrimSpace(strings.ToLower(keyName))] = keyValue + } + + sig, err := DecodeUnpaddedBase64String(params["sig"]) + if err != nil { + return nil, err + } + auth := XMatrixAuth{ + Origin: params["origin"], + Destination: params["destination"], + KeyId: params["key"], + Signature: sig, + } + if auth.Origin == "" || auth.KeyId == "" || len(auth.Signature) == 0 { + continue + } + auths = append(auths, auth) + } + + return auths, nil +} diff --git a/util/readers/multipart_reader.go b/util/readers/multipart_reader.go new file mode 100644 index 00000000..ea3c901d --- /dev/null +++ b/util/readers/multipart_reader.go @@ -0,0 +1,57 @@ +package readers + +import ( + "io" + "mime/multipart" + "net/textproto" + "net/url" + + "github.com/alioygur/is" +) + +type MultipartPart struct { + ContentType string + FileName string + Reader io.ReadCloser +} + +func NewMultipartReader(parts ...*MultipartPart) io.ReadCloser { + r, w := io.Pipe() + go func() { + mpw := multipart.NewWriter(w) + + for _, part := range parts { + headers := textproto.MIMEHeader{} + if part.ContentType != "" { + headers.Set("Content-Type", part.ContentType) + } + if part.FileName != "" { + if is.ASCII(part.FileName) { + headers.Set("Content-Disposition", "attachment; filename="+url.QueryEscape(part.FileName)) + } else { + headers.Set("Content-Disposition", "attachment; filename*=utf-8''"+url.QueryEscape(part.FileName)) + } + } + + partW, err := mpw.CreatePart(headers) + if err != nil { + _ = w.CloseWithError(err) + return + } + if _, err = io.Copy(partW, part.Reader); err != nil { + _ = w.CloseWithError(err) + return + } + if err = part.Reader.Close(); err != nil { + _ = w.CloseWithError(err) + return + } + } + + if err := mpw.Close(); err != nil { + _ = w.CloseWithError(err) + } + _ = w.Close() + }() + return MakeCloser(r) +} From a004bbf4b72d1949e46442a63016eaf4dbb6c541 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 25 Nov 2023 14:42:32 -0700 Subject: [PATCH 02/51] Add changelog --- CHANGELOG.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b953bb7c..94eacab4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * S3 datastores can now specify a `prefixLength` to improve S3 performance on some providers. See `config.sample.yaml` for details. * Add `multipartUploads` flag for running MMR against unsupported S3 providers. See `config.sample.yaml` for details. * A new "leaky bucket" rate limit algorithm has been applied to downloads. See `rateLimit.buckets` in the config for details. +* Add *unstable* support for [MSC3916: Authentication for media](https://github.com/matrix-org/matrix-spec-proposals/pull/3916). + * **Note**: MMR will *not* attempt to use authentication to download media over federation in this version. + * ***Subject to change during development.*** ### Changed @@ -35,10 +38,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Added * Dendrite homeservers can now have their media imported safely, and `adminApiKind` may be set to `dendrite`. -* Exporting MMR's data to Synapse is now possible with `import_to_synapse`. To use it, first run `gdpr_export` or similar. -* Errors encountered during a background task, such as an API-induced export, are exposed as `error_message` in the admin API. -* MMR will follow redirects on federated downloads up to 5 hops. -* S3-backed datastores can have download requests redirected to a public-facing CDN rather than being proxied through MMR. See `publicBaseUrl` under the S3 datastore config. ### Changed From f9d2316d76b3f1e454cdf7ef115e346d2f6818dd Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 25 Nov 2023 15:54:02 -0700 Subject: [PATCH 03/51] Add tests for preview_url and config authenticated endpoints --- ...sc3916_misc_client_endpoints_suite_test.go | 91 +++++++++++++++++++ test/templates/mmr.config.yaml | 17 ++++ test/test_internals/inline_dep_host_file.go | 82 +++++++++++++++++ 3 files changed, 190 insertions(+) create mode 100644 test/msc3916_misc_client_endpoints_suite_test.go create mode 100644 test/test_internals/inline_dep_host_file.go diff --git a/test/msc3916_misc_client_endpoints_suite_test.go b/test/msc3916_misc_client_endpoints_suite_test.go new file mode 100644 index 00000000..3d392b6b --- /dev/null +++ b/test/msc3916_misc_client_endpoints_suite_test.go @@ -0,0 +1,91 @@ +package test + +import ( + "log" + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" + "github.com/turt2live/matrix-media-repo/test/test_internals" +) + +type MSC3916MiscClientEndpointsSuite struct { + suite.Suite + deps *test_internals.ContainerDeps + htmlPage *test_internals.HostedFile +} + +func (s *MSC3916MiscClientEndpointsSuite) SetupSuite() { + deps, err := test_internals.MakeTestDeps() + if err != nil { + log.Fatal(err) + } + s.deps = deps + + file, err := test_internals.ServeFile("index.html", deps, "

This is a test file

") + if err != nil { + log.Fatal(err) + } + s.htmlPage = file +} + +func (s *MSC3916MiscClientEndpointsSuite) TearDownSuite() { + if s.htmlPage != nil { + s.htmlPage.Teardown() + } + if s.deps != nil { + if s.T().Failed() { + s.deps.Debug() + } + s.deps.Teardown() + } +} + +func (s *MSC3916MiscClientEndpointsSuite) TestPreviewUrlRequiresAuth() { + t := s.T() + + client1 := s.deps.Homeservers[0].UnprivilegedUsers[0].WithCsUrl(s.deps.Machines[0].HttpUrl) + client2 := &test_internals.MatrixClient{ + ClientServerUrl: s.deps.Machines[0].HttpUrl, + ServerName: s.deps.Homeservers[0].ServerName, + AccessToken: "", // no auth on this client + UserId: "", // no auth on this client + } + + qs := url.Values{ + "url": []string{s.htmlPage.PublicUrl}, + } + raw, err := client2.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/preview_url", qs, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) + + raw, err = client1.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/preview_url", qs, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, raw.StatusCode) +} + +func (s *MSC3916MiscClientEndpointsSuite) TestConfigRequiresAuth() { + t := s.T() + + client1 := s.deps.Homeservers[0].UnprivilegedUsers[0].WithCsUrl(s.deps.Machines[0].HttpUrl) + client2 := &test_internals.MatrixClient{ + ClientServerUrl: s.deps.Machines[0].HttpUrl, + ServerName: s.deps.Homeservers[0].ServerName, + AccessToken: "", // no auth on this client + UserId: "", // no auth on this client + } + + raw, err := client2.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/config", nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) + + raw, err = client1.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/config", nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, raw.StatusCode) +} + +func TestMSC3916MiscClientEndpointsSuite(t *testing.T) { + suite.Run(t, new(MSC3916MiscClientEndpointsSuite)) +} diff --git a/test/templates/mmr.config.yaml b/test/templates/mmr.config.yaml index 40fbf6d6..4d82dd3a 100644 --- a/test/templates/mmr.config.yaml +++ b/test/templates/mmr.config.yaml @@ -40,3 +40,20 @@ datastores: ssl: false rateLimit: enabled: false # we've got tests which intentionally spam +urlPreviews: + enabled: true + maxPageSizeBytes: 10485760 + previewUnsafeCertificates: false + numWords: 50 + maxLength: 200 + numTitleWords: 30 + maxTitleLength: 150 + filePreviewTypes: + - "image/*" + numWorkers: 10 + disallowedNetworks: [] + allowedNetworks: ["0.0.0.0/0"] + expireAfterDays: 0 + defaultLanguage: "en-US,en" + userAgent: "matrix-media-repo" + oEmbed: true diff --git a/test/test_internals/inline_dep_host_file.go b/test/test_internals/inline_dep_host_file.go new file mode 100644 index 00000000..dd486052 --- /dev/null +++ b/test/test_internals/inline_dep_host_file.go @@ -0,0 +1,82 @@ +package test_internals + +import ( + "fmt" + "log" + "os" + "path" + + "github.com/testcontainers/testcontainers-go" + "github.com/testcontainers/testcontainers-go/wait" +) + +type HostedFile struct { + upstream *ContainerDeps + nginx testcontainers.Container + tempDirectoryPath string + + PublicUrl string +} + +func ServeFile(fileName string, deps *ContainerDeps, contents string) (*HostedFile, error) { + tmp, err := os.MkdirTemp(os.TempDir(), "mmr-nginx") + if err != nil { + return nil, err + } + + f, err := os.Create(path.Join(tmp, fileName)) + if err != nil { + return nil, err + } + defer func(f *os.File) { + _ = f.Close() + }(f) + + _, err = f.Write([]byte(contents)) + if err != nil { + return nil, err + } + + err = f.Close() + if err != nil { + return nil, err + } + + nginx, err := testcontainers.GenericContainer(deps.ctx, testcontainers.GenericContainerRequest{ + ContainerRequest: testcontainers.ContainerRequest{ + Image: "docker.io/library/nginx:latest", + ExposedPorts: []string{"80/tcp"}, + Mounts: []testcontainers.ContainerMount{ + testcontainers.BindMount(tmp, "/usr/share/nginx/html"), + }, + Networks: []string{deps.depNet.NetId}, + WaitingFor: wait.ForListeningPort("80/tcp"), + }, + Started: true, + }) + if err != nil { + return nil, err + } + + nginxIp, err := nginx.ContainerIP(deps.ctx) + if err != nil { + return nil, err + } + + //goland:noinspection HttpUrlsUsage + return &HostedFile{ + upstream: deps, + nginx: nginx, + tempDirectoryPath: tmp, + PublicUrl: fmt.Sprintf("http://%s:%d/%s", nginxIp, 80, fileName), + }, nil +} + +func (f *HostedFile) Teardown() { + if err := f.nginx.Terminate(f.upstream.ctx); err != nil { + log.Fatalf("Error shutting down nginx container: %s", err.Error()) + } + if err := os.RemoveAll(f.tempDirectoryPath); err != nil { + log.Fatalf("Error cleaning up temporarily hosted file: %s", err.Error()) + } +} From 24058270f66dccd26f933a21d940d4e21be6d1aa Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 25 Nov 2023 16:27:28 -0700 Subject: [PATCH 04/51] Add placeholder tests for downloads and thumbnails --- test/msc3916_downloads_suite_test.go | 87 +++++++++++++++++++++++++++ test/msc3916_thumbnails_suite_test.go | 87 +++++++++++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 test/msc3916_downloads_suite_test.go create mode 100644 test/msc3916_thumbnails_suite_test.go diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go new file mode 100644 index 00000000..fc0edf83 --- /dev/null +++ b/test/msc3916_downloads_suite_test.go @@ -0,0 +1,87 @@ +package test + +import ( + "fmt" + "log" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" + "github.com/turt2live/matrix-media-repo/test/test_internals" + "github.com/turt2live/matrix-media-repo/util" +) + +type MSC3916DownloadsSuite struct { + suite.Suite + deps *test_internals.ContainerDeps +} + +func (s *MSC3916DownloadsSuite) SetupSuite() { + deps, err := test_internals.MakeTestDeps() + if err != nil { + log.Fatal(err) + } + s.deps = deps +} + +func (s *MSC3916DownloadsSuite) TearDownSuite() { + if s.deps != nil { + if s.T().Failed() { + s.deps.Debug() + } + s.deps.Teardown() + } +} + +func (s *MSC3916DownloadsSuite) TestClientDownloads() { + t := s.T() + + client1 := s.deps.Homeservers[0].UnprivilegedUsers[0].WithCsUrl(s.deps.Machines[0].HttpUrl) + client2 := &test_internals.MatrixClient{ + ClientServerUrl: s.deps.Machines[0].HttpUrl, + ServerName: s.deps.Homeservers[0].ServerName, + AccessToken: "", // this client isn't authed + UserId: "", // this client isn't authed + } + + contentType, img, err := test_internals.MakeTestImage(512, 512) + assert.NoError(t, err) + fname := "image" + util.ExtensionForContentType(contentType) + + //res := new(test_internals.MatrixUploadResponse) + //err = client1.DoReturnJson("POST", "/_matrix/client/unstable/org.matrix.msc3916/media/upload", url.Values{"filename": []string{fname}}, contentType, img, res) + res, err := client1.Upload(fname, contentType, img) + assert.NoError(t, err) + assert.NotEmpty(t, res.MxcUri) + + origin, mediaId, err := util.SplitMxc(res.MxcUri) + assert.NoError(t, err) + assert.Equal(t, client1.ServerName, origin) + assert.NotEmpty(t, mediaId) + + raw, err := client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) + raw, err = client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s/whatever.png", origin, mediaId), nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) + + raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, raw.StatusCode) + test_internals.AssertIsTestImage(t, raw.Body) + raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s/whatever.png", origin, mediaId), nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, raw.StatusCode) + test_internals.AssertIsTestImage(t, raw.Body) +} + +func (s *MSC3916DownloadsSuite) TestFederationDownloads() { + t := s.T() + t.Error("Not yet implemented") +} + +func TestMSC3916DownloadsSuite(t *testing.T) { + suite.Run(t, new(MSC3916DownloadsSuite)) +} diff --git a/test/msc3916_thumbnails_suite_test.go b/test/msc3916_thumbnails_suite_test.go new file mode 100644 index 00000000..43cf5c77 --- /dev/null +++ b/test/msc3916_thumbnails_suite_test.go @@ -0,0 +1,87 @@ +package test + +import ( + "fmt" + "log" + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" + "github.com/turt2live/matrix-media-repo/test/test_internals" + "github.com/turt2live/matrix-media-repo/util" +) + +type MSC3916ThumbnailsSuite struct { + suite.Suite + deps *test_internals.ContainerDeps +} + +func (s *MSC3916ThumbnailsSuite) SetupSuite() { + deps, err := test_internals.MakeTestDeps() + if err != nil { + log.Fatal(err) + } + s.deps = deps +} + +func (s *MSC3916ThumbnailsSuite) TearDownSuite() { + if s.deps != nil { + if s.T().Failed() { + s.deps.Debug() + } + s.deps.Teardown() + } +} + +func (s *MSC3916ThumbnailsSuite) TestClientThumbnails() { + t := s.T() + + client1 := s.deps.Homeservers[0].UnprivilegedUsers[0].WithCsUrl(s.deps.Machines[0].HttpUrl) + client2 := &test_internals.MatrixClient{ + ClientServerUrl: s.deps.Machines[0].HttpUrl, + ServerName: s.deps.Homeservers[0].ServerName, + AccessToken: "", // this client isn't authed + UserId: "", // this client isn't authed + } + + contentType, img, err := test_internals.MakeTestImage(512, 512) + assert.NoError(t, err) + fname := "image" + util.ExtensionForContentType(contentType) + + //res := new(test_internals.MatrixUploadResponse) + //err = client1.DoReturnJson("POST", "/_matrix/client/unstable/org.matrix.msc3916/media/upload", url.Values{"filename": []string{fname}}, contentType, img, res) + res, err := client1.Upload(fname, contentType, img) + assert.NoError(t, err) + assert.NotEmpty(t, res.MxcUri) + + origin, mediaId, err := util.SplitMxc(res.MxcUri) + assert.NoError(t, err) + assert.Equal(t, client1.ServerName, origin) + assert.NotEmpty(t, mediaId) + + qs := url.Values{ + "width": []string{"96"}, + "height": []string{"96"}, + "method": []string{"scale"}, + } + + raw, err := client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/thumbnail/%s/%s", origin, mediaId), qs, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) + + raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/thumbnail/%s/%s", origin, mediaId), qs, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, raw.StatusCode) + //test_internals.AssertIsTestImage(t, raw.Body) // we can't verify that the resulting image is correct +} + +func (s *MSC3916ThumbnailsSuite) TestFederationThumbnails() { + t := s.T() + t.Error("Not yet implemented") +} + +func TestMSC3916ThumbnailsSuite(t *testing.T) { + suite.Run(t, new(MSC3916ThumbnailsSuite)) +} From f3d20d9f5437851477c5f8c2192a8261fa1164ce Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 25 Nov 2023 20:08:32 -0700 Subject: [PATCH 05/51] Test X-Matrix auth header stuff --- common/config/access.go | 9 +++++ matrix/requests_signing.go | 12 +++---- matrix/xmatrix.go | 72 ++++++++++++++++++++++++++----------- test/xmatrix_header_test.go | 37 +++++++++++++++++++ util/canonical_json.go | 2 +- util/http.go | 3 +- 6 files changed, 106 insertions(+), 29 deletions(-) create mode 100644 test/xmatrix_header_test.go diff --git a/common/config/access.go b/common/config/access.go index 6b1bab44..b95a613c 100644 --- a/common/config/access.go +++ b/common/config/access.go @@ -222,6 +222,15 @@ func GetDomain(domain string) *DomainRepoConfig { return domains[domain] } +func AddDomainForTesting(domain string, config *DomainRepoConfig) { + Get() // Ensure the "main" config was loaded first + if config == nil { + c := NewDefaultDomainConfig() + config = &c + } + domains[domain] = config +} + func DomainConfigFrom(c MainRepoConfig) DomainRepoConfig { // HACK: We should be better at this kind of inheritance dc := NewDefaultDomainConfig() diff --git a/matrix/requests_signing.go b/matrix/requests_signing.go index 15e37997..8694139f 100644 --- a/matrix/requests_signing.go +++ b/matrix/requests_signing.go @@ -30,18 +30,18 @@ type serverKeyResult struct { type ServerSigningKeys map[string]ed25519.PublicKey -var signingKeySf = new(typedsf.Group[*ServerSigningKeys]) +var signingKeySf = new(typedsf.Group[ServerSigningKeys]) var signingKeyCache = cache.New(cache.NoExpiration, 30*time.Second) var signingKeyRWLock = new(sync.RWMutex) -func querySigningKeyCache(serverName string) *ServerSigningKeys { +func querySigningKeyCache(serverName string) ServerSigningKeys { if val, ok := signingKeyCache.Get(serverName); ok { - return val.(*ServerSigningKeys) + return val.(ServerSigningKeys) } return nil } -func QuerySigningKeys(serverName string) (*ServerSigningKeys, error) { +func QuerySigningKeys(serverName string) (ServerSigningKeys, error) { signingKeyRWLock.RLock() keys := querySigningKeyCache(serverName) signingKeyRWLock.RUnlock() @@ -49,7 +49,7 @@ func QuerySigningKeys(serverName string) (*ServerSigningKeys, error) { return keys, nil } - keys, err, _ := signingKeySf.Do(serverName, func() (*ServerSigningKeys, error) { + keys, err, _ := signingKeySf.Do(serverName, func() (ServerSigningKeys, error) { ctx := rcontext.Initial().LogWithFields(logrus.Fields{ "keysForServer": serverName, }) @@ -144,7 +144,7 @@ func QuerySigningKeys(serverName string) (*ServerSigningKeys, error) { // Cache & return (unlock was deferred) signingKeyCache.Set(serverName, &serverKeys, cacheUntil) - return &serverKeys, nil + return serverKeys, nil }) return keys, err } diff --git a/matrix/xmatrix.go b/matrix/xmatrix.go index 72482f13..7b9889d3 100644 --- a/matrix/xmatrix.go +++ b/matrix/xmatrix.go @@ -4,8 +4,10 @@ import ( "crypto/ed25519" "errors" "fmt" - "github.com/turt2live/matrix-media-repo/util" "net/http" + + "github.com/turt2live/matrix-media-repo/database" + "github.com/turt2live/matrix-media-repo/util" ) var ErrNoXMatrixAuth = errors.New("no X-Matrix auth headers") @@ -15,51 +17,81 @@ func ValidateXMatrixAuth(request *http.Request, expectNoContent bool) (string, e panic("development error: X-Matrix auth validation can only be done with an empty body for now") } - auths, err := util.GetXMatrixAuth(request) + auths, err := util.GetXMatrixAuth(request.Header.Values("Authorization")) if err != nil { return "", err } - if len(auths) == 0 { return "", ErrNoXMatrixAuth } - obj := map[string]interface{}{ - "method": request.Method, - "uri": request.RequestURI, - "origin": auths[0].Origin, - "destination": auths[0].Destination, - "content": "{}", - } - canonical, err := util.EncodeCanonicalJson(obj) + keys, err := QuerySigningKeys(auths[0].Origin) if err != nil { return "", err } - keys, err := QuerySigningKeys(auths[0].Origin) + err = ValidateXMatrixAuthHeader(request.Method, request.RequestURI, &database.AnonymousJson{}, auths, keys) if err != nil { return "", err } + return auths[0].Origin, nil +} + +func ValidateXMatrixAuthHeader(requestMethod string, requestUri string, content any, headers []util.XMatrixAuth, originKeys ServerSigningKeys) error { + if len(headers) == 0 { + return ErrNoXMatrixAuth + } + + obj := map[string]interface{}{ + "method": requestMethod, + "uri": requestUri, + "origin": headers[0].Origin, + "destination": headers[0].Destination, + "content": content, + } + canonical, err := util.EncodeCanonicalJson(obj) + if err != nil { + return err + } - for _, h := range auths { + for _, h := range headers { if h.Origin != obj["origin"] { - return "", errors.New("auth is from multiple servers") + return errors.New("auth is from multiple servers") } if h.Destination != obj["destination"] { - return "", errors.New("auth is for multiple servers") + return errors.New("auth is for multiple servers") } if h.Destination != "" && !util.IsServerOurs(h.Destination) { - return "", errors.New("unknown destination") + return errors.New("unknown destination") } - if key, ok := (*keys)[h.KeyId]; ok { + if key, ok := (originKeys)[h.KeyId]; ok { if !ed25519.Verify(key, canonical, h.Signature) { - return "", fmt.Errorf("failed signatures on '%s'", h.KeyId) + return fmt.Errorf("failed signatures on '%s'", h.KeyId) } } else { - return "", fmt.Errorf("unknown key '%s'", h.KeyId) + return fmt.Errorf("unknown key '%s'", h.KeyId) } } - return auths[0].Origin, nil + return nil +} + +func CreateXMatrixHeader(origin string, destination string, requestMethod string, requestUri string, content any, key *ed25519.PrivateKey, keyVersion string) (string, error) { + obj := map[string]interface{}{ + "method": requestMethod, + "uri": requestUri, + "origin": origin, + "destination": destination, + "content": content, + } + canonical, err := util.EncodeCanonicalJson(obj) + if err != nil { + return "", err + } + + b := ed25519.Sign(*key, canonical) + sig := util.EncodeUnpaddedBase64ToString(b) + + return fmt.Sprintf("X-Matrix origin=\"%s\",destination=\"%s\",key=\"ed25519:%s\",sig=\"%s\"", origin, destination, keyVersion, sig), nil } diff --git a/test/xmatrix_header_test.go b/test/xmatrix_header_test.go new file mode 100644 index 00000000..17019108 --- /dev/null +++ b/test/xmatrix_header_test.go @@ -0,0 +1,37 @@ +package test + +import ( + "crypto/ed25519" + "testing" + + "github.com/turt2live/matrix-media-repo/common/config" + "github.com/turt2live/matrix-media-repo/database" + "github.com/turt2live/matrix-media-repo/matrix" + "github.com/turt2live/matrix-media-repo/util" +) + +func TestXMatrixAuthHeader(t *testing.T) { + config.AddDomainForTesting("localhost", nil) + + pub, priv, err := ed25519.GenerateKey(nil) + if err != nil { + t.Fatal(err) + } + + header, err := matrix.CreateXMatrixHeader("localhost:8008", "localhost", "GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, &priv, "0") + if err != nil { + t.Fatal(err) + } + + auths, err := util.GetXMatrixAuth([]string{header}) + if err != nil { + t.Fatal(err) + } + + keys := make(matrix.ServerSigningKeys) + keys["ed25519:0"] = pub + err = matrix.ValidateXMatrixAuthHeader("GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, auths, keys) + if err != nil { + t.Error(err) + } +} diff --git a/util/canonical_json.go b/util/canonical_json.go index c514f311..939da9f6 100644 --- a/util/canonical_json.go +++ b/util/canonical_json.go @@ -5,7 +5,7 @@ import ( "encoding/json" ) -func EncodeCanonicalJson(obj map[string]interface{}) ([]byte, error) { +func EncodeCanonicalJson(obj any) ([]byte, error) { b, err := json.Marshal(obj) if err != nil { return nil, err diff --git a/util/http.go b/util/http.go index ac5144a4..0892f661 100644 --- a/util/http.go +++ b/util/http.go @@ -49,8 +49,7 @@ func GetLogSafeUrl(r *http.Request) string { return copyUrl.String() } -func GetXMatrixAuth(request *http.Request) ([]XMatrixAuth, error) { - headers := request.Header.Values("Authorization") +func GetXMatrixAuth(headers []string) ([]XMatrixAuth, error) { auths := make([]XMatrixAuth, 0) for _, h := range headers { if !strings.HasPrefix(h, "X-Matrix ") { From 535650651318b512ea62c0b8d65e81d15e2c6816 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 25 Nov 2023 20:58:23 -0700 Subject: [PATCH 06/51] Validate signing keys more correctly --- matrix/requests_signing.go | 93 ++++++++++++++++++++++---------------- test/signing_keys_test.go | 73 ++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 39 deletions(-) create mode 100644 test/signing_keys_test.go diff --git a/matrix/requests_signing.go b/matrix/requests_signing.go index 8694139f..57bb1677 100644 --- a/matrix/requests_signing.go +++ b/matrix/requests_signing.go @@ -20,7 +20,7 @@ type signingKey struct { Key string `json:"key"` } -type serverKeyResult struct { +type ServerKeyResult struct { ServerName string `json:"server_name"` ValidUntilTs int64 `json:"valid_until_ts"` VerifyKeys map[string]signingKey `json:"verify_keys"` // unpadded base64 @@ -83,7 +83,7 @@ func QuerySigningKeys(serverName string) (ServerSigningKeys, error) { if err = decoder.Decode(&raw); err != nil { return nil, err } - keyInfo := new(serverKeyResult) + keyInfo := new(ServerKeyResult) if err = raw.ApplyTo(keyInfo); err != nil { return nil, err } @@ -100,51 +100,66 @@ func QuerySigningKeys(serverName string) (ServerSigningKeys, error) { return nil, errors.New("returned server keys would expire too quickly") } - // Convert to something useful - serverKeys := make(ServerSigningKeys) - for keyId, keyObj := range keyInfo.VerifyKeys { - b, err := util.DecodeUnpaddedBase64String(keyObj.Key) - if err != nil { - return nil, errors.Join(fmt.Errorf("bad base64 for key ID '%s' for '%s'", keyId, serverName), err) - } - - serverKeys[keyId] = b + // Convert keys to something useful, and check signatures + serverKeys, err := CheckSigningKeySignatures(serverName, keyInfo, raw) + if err != nil { + return nil, err } - // Check signatures - if len(keyInfo.Signatures) == 0 || len(keyInfo.Signatures[serverName]) == 0 { - return nil, fmt.Errorf("missing signatures from '%s'", serverName) - } - delete(raw, "signatures") - canonical, err := util.EncodeCanonicalJson(raw) + // Cache & return (unlock was deferred) + signingKeyCache.Set(serverName, &serverKeys, cacheUntil) + return serverKeys, nil + }) + return keys, err +} + +func CheckSigningKeySignatures(serverName string, keyInfo *ServerKeyResult, raw database.AnonymousJson) (ServerSigningKeys, error) { + serverKeys := make(ServerSigningKeys) + for keyId, keyObj := range keyInfo.VerifyKeys { + b, err := util.DecodeUnpaddedBase64String(keyObj.Key) if err != nil { - return nil, err + return nil, errors.Join(fmt.Errorf("bad base64 for key ID '%s' for '%s'", keyId, serverName), err) } - for domain, sig := range keyInfo.Signatures { - if domain != serverName { - return nil, fmt.Errorf("unexpected signature from '%s' (expected '%s')", domain, serverName) - } - for keyId, b64 := range sig { - signatureBytes, err := util.DecodeUnpaddedBase64String(b64) - if err != nil { - return nil, errors.Join(fmt.Errorf("bad base64 signature for key ID '%s' for '%s'", keyId, serverName), err) - } + serverKeys[keyId] = b + } - key, ok := serverKeys[keyId] - if !ok { - return nil, fmt.Errorf("unknown key ID '%s' for signature from '%s'", keyId, serverName) - } + if len(keyInfo.Signatures) == 0 || len(keyInfo.Signatures[serverName]) == 0 { + return nil, fmt.Errorf("missing signatures from '%s'", serverName) + } + delete(raw, "signatures") + canonical, err := util.EncodeCanonicalJson(raw) + if err != nil { + return nil, err + } + for domain, sig := range keyInfo.Signatures { + if domain != serverName { + return nil, fmt.Errorf("unexpected signature from '%s' (expected '%s')", domain, serverName) + } + + for keyId, b64 := range sig { + signatureBytes, err := util.DecodeUnpaddedBase64String(b64) + if err != nil { + return nil, errors.Join(fmt.Errorf("bad base64 signature for key ID '%s' for '%s'", keyId, serverName), err) + } + + key, ok := serverKeys[keyId] + if !ok { + return nil, fmt.Errorf("unknown key ID '%s' for signature from '%s'", keyId, serverName) + } - if !ed25519.Verify(key, canonical, signatureBytes) { - return nil, fmt.Errorf("invalid signature '%s' from key ID '%s' for '%s'", b64, keyId, serverName) - } + if !ed25519.Verify(key, canonical, signatureBytes) { + return nil, fmt.Errorf("invalid signature '%s' from key ID '%s' for '%s'", b64, keyId, serverName) } } + } + + // Ensure *all* keys have signed the response + for keyId, _ := range serverKeys { + if _, ok := keyInfo.Signatures[serverName][keyId]; !ok { + return nil, fmt.Errorf("missing signature from key '%s'", keyId) + } + } - // Cache & return (unlock was deferred) - signingKeyCache.Set(serverName, &serverKeys, cacheUntil) - return serverKeys, nil - }) - return keys, err + return serverKeys, nil } diff --git a/test/signing_keys_test.go b/test/signing_keys_test.go new file mode 100644 index 00000000..13edf013 --- /dev/null +++ b/test/signing_keys_test.go @@ -0,0 +1,73 @@ +package test + +import ( + "crypto/ed25519" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/turt2live/matrix-media-repo/database" + "github.com/turt2live/matrix-media-repo/matrix" + "github.com/turt2live/matrix-media-repo/util" +) + +func TestFailInjectedKeys(t *testing.T) { + raw := database.AnonymousJson{ + "old_verify_keys": database.AnonymousJson{}, + "server_name": "x.resolvematrix.dev", + "signatures": database.AnonymousJson{ + "x.resolvematrix.dev": database.AnonymousJson{ + "ed25519:injected": "FB93YAF+fOPyWcsx285Q/xFzRiG5sr7/u1iX9XWaIcOwDyDDwx7daS1eYxuM9PfosWE5vqUyTsCxmB40JTzdCw", + }, + }, + "valid_until_ts": 1701055573679, + "verify_keys": database.AnonymousJson{ + "ed25519:AY4k3ADlto8": database.AnonymousJson{"key": "VF7dl9W/tFWAjZSXm42Ef22k3v4WKBYLXZF9I7ErU00"}, + "ed25519:injected": database.AnonymousJson{"key": "w48CLiV1IkWoEbqJLFmniGUYtxwT+c2zm87X8oEpRO8"}, + }, + } + keyInfo := new(matrix.ServerKeyResult) + err := raw.ApplyTo(keyInfo) + if err != nil { + t.Fatal(err) + } + + _, err = matrix.CheckSigningKeySignatures("x.resolvematrix.dev", keyInfo, raw) + assert.Error(t, err) + assert.Equal(t, "missing signature from key 'ed25519:AY4k3ADlto8'", err.Error()) +} + +func TestRegularKeys(t *testing.T) { + raw := database.AnonymousJson{ + "old_verify_keys": database.AnonymousJson{}, + "server_name": "x.resolvematrix.dev", + "signatures": database.AnonymousJson{ + "x.resolvematrix.dev": database.AnonymousJson{ + "ed25519:AY4k3ADlto8": "3WlsmHFTVjywCoDYyrtx3ies+VufTuBuw1Prlgmoqh+a4XrJT+isEwhTX+I5FBvtJTKTt6vLH3gaP7BA6712CA", + }, + }, + "valid_until_ts": 1701057124839, + "verify_keys": database.AnonymousJson{ + "ed25519:AY4k3ADlto8": database.AnonymousJson{"key": "VF7dl9W/tFWAjZSXm42Ef22k3v4WKBYLXZF9I7ErU00"}, + }, + } + keyInfo := new(matrix.ServerKeyResult) + err := raw.ApplyTo(keyInfo) + if err != nil { + t.Fatal(err) + } + + keys, err := matrix.CheckSigningKeySignatures("x.resolvematrix.dev", keyInfo, raw) + assert.NoError(t, err) + for keyId, keyVal := range keys { + if b64, ok := keyInfo.VerifyKeys[keyId]; !ok { + t.Errorf("got key for '%s' but wasn't expecting it", keyId) + } else { + keySelf, err := util.DecodeUnpaddedBase64String(b64.Key) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, ed25519.PublicKey(keySelf), keyVal) + } + } +} From 3fb16f56313f449416a1165fe13c4a23d230e34a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sun, 26 Nov 2023 22:53:33 -0700 Subject: [PATCH 07/51] Add early documentation for what this setup will look like --- docs/msc3916.md | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 docs/msc3916.md diff --git a/docs/msc3916.md b/docs/msc3916.md new file mode 100644 index 00000000..c1f9755e --- /dev/null +++ b/docs/msc3916.md @@ -0,0 +1,51 @@ +# MSC3916 Support in MMR + +[MSC3916](https://github.com/matrix-org/matrix-spec-proposals/pull/3916) is a proposal to add new endpoints to the Matrix +Client-Server API for authenticated media downloads. The MSC itself does not implement restrictions on who can download +a given piece of media, but does require that both users and servers identify themselves when downloading media. + +MMR supports the MSC by default, allowing the following additional endpoints to be routed to it: + +* `GET /_matrix/client/unstable/org.matrix.msc3916/media/download/:origin/:mediaId` +* `GET /_matrix/client/unstable/org.matrix.msc3916/media/download/:origin/:mediaId/:fileName` +* `GET /_matrix/client/unstable/org.matrix.msc3916/media/thumbnail/:origin/:mediaId` +* `GET /_matrix/client/unstable/org.matrix.msc3916/media/preview_url` +* `GET /_matrix/client/unstable/org.matrix.msc3916/media/config` +* `GET /_matrix/federation/unstable/org.matrix.msc3916/media/download/:origin/:mediaId` +* `GET /_matrix/federation/unstable/org.matrix.msc3916/media/thumbnail/:origin/:mediaId` + +Note that the new endpoints are *not* in the traditional `/_matrix/media` namespace. Note also that the upload endpoint +does not appear in this list - the endpoint is modified by [MSC3911](https://github.com/matrix-org/matrix-spec-proposals/pull/3911) +instead. + +MMR will allow the new `/_matrix/client` endpoints to be used if it is configured with a signing key for the server the +request is being made to. If MMR is not configured with a signing key, clients will receive the normal "not implemented" +error responses. When a signing key is configured for the server, MMR will *always* try to use the new federation endpoints +to download media, regardless as to how it was approached. This will cause a fallback to the legacy `/_matrix/media` +endpoints if the remote server does not support the new endpoints. + +**It is strongly recommended that a signing key be configured.** Future versions of MMR may refuse to start up without a +signing key configured. + +To set up a signing key for MMR: + +1. Back up your existing homeserver signing key, and store it in a safe place. The signing key effectively grants full + access to your server and events, and should not be disclosed to anyone. +2. Download the `generate_signing_key` and `combine_signing_keys` tools for your version of MMR from the + [GitHub releases page](https://github.com/turt2live/matrix-media-repo/releases). +3. Run `./generate_signing_key -output mmr.signing.key` to create a signing key usable with MMR. +4. If you're using Synapse as your homeserver software, run `./combine_signing_keys -format synapse -output ./merged.signing.key ./existing.signing.key ./mmr.signing.key` + to combine the signing keys, being sure to list Synapse's existing signing key *first* in the arguments. For all other + homeserver software, please consult the homeserver documentation for how to deploy multiple signing keys. Note that + not all homeserver software options support multiple signing keys. +5. Run `cat ./merged.signing.key` to verify that your existing signing key ID is on the first line. You can get your key + ID from `GET /_matrix/key/v2/server` against your homeserver in a web browser. If your existing signing key is *not* + first, re-run the steps above, noting the order of keys supplied to `./combine_signing_keys` is important. +6. Deploy `./merged.signing.key` to your Synapse server in place of the existing signing key, restarting it. +7. Deploy `./mmr.signing.key` alongside MMR and specified as `signingKeyPath` for that homeserver in your MMR config. + +In the event that you ever need to revoke MMR's signing key from your homeserver, restore your signing key from the most +recent backup. If your homeserver's signing key changes after running the above steps, re-run the steps above to set up +your server with the new key. Note that it's considered good practice to list your old signing keys, including MMR's +revoked keys, under `old_verify_keys` on `GET /_matrix/key/v2/server` - many homeservers offer a config option to +populate this field. From beac841da93e63ef225bf86aa73ca632b6fff840 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 9 Feb 2024 22:13:44 -0700 Subject: [PATCH 08/51] Fix imports --- api/_routers/97-require-server-auth.go | 10 +++++----- api/unstable/msc3916_download.go | 10 +++++----- api/unstable/msc3916_thumbnail.go | 10 +++++----- docs/msc3916.md | 2 +- matrix/requests_signing.go | 6 +++--- matrix/xmatrix.go | 4 ++-- test/msc3916_downloads_suite_test.go | 4 ++-- test/msc3916_misc_client_endpoints_suite_test.go | 2 +- test/msc3916_thumbnails_suite_test.go | 4 ++-- test/signing_keys_test.go | 6 +++--- test/xmatrix_header_test.go | 8 ++++---- 11 files changed, 33 insertions(+), 33 deletions(-) diff --git a/api/_routers/97-require-server-auth.go b/api/_routers/97-require-server-auth.go index b7ca3bac..0b128523 100644 --- a/api/_routers/97-require-server-auth.go +++ b/api/_routers/97-require-server-auth.go @@ -3,11 +3,11 @@ package _routers import ( "net/http" - "github.com/turt2live/matrix-media-repo/api/_apimeta" - "github.com/turt2live/matrix-media-repo/api/_responses" - "github.com/turt2live/matrix-media-repo/common" - "github.com/turt2live/matrix-media-repo/common/rcontext" - "github.com/turt2live/matrix-media-repo/matrix" + "github.com/t2bot/matrix-media-repo/api/_apimeta" + "github.com/t2bot/matrix-media-repo/api/_responses" + "github.com/t2bot/matrix-media-repo/common" + "github.com/t2bot/matrix-media-repo/common/rcontext" + "github.com/t2bot/matrix-media-repo/matrix" ) type GeneratorWithServerFn = func(r *http.Request, ctx rcontext.RequestContext, server _apimeta.ServerInfo) interface{} diff --git a/api/unstable/msc3916_download.go b/api/unstable/msc3916_download.go index 64d9d5b1..6089976a 100644 --- a/api/unstable/msc3916_download.go +++ b/api/unstable/msc3916_download.go @@ -4,11 +4,11 @@ import ( "bytes" "net/http" - "github.com/turt2live/matrix-media-repo/api/_apimeta" - "github.com/turt2live/matrix-media-repo/api/_responses" - "github.com/turt2live/matrix-media-repo/api/r0" - "github.com/turt2live/matrix-media-repo/common/rcontext" - "github.com/turt2live/matrix-media-repo/util/readers" + "github.com/t2bot/matrix-media-repo/api/_apimeta" + "github.com/t2bot/matrix-media-repo/api/_responses" + "github.com/t2bot/matrix-media-repo/api/r0" + "github.com/t2bot/matrix-media-repo/common/rcontext" + "github.com/t2bot/matrix-media-repo/util/readers" ) func ClientDownloadMedia(r *http.Request, rctx rcontext.RequestContext, user _apimeta.UserInfo) interface{} { diff --git a/api/unstable/msc3916_thumbnail.go b/api/unstable/msc3916_thumbnail.go index 81b77d20..e6de68ec 100644 --- a/api/unstable/msc3916_thumbnail.go +++ b/api/unstable/msc3916_thumbnail.go @@ -4,11 +4,11 @@ import ( "bytes" "net/http" - "github.com/turt2live/matrix-media-repo/api/_apimeta" - "github.com/turt2live/matrix-media-repo/api/_responses" - "github.com/turt2live/matrix-media-repo/api/r0" - "github.com/turt2live/matrix-media-repo/common/rcontext" - "github.com/turt2live/matrix-media-repo/util/readers" + "github.com/t2bot/matrix-media-repo/api/_apimeta" + "github.com/t2bot/matrix-media-repo/api/_responses" + "github.com/t2bot/matrix-media-repo/api/r0" + "github.com/t2bot/matrix-media-repo/common/rcontext" + "github.com/t2bot/matrix-media-repo/util/readers" ) func FederationThumbnailMedia(r *http.Request, rctx rcontext.RequestContext, server _apimeta.ServerInfo) interface{} { diff --git a/docs/msc3916.md b/docs/msc3916.md index c1f9755e..5d41d293 100644 --- a/docs/msc3916.md +++ b/docs/msc3916.md @@ -32,7 +32,7 @@ To set up a signing key for MMR: 1. Back up your existing homeserver signing key, and store it in a safe place. The signing key effectively grants full access to your server and events, and should not be disclosed to anyone. 2. Download the `generate_signing_key` and `combine_signing_keys` tools for your version of MMR from the - [GitHub releases page](https://github.com/turt2live/matrix-media-repo/releases). + [GitHub releases page](https://github.com/t2bot/matrix-media-repo/releases). 3. Run `./generate_signing_key -output mmr.signing.key` to create a signing key usable with MMR. 4. If you're using Synapse as your homeserver software, run `./combine_signing_keys -format synapse -output ./merged.signing.key ./existing.signing.key ./mmr.signing.key` to combine the signing keys, being sure to list Synapse's existing signing key *first* in the arguments. For all other diff --git a/matrix/requests_signing.go b/matrix/requests_signing.go index 57bb1677..f013f047 100644 --- a/matrix/requests_signing.go +++ b/matrix/requests_signing.go @@ -11,9 +11,9 @@ import ( "github.com/patrickmn/go-cache" "github.com/sirupsen/logrus" "github.com/t2bot/go-typed-singleflight" - "github.com/turt2live/matrix-media-repo/common/rcontext" - "github.com/turt2live/matrix-media-repo/database" - "github.com/turt2live/matrix-media-repo/util" + "github.com/t2bot/matrix-media-repo/common/rcontext" + "github.com/t2bot/matrix-media-repo/database" + "github.com/t2bot/matrix-media-repo/util" ) type signingKey struct { diff --git a/matrix/xmatrix.go b/matrix/xmatrix.go index 7b9889d3..dd8c662e 100644 --- a/matrix/xmatrix.go +++ b/matrix/xmatrix.go @@ -6,8 +6,8 @@ import ( "fmt" "net/http" - "github.com/turt2live/matrix-media-repo/database" - "github.com/turt2live/matrix-media-repo/util" + "github.com/t2bot/matrix-media-repo/database" + "github.com/t2bot/matrix-media-repo/util" ) var ErrNoXMatrixAuth = errors.New("no X-Matrix auth headers") diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index fc0edf83..5cc12256 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -8,8 +8,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" - "github.com/turt2live/matrix-media-repo/test/test_internals" - "github.com/turt2live/matrix-media-repo/util" + "github.com/t2bot/matrix-media-repo/test/test_internals" + "github.com/t2bot/matrix-media-repo/util" ) type MSC3916DownloadsSuite struct { diff --git a/test/msc3916_misc_client_endpoints_suite_test.go b/test/msc3916_misc_client_endpoints_suite_test.go index 3d392b6b..dac970e3 100644 --- a/test/msc3916_misc_client_endpoints_suite_test.go +++ b/test/msc3916_misc_client_endpoints_suite_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" - "github.com/turt2live/matrix-media-repo/test/test_internals" + "github.com/t2bot/matrix-media-repo/test/test_internals" ) type MSC3916MiscClientEndpointsSuite struct { diff --git a/test/msc3916_thumbnails_suite_test.go b/test/msc3916_thumbnails_suite_test.go index 43cf5c77..89132fac 100644 --- a/test/msc3916_thumbnails_suite_test.go +++ b/test/msc3916_thumbnails_suite_test.go @@ -9,8 +9,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" - "github.com/turt2live/matrix-media-repo/test/test_internals" - "github.com/turt2live/matrix-media-repo/util" + "github.com/t2bot/matrix-media-repo/test/test_internals" + "github.com/t2bot/matrix-media-repo/util" ) type MSC3916ThumbnailsSuite struct { diff --git a/test/signing_keys_test.go b/test/signing_keys_test.go index 13edf013..b23ad54b 100644 --- a/test/signing_keys_test.go +++ b/test/signing_keys_test.go @@ -5,9 +5,9 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/turt2live/matrix-media-repo/database" - "github.com/turt2live/matrix-media-repo/matrix" - "github.com/turt2live/matrix-media-repo/util" + "github.com/t2bot/matrix-media-repo/database" + "github.com/t2bot/matrix-media-repo/matrix" + "github.com/t2bot/matrix-media-repo/util" ) func TestFailInjectedKeys(t *testing.T) { diff --git a/test/xmatrix_header_test.go b/test/xmatrix_header_test.go index 17019108..e3038c39 100644 --- a/test/xmatrix_header_test.go +++ b/test/xmatrix_header_test.go @@ -4,10 +4,10 @@ import ( "crypto/ed25519" "testing" - "github.com/turt2live/matrix-media-repo/common/config" - "github.com/turt2live/matrix-media-repo/database" - "github.com/turt2live/matrix-media-repo/matrix" - "github.com/turt2live/matrix-media-repo/util" + "github.com/t2bot/matrix-media-repo/common/config" + "github.com/t2bot/matrix-media-repo/database" + "github.com/t2bot/matrix-media-repo/matrix" + "github.com/t2bot/matrix-media-repo/util" ) func TestXMatrixAuthHeader(t *testing.T) { From 5a51564e3404a4cbf34fd39181e9f55f6c72ee86 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 19 Feb 2024 23:16:35 -0700 Subject: [PATCH 09/51] Update tests --- test/xmatrix_header_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/xmatrix_header_test.go b/test/xmatrix_header_test.go index e3038c39..8ec41175 100644 --- a/test/xmatrix_header_test.go +++ b/test/xmatrix_header_test.go @@ -4,6 +4,7 @@ import ( "crypto/ed25519" "testing" + "github.com/stretchr/testify/assert" "github.com/t2bot/matrix-media-repo/common/config" "github.com/t2bot/matrix-media-repo/database" "github.com/t2bot/matrix-media-repo/matrix" @@ -31,7 +32,5 @@ func TestXMatrixAuthHeader(t *testing.T) { keys := make(matrix.ServerSigningKeys) keys["ed25519:0"] = pub err = matrix.ValidateXMatrixAuthHeader("GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, auths, keys) - if err != nil { - t.Error(err) - } + assert.NoError(t, err) } From 4adc55ae8d9903c7464141aac6462a3042be963e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 19 Feb 2024 23:16:42 -0700 Subject: [PATCH 10/51] Add resolvematrix.dev tests --- test/matrix_resolve_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 test/matrix_resolve_test.go diff --git a/test/matrix_resolve_test.go b/test/matrix_resolve_test.go new file mode 100644 index 00000000..e62da699 --- /dev/null +++ b/test/matrix_resolve_test.go @@ -0,0 +1,26 @@ +package test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/t2bot/matrix-media-repo/matrix" +) + +func doResolve(t *testing.T, origin string, expectedAddress string, expectedHost string) { + url, host, err := matrix.GetServerApiUrl(origin) + assert.NoError(t, err, origin) + assert.Equal(t, expectedAddress, url, origin) + assert.Equal(t, expectedHost, host, origin) +} + +func TestResolveMatrix(t *testing.T) { + doResolve(t, "2.s.resolvematrix.dev:7652", "https://2.s.resolvematrix.dev:7652", "2.s.resolvematrix.dev") + doResolve(t, "3b.s.resolvematrix.dev", "https://wk.3b.s.resolvematrix.dev:7753", "wk.3b.s.resolvematrix.dev:7753") + doResolve(t, "3c.s.resolvematrix.dev", "https://srv.wk.3c.s.resolvematrix.dev:7754", "wk.3c.s.resolvematrix.dev") + doResolve(t, "3d.s.resolvematrix.dev", "https://wk.3d.s.resolvematrix.dev:8448", "wk.3d.s.resolvematrix.dev") + doResolve(t, "4.s.resolvematrix.dev", "https://srv.4.s.resolvematrix.dev:7855", "4.s.resolvematrix.dev") + doResolve(t, "5.s.resolvematrix.dev", "https://5.s.resolvematrix.dev:8448", "5.s.resolvematrix.dev") + doResolve(t, "3c.msc4040.s.resolvematrix.dev", "https://srv.wk.3c.msc4040.s.resolvematrix.dev:7053", "wk.3c.msc4040.s.resolvematrix.dev") + doResolve(t, "4.msc4040.s.resolvematrix.dev", "https://srv.4.msc4040.s.resolvematrix.dev:7054", "4.msc4040.s.resolvematrix.dev") +} From 05ac6a8035d241a69dbbc85d86062684e9b6f503 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 19 Feb 2024 23:22:37 -0700 Subject: [PATCH 11/51] Fix URL preview test --- ...sc3916_misc_client_endpoints_suite_test.go | 4 +++ test/test_internals/deps.go | 32 +++++++++++-------- test/test_internals/inline_dep_host_file.go | 11 +++++++ 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/test/msc3916_misc_client_endpoints_suite_test.go b/test/msc3916_misc_client_endpoints_suite_test.go index dac970e3..2aaa2cb3 100644 --- a/test/msc3916_misc_client_endpoints_suite_test.go +++ b/test/msc3916_misc_client_endpoints_suite_test.go @@ -33,6 +33,10 @@ func (s *MSC3916MiscClientEndpointsSuite) SetupSuite() { func (s *MSC3916MiscClientEndpointsSuite) TearDownSuite() { if s.htmlPage != nil { + if s.T().Failed() { + staticLogs, err := s.htmlPage.Logs() + s.deps.DumpDebugLogs(staticLogs, err, -1, s.htmlPage.PublicUrl) + } s.htmlPage.Teardown() } if s.deps != nil { diff --git a/test/test_internals/deps.go b/test/test_internals/deps.go index 18ba27dd..da0c65db 100644 --- a/test/test_internals/deps.go +++ b/test/test_internals/deps.go @@ -182,19 +182,23 @@ func (c *ContainerDeps) Teardown() { func (c *ContainerDeps) Debug() { for i, m := range c.Machines { logs, err := m.Logs() - if err != nil { - log.Fatal(err) - } - b, err := io.ReadAll(logs) - if err != nil { - log.Fatal(err) - } - fmt.Printf("[MMR Deps] Logs from index %d (%s)", i, m.HttpUrl) - fmt.Println() - fmt.Println(string(b)) - err = logs.Close() - if err != nil { - log.Fatal(err) - } + c.DumpDebugLogs(logs, err, i, m.HttpUrl) + } +} + +func (c *ContainerDeps) DumpDebugLogs(logs io.ReadCloser, err error, i int, url string) { + if err != nil { + log.Fatal(err) + } + b, err := io.ReadAll(logs) + if err != nil { + log.Fatal(err) + } + fmt.Printf("[MMR Deps] Logs from index %d (%s)", i, url) + fmt.Println() + fmt.Println(string(b)) + err = logs.Close() + if err != nil { + log.Fatal(err) } } diff --git a/test/test_internals/inline_dep_host_file.go b/test/test_internals/inline_dep_host_file.go index dd486052..5fc9cf17 100644 --- a/test/test_internals/inline_dep_host_file.go +++ b/test/test_internals/inline_dep_host_file.go @@ -1,7 +1,9 @@ package test_internals import ( + "context" "fmt" + "io" "log" "os" "path" @@ -24,6 +26,11 @@ func ServeFile(fileName string, deps *ContainerDeps, contents string) (*HostedFi return nil, err } + err = os.Chmod(tmp, 0755) + if err != nil { + return nil, err + } + f, err := os.Create(path.Join(tmp, fileName)) if err != nil { return nil, err @@ -80,3 +87,7 @@ func (f *HostedFile) Teardown() { log.Fatalf("Error cleaning up temporarily hosted file: %s", err.Error()) } } + +func (f *HostedFile) Logs() (io.ReadCloser, error) { + return f.nginx.Logs(context.Background()) +} From 6aae7dee5cfcef9d0986377cb61de35c7217e1cd Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 11 Apr 2024 10:42:39 -0600 Subject: [PATCH 12/51] Support receiving `/versions` and enabling MSC3916 support --- api/r0/versions.go | 26 ++++++++++++++++++++++++++ api/routes.go | 5 +++++ dev/homeserver.nginx.conf | 10 ++++++++++ matrix/breakers.go | 4 ---- matrix/requests_info.go | 17 +++++++++++++++++ 5 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 api/r0/versions.go create mode 100644 matrix/requests_info.go diff --git a/api/r0/versions.go b/api/r0/versions.go new file mode 100644 index 00000000..023f9a66 --- /dev/null +++ b/api/r0/versions.go @@ -0,0 +1,26 @@ +package r0 + +import ( + "github.com/getsentry/sentry-go" + "github.com/t2bot/matrix-media-repo/api/_apimeta" + "github.com/t2bot/matrix-media-repo/api/_responses" + "github.com/t2bot/matrix-media-repo/matrix" + + "net/http" + + "github.com/t2bot/matrix-media-repo/common/rcontext" +) + +func ClientVersions(r *http.Request, rctx rcontext.RequestContext, user _apimeta.UserInfo) interface{} { + versions, err := matrix.ClientVersions(rctx, r.Host, user.UserId, user.AccessToken, r.RemoteAddr) + if err != nil { + rctx.Log.Error(err) + sentry.CaptureException(err) + return _responses.InternalServerError("unable to get versions") + } + if versions.UnstableFeatures == nil { + versions.UnstableFeatures = make(map[string]bool) + } + versions.UnstableFeatures["org.matrix.msc3916"] = true + return versions +} diff --git a/api/routes.go b/api/routes.go index d30feea9..d7393a52 100644 --- a/api/routes.go +++ b/api/routes.go @@ -45,6 +45,7 @@ func buildRoutes() http.Handler { register([]string{"POST"}, PrefixClient, "logout", mxSpecV3TransitionCS, router, makeRoute(_routers.RequireAccessToken(r0.Logout), "logout", counter)) register([]string{"POST"}, PrefixClient, "logout/all", mxSpecV3TransitionCS, router, makeRoute(_routers.RequireAccessToken(r0.LogoutAll), "logout_all", counter)) register([]string{"POST"}, PrefixMedia, "create", mxV1, router, makeRoute(_routers.RequireAccessToken(v1.CreateMedia), "create", counter)) + register([]string{"GET"}, PrefixClient, "versions", mxNoVersion, router, makeRoute(_routers.OptionalAccessToken(r0.ClientVersions), "client_versions", counter)) // MSC3916 - Authentication & endpoint API separation register([]string{"GET"}, PrefixClient, "media/preview_url", msc3916, router, previewUrlRoute) @@ -148,12 +149,16 @@ var ( mxR0 matrixVersions = []string{"r0"} mxV1 matrixVersions = []string{"v1"} mxV3 matrixVersions = []string{"v3"} + mxNoVersion matrixVersions = []string{""} ) func register(methods []string, prefix string, postfix string, versions matrixVersions, router *httprouter.Router, handler http.Handler) { for _, method := range methods { for _, version := range versions { path := fmt.Sprintf("%s/%s/%s", prefix, version, postfix) + if version == "" { + path = fmt.Sprintf("%s/%s", prefix, postfix) + } router.Handler(method, path, http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { defer func() { // hopefully the body was already closed, but maybe it wasn't diff --git a/dev/homeserver.nginx.conf b/dev/homeserver.nginx.conf index 10f39142..8a975aa1 100644 --- a/dev/homeserver.nginx.conf +++ b/dev/homeserver.nginx.conf @@ -9,6 +9,16 @@ server { proxy_pass http://host.docker.internal:8001; } + location /_matrix/client/versions { + proxy_set_header Host localhost; + proxy_pass http://host.docker.internal:8001; + } + + location /_matrix/client/unstable/org.matrix.msc3916 { + proxy_set_header Host localhost; + proxy_pass http://host.docker.internal:8001; + } + location /_matrix { proxy_pass http://media_repo_synapse:8008; } diff --git a/matrix/breakers.go b/matrix/breakers.go index 5f6ee6a9..911f92f2 100644 --- a/matrix/breakers.go +++ b/matrix/breakers.go @@ -50,10 +50,6 @@ func getFederationBreaker(hostname string) *circuit.Breaker { } func doBreakerRequest(ctx rcontext.RequestContext, serverName string, accessToken string, appserviceUserId string, ipAddr string, method string, path string, resp interface{}) error { - if accessToken == "" { - return ErrInvalidToken - } - hs, cb := getBreakerAndConfig(serverName) var replyError error diff --git a/matrix/requests_info.go b/matrix/requests_info.go new file mode 100644 index 00000000..c2cc1de0 --- /dev/null +++ b/matrix/requests_info.go @@ -0,0 +1,17 @@ +package matrix + +import "github.com/t2bot/matrix-media-repo/common/rcontext" + +type ClientVersionsResponse struct { + Versions []string `json:"versions"` + UnstableFeatures map[string]bool `json:"unstable_features"` +} + +func ClientVersions(ctx rcontext.RequestContext, serverName string, accessToken string, appserviceUserId string, ipAddr string) (*ClientVersionsResponse, error) { + response := &ClientVersionsResponse{} + err := doBreakerRequest(ctx, serverName, accessToken, appserviceUserId, ipAddr, "GET", "/_matrix/client/versions", response) + if err != nil { + return nil, err + } + return response, nil +} From 177389139abcd52e18511126328310ddb9dee8a7 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 1 May 2024 13:27:36 -0600 Subject: [PATCH 13/51] Remove placeholder docs --- docs/msc3916.md | 51 ------------------------------------------------- 1 file changed, 51 deletions(-) delete mode 100644 docs/msc3916.md diff --git a/docs/msc3916.md b/docs/msc3916.md deleted file mode 100644 index 5d41d293..00000000 --- a/docs/msc3916.md +++ /dev/null @@ -1,51 +0,0 @@ -# MSC3916 Support in MMR - -[MSC3916](https://github.com/matrix-org/matrix-spec-proposals/pull/3916) is a proposal to add new endpoints to the Matrix -Client-Server API for authenticated media downloads. The MSC itself does not implement restrictions on who can download -a given piece of media, but does require that both users and servers identify themselves when downloading media. - -MMR supports the MSC by default, allowing the following additional endpoints to be routed to it: - -* `GET /_matrix/client/unstable/org.matrix.msc3916/media/download/:origin/:mediaId` -* `GET /_matrix/client/unstable/org.matrix.msc3916/media/download/:origin/:mediaId/:fileName` -* `GET /_matrix/client/unstable/org.matrix.msc3916/media/thumbnail/:origin/:mediaId` -* `GET /_matrix/client/unstable/org.matrix.msc3916/media/preview_url` -* `GET /_matrix/client/unstable/org.matrix.msc3916/media/config` -* `GET /_matrix/federation/unstable/org.matrix.msc3916/media/download/:origin/:mediaId` -* `GET /_matrix/federation/unstable/org.matrix.msc3916/media/thumbnail/:origin/:mediaId` - -Note that the new endpoints are *not* in the traditional `/_matrix/media` namespace. Note also that the upload endpoint -does not appear in this list - the endpoint is modified by [MSC3911](https://github.com/matrix-org/matrix-spec-proposals/pull/3911) -instead. - -MMR will allow the new `/_matrix/client` endpoints to be used if it is configured with a signing key for the server the -request is being made to. If MMR is not configured with a signing key, clients will receive the normal "not implemented" -error responses. When a signing key is configured for the server, MMR will *always* try to use the new federation endpoints -to download media, regardless as to how it was approached. This will cause a fallback to the legacy `/_matrix/media` -endpoints if the remote server does not support the new endpoints. - -**It is strongly recommended that a signing key be configured.** Future versions of MMR may refuse to start up without a -signing key configured. - -To set up a signing key for MMR: - -1. Back up your existing homeserver signing key, and store it in a safe place. The signing key effectively grants full - access to your server and events, and should not be disclosed to anyone. -2. Download the `generate_signing_key` and `combine_signing_keys` tools for your version of MMR from the - [GitHub releases page](https://github.com/t2bot/matrix-media-repo/releases). -3. Run `./generate_signing_key -output mmr.signing.key` to create a signing key usable with MMR. -4. If you're using Synapse as your homeserver software, run `./combine_signing_keys -format synapse -output ./merged.signing.key ./existing.signing.key ./mmr.signing.key` - to combine the signing keys, being sure to list Synapse's existing signing key *first* in the arguments. For all other - homeserver software, please consult the homeserver documentation for how to deploy multiple signing keys. Note that - not all homeserver software options support multiple signing keys. -5. Run `cat ./merged.signing.key` to verify that your existing signing key ID is on the first line. You can get your key - ID from `GET /_matrix/key/v2/server` against your homeserver in a web browser. If your existing signing key is *not* - first, re-run the steps above, noting the order of keys supplied to `./combine_signing_keys` is important. -6. Deploy `./merged.signing.key` to your Synapse server in place of the existing signing key, restarting it. -7. Deploy `./mmr.signing.key` alongside MMR and specified as `signingKeyPath` for that homeserver in your MMR config. - -In the event that you ever need to revoke MMR's signing key from your homeserver, restore your signing key from the most -recent backup. If your homeserver's signing key changes after running the above steps, re-run the steps above to set up -your server with the new key. Note that it's considered good practice to list your old signing keys, including MMR's -revoked keys, under `old_verify_keys` on `GET /_matrix/key/v2/server` - many homeservers offer a config option to -populate this field. From 9f4b29fff8d9be37b1909cfc166b94bbf9cc3095 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 1 May 2024 16:08:49 -0600 Subject: [PATCH 14/51] Make outbound federation requests using MSC3916 --- CHANGELOG.md | 5 +- api/custom/federation.go | 2 +- cmd/workers/media_repo/reloads.go | 18 ++++ common/config/conf_domain.go | 1 + common/config/models_main.go | 1 + common/config/watch.go | 5 +- common/errorcodes.go | 1 + common/globals/reload.go | 1 + config.sample.yaml | 5 + matrix/errors.go | 18 +++- matrix/requests.go | 29 +++++- matrix/requests_signing.go | 2 +- matrix/signing_key_cache.go | 43 ++++++++ pipelines/_steps/download/try_download.go | 115 +++++++++++++++++++--- pipelines/pipeline_download/pipeline.go | 9 ++ test/xmatrix_header_test.go | 28 +++++- util/matrix_media_part.go | 27 +++++ 17 files changed, 284 insertions(+), 26 deletions(-) create mode 100644 matrix/signing_key_cache.go create mode 100644 util/matrix_media_part.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 94eacab4..99d51326 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,9 +14,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * S3 datastores can now specify a `prefixLength` to improve S3 performance on some providers. See `config.sample.yaml` for details. * Add `multipartUploads` flag for running MMR against unsupported S3 providers. See `config.sample.yaml` for details. * A new "leaky bucket" rate limit algorithm has been applied to downloads. See `rateLimit.buckets` in the config for details. -* Add *unstable* support for [MSC3916: Authentication for media](https://github.com/matrix-org/matrix-spec-proposals/pull/3916). - * **Note**: MMR will *not* attempt to use authentication to download media over federation in this version. - * ***Subject to change during development.*** +* Add *unstable* support for [MSC3916: Authentication for media](https://github.com/matrix-org/matrix-spec-proposals/pull/3916). + * To enable full support, use `signingKeyPath` in your config. See sample config for details. ### Changed diff --git a/api/custom/federation.go b/api/custom/federation.go index ceee40a4..e92a1393 100644 --- a/api/custom/federation.go +++ b/api/custom/federation.go @@ -33,7 +33,7 @@ func GetFederationInfo(r *http.Request, rctx rcontext.RequestContext, user _apim } versionUrl := url + "/_matrix/federation/v1/version" - versionResponse, err := matrix.FederatedGet(versionUrl, hostname, rctx) + versionResponse, err := matrix.FederatedGet(rctx, versionUrl, hostname, matrix.NoSigningKey) if versionResponse != nil { defer versionResponse.Body.Close() } diff --git a/cmd/workers/media_repo/reloads.go b/cmd/workers/media_repo/reloads.go index dbf201c5..4dbde321 100644 --- a/cmd/workers/media_repo/reloads.go +++ b/cmd/workers/media_repo/reloads.go @@ -10,6 +10,7 @@ import ( "github.com/t2bot/matrix-media-repo/database" "github.com/t2bot/matrix-media-repo/errcache" "github.com/t2bot/matrix-media-repo/limits" + "github.com/t2bot/matrix-media-repo/matrix" "github.com/t2bot/matrix-media-repo/metrics" "github.com/t2bot/matrix-media-repo/pgo_internal" "github.com/t2bot/matrix-media-repo/plugins" @@ -29,6 +30,7 @@ func setupReloads() { reloadPluginsOnChan(globals.PluginReloadChan) reloadPoolOnChan(globals.PoolReloadChan) reloadErrorCachesOnChan(globals.ErrorCacheReloadChan) + reloadMatrixCachesOnChan(globals.MatrixCachesReloadChan) reloadPGOOnChan(globals.PGOReloadChan) reloadBucketsOnChan(globals.BucketsReloadChan) } @@ -55,6 +57,8 @@ func stopReloads() { globals.PoolReloadChan <- false logrus.Debug("Stopping ErrorCacheReloadChan") globals.ErrorCacheReloadChan <- false + logrus.Debug("Stopping MatrixCachesReloadChan") + globals.MatrixCachesReloadChan <- false logrus.Debug("Stopping PGOReloadChan") globals.PGOReloadChan <- false logrus.Debug("Stopping BucketsReloadChan") @@ -207,6 +211,20 @@ func reloadErrorCachesOnChan(reloadChan chan bool) { }() } +func reloadMatrixCachesOnChan(reloadChan chan bool) { + go func() { + defer close(reloadChan) + for { + shouldReload := <-reloadChan + if shouldReload { + matrix.FlushSigningKeyCache() + } else { + return // received stop + } + } + }() +} + func reloadPGOOnChan(reloadChan chan bool) { go func() { defer close(reloadChan) diff --git a/common/config/conf_domain.go b/common/config/conf_domain.go index f7784f43..5443efce 100644 --- a/common/config/conf_domain.go +++ b/common/config/conf_domain.go @@ -16,6 +16,7 @@ func NewDefaultDomainConfig() DomainRepoConfig { ClientServerApi: "https://UNDEFINED", BackoffAt: 10, AdminApiKind: "matrix", + SigningKeyPath: "", }, Downloads: DownloadsConfig{ MaxSizeBytes: 104857600, // 100mb diff --git a/common/config/models_main.go b/common/config/models_main.go index 4f4a89f8..6b80d272 100644 --- a/common/config/models_main.go +++ b/common/config/models_main.go @@ -16,6 +16,7 @@ type HomeserverConfig struct { ClientServerApi string `yaml:"csApi"` BackoffAt int `yaml:"backoffAt"` AdminApiKind string `yaml:"adminApiKind"` + SigningKeyPath string `yaml:"signingKeyPath"` } type DatabaseConfig struct { diff --git a/common/config/watch.go b/common/config/watch.go index 6ba7f910..4ad281cd 100644 --- a/common/config/watch.go +++ b/common/config/watch.go @@ -61,10 +61,13 @@ func onFileChanged() { PrintDomainInfo() CheckDeprecations() - logrus.Info("Reloading pool & cache configuration") + logrus.Info("Reloading pool & error cache configurations") globals.PoolReloadChan <- true globals.ErrorCacheReloadChan <- true + logrus.Info("Reloading matrix caches") + globals.MatrixCachesReloadChan <- true + bindAddressChange := configNew.General.BindAddress != configNow.General.BindAddress bindPortChange := configNew.General.Port != configNow.General.Port forwardAddressChange := configNew.General.TrustAnyForward != configNow.General.TrustAnyForward diff --git a/common/errorcodes.go b/common/errorcodes.go index 2ba7fca2..a04aeff3 100644 --- a/common/errorcodes.go +++ b/common/errorcodes.go @@ -13,6 +13,7 @@ const ErrCodeBadRequest = "M_BAD_REQUEST" const ErrCodeRateLimitExceeded = "M_LIMIT_EXCEEDED" const ErrCodeUnknown = "M_UNKNOWN" const ErrCodeForbidden = "M_FORBIDDEN" +const ErrCodeUnauthorized = "M_UNAUTHORIZED" const ErrCodeQuotaExceeded = "M_QUOTA_EXCEEDED" const ErrCodeCannotOverwrite = "M_CANNOT_OVERWRITE_MEDIA" const ErrCodeNotYetUploaded = "M_NOT_YET_UPLOADED" diff --git a/common/globals/reload.go b/common/globals/reload.go index c4c942b2..251742ba 100644 --- a/common/globals/reload.go +++ b/common/globals/reload.go @@ -10,5 +10,6 @@ var CacheReplaceChan = make(chan bool) var PluginReloadChan = make(chan bool) var PoolReloadChan = make(chan bool) var ErrorCacheReloadChan = make(chan bool) +var MatrixCachesReloadChan = make(chan bool) var PGOReloadChan = make(chan bool) var BucketsReloadChan = make(chan bool) diff --git a/config.sample.yaml b/config.sample.yaml index 6798fe11..3cf6152a 100644 --- a/config.sample.yaml +++ b/config.sample.yaml @@ -92,6 +92,11 @@ homeservers: # to "matrix", most functionality requiring the admin API will not work. adminApiKind: "synapse" + # The signing key to use for authorizing outbound federation requests. If not specified, + # requests will not be authorized. See https://docs.t2bot.io/matrix-media-repo/v1.3.5/installation/signing-key/ + # for details. + #signingKeyPath: "/data/example.org.key" + # Options for controlling how access tokens work with the media repo. It is recommended that if # you are going to use these options that the `/logout` and `/logout/all` client-server endpoints # be proxied through this process. They will also be called on the homeserver, and the response diff --git a/matrix/errors.go b/matrix/errors.go index 965b1851..fb6a7458 100644 --- a/matrix/errors.go +++ b/matrix/errors.go @@ -7,12 +7,12 @@ import ( "github.com/t2bot/matrix-media-repo/common" ) -type errorResponse struct { +type ErrorResponse struct { ErrorCode string `json:"errcode"` Message string `json:"error"` } -func (e errorResponse) Error() string { +func (e ErrorResponse) Error() string { return fmt.Sprintf("code=%s message=%s", e.ErrorCode, e.Message) } @@ -22,7 +22,7 @@ func filterError(err error) (error, error) { } // Unknown token errors should be filtered out explicitly to ensure we don't break on bad requests - var httpErr *errorResponse + var httpErr *ErrorResponse if errors.As(err, &httpErr) { // We send back our own version of errors to ensure we can filter them out elsewhere if httpErr.ErrorCode == common.ErrCodeUnknownToken { @@ -34,3 +34,15 @@ func filterError(err error) (error, error) { return err, err } + +type ServerNotAllowedError struct { + error + ServerName string +} + +func MakeServerNotAllowedError(serverName string) ServerNotAllowedError { + return ServerNotAllowedError{ + error: errors.New("server " + serverName + " is not allowed"), + ServerName: serverName, + } +} diff --git a/matrix/requests.go b/matrix/requests.go index d075d28b..740ea432 100644 --- a/matrix/requests.go +++ b/matrix/requests.go @@ -10,12 +10,16 @@ import ( "io" "net" "net/http" + "net/url" "os" "time" "github.com/t2bot/matrix-media-repo/common/rcontext" + "github.com/t2bot/matrix-media-repo/database" ) +const NoSigningKey = "" + // Based in part on https://github.com/matrix-org/gomatrix/blob/072b39f7fa6b40257b4eead8c958d71985c28bdd/client.go#L180-L243 func doRequest(ctx rcontext.RequestContext, method string, urlStr string, body interface{}, result interface{}, accessToken string, ipAddr string) error { ctx.Log.Debugf("Calling %s %s", method, urlStr) @@ -60,7 +64,7 @@ func doRequest(ctx rcontext.RequestContext, method string, urlStr string, body i return err } if res.StatusCode != http.StatusOK { - mtxErr := &errorResponse{} + mtxErr := &ErrorResponse{} err = json.Unmarshal(contents, mtxErr) if err == nil && mtxErr.ErrorCode != "" { return mtxErr @@ -78,14 +82,14 @@ func doRequest(ctx rcontext.RequestContext, method string, urlStr string, body i return nil } -func FederatedGet(url string, realHost string, ctx rcontext.RequestContext) (*http.Response, error) { - ctx.Log.Debug("Doing federated GET to " + url + " with host " + realHost) +func FederatedGet(ctx rcontext.RequestContext, reqUrl string, realHost string, useSigningKeyPath string) (*http.Response, error) { + ctx.Log.Debug("Doing federated GET to " + reqUrl + " with host " + realHost) cb := getFederationBreaker(realHost) var resp *http.Response replyError := cb.CallContext(ctx, func() error { - req, err := http.NewRequest("GET", url, nil) + req, err := http.NewRequest(http.MethodGet, reqUrl, nil) if err != nil { return err } @@ -95,6 +99,23 @@ func FederatedGet(url string, realHost string, ctx rcontext.RequestContext) (*ht req.Header.Set("User-Agent", "matrix-media-repo") req.Host = realHost + if useSigningKeyPath != NoSigningKey { + ctx.Log.Debug("Reading signing key and adding authentication headers") + key, err := getLocalSigningKey(useSigningKeyPath) + if err != nil { + return err + } + parsed, err := url.Parse(reqUrl) + if err != nil { + return err + } + auth, err := CreateXMatrixHeader(ctx.Request.Host, realHost, http.MethodGet, parsed.RequestURI(), &database.AnonymousJson{}, key.Key, key.Version) + if err != nil { + return err + } + req.Header.Set("Authorization", auth) + } + var client *http.Client if os.Getenv("MEDIA_REPO_UNSAFE_FEDERATION") != "true" { // This is how we verify the certificate is valid for the host we expect. diff --git a/matrix/requests_signing.go b/matrix/requests_signing.go index f013f047..e2b5ef8c 100644 --- a/matrix/requests_signing.go +++ b/matrix/requests_signing.go @@ -70,7 +70,7 @@ func QuerySigningKeys(serverName string) (ServerSigningKeys, error) { } keysUrl := url + "/_matrix/key/v2/server" - keysResponse, err := FederatedGet(keysUrl, hostname, ctx) + keysResponse, err := FederatedGet(ctx, keysUrl, hostname, NoSigningKey) if keysResponse != nil { defer keysResponse.Body.Close() } diff --git a/matrix/signing_key_cache.go b/matrix/signing_key_cache.go new file mode 100644 index 00000000..f3b8bc63 --- /dev/null +++ b/matrix/signing_key_cache.go @@ -0,0 +1,43 @@ +package matrix + +import ( + "crypto/ed25519" + "os" + "time" + + "github.com/patrickmn/go-cache" + "github.com/t2bot/matrix-media-repo/homeserver_interop/mmr" +) + +type LocalSigningKey struct { + Key ed25519.PrivateKey + Version string +} + +var localSigningKeyCache = cache.New(5*time.Minute, 10*time.Minute) + +func FlushSigningKeyCache() { + localSigningKeyCache.Flush() +} + +func getLocalSigningKey(fromPath string) (*LocalSigningKey, error) { + if val, ok := localSigningKeyCache.Get(fromPath); ok { + return val.(*LocalSigningKey), nil + } + + f, err := os.Open(fromPath) + defer f.Close() + if err != nil { + return nil, err + } + key, err := mmr.DecodeSigningKey(f) + if err != nil { + return nil, err + } + sk := &LocalSigningKey{ + Key: key.PrivateKey, + Version: key.KeyVersion, + } + localSigningKeyCache.Set(fromPath, sk, cache.DefaultExpiration) + return sk, nil +} diff --git a/pipelines/_steps/download/try_download.go b/pipelines/_steps/download/try_download.go index bda7aa62..c087dce6 100644 --- a/pipelines/_steps/download/try_download.go +++ b/pipelines/_steps/download/try_download.go @@ -1,13 +1,16 @@ package download import ( + "encoding/json" "errors" "fmt" "io" "mime" + "mime/multipart" "net/http" "net/url" "strconv" + "strings" "github.com/prometheus/client_golang/prometheus" "github.com/t2bot/matrix-media-repo/common" @@ -25,6 +28,7 @@ import ( type downloadResult struct { r io.ReadCloser + metadata *database.AnonymousJson filename string contentType string err error @@ -55,12 +59,52 @@ func TryDownload(ctx rcontext.RequestContext, origin string, mediaId string) (*d return } - downloadUrl := fmt.Sprintf("%s/_matrix/media/v3/download/%s/%s?allow_remote=false&allow_redirect=true", baseUrl, url.PathEscape(origin), url.PathEscape(mediaId)) - resp, err := matrix.FederatedGet(downloadUrl, realHost, ctx) - metrics.MediaDownloaded.With(prometheus.Labels{"origin": origin}).Inc() - if err != nil { - errFn(err) - return + var resp *http.Response + var downloadUrl string + usesMultipartFormat := false + if ctx.Config.SigningKeyPath != "" { + downloadUrl = fmt.Sprintf("%s/_matrix/federation/unstable/org.matrix.msc3916/media/download/%s/%s?a=b", baseUrl, url.PathEscape(origin), url.PathEscape(mediaId)) + resp, err = matrix.FederatedGet(ctx, downloadUrl, realHost, ctx.Config.SigningKeyPath) + metrics.MediaDownloaded.With(prometheus.Labels{"origin": origin}).Inc() + if err != nil { + errFn(err) + return + } + if resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusUnauthorized { + errFn(matrix.MakeServerNotAllowedError(ctx.Request.Host)) + return + } else if resp.StatusCode == http.StatusNotFound { + decoder := json.NewDecoder(resp.Body) + resp2 := resp // copy response in case we clear it out later + defer resp2.Body.Close() + mxerr := &matrix.ErrorResponse{} + if err = decoder.Decode(&mxerr); err != nil { + // we probably got not-json - ignore and move on + ctx.Log.Debugf("Ignoring JSON decoding error on download error %d: %v", resp.StatusCode, err) + resp = nil // indicate we want to use fallback + } else { + if mxerr.ErrorCode == "M_UNRECOGNIZED" { + ctx.Log.Debugf("Destination doesn't support MSC3916") + resp = nil // indicate we want to use fallback + } + } + } else if resp.StatusCode == http.StatusOK { + usesMultipartFormat = true + } + } else { + // Yes, we are deliberately loud about this. People should configure this. + ctx.Log.Warn("No signing key is configured for this domain! See `signingKeyPath` in the sample config for details.") + } + + // Try fallback (unauthenticated) + if resp == nil { + downloadUrl = fmt.Sprintf("%s/_matrix/media/v3/download/%s/%s?allow_remote=false&allow_redirect=true", baseUrl, url.PathEscape(origin), url.PathEscape(mediaId)) + resp, err = matrix.FederatedGet(ctx, downloadUrl, realHost, matrix.NoSigningKey) + metrics.MediaDownloaded.With(prometheus.Labels{"origin": origin}).Inc() + if err != nil { + errFn(err) + return + } } if resp.StatusCode == http.StatusNotFound { @@ -85,24 +129,72 @@ func TryDownload(ctx rcontext.RequestContext, origin string, mediaId string) (*d return } - r := resp.Body if ctx.Config.Downloads.MaxSizeBytes > 0 { - r = readers.LimitReaderWithOverrunError(resp.Body, ctx.Config.Downloads.MaxSizeBytes) + resp.Body = readers.LimitReaderWithOverrunError(resp.Body, ctx.Config.Downloads.MaxSizeBytes) + } + + contentType := resp.Header.Get("Content-Type") // we default Content-Type after we inspect for multiparts + + metadata := &database.AnonymousJson{} + mediaPart := util.MatrixMediaPartFromResponse(resp) + if usesMultipartFormat { + if !strings.HasPrefix(contentType, "multipart/mixed;") { + errFn(fmt.Errorf("expected multipart/mixed, got %s", contentType)) + return + } + + _, params, err := mime.ParseMediaType(contentType) + if err != nil { + errFn(err) + return + } + + partReader := multipart.NewReader(resp.Body, params["boundary"]) + + // The first part should always be the metadata + jsonPart, err := partReader.NextPart() + if err != nil { + errFn(err) + return + } + partType := jsonPart.Header.Get("Content-Type") + if partType == "" || partType == "application/json" { + decoder := json.NewDecoder(jsonPart) + err = decoder.Decode(&metadata) + if err != nil { + errFn(err) + return + } + } else { + errFn(fmt.Errorf("expected application/json as the first part, got %s instead", partType)) + } + + ctx.Log.Debugf("Got metadata: %v", metadata) + + // The second part should always be the media itself + bodyPart, err := partReader.NextPart() + if err != nil { + errFn(err) + return + } + mediaPart = util.MatrixMediaPartFromMimeMultipart(bodyPart) + contentType = mediaPart.Header.Get("Content-Type") // Content-Type should really be the media content type } - contentType := resp.Header.Get("Content-Type") + // Default the Content-Type if we haven't already if contentType == "" { contentType = "application/octet-stream" // binary } fileName := "download" - _, params, err := mime.ParseMediaType(resp.Header.Get("Content-Disposition")) + _, params, err := mime.ParseMediaType(mediaPart.Header.Get("Content-Disposition")) if err == nil && params["filename"] != "" { fileName = params["filename"] } ch <- downloadResult{ - r: r, + r: mediaPart.Body, + metadata: metadata, filename: fileName, contentType: contentType, err: nil, @@ -117,6 +209,7 @@ func TryDownload(ctx rcontext.RequestContext, origin string, mediaId string) (*d } // At this point, res.r is our http response body. + // TODO: Do something with res.metadata (MSC3911) return datastore_op.PutAndReturnStream(ctx, origin, mediaId, res.r, res.contentType, res.filename, datastores.RemoteMediaKind) } diff --git a/pipelines/pipeline_download/pipeline.go b/pipelines/pipeline_download/pipeline.go index 3a600875..fa45c514 100644 --- a/pipelines/pipeline_download/pipeline.go +++ b/pipelines/pipeline_download/pipeline.go @@ -14,6 +14,7 @@ import ( "github.com/t2bot/matrix-media-repo/common/rcontext" "github.com/t2bot/matrix-media-repo/database" "github.com/t2bot/matrix-media-repo/limits" + "github.com/t2bot/matrix-media-repo/matrix" "github.com/t2bot/matrix-media-repo/pipelines/_steps/download" "github.com/t2bot/matrix-media-repo/pipelines/_steps/meta" "github.com/t2bot/matrix-media-repo/pipelines/_steps/quarantine" @@ -141,6 +142,14 @@ func Execute(ctx rcontext.RequestContext, origin string, mediaId string, opts Do cancel() return nil, r, err } + var notAllowedErr *matrix.ServerNotAllowedError + if errors.As(err, ¬AllowedErr) { + if notAllowedErr.ServerName != ctx.Request.Host { + ctx.Log.Debug("'Not allowed' error is for another server - retrying") + cancel() + return Execute(ctx, origin, mediaId, opts) + } + } if err != nil { cancel() return nil, nil, err diff --git a/test/xmatrix_header_test.go b/test/xmatrix_header_test.go index 8ec41175..5572f0b8 100644 --- a/test/xmatrix_header_test.go +++ b/test/xmatrix_header_test.go @@ -19,7 +19,7 @@ func TestXMatrixAuthHeader(t *testing.T) { t.Fatal(err) } - header, err := matrix.CreateXMatrixHeader("localhost:8008", "localhost", "GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, &priv, "0") + header, err := matrix.CreateXMatrixHeader("localhost:8008", "localhost", "GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, priv, "0") if err != nil { t.Fatal(err) } @@ -31,6 +31,30 @@ func TestXMatrixAuthHeader(t *testing.T) { keys := make(matrix.ServerSigningKeys) keys["ed25519:0"] = pub - err = matrix.ValidateXMatrixAuthHeader("GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, auths, keys) + err = matrix.ValidateXMatrixAuthHeader("GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, auths, keys, "localhost") assert.NoError(t, err) } + +func TestXMatrixAuthDestinationMismatch(t *testing.T) { + config.AddDomainForTesting("localhost", nil) + + pub, priv, err := ed25519.GenerateKey(nil) + if err != nil { + t.Fatal(err) + } + + header, err := matrix.CreateXMatrixHeader("localhost:8008", "localhost:1234", "GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, priv, "0") + if err != nil { + t.Fatal(err) + } + + auths, err := util.GetXMatrixAuth([]string{header}) + if err != nil { + t.Fatal(err) + } + + keys := make(matrix.ServerSigningKeys) + keys["ed25519:0"] = pub + err = matrix.ValidateXMatrixAuthHeader("GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, auths, keys, "localhost:1234") + assert.ErrorIs(t, err, matrix.ErrWrongDestination) +} diff --git a/util/matrix_media_part.go b/util/matrix_media_part.go new file mode 100644 index 00000000..689113fb --- /dev/null +++ b/util/matrix_media_part.go @@ -0,0 +1,27 @@ +package util + +import ( + "io" + "mime/multipart" + "net/http" + "net/textproto" +) + +type MatrixMediaPart struct { + Header textproto.MIMEHeader + Body io.ReadCloser +} + +func MatrixMediaPartFromResponse(r *http.Response) *MatrixMediaPart { + return &MatrixMediaPart{ + Header: textproto.MIMEHeader(r.Header), + Body: r.Body, + } +} + +func MatrixMediaPartFromMimeMultipart(p *multipart.Part) *MatrixMediaPart { + return &MatrixMediaPart{ + Header: p.Header, + Body: p, + } +} From 237e15348bc73b8121c6c22ef63b467ddd6cf4b7 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 1 May 2024 16:09:02 -0600 Subject: [PATCH 15/51] Validate X-Matrix destination correctly --- api/_routers/97-require-server-auth.go | 19 +++++++++++++++++-- matrix/xmatrix.go | 13 +++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/api/_routers/97-require-server-auth.go b/api/_routers/97-require-server-auth.go index 0b128523..28ad632d 100644 --- a/api/_routers/97-require-server-auth.go +++ b/api/_routers/97-require-server-auth.go @@ -1,6 +1,7 @@ package _routers import ( + "errors" "net/http" "github.com/t2bot/matrix-media-repo/api/_apimeta" @@ -17,10 +18,24 @@ func RequireServerAuth(generator GeneratorWithServerFn) GeneratorFn { serverName, err := matrix.ValidateXMatrixAuth(r, true) if err != nil { ctx.Log.Debug("Error with X-Matrix auth: ", err) + if errors.Is(err, matrix.ErrNoXMatrixAuth) { + return &_responses.ErrorResponse{ + Code: common.ErrCodeUnauthorized, + Message: "no auth provided (required)", + InternalCode: common.ErrCodeMissingToken, + } + } + if errors.Is(err, matrix.ErrWrongDestination) { + return &_responses.ErrorResponse{ + Code: common.ErrCodeUnauthorized, + Message: "no auth provided for this destination (required)", + InternalCode: common.ErrCodeBadRequest, + } + } return &_responses.ErrorResponse{ Code: common.ErrCodeForbidden, - Message: "no auth provided (required)", - InternalCode: common.ErrCodeMissingToken, + Message: "invalid auth provided (required)", + InternalCode: common.ErrCodeBadRequest, } } return generator(r, ctx, _apimeta.ServerInfo{ diff --git a/matrix/xmatrix.go b/matrix/xmatrix.go index dd8c662e..6a95e7d9 100644 --- a/matrix/xmatrix.go +++ b/matrix/xmatrix.go @@ -11,6 +11,7 @@ import ( ) var ErrNoXMatrixAuth = errors.New("no X-Matrix auth headers") +var ErrWrongDestination = errors.New("wrong destination") func ValidateXMatrixAuth(request *http.Request, expectNoContent bool) (string, error) { if !expectNoContent { @@ -30,14 +31,14 @@ func ValidateXMatrixAuth(request *http.Request, expectNoContent bool) (string, e return "", err } - err = ValidateXMatrixAuthHeader(request.Method, request.RequestURI, &database.AnonymousJson{}, auths, keys) + err = ValidateXMatrixAuthHeader(request.Method, request.RequestURI, &database.AnonymousJson{}, auths, keys, request.Host) if err != nil { return "", err } return auths[0].Origin, nil } -func ValidateXMatrixAuthHeader(requestMethod string, requestUri string, content any, headers []util.XMatrixAuth, originKeys ServerSigningKeys) error { +func ValidateXMatrixAuthHeader(requestMethod string, requestUri string, content any, headers []util.XMatrixAuth, originKeys ServerSigningKeys, destinationHost string) error { if len(headers) == 0 { return ErrNoXMatrixAuth } @@ -61,8 +62,8 @@ func ValidateXMatrixAuthHeader(requestMethod string, requestUri string, content if h.Destination != obj["destination"] { return errors.New("auth is for multiple servers") } - if h.Destination != "" && !util.IsServerOurs(h.Destination) { - return errors.New("unknown destination") + if h.Destination != "" && (!util.IsServerOurs(h.Destination) || destinationHost != h.Destination) { + return ErrWrongDestination } if key, ok := (originKeys)[h.KeyId]; ok { @@ -77,7 +78,7 @@ func ValidateXMatrixAuthHeader(requestMethod string, requestUri string, content return nil } -func CreateXMatrixHeader(origin string, destination string, requestMethod string, requestUri string, content any, key *ed25519.PrivateKey, keyVersion string) (string, error) { +func CreateXMatrixHeader(origin string, destination string, requestMethod string, requestUri string, content any, key ed25519.PrivateKey, keyVersion string) (string, error) { obj := map[string]interface{}{ "method": requestMethod, "uri": requestUri, @@ -90,7 +91,7 @@ func CreateXMatrixHeader(origin string, destination string, requestMethod string return "", err } - b := ed25519.Sign(*key, canonical) + b := ed25519.Sign(key, canonical) sig := util.EncodeUnpaddedBase64ToString(b) return fmt.Sprintf("X-Matrix origin=\"%s\",destination=\"%s\",key=\"ed25519:%s\",sig=\"%s\"", origin, destination, keyVersion, sig), nil From 579987b798b7d5b3acc65e4bf0344c197eac151f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 3 May 2024 20:00:11 -0600 Subject: [PATCH 16/51] Factor out signing key generation --- cmd/utilities/generate_signing_key/main.go | 38 +------------------- homeserver_interop/signing_key.go | 41 ++++++++++++++++++++++ 2 files changed, 42 insertions(+), 37 deletions(-) diff --git a/cmd/utilities/generate_signing_key/main.go b/cmd/utilities/generate_signing_key/main.go index 3367164d..99afc745 100644 --- a/cmd/utilities/generate_signing_key/main.go +++ b/cmd/utilities/generate_signing_key/main.go @@ -1,13 +1,8 @@ package main import ( - "crypto/ed25519" - "crypto/rand" "flag" - "fmt" "os" - "sort" - "strings" "github.com/sirupsen/logrus" "github.com/t2bot/matrix-media-repo/cmd/utilities/_common" @@ -27,16 +22,7 @@ func main() { if *inputFile != "" { key, err = decodeKey(*inputFile) } else { - keyVersion := makeKeyVersion() - - var priv ed25519.PrivateKey - _, priv, err = ed25519.GenerateKey(nil) - priv = priv[len(priv)-32:] - - key = &homeserver_interop.SigningKey{ - PrivateKey: priv, - KeyVersion: keyVersion, - } + key, err = homeserver_interop.GenerateSigningKey() } if err != nil { logrus.Fatal(err) @@ -47,28 +33,6 @@ func main() { _common.EncodeSigningKeys([]*homeserver_interop.SigningKey{key}, *outputFormat, *outputFile) } -func makeKeyVersion() string { - buf := make([]byte, 2) - chars := strings.Split("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789", "") - for i := 0; i < len(chars); i++ { - sort.Slice(chars, func(i int, j int) bool { - c, err := rand.Read(buf) - - // "should never happen" clauses - if err != nil { - panic(err) - } - if c != len(buf) || c != 2 { - panic(fmt.Sprintf("crypto rand read %d bytes, expected %d", c, len(buf))) - } - - return buf[0] < buf[1] - }) - } - - return strings.Join(chars[:6], "") -} - func decodeKey(fileName string) (*homeserver_interop.SigningKey, error) { f, err := os.Open(fileName) if err != nil { diff --git a/homeserver_interop/signing_key.go b/homeserver_interop/signing_key.go index 29c0af81..8a1d887c 100644 --- a/homeserver_interop/signing_key.go +++ b/homeserver_interop/signing_key.go @@ -2,9 +2,50 @@ package homeserver_interop import ( "crypto/ed25519" + "crypto/rand" + "fmt" + "sort" + "strings" ) type SigningKey struct { PrivateKey ed25519.PrivateKey KeyVersion string } + +func GenerateSigningKey() (*SigningKey, error) { + keyVersion := makeKeyVersion() + + _, priv, err := ed25519.GenerateKey(nil) + priv = priv[len(priv)-32:] + if err != nil { + return nil, err + } + + return &SigningKey{ + PrivateKey: priv, + KeyVersion: keyVersion, + }, nil +} + +func makeKeyVersion() string { + buf := make([]byte, 2) + chars := strings.Split("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789", "") + for i := 0; i < len(chars); i++ { + sort.Slice(chars, func(i int, j int) bool { + c, err := rand.Read(buf) + + // "should never happen" clauses + if err != nil { + panic(err) + } + if c != len(buf) || c != 2 { + panic(fmt.Errorf("crypto rand read %d bytes, expected %d", c, len(buf))) + } + + return buf[0] < buf[1] + }) + } + + return strings.Join(chars[:6], "") +} From 4bfddf30e40d91fad04df39c5ed507bb0c3053ca Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 3 May 2024 20:00:26 -0600 Subject: [PATCH 17/51] Allow overriding the auth header in tests --- test/test_internals/util_client.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/test_internals/util_client.go b/test/test_internals/util_client.go index 443ca507..e95c799e 100644 --- a/test/test_internals/util_client.go +++ b/test/test_internals/util_client.go @@ -11,10 +11,11 @@ import ( ) type MatrixClient struct { - AccessToken string - ClientServerUrl string - UserId string - ServerName string + AccessToken string + ClientServerUrl string + UserId string + ServerName string + AuthHeaderOverride string } func (c *MatrixClient) WithCsUrl(newUrl string) *MatrixClient { @@ -80,10 +81,14 @@ func (c *MatrixClient) DoRaw(method string, endpoint string, qs url.Values, cont if contentType != "" { req.Header.Set("Content-Type", contentType) } + if c.AccessToken != "" { req.Header.Set("Authorization", "Bearer "+c.AccessToken) } + if c.AuthHeaderOverride != "" { + req.Header.Set("Authorization", c.AuthHeaderOverride) + } - log.Printf("[HTTP] [Auth=%s] [Host=%s] %s %s", c.AccessToken, c.ServerName, req.Method, req.URL.String()) + log.Printf("[HTTP] [Auth=%s] [Host=%s] %s %s", req.Header.Get("Authorization"), c.ServerName, req.Method, req.URL.String()) return http.DefaultClient.Do(req) } From 0944a9a367fbc7d7ecf8d0a985f3cd3d31a609fc Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 3 May 2024 20:00:44 -0600 Subject: [PATCH 18/51] Print signing key path when printing domains --- common/config/access.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/config/access.go b/common/config/access.go index b95a613c..04099653 100644 --- a/common/config/access.go +++ b/common/config/access.go @@ -274,7 +274,7 @@ func UniqueDatastores() []DatastoreConfig { func PrintDomainInfo() { logrus.Info("Domains loaded:") for _, d := range domains { - logrus.Info(fmt.Sprintf("\t%s (%s)", d.Name, d.ClientServerApi)) + logrus.Info(fmt.Sprintf("\t%s (%s | Signing Key Path=%s)", d.Name, d.ClientServerApi, d.SigningKeyPath)) } } From b5472bd2d57221593665e0a8d5403779f93d69aa Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 3 May 2024 20:01:10 -0600 Subject: [PATCH 19/51] Configure test MMR instances with a signing key --- test/templates/mmr.config.yaml | 1 + test/test_internals/deps.go | 60 +++++++++++++++++++++++++-------- test/test_internals/deps_mmr.go | 1 + 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/test/templates/mmr.config.yaml b/test/templates/mmr.config.yaml index 4d82dd3a..965490d5 100644 --- a/test/templates/mmr.config.yaml +++ b/test/templates/mmr.config.yaml @@ -18,6 +18,7 @@ homeservers: csApi: "{{.ClientServerApiUrl}}" backoffAt: 10 adminApiKind: "synapse" + signingKeyPath: "{{.SigningKeyPath}}" {{end}} redis: enabled: true diff --git a/test/test_internals/deps.go b/test/test_internals/deps.go index da0c65db..ecc8bbbc 100644 --- a/test/test_internals/deps.go +++ b/test/test_internals/deps.go @@ -7,22 +7,26 @@ import ( "log" "os" "path" + "strings" "time" "github.com/t2bot/matrix-media-repo/common/assets" "github.com/t2bot/matrix-media-repo/common/config" + "github.com/t2bot/matrix-media-repo/homeserver_interop" + "github.com/t2bot/matrix-media-repo/homeserver_interop/mmr" "github.com/testcontainers/testcontainers-go" "github.com/testcontainers/testcontainers-go/modules/postgres" "github.com/testcontainers/testcontainers-go/wait" ) type ContainerDeps struct { - ctx context.Context - pgContainer *postgres.PostgresContainer - redisContainer testcontainers.Container - minioDep *MinioDep - depNet *NetworkDep - mmrExtConfigPath string + ctx context.Context + pgContainer *postgres.PostgresContainer + redisContainer testcontainers.Container + minioDep *MinioDep + depNet *NetworkDep + mmrExtConfigPath string + mmrSigningKeyPath string Homeservers []*SynapseDep Machines []*mmrContainer @@ -111,16 +115,40 @@ func MakeTestDeps() (*ContainerDeps, error) { return nil, err } + // Create a shared signing key for the MMR instances + signingKeyFile, err := os.CreateTemp(os.TempDir(), "mmr-signing-key") + if err != nil { + return nil, err + } + signingKey, err := homeserver_interop.GenerateSigningKey() + if err != nil { + return nil, err + } + b, err := mmr.EncodeSigningKey(signingKey) + if err != nil { + return nil, err + } + _, err = signingKeyFile.Write(b) + if err != nil { + return nil, err + } + err = signingKeyFile.Close() + if err != nil { + return nil, err + } + // Start two MMRs for testing tmplArgs := mmrTmplArgs{ Homeservers: []mmrHomeserverTmplArgs{ { ServerName: syn1.ServerName, ClientServerApiUrl: syn1.InternalClientServerApiUrl, + SigningKeyPath: strings.ReplaceAll(signingKeyFile.Name(), "\\", "\\\\"), }, { ServerName: syn2.ServerName, ClientServerApiUrl: syn2.InternalClientServerApiUrl, + SigningKeyPath: strings.ReplaceAll(signingKeyFile.Name(), "\\", "\\\\"), }, }, RedisAddr: fmt.Sprintf("%s:%d", redisIp, 6379), // we're behind the network for redis @@ -148,14 +176,15 @@ func MakeTestDeps() (*ContainerDeps, error) { assets.SetupAssets(config.DefaultAssetsPath) return &ContainerDeps{ - ctx: ctx, - pgContainer: pgContainer, - redisContainer: redisContainer, - minioDep: minioDep, - mmrExtConfigPath: tmpPath, - Homeservers: []*SynapseDep{syn1, syn2}, - Machines: mmrs, - depNet: depNet, + ctx: ctx, + pgContainer: pgContainer, + redisContainer: redisContainer, + minioDep: minioDep, + mmrExtConfigPath: tmpPath, + mmrSigningKeyPath: signingKeyFile.Name(), + Homeservers: []*SynapseDep{syn1, syn2}, + Machines: mmrs, + depNet: depNet, }, nil } @@ -177,6 +206,9 @@ func (c *ContainerDeps) Teardown() { if err := os.Remove(c.mmrExtConfigPath); err != nil && !os.IsNotExist(err) { log.Fatalf("Error cleaning up MMR-External config file '%s': %s", c.mmrExtConfigPath, err.Error()) } + if err := os.Remove(c.mmrSigningKeyPath); err != nil && !os.IsNotExist(err) { + log.Fatalf("Error cleaning up MMR-Signing Key file '%s': %s", c.mmrSigningKeyPath, err.Error()) + } } func (c *ContainerDeps) Debug() { diff --git a/test/test_internals/deps_mmr.go b/test/test_internals/deps_mmr.go index a3d3b9af..407d182a 100644 --- a/test/test_internals/deps_mmr.go +++ b/test/test_internals/deps_mmr.go @@ -19,6 +19,7 @@ import ( type mmrHomeserverTmplArgs struct { ServerName string ClientServerApiUrl string + SigningKeyPath string } type mmrTmplArgs struct { From 476e92d8c8671431c8e09c5d4f89cd005acafa75 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 3 May 2024 20:01:28 -0600 Subject: [PATCH 20/51] Allow lazy ServeFile implementations --- test/test_internals/inline_dep_host_file.go | 58 ++++++++++++++------- 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/test/test_internals/inline_dep_host_file.go b/test/test_internals/inline_dep_host_file.go index 5fc9cf17..953c0971 100644 --- a/test/test_internals/inline_dep_host_file.go +++ b/test/test_internals/inline_dep_host_file.go @@ -17,36 +17,57 @@ type HostedFile struct { nginx testcontainers.Container tempDirectoryPath string - PublicUrl string + PublicUrl string + PublicHostname string } func ServeFile(fileName string, deps *ContainerDeps, contents string) (*HostedFile, error) { + container, writeFn, err := LazyServeFile(fileName, deps) + if writeFn != nil { + err2 := writeFn(contents) + if err2 != nil { + return nil, err2 + } + } + return container, err +} + +func LazyServeFile(fileName string, deps *ContainerDeps) (*HostedFile, func(string) error, error) { tmp, err := os.MkdirTemp(os.TempDir(), "mmr-nginx") if err != nil { - return nil, err + return nil, nil, err } err = os.Chmod(tmp, 0755) if err != nil { - return nil, err + return nil, nil, err } - f, err := os.Create(path.Join(tmp, fileName)) + err = os.MkdirAll(path.Join(tmp, path.Dir(fileName)), 0755) if err != nil { - return nil, err + return nil, nil, err } - defer func(f *os.File) { - _ = f.Close() - }(f) - _, err = f.Write([]byte(contents)) - if err != nil { - return nil, err - } + writeFn := func(contents string) error { + f, err := os.Create(path.Join(tmp, fileName)) + if err != nil { + return err + } + defer func(f *os.File) { + _ = f.Close() + }(f) - err = f.Close() - if err != nil { - return nil, err + _, err = f.Write([]byte(contents)) + if err != nil { + return err + } + + err = f.Close() + if err != nil { + return err + } + + return nil } nginx, err := testcontainers.GenericContainer(deps.ctx, testcontainers.GenericContainerRequest{ @@ -62,12 +83,12 @@ func ServeFile(fileName string, deps *ContainerDeps, contents string) (*HostedFi Started: true, }) if err != nil { - return nil, err + return nil, nil, err } nginxIp, err := nginx.ContainerIP(deps.ctx) if err != nil { - return nil, err + return nil, nil, err } //goland:noinspection HttpUrlsUsage @@ -76,7 +97,8 @@ func ServeFile(fileName string, deps *ContainerDeps, contents string) (*HostedFi nginx: nginx, tempDirectoryPath: tmp, PublicUrl: fmt.Sprintf("http://%s:%d/%s", nginxIp, 80, fileName), - }, nil + PublicHostname: fmt.Sprintf("%s:%d", nginxIp, 80), + }, writeFn, nil } func (f *HostedFile) Teardown() { From 987909956d6cc040b155272c9284dabe3a309613 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 3 May 2024 20:04:30 -0600 Subject: [PATCH 21/51] Add federation download test --- test/msc3916_downloads_suite_test.go | 105 ++++++++++++++++++++++++++- 1 file changed, 101 insertions(+), 4 deletions(-) diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index 5cc12256..01d72871 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -1,6 +1,9 @@ package test import ( + "bytes" + "crypto/ed25519" + "encoding/json" "fmt" "log" "net/http" @@ -8,13 +11,19 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" + "github.com/t2bot/matrix-media-repo/database" + "github.com/t2bot/matrix-media-repo/homeserver_interop" + "github.com/t2bot/matrix-media-repo/homeserver_interop/mmr" + "github.com/t2bot/matrix-media-repo/matrix" "github.com/t2bot/matrix-media-repo/test/test_internals" "github.com/t2bot/matrix-media-repo/util" ) type MSC3916DownloadsSuite struct { suite.Suite - deps *test_internals.ContainerDeps + deps *test_internals.ContainerDeps + keyServer *test_internals.HostedFile + keyServerKey *homeserver_interop.SigningKey } func (s *MSC3916DownloadsSuite) SetupSuite() { @@ -23,6 +32,50 @@ func (s *MSC3916DownloadsSuite) SetupSuite() { log.Fatal(err) } s.deps = deps + + // We'll use a pre-computed signing key for simplicity + signingKey, err := mmr.DecodeSigningKey(bytes.NewReader([]byte(`-----BEGIN MMR PRIVATE KEY----- +Key-ID: ed25519:e5d0oC +Version: 1 + +PJt0OaIImDJk8P/PDb4TNQHgI/1AA1C+AaQaABxAcgc= +-----END MMR PRIVATE KEY----- +`))) + if err != nil { + log.Fatal(err) + } + s.keyServerKey = signingKey + // Create a /_matrix/key/v2/server response file (signed JSON) + keyServer, writeFn, err := test_internals.LazyServeFile("_matrix/key/v2/server", deps) + if err != nil { + log.Fatal(err) + } + s.keyServer = keyServer + serverKey := database.AnonymousJson{ + "old_verify_keys": database.AnonymousJson{}, + "server_name": keyServer.PublicHostname, + "valid_until_ts": util.NowMillis() + (60 * 60 * 1000), // +1hr + "verify_keys": database.AnonymousJson{ + "ed25519:e5d0oC": database.AnonymousJson{ + "key": "TohekYXzLx7VzV8FtLQlI3XsSdPv1CjhVYY5rZmFCvU", + }, + }, + } + canonical, err := util.EncodeCanonicalJson(serverKey) + signature := util.EncodeUnpaddedBase64ToString(ed25519.Sign(signingKey.PrivateKey, canonical)) + serverKey["signatures"] = database.AnonymousJson{ + keyServer.PublicHostname: database.AnonymousJson{ + "ed25519:e5d0oC": signature, + }, + } + b, err := json.Marshal(serverKey) + if err != nil { + log.Fatal(err) + } + err = writeFn(string(b)) + if err != nil { + log.Fatal(err) + } } func (s *MSC3916DownloadsSuite) TearDownSuite() { @@ -49,8 +102,6 @@ func (s *MSC3916DownloadsSuite) TestClientDownloads() { assert.NoError(t, err) fname := "image" + util.ExtensionForContentType(contentType) - //res := new(test_internals.MatrixUploadResponse) - //err = client1.DoReturnJson("POST", "/_matrix/client/unstable/org.matrix.msc3916/media/upload", url.Values{"filename": []string{fname}}, contentType, img, res) res, err := client1.Upload(fname, contentType, img) assert.NoError(t, err) assert.NotEmpty(t, res.MxcUri) @@ -79,9 +130,55 @@ func (s *MSC3916DownloadsSuite) TestClientDownloads() { func (s *MSC3916DownloadsSuite) TestFederationDownloads() { t := s.T() - t.Error("Not yet implemented") + + client1 := s.deps.Homeservers[0].UnprivilegedUsers[0].WithCsUrl(s.deps.Machines[0].HttpUrl) + remoteClient := &test_internals.MatrixClient{ + ClientServerUrl: s.deps.Machines[0].HttpUrl, + ServerName: s.deps.Homeservers[0].ServerName, + AccessToken: "", // this client isn't authed over the CS API + UserId: "", // this client isn't authed over the CS API + } + + contentType, img, err := test_internals.MakeTestImage(512, 512) + assert.NoError(t, err) + fname := "image" + util.ExtensionForContentType(contentType) + + res, err := client1.Upload(fname, contentType, img) + assert.NoError(t, err) + assert.NotEmpty(t, res.MxcUri) + + origin, mediaId, err := util.SplitMxc(res.MxcUri) + assert.NoError(t, err) + assert.Equal(t, client1.ServerName, origin) + assert.NotEmpty(t, mediaId) + + // Verify the federation download *fails* when lacking auth + uri := fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId) + raw, err := remoteClient.DoRaw("GET", uri, nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) + + // Now add the X-Matrix auth and try again + // TODO: We probably need to tell MMR to use an insecure environment to pass the federation test. + header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", uri, &database.AnonymousJson{}, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion) + assert.NoError(t, err) + remoteClient.AuthHeaderOverride = header + raw, err = remoteClient.DoRaw("GET", uri, nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, raw.StatusCode) } +//func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { +// t := s.T() +// +// // TODO: Tests for: +// // * Actually tries MSC3916 for downloads +// // * Falls back on failure +// // * Doesn't call unauthenticated endpoint if MSC3916 was successful +// // * Sets correct auth +// t.Error("not yet implemented") +//} + func TestMSC3916DownloadsSuite(t *testing.T) { suite.Run(t, new(MSC3916DownloadsSuite)) } From 2c0fcdef67812d25b56556e9729ab61bf8df5c19 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 3 May 2024 20:07:46 -0600 Subject: [PATCH 22/51] Re-add merge conflicts in changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99d51326..a4271d3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Added * Dendrite homeservers can now have their media imported safely, and `adminApiKind` may be set to `dendrite`. +* Exporting MMR's data to Synapse is now possible with `import_to_synapse`. To use it, first run `gdpr_export` or similar. +* Errors encountered during a background task, such as an API-induced export, are exposed as `error_message` in the admin API. +* MMR will follow redirects on federated downloads up to 5 hops. +* S3-backed datastores can have download requests redirected to a public-facing CDN rather than being proxied through MMR. See `publicBaseUrl` under the S3 datastore config. ### Changed From dcb32492505c074c5e013ac1c65c563a36facba6 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 15 May 2024 20:36:03 -0600 Subject: [PATCH 23/51] Support http-only federation for tests --- matrix/server_discovery.go | 29 +++++++++++++++++----------- test/msc3916_downloads_suite_test.go | 1 - test/test_internals/deps_mmr.go | 3 ++- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/matrix/server_discovery.go b/matrix/server_discovery.go index 02e5ce37..43ee1996 100644 --- a/matrix/server_discovery.go +++ b/matrix/server_discovery.go @@ -5,6 +5,7 @@ import ( "fmt" "net" "net/http" + "os" "strconv" "strings" "sync" @@ -36,6 +37,12 @@ func GetServerApiUrl(hostname string) (string, string, error) { logrus.Debug("Getting server API URL for " + hostname) + scheme := "https" + if os.Getenv("MEDIA_REPO_HTTP_ONLY_FEDERATION") == "true" { + logrus.Warnf("Making non-https request to hostname %s because MEDIA_REPO_HTTP_ONLY_FEDERATION is set to true", hostname) + scheme = "http" + } + // Check to see if we've cached this hostname at all setupCache() record, found := apiUrlCacheInstance.Get(hostname) @@ -58,7 +65,7 @@ func GetServerApiUrl(hostname string) (string, string, error) { // Step 1 of the discovery process: if the hostname is an IP, use that with explicit or default port logrus.Debug("Testing if " + h + " is an IP address") if is.IP(h) { - url := fmt.Sprintf("https://%s", net.JoinHostPort(h, p)) + url := fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(h, p)) server := cachedServer{url, hostname} apiUrlCacheInstance.Set(hostname, server, cache.DefaultExpiration) logrus.Debug("Server API URL for " + hostname + " is " + url + " (IP address)") @@ -68,7 +75,7 @@ func GetServerApiUrl(hostname string) (string, string, error) { // Step 2: if the hostname is not an IP address, and an explicit port is given, use that logrus.Debug("Testing if a default port was used. Using default = ", defPort) if !defPort { - url := fmt.Sprintf("https://%s", net.JoinHostPort(h, p)) + url := fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(h, p)) server := cachedServer{url, h} apiUrlCacheInstance.Set(hostname, server, cache.DefaultExpiration) logrus.Debug("Server API URL for " + hostname + " is " + url + " (explicit port)") @@ -78,7 +85,7 @@ func GetServerApiUrl(hostname string) (string, string, error) { // Step 3: if the hostname is not an IP address and no explicit port is given, do .well-known // Note that we have sprawling branches here because we need to fall through to step 4 if parsing fails logrus.Debug("Doing .well-known lookup on " + h) - r, err := http.Get(fmt.Sprintf("https://%s/.well-known/matrix/server", h)) + r, err := http.Get(fmt.Sprintf("%s://%s/.well-known/matrix/server", scheme, h)) if r != nil { defer r.Body.Close() } @@ -98,7 +105,7 @@ func GetServerApiUrl(hostname string) (string, string, error) { // Step 3a: if the delegated host is an IP address, use that (regardless of port) logrus.Debug("Checking if WK host is an IP: " + wkHost) if is.IP(wkHost) { - url := fmt.Sprintf("https://%s", net.JoinHostPort(wkHost, wkPort)) + url := fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(wkHost, wkPort)) server := cachedServer{url, wk.ServerAddr} apiUrlCacheInstance.Set(hostname, server, cache.DefaultExpiration) logrus.Debug("Server API URL for " + hostname + " is " + url + " (WK; IP address)") @@ -109,7 +116,7 @@ func GetServerApiUrl(hostname string) (string, string, error) { logrus.Debug("Checking if WK is using default port? ", wkDefPort) if !wkDefPort { wkHost = net.JoinHostPort(wkHost, wkPort) - url := fmt.Sprintf("https://%s", wkHost) + url := fmt.Sprintf("%s://%s", scheme, wkHost) server := cachedServer{url, wkHost} apiUrlCacheInstance.Set(hostname, server, cache.DefaultExpiration) logrus.Debug("Server API URL for " + hostname + " is " + url + " (WK; explicit port)") @@ -126,7 +133,7 @@ func GetServerApiUrl(hostname string) (string, string, error) { if realAddr[len(realAddr)-1:] == "." { realAddr = realAddr[0 : len(realAddr)-1] } - url := fmt.Sprintf("https://%s", net.JoinHostPort(realAddr, strconv.Itoa(int(addrs[0].Port)))) + url := fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(realAddr, strconv.Itoa(int(addrs[0].Port)))) server := cachedServer{url, wkHost} apiUrlCacheInstance.Set(hostname, server, cache.DefaultExpiration) logrus.Debug("Server API URL for " + hostname + " is " + url + " (WK; SRV)") @@ -144,7 +151,7 @@ func GetServerApiUrl(hostname string) (string, string, error) { if realAddr[len(realAddr)-1:] == "." { realAddr = realAddr[0 : len(realAddr)-1] } - url := fmt.Sprintf("https://%s", net.JoinHostPort(realAddr, strconv.Itoa(int(addrs[0].Port)))) + url := fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(realAddr, strconv.Itoa(int(addrs[0].Port)))) server := cachedServer{url, wkHost} apiUrlCacheInstance.Set(hostname, server, cache.DefaultExpiration) logrus.Debug("Server API URL for " + hostname + " is " + url + " (WK; SRV-Deprecated)") @@ -153,7 +160,7 @@ func GetServerApiUrl(hostname string) (string, string, error) { // Step 3d: use the delegated host as-is logrus.Debug("Using .well-known as-is for ", wkHost) - url := fmt.Sprintf("https://%s", net.JoinHostPort(wkHost, wkPort)) + url := fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(wkHost, wkPort)) server := cachedServer{url, wkHost} apiUrlCacheInstance.Set(hostname, server, cache.DefaultExpiration) logrus.Debug("Server API URL for " + hostname + " is " + url + " (WK; fallback)") @@ -176,7 +183,7 @@ func GetServerApiUrl(hostname string) (string, string, error) { if realAddr[len(realAddr)-1:] == "." { realAddr = realAddr[0 : len(realAddr)-1] } - url := fmt.Sprintf("https://%s", net.JoinHostPort(realAddr, strconv.Itoa(int(addrs[0].Port)))) + url := fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(realAddr, strconv.Itoa(int(addrs[0].Port)))) server := cachedServer{url, h} apiUrlCacheInstance.Set(hostname, server, cache.DefaultExpiration) logrus.Debug("Server API URL for " + hostname + " is " + url + " (SRV)") @@ -193,7 +200,7 @@ func GetServerApiUrl(hostname string) (string, string, error) { if realAddr[len(realAddr)-1:] == "." { realAddr = realAddr[0 : len(realAddr)-1] } - url := fmt.Sprintf("https://%s", net.JoinHostPort(realAddr, strconv.Itoa(int(addrs[0].Port)))) + url := fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(realAddr, strconv.Itoa(int(addrs[0].Port)))) server := cachedServer{url, h} apiUrlCacheInstance.Set(hostname, server, cache.DefaultExpiration) logrus.Debug("Server API URL for " + hostname + " is " + url + " (SRV-Deprecated)") @@ -202,7 +209,7 @@ func GetServerApiUrl(hostname string) (string, string, error) { // Step 6: use the target host as-is logrus.Debug("Using host as-is: ", hostname) - url := fmt.Sprintf("https://%s", net.JoinHostPort(h, p)) + url := fmt.Sprintf("%s://%s", scheme, net.JoinHostPort(h, p)) server := cachedServer{url, h} apiUrlCacheInstance.Set(hostname, server, cache.DefaultExpiration) logrus.Debug("Server API URL for " + hostname + " is " + url + " (fallback)") diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index 01d72871..57fab61e 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -159,7 +159,6 @@ func (s *MSC3916DownloadsSuite) TestFederationDownloads() { assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) // Now add the X-Matrix auth and try again - // TODO: We probably need to tell MMR to use an insecure environment to pass the federation test. header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", uri, &database.AnonymousJson{}, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion) assert.NoError(t, err) remoteClient.AuthHeaderOverride = header diff --git a/test/test_internals/deps_mmr.go b/test/test_internals/deps_mmr.go index 407d182a..28c5f722 100644 --- a/test/test_internals/deps_mmr.go +++ b/test/test_internals/deps_mmr.go @@ -100,7 +100,8 @@ func makeMmrInstances(ctx context.Context, count int, depNet *NetworkDep, tmplAr testcontainers.BindMount(intTmpName, "/data/media-repo.yaml"), }, Env: map[string]string{ - "MACHINE_ID": strconv.Itoa(i), + "MACHINE_ID": strconv.Itoa(i), + "MEDIA_REPO_HTTP_ONLY_FEDERATION": "true", }, Networks: []string{depNet.NetId}, WaitingFor: wait.ForHTTP("/healthz").WithPort(p), From f7e1504f42ea2de04c3bf33f32f75ebef31a0d09 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 15 May 2024 20:36:18 -0600 Subject: [PATCH 24/51] Strip Go-added URI segments --- matrix/xmatrix.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/matrix/xmatrix.go b/matrix/xmatrix.go index 6a95e7d9..6b9e890e 100644 --- a/matrix/xmatrix.go +++ b/matrix/xmatrix.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "strings" "github.com/t2bot/matrix-media-repo/database" "github.com/t2bot/matrix-media-repo/util" @@ -31,7 +32,12 @@ func ValidateXMatrixAuth(request *http.Request, expectNoContent bool) (string, e return "", err } - err = ValidateXMatrixAuthHeader(request.Method, request.RequestURI, &database.AnonymousJson{}, auths, keys, request.Host) + uri := request.RequestURI + if strings.HasSuffix(uri, "?") { + uri = uri[:len(uri)-1] + } + + err = ValidateXMatrixAuthHeader(request.Method, uri, &database.AnonymousJson{}, auths, keys, request.Host) if err != nil { return "", err } @@ -55,7 +61,7 @@ func ValidateXMatrixAuthHeader(requestMethod string, requestUri string, content return err } - for _, h := range headers { + for i, h := range headers { if h.Origin != obj["origin"] { return errors.New("auth is from multiple servers") } @@ -68,7 +74,7 @@ func ValidateXMatrixAuthHeader(requestMethod string, requestUri string, content if key, ok := (originKeys)[h.KeyId]; ok { if !ed25519.Verify(key, canonical, h.Signature) { - return fmt.Errorf("failed signatures on '%s'", h.KeyId) + return fmt.Errorf("failed signatures on '%s', header %d", h.KeyId, i) } } else { return fmt.Errorf("unknown key '%s'", h.KeyId) From 2cb930b81252a1fbc5ed9aa59a730b1e16c13669 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 15 May 2024 21:00:58 -0600 Subject: [PATCH 25/51] Fix test shutdown --- test/test_internals/deps.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/test_internals/deps.go b/test/test_internals/deps.go index ecc8bbbc..ec71e374 100644 --- a/test/test_internals/deps.go +++ b/test/test_internals/deps.go @@ -189,8 +189,8 @@ func MakeTestDeps() (*ContainerDeps, error) { } func (c *ContainerDeps) Teardown() { - for _, mmr := range c.Machines { - mmr.Teardown() + for _, machine := range c.Machines { + machine.Teardown() } for _, hs := range c.Homeservers { hs.Teardown() @@ -202,13 +202,15 @@ func (c *ContainerDeps) Teardown() { log.Fatalf("Error shutting down mmr-postgres container: %s", err.Error()) } c.minioDep.Teardown() - c.depNet.Teardown() if err := os.Remove(c.mmrExtConfigPath); err != nil && !os.IsNotExist(err) { log.Fatalf("Error cleaning up MMR-External config file '%s': %s", c.mmrExtConfigPath, err.Error()) } if err := os.Remove(c.mmrSigningKeyPath); err != nil && !os.IsNotExist(err) { log.Fatalf("Error cleaning up MMR-Signing Key file '%s': %s", c.mmrSigningKeyPath, err.Error()) } + + // XXX: We should be shutting this down, but it appears testcontainers leaves something attached :( + //c.depNet.Teardown() } func (c *ContainerDeps) Debug() { From 8f79ea0319a6512d2982210a6d35a96cbb9eb61c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 16 May 2024 14:23:37 -0600 Subject: [PATCH 26/51] Remove unused test --- test/msc3916_thumbnails_suite_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/msc3916_thumbnails_suite_test.go b/test/msc3916_thumbnails_suite_test.go index 89132fac..996807db 100644 --- a/test/msc3916_thumbnails_suite_test.go +++ b/test/msc3916_thumbnails_suite_test.go @@ -77,11 +77,6 @@ func (s *MSC3916ThumbnailsSuite) TestClientThumbnails() { //test_internals.AssertIsTestImage(t, raw.Body) // we can't verify that the resulting image is correct } -func (s *MSC3916ThumbnailsSuite) TestFederationThumbnails() { - t := s.T() - t.Error("Not yet implemented") -} - func TestMSC3916ThumbnailsSuite(t *testing.T) { suite.Run(t, new(MSC3916ThumbnailsSuite)) } From c793319d792b20362262972c1e2e033d874a7fbb Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 16 May 2024 14:25:45 -0600 Subject: [PATCH 27/51] Enable failing tests --- test/msc3916_downloads_suite_test.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index 57fab61e..56a5ca4f 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -167,16 +167,20 @@ func (s *MSC3916DownloadsSuite) TestFederationDownloads() { assert.Equal(t, http.StatusOK, raw.StatusCode) } -//func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { -// t := s.T() -// -// // TODO: Tests for: -// // * Actually tries MSC3916 for downloads -// // * Falls back on failure -// // * Doesn't call unauthenticated endpoint if MSC3916 was successful -// // * Sets correct auth -// t.Error("not yet implemented") -//} +// TODO: Tests for: +// * Actually tries MSC3916 for downloads +// * Doesn't call unauthenticated endpoint if MSC3916 was successful & sets correct auth +// * Falls back on failure + +func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { + t := s.T() + t.Error("not yet implemented") +} + +func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloadsAndFallsBack() { + t := s.T() + t.Error("not yet implemented") +} func TestMSC3916DownloadsSuite(t *testing.T) { suite.Run(t, new(MSC3916DownloadsSuite)) From 03dd83ec132d16d0cadf717a72ea063b3f70ee2d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Sat, 18 May 2024 21:29:50 -0600 Subject: [PATCH 28/51] Ensure signing keys exist inside container --- test/msc3916_downloads_suite_test.go | 24 +++++++++++++++++++++++- test/test_internals/deps_mmr.go | 15 +++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index 56a5ca4f..043379a4 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -7,6 +7,8 @@ import ( "fmt" "log" "net/http" + "net/http/httptest" + "net/url" "testing" "github.com/stretchr/testify/assert" @@ -174,7 +176,27 @@ func (s *MSC3916DownloadsSuite) TestFederationDownloads() { func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { t := s.T() - t.Error("not yet implemented") + + client1 := s.deps.Homeservers[0].UnprivilegedUsers[0].WithCsUrl(s.deps.Machines[0].HttpUrl) + + origin := "" + mediaId := "abc123" + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), r.URL.Path) + origin, err := matrix.ValidateXMatrixAuth(r, true) + assert.NoError(t, err) + assert.Equal(t, client1.ServerName, origin) + w.Header().Set("Content-Type", "multipart/mixed; boundary=gc0p4Jq0M2Yt08jU534c0p") + _, _ = w.Write([]byte("--gc0p4Jq0M2Yt08jU534c0p\nContent-Type: application/json\n\n{}\n\n--gc0p4Jq0M2Yt08jU534c0p\nContent-Type: text/plain\n\nThis media is plain text. Maybe somebody used it as a paste bin.\n\n--gc0p4Jq0M2Yt08jU534c0p")) + })) + defer testServer.Close() + + u, _ := url.Parse(testServer.URL) + origin = fmt.Sprintf("host.docker.internal:%s", u.Port()) + + raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, raw.StatusCode) } func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloadsAndFallsBack() { diff --git a/test/test_internals/deps_mmr.go b/test/test_internals/deps_mmr.go index 28c5f722..533c3657 100644 --- a/test/test_internals/deps_mmr.go +++ b/test/test_internals/deps_mmr.go @@ -72,6 +72,17 @@ func writeMmrConfig(tmplArgs mmrTmplArgs) (string, error) { } func makeMmrInstances(ctx context.Context, count int, depNet *NetworkDep, tmplArgs mmrTmplArgs) ([]*mmrContainer, error) { + // We need to relocate the signing key paths for a Docker mount + additionalMounts := make([]testcontainers.ContainerMount, 0) + for i, hs := range tmplArgs.Homeservers { + if hs.SigningKeyPath != "" { + inContainerName := fmt.Sprintf("/data/hs%d.key", i) + additionalMounts = append(additionalMounts, testcontainers.BindMount(hs.SigningKeyPath, testcontainers.ContainerMountTarget(inContainerName))) + hs.SigningKeyPath = inContainerName + } + } + + // ... then we can write the config and get the temp file path for it intTmpName, err := writeMmrConfig(tmplArgs) if err != nil { return nil, err @@ -96,9 +107,9 @@ func makeMmrInstances(ctx context.Context, count int, depNet *NetworkDep, tmplAr KeepImage: true, }, ExposedPorts: []string{"8000/tcp"}, - Mounts: []testcontainers.ContainerMount{ + Mounts: append([]testcontainers.ContainerMount{ testcontainers.BindMount(intTmpName, "/data/media-repo.yaml"), - }, + }, additionalMounts...), Env: map[string]string{ "MACHINE_ID": strconv.Itoa(i), "MEDIA_REPO_HTTP_ONLY_FEDERATION": "true", From 9405db73c4d5af0b01ffeb3b4a2b08595f941446 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 May 2024 18:31:14 -0600 Subject: [PATCH 29/51] Fix signing key alignment between dependencies --- matrix/requests_signing.go | 38 +++++++++++++- test/msc3916_downloads_suite_test.go | 14 ++++++ test/templates/synapse.homeserver.yaml | 2 +- test/test_internals/deps.go | 69 ++++++++++++++++---------- test/test_internals/deps_synapse.go | 3 +- 5 files changed, 98 insertions(+), 28 deletions(-) diff --git a/matrix/requests_signing.go b/matrix/requests_signing.go index e2b5ef8c..54e09b71 100644 --- a/matrix/requests_signing.go +++ b/matrix/requests_signing.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "net/http" "sync" "time" @@ -34,9 +35,44 @@ var signingKeySf = new(typedsf.Group[ServerSigningKeys]) var signingKeyCache = cache.New(cache.NoExpiration, 30*time.Second) var signingKeyRWLock = new(sync.RWMutex) +// TestsOnlyInjectSigningKey +// Deprecated: For tests only. +func TestsOnlyInjectSigningKey(serverName string, httpFederationUrl string) error { + resp, err := http.Get(httpFederationUrl + "/_matrix/key/v2/server") + if err != nil { + return err + } + defer resp.Body.Close() + + decoder := json.NewDecoder(resp.Body) + raw := database.AnonymousJson{} + if err = decoder.Decode(&raw); err != nil { + return err + } + keyInfo := new(ServerKeyResult) + if err = raw.ApplyTo(keyInfo); err != nil { + return err + } + + // Convert keys to something useful, and check signatures + serverKeys, err := CheckSigningKeySignatures(serverName, keyInfo, raw) + if err != nil { + return err + } + + // Cache & return (unlock is deferred) + signingKeyRWLock.Lock() + defer signingKeyRWLock.Unlock() + cacheUntil := time.Until(time.UnixMilli(keyInfo.ValidUntilTs)) / 2 + signingKeyCache.Set(serverName, &serverKeys, cacheUntil) + + return nil +} + func querySigningKeyCache(serverName string) ServerSigningKeys { if val, ok := signingKeyCache.Get(serverName); ok { - return val.(ServerSigningKeys) + ptr := val.(*ServerSigningKeys) + return *ptr } return nil } diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index 043379a4..c65f8bf4 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -9,10 +9,12 @@ import ( "net/http" "net/http/httptest" "net/url" + "os" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" + "github.com/t2bot/matrix-media-repo/common/config" "github.com/t2bot/matrix-media-repo/database" "github.com/t2bot/matrix-media-repo/homeserver_interop" "github.com/t2bot/matrix-media-repo/homeserver_interop/mmr" @@ -29,6 +31,11 @@ type MSC3916DownloadsSuite struct { } func (s *MSC3916DownloadsSuite) SetupSuite() { + err := os.Setenv("MEDIA_REPO_HTTP_ONLY_FEDERATION", "true") + if err != nil { + s.T().Fatal(err) + } + deps, err := test_internals.MakeTestDeps() if err != nil { log.Fatal(err) @@ -81,6 +88,10 @@ PJt0OaIImDJk8P/PDb4TNQHgI/1AA1C+AaQaABxAcgc= } func (s *MSC3916DownloadsSuite) TearDownSuite() { + err := os.Unsetenv("MEDIA_REPO_HTTP_ONLY_FEDERATION") + if err != nil { + s.T().Fatal(err) + } if s.deps != nil { if s.T().Failed() { s.deps.Debug() @@ -181,6 +192,8 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { origin := "" mediaId := "abc123" + err := matrix.TestsOnlyInjectSigningKey(s.deps.Homeservers[0].ServerName, s.deps.Homeservers[0].ExternalClientServerApiUrl) + assert.NoError(t, err) testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), r.URL.Path) origin, err := matrix.ValidateXMatrixAuth(r, true) @@ -193,6 +206,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { u, _ := url.Parse(testServer.URL) origin = fmt.Sprintf("host.docker.internal:%s", u.Port()) + config.AddDomainForTesting("host.docker.internal", nil) // no port for config lookup raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) diff --git a/test/templates/synapse.homeserver.yaml b/test/templates/synapse.homeserver.yaml index 750730c6..556dd611 100644 --- a/test/templates/synapse.homeserver.yaml +++ b/test/templates/synapse.homeserver.yaml @@ -25,7 +25,7 @@ registration_shared_secret: "l,jbms,sR_Z82JNP2,sv-~^5bXqFTV-T=j,,~=OKZ8I_Tardk;" report_stats: false macaroon_secret_key: "KV*8qANyBE28e*pZ-9RP+u86~i8+.j9IZEKU8Vb4+jdIoe~ncw" form_secret: "mQrUxtt6^F3uQ3nVrGdg7yAK64p*#Uf@2n=e9y8ggLbhy3-QIy" -signing_key_path: "/app/signing.key" +signing_key_path: "/data/signing.key" enable_media_repo: false enable_registration: true enable_registration_without_verification: true diff --git a/test/test_internals/deps.go b/test/test_internals/deps.go index ec71e374..c419d457 100644 --- a/test/test_internals/deps.go +++ b/test/test_internals/deps.go @@ -14,6 +14,7 @@ import ( "github.com/t2bot/matrix-media-repo/common/config" "github.com/t2bot/matrix-media-repo/homeserver_interop" "github.com/t2bot/matrix-media-repo/homeserver_interop/mmr" + "github.com/t2bot/matrix-media-repo/homeserver_interop/synapse" "github.com/testcontainers/testcontainers-go" "github.com/testcontainers/testcontainers-go/modules/postgres" "github.com/testcontainers/testcontainers-go/wait" @@ -41,12 +42,52 @@ func MakeTestDeps() (*ContainerDeps, error) { return nil, err } + // Create a shared signing key for the MMR instances + signingKeyFile, err := os.CreateTemp(os.TempDir(), "mmr-signing-key") + if err != nil { + return nil, err + } + signingKey, err := homeserver_interop.GenerateSigningKey() + if err != nil { + return nil, err + } + b, err := mmr.EncodeSigningKey(signingKey) + if err != nil { + return nil, err + } + _, err = signingKeyFile.Write(b) + if err != nil { + return nil, err + } + err = signingKeyFile.Close() + if err != nil { + return nil, err + } + + // And use that same signing key for Synapse + synapseSigningKeyFile, err := os.CreateTemp(os.TempDir(), "mmr-synapse-signing-key") + if err != nil { + return nil, err + } + b, err = synapse.EncodeSigningKey(signingKey) + if err != nil { + return nil, err + } + _, err = synapseSigningKeyFile.Write(b) + if err != nil { + return nil, err + } + err = synapseSigningKeyFile.Close() + if err != nil { + return nil, err + } + // Start two synapses for testing - syn1, err := MakeSynapse("first.example.org", depNet) + syn1, err := MakeSynapse("first.example.org", depNet, synapseSigningKeyFile.Name()) if err != nil { return nil, err } - syn2, err := MakeSynapse("second.example.org", depNet) + syn2, err := MakeSynapse("second.example.org", depNet, synapseSigningKeyFile.Name()) if err != nil { return nil, err } @@ -115,31 +156,9 @@ func MakeTestDeps() (*ContainerDeps, error) { return nil, err } - // Create a shared signing key for the MMR instances - signingKeyFile, err := os.CreateTemp(os.TempDir(), "mmr-signing-key") - if err != nil { - return nil, err - } - signingKey, err := homeserver_interop.GenerateSigningKey() - if err != nil { - return nil, err - } - b, err := mmr.EncodeSigningKey(signingKey) - if err != nil { - return nil, err - } - _, err = signingKeyFile.Write(b) - if err != nil { - return nil, err - } - err = signingKeyFile.Close() - if err != nil { - return nil, err - } - // Start two MMRs for testing tmplArgs := mmrTmplArgs{ - Homeservers: []mmrHomeserverTmplArgs{ + Homeservers: []*mmrHomeserverTmplArgs{ { ServerName: syn1.ServerName, ClientServerApiUrl: syn1.InternalClientServerApiUrl, diff --git a/test/test_internals/deps_synapse.go b/test/test_internals/deps_synapse.go index f73b1799..d5bc7645 100644 --- a/test/test_internals/deps_synapse.go +++ b/test/test_internals/deps_synapse.go @@ -42,7 +42,7 @@ type SynapseDep struct { UnprivilegedUsers []*MatrixClient // uses ExternalClientServerApiUrl } -func MakeSynapse(domainName string, depNet *NetworkDep) (*SynapseDep, error) { +func MakeSynapse(domainName string, depNet *NetworkDep, signingKeyFilePath string) (*SynapseDep, error) { ctx := context.Background() // Start postgresql database @@ -117,6 +117,7 @@ func MakeSynapse(domainName string, depNet *NetworkDep) (*SynapseDep, error) { ExposedPorts: []string{"8008/tcp"}, Mounts: []testcontainers.ContainerMount{ testcontainers.BindMount(f.Name(), "/data/homeserver.yaml"), + testcontainers.BindMount(signingKeyFilePath, "/data/signing.key"), testcontainers.BindMount(path.Join(cwd, ".", "test", "templates", "synapse.log.config"), "/data/log.config"), testcontainers.BindMount(d, "/app"), }, From d2862d05f1281d3c58a5339470b495874feceef7 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 May 2024 18:31:27 -0600 Subject: [PATCH 30/51] Ensure signing key information is carried into the config object --- common/config/access.go | 1 + 1 file changed, 1 insertion(+) diff --git a/common/config/access.go b/common/config/access.go index 04099653..73a1e5a3 100644 --- a/common/config/access.go +++ b/common/config/access.go @@ -153,6 +153,7 @@ func reloadConfig() (*MainRepoConfig, map[string]*DomainRepoConfig, error) { dc.ClientServerApi = d.ClientServerApi dc.BackoffAt = d.BackoffAt dc.AdminApiKind = d.AdminApiKind + dc.SigningKeyPath = d.SigningKeyPath m, err := objToMapYaml(dc) if err != nil { From 5af30359cc690caabb9d85233950f5550b37efd0 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 May 2024 18:31:44 -0600 Subject: [PATCH 31/51] Generally treat homeserver config a bite more safely --- test/test_internals/deps_mmr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_internals/deps_mmr.go b/test/test_internals/deps_mmr.go index 533c3657..5a01504b 100644 --- a/test/test_internals/deps_mmr.go +++ b/test/test_internals/deps_mmr.go @@ -23,7 +23,7 @@ type mmrHomeserverTmplArgs struct { } type mmrTmplArgs struct { - Homeservers []mmrHomeserverTmplArgs + Homeservers []*mmrHomeserverTmplArgs RedisAddr string PgConnectionString string S3Endpoint string From 4572673063d2508295f0ea3b3963b2a914948262 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 21 May 2024 13:52:48 -0600 Subject: [PATCH 32/51] Support and use new 3916v2 federation download URL --- api/routes.go | 3 ++- api/unstable/msc3916_download.go | 2 ++ pipelines/_steps/download/try_download.go | 2 +- test/msc3916_downloads_suite_test.go | 4 ++-- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/api/routes.go b/api/routes.go index d7393a52..2fbc652d 100644 --- a/api/routes.go +++ b/api/routes.go @@ -55,6 +55,7 @@ func buildRoutes() http.Handler { register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId", msc3916, router, authedDownloadRoute) register([]string{"GET"}, PrefixClient, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireAccessToken(r0.ThumbnailMedia), "thumbnail", counter)) register([]string{"GET"}, PrefixFederation, "media/download/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) + register([]string{"GET"}, PrefixFederation, "media/download/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) register([]string{"GET"}, PrefixFederation, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) // Custom features @@ -143,7 +144,7 @@ var ( //mxAllSpec matrixVersions = []string{"r0", "v1", "v3", "unstable", "unstable/io.t2bot.media" /* and MSC routes */} mxUnstable matrixVersions = []string{"unstable", "unstable/io.t2bot.media"} msc4034 matrixVersions = []string{"unstable/org.matrix.msc4034"} - msc3916 matrixVersions = []string{"unstable/org.matrix.msc3916"} + msc3916 matrixVersions = []string{"unstable/org.matrix.msc3916", "unstable/org.matrix.msc3916.v2"} mxSpecV3Transition matrixVersions = []string{"r0", "v1", "v3"} mxSpecV3TransitionCS matrixVersions = []string{"r0", "v3"} mxR0 matrixVersions = []string{"r0"} diff --git a/api/unstable/msc3916_download.go b/api/unstable/msc3916_download.go index 6089976a..c357cf56 100644 --- a/api/unstable/msc3916_download.go +++ b/api/unstable/msc3916_download.go @@ -6,6 +6,7 @@ import ( "github.com/t2bot/matrix-media-repo/api/_apimeta" "github.com/t2bot/matrix-media-repo/api/_responses" + "github.com/t2bot/matrix-media-repo/api/_routers" "github.com/t2bot/matrix-media-repo/api/r0" "github.com/t2bot/matrix-media-repo/common/rcontext" "github.com/t2bot/matrix-media-repo/util/readers" @@ -18,6 +19,7 @@ func ClientDownloadMedia(r *http.Request, rctx rcontext.RequestContext, user _ap func FederationDownloadMedia(r *http.Request, rctx rcontext.RequestContext, server _apimeta.ServerInfo) interface{} { r.URL.Query().Set("allow_remote", "false") + r = _routers.ForceSetParam("server", r.Host, r) res := r0.DownloadMedia(r, rctx, _apimeta.UserInfo{}) if dl, ok := res.(*_responses.DownloadResponse); ok { diff --git a/pipelines/_steps/download/try_download.go b/pipelines/_steps/download/try_download.go index c087dce6..d1fbcbec 100644 --- a/pipelines/_steps/download/try_download.go +++ b/pipelines/_steps/download/try_download.go @@ -63,7 +63,7 @@ func TryDownload(ctx rcontext.RequestContext, origin string, mediaId string) (*d var downloadUrl string usesMultipartFormat := false if ctx.Config.SigningKeyPath != "" { - downloadUrl = fmt.Sprintf("%s/_matrix/federation/unstable/org.matrix.msc3916/media/download/%s/%s?a=b", baseUrl, url.PathEscape(origin), url.PathEscape(mediaId)) + downloadUrl = fmt.Sprintf("%s/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", baseUrl, url.PathEscape(mediaId)) resp, err = matrix.FederatedGet(ctx, downloadUrl, realHost, ctx.Config.SigningKeyPath) metrics.MediaDownloaded.With(prometheus.Labels{"origin": origin}).Inc() if err != nil { diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index c65f8bf4..77c47a45 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -166,7 +166,7 @@ func (s *MSC3916DownloadsSuite) TestFederationDownloads() { assert.NotEmpty(t, mediaId) // Verify the federation download *fails* when lacking auth - uri := fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId) + uri := fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId) raw, err := remoteClient.DoRaw("GET", uri, nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) @@ -195,7 +195,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { err := matrix.TestsOnlyInjectSigningKey(s.deps.Homeservers[0].ServerName, s.deps.Homeservers[0].ExternalClientServerApiUrl) assert.NoError(t, err) testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), r.URL.Path) + assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId), r.URL.Path) origin, err := matrix.ValidateXMatrixAuth(r, true) assert.NoError(t, err) assert.Equal(t, client1.ServerName, origin) From 1cc666df7edf3da3e86a126fa45d8d096e0ea024 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 May 2024 18:35:23 -0600 Subject: [PATCH 33/51] Fix signing key permissions? --- test/test_internals/deps.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_internals/deps.go b/test/test_internals/deps.go index c419d457..06999673 100644 --- a/test/test_internals/deps.go +++ b/test/test_internals/deps.go @@ -81,6 +81,10 @@ func MakeTestDeps() (*ContainerDeps, error) { if err != nil { return nil, err } + err = os.Chmod(synapseSigningKeyFile.Name(), 0777) // XXX: Not great, but works. + if err != nil { + return nil, err + } // Start two synapses for testing syn1, err := MakeSynapse("first.example.org", depNet, synapseSigningKeyFile.Name()) From b0ba084266a12b4d747ed8c1c7cee72b35deda53 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 24 May 2024 18:40:59 -0600 Subject: [PATCH 34/51] Fix routing --- api/routes.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/api/routes.go b/api/routes.go index 2fbc652d..24cd9fe3 100644 --- a/api/routes.go +++ b/api/routes.go @@ -55,7 +55,7 @@ func buildRoutes() http.Handler { register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId", msc3916, router, authedDownloadRoute) register([]string{"GET"}, PrefixClient, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireAccessToken(r0.ThumbnailMedia), "thumbnail", counter)) register([]string{"GET"}, PrefixFederation, "media/download/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) - register([]string{"GET"}, PrefixFederation, "media/download/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) + register([]string{"GET"}, PrefixFederation, "media/download/:mediaId", msc3916v2, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) register([]string{"GET"}, PrefixFederation, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) // Custom features @@ -144,7 +144,8 @@ var ( //mxAllSpec matrixVersions = []string{"r0", "v1", "v3", "unstable", "unstable/io.t2bot.media" /* and MSC routes */} mxUnstable matrixVersions = []string{"unstable", "unstable/io.t2bot.media"} msc4034 matrixVersions = []string{"unstable/org.matrix.msc4034"} - msc3916 matrixVersions = []string{"unstable/org.matrix.msc3916", "unstable/org.matrix.msc3916.v2"} + msc3916 matrixVersions = []string{"unstable/org.matrix.msc3916"} + msc3916v2 matrixVersions = []string{"unstable/org.matrix.msc3916.v2"} mxSpecV3Transition matrixVersions = []string{"r0", "v1", "v3"} mxSpecV3TransitionCS matrixVersions = []string{"r0", "v3"} mxR0 matrixVersions = []string{"r0"} From c7776f043868a6160292177af82682efca2f9627 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 28 May 2024 17:21:50 -0600 Subject: [PATCH 35/51] Update redirect-supporting behaviour --- api/routes.go | 2 +- api/unstable/msc3916_download.go | 2 ++ api/unstable/msc3916_thumbnail.go | 7 +++++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/api/routes.go b/api/routes.go index 24cd9fe3..48f96868 100644 --- a/api/routes.go +++ b/api/routes.go @@ -53,7 +53,7 @@ func buildRoutes() http.Handler { authedDownloadRoute := makeRoute(_routers.RequireAccessToken(unstable.ClientDownloadMedia), "download", counter) register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId/:filename", msc3916, router, authedDownloadRoute) register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId", msc3916, router, authedDownloadRoute) - register([]string{"GET"}, PrefixClient, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireAccessToken(r0.ThumbnailMedia), "thumbnail", counter)) + register([]string{"GET"}, PrefixClient, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireAccessToken(unstable.ClientThumbnailMedia), "thumbnail", counter)) register([]string{"GET"}, PrefixFederation, "media/download/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) register([]string{"GET"}, PrefixFederation, "media/download/:mediaId", msc3916v2, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) register([]string{"GET"}, PrefixFederation, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) diff --git a/api/unstable/msc3916_download.go b/api/unstable/msc3916_download.go index c357cf56..c55ee1b0 100644 --- a/api/unstable/msc3916_download.go +++ b/api/unstable/msc3916_download.go @@ -14,11 +14,13 @@ import ( func ClientDownloadMedia(r *http.Request, rctx rcontext.RequestContext, user _apimeta.UserInfo) interface{} { r.URL.Query().Set("allow_remote", "true") + r.URL.Query().Set("allow_redirect", "true") return r0.DownloadMedia(r, rctx, user) } func FederationDownloadMedia(r *http.Request, rctx rcontext.RequestContext, server _apimeta.ServerInfo) interface{} { r.URL.Query().Set("allow_remote", "false") + r.URL.Query().Set("allow_redirect", "false") // not supported at the top level r = _routers.ForceSetParam("server", r.Host, r) res := r0.DownloadMedia(r, rctx, _apimeta.UserInfo{}) diff --git a/api/unstable/msc3916_thumbnail.go b/api/unstable/msc3916_thumbnail.go index e6de68ec..b22f81f9 100644 --- a/api/unstable/msc3916_thumbnail.go +++ b/api/unstable/msc3916_thumbnail.go @@ -11,8 +11,15 @@ import ( "github.com/t2bot/matrix-media-repo/util/readers" ) +func ClientThumbnailMedia(r *http.Request, rctx rcontext.RequestContext, user _apimeta.UserInfo) interface{} { + r.URL.Query().Set("allow_remote", "true") + r.URL.Query().Set("allow_redirect", "true") + return r0.ThumbnailMedia(r, rctx, user) +} + func FederationThumbnailMedia(r *http.Request, rctx rcontext.RequestContext, server _apimeta.ServerInfo) interface{} { r.URL.Query().Set("allow_remote", "false") + r.URL.Query().Set("allow_redirect", "false") // not supported at the top level res := r0.ThumbnailMedia(r, rctx, _apimeta.UserInfo{}) if dl, ok := res.(*_responses.DownloadResponse); ok { From bf17b97478b64c5da2bb4bbd76462c5ecf31d406 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 28 May 2024 17:29:03 -0600 Subject: [PATCH 36/51] Support redirects --- pipelines/_steps/download/try_download.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pipelines/_steps/download/try_download.go b/pipelines/_steps/download/try_download.go index d1fbcbec..dea01dbd 100644 --- a/pipelines/_steps/download/try_download.go +++ b/pipelines/_steps/download/try_download.go @@ -12,6 +12,7 @@ import ( "strconv" "strings" + "github.com/getsentry/sentry-go" "github.com/prometheus/client_golang/prometheus" "github.com/t2bot/matrix-media-repo/common" "github.com/t2bot/matrix-media-repo/common/rcontext" @@ -179,6 +180,26 @@ func TryDownload(ctx rcontext.RequestContext, origin string, mediaId string) (*d } mediaPart = util.MatrixMediaPartFromMimeMultipart(bodyPart) contentType = mediaPart.Header.Get("Content-Type") // Content-Type should really be the media content type + + locationHeader := mediaPart.Header.Get("Location") + if locationHeader != "" { + // the media part body won't have anything for us - go `GET` the URL. + ctx.Log.Debugf("Redirecting to %s", locationHeader) + + err = mediaPart.Body.Close() + if err != nil { + sentry.CaptureException(errors.Join(errors.New("non-fatal error closing redirected MSC3916 body"), err)) + ctx.Log.Debug("Non-fatal error closing redirected MSC3916 body: ", err) + } + + resp, err = http.DefaultClient.Get(locationHeader) + if err != nil { + errFn(err) + return + } + mediaPart = util.MatrixMediaPartFromResponse(resp) + contentType = mediaPart.Header.Get("Content-Type") + } } // Default the Content-Type if we haven't already From 21e82819f3442bdba92bee8162e0e06040f861e3 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 29 May 2024 17:30:00 -0600 Subject: [PATCH 37/51] Finish tests --- test/msc3916_downloads_suite_test.go | 91 ++++++++++++++++++++++++++-- 1 file changed, 85 insertions(+), 6 deletions(-) diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index 77c47a45..b4905de6 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -5,6 +5,7 @@ import ( "crypto/ed25519" "encoding/json" "fmt" + "io" "log" "net/http" "net/http/httptest" @@ -180,11 +181,6 @@ func (s *MSC3916DownloadsSuite) TestFederationDownloads() { assert.Equal(t, http.StatusOK, raw.StatusCode) } -// TODO: Tests for: -// * Actually tries MSC3916 for downloads -// * Doesn't call unauthenticated endpoint if MSC3916 was successful & sets correct auth -// * Falls back on failure - func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { t := s.T() @@ -213,9 +209,92 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { assert.Equal(t, http.StatusOK, raw.StatusCode) } +func (s *MSC3916DownloadsSuite) TestFederationFollowsRedirects() { + t := s.T() + + client1 := s.deps.Homeservers[0].UnprivilegedUsers[0].WithCsUrl(s.deps.Machines[0].HttpUrl) + + origin := "" + mediaId := "abc123" + fileContents := "hello world! This is a test file" + err := matrix.TestsOnlyInjectSigningKey(s.deps.Homeservers[0].ServerName, s.deps.Homeservers[0].ExternalClientServerApiUrl) + assert.NoError(t, err) + + // Mock CDN (2nd hop) + testServer2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/cdn/file", r.URL.Path) + w.Header().Set("Content-Type", "text/plain") + _, _ = w.Write([]byte(fileContents)) + })) + defer testServer2.Close() + u, _ := url.Parse(testServer2.URL) + //goland:noinspection HttpUrlsUsage + redirectUrl := fmt.Sprintf("http://host.docker.internal:%s/cdn/file", u.Port()) + + // Mock homeserver (1st hop) + testServer1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId), r.URL.Path) + origin, err := matrix.ValidateXMatrixAuth(r, true) + assert.NoError(t, err) + assert.Equal(t, client1.ServerName, origin) + w.Header().Set("Content-Type", "multipart/mixed; boundary=gc0p4Jq0M2Yt08jU534c0p") + _, _ = w.Write([]byte(fmt.Sprintf("--gc0p4Jq0M2Yt08jU534c0p\nContent-Type: application/json\n\n{}\n\n--gc0p4Jq0M2Yt08jU534c0p\nLocation: %s\n\n-gc0p4Jq0M2Yt08jU534c0p", redirectUrl))) + })) + defer testServer1.Close() + + u, _ = url.Parse(testServer1.URL) + origin = fmt.Sprintf("host.docker.internal:%s", u.Port()) + config.AddDomainForTesting("host.docker.internal", nil) // no port for config lookup + + raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, raw.StatusCode) + + b, err := io.ReadAll(raw.Body) + assert.NoError(t, err) + assert.Equal(t, fileContents, string(b)) +} + func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloadsAndFallsBack() { t := s.T() - t.Error("not yet implemented") + + client1 := s.deps.Homeservers[0].UnprivilegedUsers[0].WithCsUrl(s.deps.Machines[0].HttpUrl) + + origin := "" + mediaId := "abc123" + fileContents := "hello world! This is a test file" + err := matrix.TestsOnlyInjectSigningKey(s.deps.Homeservers[0].ServerName, s.deps.Homeservers[0].ExternalClientServerApiUrl) + assert.NoError(t, err) + + reqNum := 0 + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if reqNum == 0 { + origin, err := matrix.ValidateXMatrixAuth(r, true) + assert.NoError(t, err) + assert.Equal(t, client1.ServerName, origin) + assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId), r.URL.Path) + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte("{\"errcode\":\"M_UNRECOGNIZED\"}")) + reqNum++ + } else { + assert.Equal(t, fmt.Sprintf("/_matrix/media/v3/download/%s/%s", origin, mediaId), r.URL.Path) + } + w.Header().Set("Content-Type", "text/plain") + _, _ = w.Write([]byte(fileContents)) + })) + defer testServer.Close() + + u, _ := url.Parse(testServer.URL) + origin = fmt.Sprintf("host.docker.internal:%s", u.Port()) + config.AddDomainForTesting("host.docker.internal", nil) // no port for config lookup + + raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, raw.StatusCode) + + b, err := io.ReadAll(raw.Body) + assert.NoError(t, err) + assert.Equal(t, fileContents, string(b)) } func TestMSC3916DownloadsSuite(t *testing.T) { From 99ab04a5c2d33596450ba4aecdfaa71d0976bb9a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 30 May 2024 11:22:53 -0600 Subject: [PATCH 38/51] Mark test function as deprecated to discourage use --- database/db.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/database/db.go b/database/db.go index 5931a481..ffae2bc4 100644 --- a/database/db.go +++ b/database/db.go @@ -61,6 +61,8 @@ func Reload() { GetInstance() } +// GetAccessorForTests +// Deprecated: For tests only. func GetAccessorForTests() *sql.DB { return GetInstance().conn } From 8f01e456ed25081b288e87a63dab940fd687487b Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 30 May 2024 11:23:11 -0600 Subject: [PATCH 39/51] Avoid testcontainers tests from overwriting the config concurrently. --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 91212987..cff31f0e 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -44,5 +44,5 @@ jobs: - name: "Run: compile assets" run: "$PWD/bin/compile_assets" - name: "Run: tests" - run: "go test -c -v ./test && ./test.test '-test.v'" # cheat and work around working directory issues + run: "go test -c -v ./test && ./test.test '-test.v' -test.parallel 1" # cheat and work around working directory issues timeout-minutes: 30 From fa406565b875110f442389d86098dfb8e05ccd0a Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 30 May 2024 11:39:00 -0600 Subject: [PATCH 40/51] host.docker.internal doesn't exist on linux --- test/msc3916_downloads_suite_test.go | 14 +++++++------- test/test_internals/util.go | 8 ++++++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index b4905de6..deba8493 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -201,8 +201,8 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { defer testServer.Close() u, _ := url.Parse(testServer.URL) - origin = fmt.Sprintf("host.docker.internal:%s", u.Port()) - config.AddDomainForTesting("host.docker.internal", nil) // no port for config lookup + origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) + config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) @@ -229,7 +229,7 @@ func (s *MSC3916DownloadsSuite) TestFederationFollowsRedirects() { defer testServer2.Close() u, _ := url.Parse(testServer2.URL) //goland:noinspection HttpUrlsUsage - redirectUrl := fmt.Sprintf("http://host.docker.internal:%s/cdn/file", u.Port()) + redirectUrl := fmt.Sprintf("http://%s:%s/cdn/file", test_internals.DockerHostAddress(), u.Port()) // Mock homeserver (1st hop) testServer1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -243,8 +243,8 @@ func (s *MSC3916DownloadsSuite) TestFederationFollowsRedirects() { defer testServer1.Close() u, _ = url.Parse(testServer1.URL) - origin = fmt.Sprintf("host.docker.internal:%s", u.Port()) - config.AddDomainForTesting("host.docker.internal", nil) // no port for config lookup + origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) + config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) @@ -285,8 +285,8 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloadsAndFallsBack() defer testServer.Close() u, _ := url.Parse(testServer.URL) - origin = fmt.Sprintf("host.docker.internal:%s", u.Port()) - config.AddDomainForTesting("host.docker.internal", nil) // no port for config lookup + origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) + config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) diff --git a/test/test_internals/util.go b/test/test_internals/util.go index fe0744b9..18ecf7ce 100644 --- a/test/test_internals/util.go +++ b/test/test_internals/util.go @@ -6,6 +6,7 @@ import ( "image" "image/color" "io" + "runtime" "testing" "github.com/disintegration/imaging" @@ -58,3 +59,10 @@ func AssertIsTestImage(t *testing.T, i io.Reader) { } } } + +func DockerHostAddress() string { + if runtime.GOOS == "linux" { + return "172.17.0.1" // XXX: This is bad + } + return "host.docker.internal" +} From 5f166484b3a695370e699fbea209bc636f3217a6 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 3 Jun 2024 11:34:40 -0600 Subject: [PATCH 41/51] Temporarily disable upload tests --- test/upload_suite_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/upload_suite_test.go b/test/upload_suite_test.go index b203b8ce..cd954310 100644 --- a/test/upload_suite_test.go +++ b/test/upload_suite_test.go @@ -8,7 +8,6 @@ import ( "net/http" "net/url" "sync" - "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" @@ -342,6 +341,6 @@ func (s *UploadTestSuite) TestUploadAsyncExpiredFlow() { assert.Equal(t, http.StatusNotFound, errRes.InjectedStatusCode) } -func TestUploadTestSuite(t *testing.T) { - suite.Run(t, new(UploadTestSuite)) -} +//func TestUploadTestSuite(t *testing.T) { +// suite.Run(t, new(UploadTestSuite)) +//} From 806464d0db909d4bde9747c029a46c5c2d5f2229 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 3 Jun 2024 11:41:04 -0600 Subject: [PATCH 42/51] Support federation thumbnails again --- api/routes.go | 1 + api/unstable/msc3916_thumbnail.go | 2 + test/msc3916_downloads_suite_test.go | 48 +-------------------- test/msc3916_thumbnails_suite_test.go | 48 ++++++++++++++++++++- test/test_internals/util_keyserver.go | 60 +++++++++++++++++++++++++++ 5 files changed, 111 insertions(+), 48 deletions(-) create mode 100644 test/test_internals/util_keyserver.go diff --git a/api/routes.go b/api/routes.go index 48f96868..054a2a1b 100644 --- a/api/routes.go +++ b/api/routes.go @@ -57,6 +57,7 @@ func buildRoutes() http.Handler { register([]string{"GET"}, PrefixFederation, "media/download/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) register([]string{"GET"}, PrefixFederation, "media/download/:mediaId", msc3916v2, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) register([]string{"GET"}, PrefixFederation, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) + register([]string{"GET"}, PrefixFederation, "media/thumbnail/:mediaId", msc3916v2, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) // Custom features register([]string{"GET"}, PrefixMedia, "local_copy/:server/:mediaId", mxUnstable, router, makeRoute(_routers.RequireAccessToken(unstable.LocalCopy), "local_copy", counter)) diff --git a/api/unstable/msc3916_thumbnail.go b/api/unstable/msc3916_thumbnail.go index b22f81f9..3d1db0d2 100644 --- a/api/unstable/msc3916_thumbnail.go +++ b/api/unstable/msc3916_thumbnail.go @@ -6,6 +6,7 @@ import ( "github.com/t2bot/matrix-media-repo/api/_apimeta" "github.com/t2bot/matrix-media-repo/api/_responses" + "github.com/t2bot/matrix-media-repo/api/_routers" "github.com/t2bot/matrix-media-repo/api/r0" "github.com/t2bot/matrix-media-repo/common/rcontext" "github.com/t2bot/matrix-media-repo/util/readers" @@ -20,6 +21,7 @@ func ClientThumbnailMedia(r *http.Request, rctx rcontext.RequestContext, user _a func FederationThumbnailMedia(r *http.Request, rctx rcontext.RequestContext, server _apimeta.ServerInfo) interface{} { r.URL.Query().Set("allow_remote", "false") r.URL.Query().Set("allow_redirect", "false") // not supported at the top level + r = _routers.ForceSetParam("server", r.Host, r) res := r0.ThumbnailMedia(r, rctx, _apimeta.UserInfo{}) if dl, ok := res.(*_responses.DownloadResponse); ok { diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index deba8493..d08a8147 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -1,9 +1,6 @@ package test import ( - "bytes" - "crypto/ed25519" - "encoding/json" "fmt" "io" "log" @@ -18,7 +15,6 @@ import ( "github.com/t2bot/matrix-media-repo/common/config" "github.com/t2bot/matrix-media-repo/database" "github.com/t2bot/matrix-media-repo/homeserver_interop" - "github.com/t2bot/matrix-media-repo/homeserver_interop/mmr" "github.com/t2bot/matrix-media-repo/matrix" "github.com/t2bot/matrix-media-repo/test/test_internals" "github.com/t2bot/matrix-media-repo/util" @@ -43,49 +39,7 @@ func (s *MSC3916DownloadsSuite) SetupSuite() { } s.deps = deps - // We'll use a pre-computed signing key for simplicity - signingKey, err := mmr.DecodeSigningKey(bytes.NewReader([]byte(`-----BEGIN MMR PRIVATE KEY----- -Key-ID: ed25519:e5d0oC -Version: 1 - -PJt0OaIImDJk8P/PDb4TNQHgI/1AA1C+AaQaABxAcgc= ------END MMR PRIVATE KEY----- -`))) - if err != nil { - log.Fatal(err) - } - s.keyServerKey = signingKey - // Create a /_matrix/key/v2/server response file (signed JSON) - keyServer, writeFn, err := test_internals.LazyServeFile("_matrix/key/v2/server", deps) - if err != nil { - log.Fatal(err) - } - s.keyServer = keyServer - serverKey := database.AnonymousJson{ - "old_verify_keys": database.AnonymousJson{}, - "server_name": keyServer.PublicHostname, - "valid_until_ts": util.NowMillis() + (60 * 60 * 1000), // +1hr - "verify_keys": database.AnonymousJson{ - "ed25519:e5d0oC": database.AnonymousJson{ - "key": "TohekYXzLx7VzV8FtLQlI3XsSdPv1CjhVYY5rZmFCvU", - }, - }, - } - canonical, err := util.EncodeCanonicalJson(serverKey) - signature := util.EncodeUnpaddedBase64ToString(ed25519.Sign(signingKey.PrivateKey, canonical)) - serverKey["signatures"] = database.AnonymousJson{ - keyServer.PublicHostname: database.AnonymousJson{ - "ed25519:e5d0oC": signature, - }, - } - b, err := json.Marshal(serverKey) - if err != nil { - log.Fatal(err) - } - err = writeFn(string(b)) - if err != nil { - log.Fatal(err) - } + s.keyServer, s.keyServerKey = test_internals.MakeKeyServer(deps) } func (s *MSC3916DownloadsSuite) TearDownSuite() { diff --git a/test/msc3916_thumbnails_suite_test.go b/test/msc3916_thumbnails_suite_test.go index 996807db..db76877c 100644 --- a/test/msc3916_thumbnails_suite_test.go +++ b/test/msc3916_thumbnails_suite_test.go @@ -9,13 +9,18 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" + "github.com/t2bot/matrix-media-repo/database" + "github.com/t2bot/matrix-media-repo/homeserver_interop" + "github.com/t2bot/matrix-media-repo/matrix" "github.com/t2bot/matrix-media-repo/test/test_internals" "github.com/t2bot/matrix-media-repo/util" ) type MSC3916ThumbnailsSuite struct { suite.Suite - deps *test_internals.ContainerDeps + deps *test_internals.ContainerDeps + keyServer *test_internals.HostedFile + keyServerKey *homeserver_interop.SigningKey } func (s *MSC3916ThumbnailsSuite) SetupSuite() { @@ -24,6 +29,8 @@ func (s *MSC3916ThumbnailsSuite) SetupSuite() { log.Fatal(err) } s.deps = deps + + s.keyServer, s.keyServerKey = test_internals.MakeKeyServer(deps) } func (s *MSC3916ThumbnailsSuite) TearDownSuite() { @@ -77,6 +84,45 @@ func (s *MSC3916ThumbnailsSuite) TestClientThumbnails() { //test_internals.AssertIsTestImage(t, raw.Body) // we can't verify that the resulting image is correct } +func (s *MSC3916ThumbnailsSuite) TestFederationThumbnails() { + t := s.T() + + client1 := s.deps.Homeservers[0].UnprivilegedUsers[0].WithCsUrl(s.deps.Machines[0].HttpUrl) + remoteClient := &test_internals.MatrixClient{ + ClientServerUrl: s.deps.Machines[0].HttpUrl, + ServerName: s.deps.Homeservers[0].ServerName, + AccessToken: "", // this client isn't authed over the CS API + UserId: "", // this client isn't authed over the CS API + } + + contentType, img, err := test_internals.MakeTestImage(512, 512) + assert.NoError(t, err) + fname := "image" + util.ExtensionForContentType(contentType) + + res, err := client1.Upload(fname, contentType, img) + assert.NoError(t, err) + assert.NotEmpty(t, res.MxcUri) + + origin, mediaId, err := util.SplitMxc(res.MxcUri) + assert.NoError(t, err) + assert.Equal(t, client1.ServerName, origin) + assert.NotEmpty(t, mediaId) + + // Verify the federation download *fails* when lacking auth + uri := fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/thumbnail/%s?width=96&height=96&method=scale", mediaId) + raw, err := remoteClient.DoRaw("GET", uri, nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) + + // Now add the X-Matrix auth and try again + header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", uri, &database.AnonymousJson{}, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion) + assert.NoError(t, err) + remoteClient.AuthHeaderOverride = header + raw, err = remoteClient.DoRaw("GET", uri, nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, raw.StatusCode) +} + func TestMSC3916ThumbnailsSuite(t *testing.T) { suite.Run(t, new(MSC3916ThumbnailsSuite)) } diff --git a/test/test_internals/util_keyserver.go b/test/test_internals/util_keyserver.go new file mode 100644 index 00000000..e8aaa914 --- /dev/null +++ b/test/test_internals/util_keyserver.go @@ -0,0 +1,60 @@ +package test_internals + +import ( + "bytes" + "crypto/ed25519" + "encoding/json" + "log" + + "github.com/t2bot/matrix-media-repo/database" + "github.com/t2bot/matrix-media-repo/homeserver_interop" + "github.com/t2bot/matrix-media-repo/homeserver_interop/mmr" + "github.com/t2bot/matrix-media-repo/util" +) + +func MakeKeyServer(deps *ContainerDeps) (*HostedFile, *homeserver_interop.SigningKey) { + // We'll use a pre-computed signing key for simplicity + signingKey, err := mmr.DecodeSigningKey(bytes.NewReader([]byte(`-----BEGIN MMR PRIVATE KEY----- +Key-ID: ed25519:e5d0oC +Version: 1 + +PJt0OaIImDJk8P/PDb4TNQHgI/1AA1C+AaQaABxAcgc= +-----END MMR PRIVATE KEY----- +`))) + if err != nil { + log.Fatal(err) + } + keyServerKey := signingKey + // Create a /_matrix/key/v2/server response file (signed JSON) + keyServer, writeFn, err := LazyServeFile("_matrix/key/v2/server", deps) + if err != nil { + log.Fatal(err) + } + serverKey := database.AnonymousJson{ + "old_verify_keys": database.AnonymousJson{}, + "server_name": keyServer.PublicHostname, + "valid_until_ts": util.NowMillis() + (60 * 60 * 1000), // +1hr + "verify_keys": database.AnonymousJson{ + "ed25519:e5d0oC": database.AnonymousJson{ + "key": "TohekYXzLx7VzV8FtLQlI3XsSdPv1CjhVYY5rZmFCvU", + }, + }, + } + canonical, err := util.EncodeCanonicalJson(serverKey) + signature := util.EncodeUnpaddedBase64ToString(ed25519.Sign(signingKey.PrivateKey, canonical)) + serverKey["signatures"] = database.AnonymousJson{ + keyServer.PublicHostname: database.AnonymousJson{ + "ed25519:e5d0oC": signature, + }, + } + b, err := json.Marshal(serverKey) + if err != nil { + log.Fatal(err) + } + err = writeFn(string(b)) + if err != nil { + log.Fatal(err) + } + + return keyServer, keyServerKey +} From 5133471f4ce79be7f2b0d1514f398e8c8a903790 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 3 Jun 2024 17:18:36 -0600 Subject: [PATCH 43/51] Fix tests for auth header --- test/msc3916_thumbnails_suite_test.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/test/msc3916_thumbnails_suite_test.go b/test/msc3916_thumbnails_suite_test.go index db76877c..5ea5bc84 100644 --- a/test/msc3916_thumbnails_suite_test.go +++ b/test/msc3916_thumbnails_suite_test.go @@ -109,16 +109,21 @@ func (s *MSC3916ThumbnailsSuite) TestFederationThumbnails() { assert.NotEmpty(t, mediaId) // Verify the federation download *fails* when lacking auth - uri := fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/thumbnail/%s?width=96&height=96&method=scale", mediaId) - raw, err := remoteClient.DoRaw("GET", uri, nil, "", nil) + uri := fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/thumbnail/%s", mediaId) + qs := url.Values{ + "width": []string{"96"}, + "height": []string{"96"}, + "method": []string{"scale"}, + } + raw, err := remoteClient.DoRaw("GET", uri, qs, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) // Now add the X-Matrix auth and try again - header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", uri, &database.AnonymousJson{}, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion) + header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", fmt.Sprintf("%s?%s", uri, qs.Encode()), &database.AnonymousJson{}, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion) assert.NoError(t, err) remoteClient.AuthHeaderOverride = header - raw, err = remoteClient.DoRaw("GET", uri, nil, "", nil) + raw, err = remoteClient.DoRaw("GET", uri, qs, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) } From 9697e1197eac268f1f4843372f3b7fc03f109abb Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 20 Jun 2024 16:26:57 -0600 Subject: [PATCH 44/51] Switch to stable endpoints --- CHANGELOG.md | 2 +- api/r0/versions.go | 20 ++++++++++++----- api/routes.go | 20 ++++++----------- pipelines/_steps/download/try_download.go | 2 +- test/msc3916_downloads_suite_test.go | 22 +++++++++---------- ...sc3916_misc_client_endpoints_suite_test.go | 8 +++---- test/msc3916_thumbnails_suite_test.go | 8 +++---- 7 files changed, 42 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4271d3f..8f86975f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * S3 datastores can now specify a `prefixLength` to improve S3 performance on some providers. See `config.sample.yaml` for details. * Add `multipartUploads` flag for running MMR against unsupported S3 providers. See `config.sample.yaml` for details. * A new "leaky bucket" rate limit algorithm has been applied to downloads. See `rateLimit.buckets` in the config for details. -* Add *unstable* support for [MSC3916: Authentication for media](https://github.com/matrix-org/matrix-spec-proposals/pull/3916). +* Add support for [MSC3916: Authentication for media](https://github.com/matrix-org/matrix-spec-proposals/pull/3916). * To enable full support, use `signingKeyPath` in your config. See sample config for details. ### Changed diff --git a/api/r0/versions.go b/api/r0/versions.go index 023f9a66..47fc5e79 100644 --- a/api/r0/versions.go +++ b/api/r0/versions.go @@ -1,13 +1,14 @@ package r0 import ( + "net/http" + "slices" + "github.com/getsentry/sentry-go" "github.com/t2bot/matrix-media-repo/api/_apimeta" "github.com/t2bot/matrix-media-repo/api/_responses" "github.com/t2bot/matrix-media-repo/matrix" - "net/http" - "github.com/t2bot/matrix-media-repo/common/rcontext" ) @@ -18,9 +19,18 @@ func ClientVersions(r *http.Request, rctx rcontext.RequestContext, user _apimeta sentry.CaptureException(err) return _responses.InternalServerError("unable to get versions") } - if versions.UnstableFeatures == nil { - versions.UnstableFeatures = make(map[string]bool) + + // This is where we'd add our feature/version support as needed + if versions.Versions == nil { + versions.Versions = make([]string, 1) + } + + // We add v1.11 by force, even though we can't reliably say the rest of the server implements it. This + // is because server admins which point `/versions` at us are effectively opting in to whatever features + // we need to advertise support for. In our case, it's at least Authenticated Media (MSC3916). + if !slices.Contains(versions.Versions, "v1.11") { + versions.Versions = append(versions.Versions, "v1.11") } - versions.UnstableFeatures["org.matrix.msc3916"] = true + return versions } diff --git a/api/routes.go b/api/routes.go index 054a2a1b..036a5f51 100644 --- a/api/routes.go +++ b/api/routes.go @@ -46,18 +46,14 @@ func buildRoutes() http.Handler { register([]string{"POST"}, PrefixClient, "logout/all", mxSpecV3TransitionCS, router, makeRoute(_routers.RequireAccessToken(r0.LogoutAll), "logout_all", counter)) register([]string{"POST"}, PrefixMedia, "create", mxV1, router, makeRoute(_routers.RequireAccessToken(v1.CreateMedia), "create", counter)) register([]string{"GET"}, PrefixClient, "versions", mxNoVersion, router, makeRoute(_routers.OptionalAccessToken(r0.ClientVersions), "client_versions", counter)) - - // MSC3916 - Authentication & endpoint API separation - register([]string{"GET"}, PrefixClient, "media/preview_url", msc3916, router, previewUrlRoute) - register([]string{"GET"}, PrefixClient, "media/config", msc3916, router, configRoute) + register([]string{"GET"}, PrefixClient, "media/preview_url", mxV1, router, previewUrlRoute) + register([]string{"GET"}, PrefixClient, "media/config", mxV1, router, configRoute) authedDownloadRoute := makeRoute(_routers.RequireAccessToken(unstable.ClientDownloadMedia), "download", counter) - register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId/:filename", msc3916, router, authedDownloadRoute) - register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId", msc3916, router, authedDownloadRoute) - register([]string{"GET"}, PrefixClient, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireAccessToken(unstable.ClientThumbnailMedia), "thumbnail", counter)) - register([]string{"GET"}, PrefixFederation, "media/download/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) - register([]string{"GET"}, PrefixFederation, "media/download/:mediaId", msc3916v2, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) - register([]string{"GET"}, PrefixFederation, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) - register([]string{"GET"}, PrefixFederation, "media/thumbnail/:mediaId", msc3916v2, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) + register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId/:filename", mxV1, router, authedDownloadRoute) + register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId", mxV1, router, authedDownloadRoute) + register([]string{"GET"}, PrefixClient, "media/thumbnail/:server/:mediaId", mxV1, router, makeRoute(_routers.RequireAccessToken(unstable.ClientThumbnailMedia), "thumbnail", counter)) + register([]string{"GET"}, PrefixFederation, "media/download/:mediaId", mxV1, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) + register([]string{"GET"}, PrefixFederation, "media/thumbnail/:mediaId", mxV1, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) // Custom features register([]string{"GET"}, PrefixMedia, "local_copy/:server/:mediaId", mxUnstable, router, makeRoute(_routers.RequireAccessToken(unstable.LocalCopy), "local_copy", counter)) @@ -145,8 +141,6 @@ var ( //mxAllSpec matrixVersions = []string{"r0", "v1", "v3", "unstable", "unstable/io.t2bot.media" /* and MSC routes */} mxUnstable matrixVersions = []string{"unstable", "unstable/io.t2bot.media"} msc4034 matrixVersions = []string{"unstable/org.matrix.msc4034"} - msc3916 matrixVersions = []string{"unstable/org.matrix.msc3916"} - msc3916v2 matrixVersions = []string{"unstable/org.matrix.msc3916.v2"} mxSpecV3Transition matrixVersions = []string{"r0", "v1", "v3"} mxSpecV3TransitionCS matrixVersions = []string{"r0", "v3"} mxR0 matrixVersions = []string{"r0"} diff --git a/pipelines/_steps/download/try_download.go b/pipelines/_steps/download/try_download.go index dea01dbd..3c2e959c 100644 --- a/pipelines/_steps/download/try_download.go +++ b/pipelines/_steps/download/try_download.go @@ -64,7 +64,7 @@ func TryDownload(ctx rcontext.RequestContext, origin string, mediaId string) (*d var downloadUrl string usesMultipartFormat := false if ctx.Config.SigningKeyPath != "" { - downloadUrl = fmt.Sprintf("%s/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", baseUrl, url.PathEscape(mediaId)) + downloadUrl = fmt.Sprintf("%s/_matrix/federation/v1/media/download/%s", baseUrl, url.PathEscape(mediaId)) resp, err = matrix.FederatedGet(ctx, downloadUrl, realHost, ctx.Config.SigningKeyPath) metrics.MediaDownloaded.With(prometheus.Labels{"origin": origin}).Inc() if err != nil { diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index d08a8147..7f37505a 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -79,18 +79,18 @@ func (s *MSC3916DownloadsSuite) TestClientDownloads() { assert.Equal(t, client1.ServerName, origin) assert.NotEmpty(t, mediaId) - raw, err := client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + raw, err := client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) - raw, err = client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s/whatever.png", origin, mediaId), nil, "", nil) + raw, err = client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s/whatever.png", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) - raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) test_internals.AssertIsTestImage(t, raw.Body) - raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s/whatever.png", origin, mediaId), nil, "", nil) + raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s/whatever.png", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) test_internals.AssertIsTestImage(t, raw.Body) @@ -121,7 +121,7 @@ func (s *MSC3916DownloadsSuite) TestFederationDownloads() { assert.NotEmpty(t, mediaId) // Verify the federation download *fails* when lacking auth - uri := fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId) + uri := fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId) raw, err := remoteClient.DoRaw("GET", uri, nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) @@ -145,7 +145,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { err := matrix.TestsOnlyInjectSigningKey(s.deps.Homeservers[0].ServerName, s.deps.Homeservers[0].ExternalClientServerApiUrl) assert.NoError(t, err) testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId), r.URL.Path) + assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId), r.URL.Path) origin, err := matrix.ValidateXMatrixAuth(r, true) assert.NoError(t, err) assert.Equal(t, client1.ServerName, origin) @@ -158,7 +158,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup - raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) } @@ -187,7 +187,7 @@ func (s *MSC3916DownloadsSuite) TestFederationFollowsRedirects() { // Mock homeserver (1st hop) testServer1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId), r.URL.Path) + assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId), r.URL.Path) origin, err := matrix.ValidateXMatrixAuth(r, true) assert.NoError(t, err) assert.Equal(t, client1.ServerName, origin) @@ -200,7 +200,7 @@ func (s *MSC3916DownloadsSuite) TestFederationFollowsRedirects() { origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup - raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) @@ -226,7 +226,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloadsAndFallsBack() origin, err := matrix.ValidateXMatrixAuth(r, true) assert.NoError(t, err) assert.Equal(t, client1.ServerName, origin) - assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId), r.URL.Path) + assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId), r.URL.Path) w.WriteHeader(http.StatusNotFound) _, _ = w.Write([]byte("{\"errcode\":\"M_UNRECOGNIZED\"}")) reqNum++ @@ -242,7 +242,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloadsAndFallsBack() origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup - raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) diff --git a/test/msc3916_misc_client_endpoints_suite_test.go b/test/msc3916_misc_client_endpoints_suite_test.go index 2aaa2cb3..1a0801fd 100644 --- a/test/msc3916_misc_client_endpoints_suite_test.go +++ b/test/msc3916_misc_client_endpoints_suite_test.go @@ -61,11 +61,11 @@ func (s *MSC3916MiscClientEndpointsSuite) TestPreviewUrlRequiresAuth() { qs := url.Values{ "url": []string{s.htmlPage.PublicUrl}, } - raw, err := client2.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/preview_url", qs, "", nil) + raw, err := client2.DoRaw("GET", "/_matrix/client/v1/media/preview_url", qs, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) - raw, err = client1.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/preview_url", qs, "", nil) + raw, err = client1.DoRaw("GET", "/_matrix/client/v1/media/preview_url", qs, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) } @@ -81,11 +81,11 @@ func (s *MSC3916MiscClientEndpointsSuite) TestConfigRequiresAuth() { UserId: "", // no auth on this client } - raw, err := client2.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/config", nil, "", nil) + raw, err := client2.DoRaw("GET", "/_matrix/client/v1/media/config", nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) - raw, err = client1.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/config", nil, "", nil) + raw, err = client1.DoRaw("GET", "/_matrix/client/v1/media/config", nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) } diff --git a/test/msc3916_thumbnails_suite_test.go b/test/msc3916_thumbnails_suite_test.go index 5ea5bc84..f2f5d0f0 100644 --- a/test/msc3916_thumbnails_suite_test.go +++ b/test/msc3916_thumbnails_suite_test.go @@ -57,8 +57,6 @@ func (s *MSC3916ThumbnailsSuite) TestClientThumbnails() { assert.NoError(t, err) fname := "image" + util.ExtensionForContentType(contentType) - //res := new(test_internals.MatrixUploadResponse) - //err = client1.DoReturnJson("POST", "/_matrix/client/unstable/org.matrix.msc3916/media/upload", url.Values{"filename": []string{fname}}, contentType, img, res) res, err := client1.Upload(fname, contentType, img) assert.NoError(t, err) assert.NotEmpty(t, res.MxcUri) @@ -74,11 +72,11 @@ func (s *MSC3916ThumbnailsSuite) TestClientThumbnails() { "method": []string{"scale"}, } - raw, err := client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/thumbnail/%s/%s", origin, mediaId), qs, "", nil) + raw, err := client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/thumbnail/%s/%s", origin, mediaId), qs, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) - raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/thumbnail/%s/%s", origin, mediaId), qs, "", nil) + raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/thumbnail/%s/%s", origin, mediaId), qs, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) //test_internals.AssertIsTestImage(t, raw.Body) // we can't verify that the resulting image is correct @@ -109,7 +107,7 @@ func (s *MSC3916ThumbnailsSuite) TestFederationThumbnails() { assert.NotEmpty(t, mediaId) // Verify the federation download *fails* when lacking auth - uri := fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/thumbnail/%s", mediaId) + uri := fmt.Sprintf("/_matrix/federation/v1.v2/media/thumbnail/%s", mediaId) qs := url.Values{ "width": []string{"96"}, "height": []string{"96"}, From 85e7cbe74cdbbd83aca68f38e998bce6b51f4e10 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 20 Jun 2024 16:52:01 -0600 Subject: [PATCH 45/51] Maybe use the correct stable endpoint too --- test/msc3916_downloads_suite_test.go | 8 ++++---- test/msc3916_thumbnails_suite_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index 7f37505a..15b56b2a 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -121,7 +121,7 @@ func (s *MSC3916DownloadsSuite) TestFederationDownloads() { assert.NotEmpty(t, mediaId) // Verify the federation download *fails* when lacking auth - uri := fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId) + uri := fmt.Sprintf("/_matrix/federation/v1/media/download/%s", mediaId) raw, err := remoteClient.DoRaw("GET", uri, nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) @@ -145,7 +145,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { err := matrix.TestsOnlyInjectSigningKey(s.deps.Homeservers[0].ServerName, s.deps.Homeservers[0].ExternalClientServerApiUrl) assert.NoError(t, err) testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId), r.URL.Path) + assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1/media/download/%s", mediaId), r.URL.Path) origin, err := matrix.ValidateXMatrixAuth(r, true) assert.NoError(t, err) assert.Equal(t, client1.ServerName, origin) @@ -187,7 +187,7 @@ func (s *MSC3916DownloadsSuite) TestFederationFollowsRedirects() { // Mock homeserver (1st hop) testServer1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId), r.URL.Path) + assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1/media/download/%s", mediaId), r.URL.Path) origin, err := matrix.ValidateXMatrixAuth(r, true) assert.NoError(t, err) assert.Equal(t, client1.ServerName, origin) @@ -226,7 +226,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloadsAndFallsBack() origin, err := matrix.ValidateXMatrixAuth(r, true) assert.NoError(t, err) assert.Equal(t, client1.ServerName, origin) - assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId), r.URL.Path) + assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1/media/download/%s", mediaId), r.URL.Path) w.WriteHeader(http.StatusNotFound) _, _ = w.Write([]byte("{\"errcode\":\"M_UNRECOGNIZED\"}")) reqNum++ diff --git a/test/msc3916_thumbnails_suite_test.go b/test/msc3916_thumbnails_suite_test.go index f2f5d0f0..4ff43a17 100644 --- a/test/msc3916_thumbnails_suite_test.go +++ b/test/msc3916_thumbnails_suite_test.go @@ -107,7 +107,7 @@ func (s *MSC3916ThumbnailsSuite) TestFederationThumbnails() { assert.NotEmpty(t, mediaId) // Verify the federation download *fails* when lacking auth - uri := fmt.Sprintf("/_matrix/federation/v1.v2/media/thumbnail/%s", mediaId) + uri := fmt.Sprintf("/_matrix/federation/v1/media/thumbnail/%s", mediaId) qs := url.Values{ "width": []string{"96"}, "height": []string{"96"}, From 69ac7b9a99674189d84aca29dc8a728d093bfb33 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 20 Jun 2024 17:17:28 -0600 Subject: [PATCH 46/51] Revert "Temporarily disable upload tests" This reverts commit e21fa0135b8fa880af7bbb7690e147a12946c28e. --- test/upload_suite_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/upload_suite_test.go b/test/upload_suite_test.go index cd954310..b203b8ce 100644 --- a/test/upload_suite_test.go +++ b/test/upload_suite_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/url" "sync" + "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" @@ -341,6 +342,6 @@ func (s *UploadTestSuite) TestUploadAsyncExpiredFlow() { assert.Equal(t, http.StatusNotFound, errRes.InjectedStatusCode) } -//func TestUploadTestSuite(t *testing.T) { -// suite.Run(t, new(UploadTestSuite)) -//} +func TestUploadTestSuite(t *testing.T) { + suite.Run(t, new(UploadTestSuite)) +} From a55b0d8373d39c1a67c5d02c7bd4df219b439e08 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 2 Jul 2024 15:13:15 -0600 Subject: [PATCH 47/51] Try fixing tests --- test/test_internals/deps_mmr.go | 4 ++++ test/test_internals/util.go | 7 +++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/test/test_internals/deps_mmr.go b/test/test_internals/deps_mmr.go index 5a01504b..07308030 100644 --- a/test/test_internals/deps_mmr.go +++ b/test/test_internals/deps_mmr.go @@ -11,6 +11,7 @@ import ( "strings" "text/template" + "github.com/docker/docker/api/types/container" "github.com/docker/go-connections/nat" "github.com/testcontainers/testcontainers-go" "github.com/testcontainers/testcontainers-go/wait" @@ -116,6 +117,9 @@ func makeMmrInstances(ctx context.Context, count int, depNet *NetworkDep, tmplAr }, Networks: []string{depNet.NetId}, WaitingFor: wait.ForHTTP("/healthz").WithPort(p), + HostConfigModifier: func(c *container.HostConfig) { + c.ExtraHosts = append(c.ExtraHosts, "host.docker.internal:host-gateway") + }, }, Started: true, }) diff --git a/test/test_internals/util.go b/test/test_internals/util.go index 18ecf7ce..d507c029 100644 --- a/test/test_internals/util.go +++ b/test/test_internals/util.go @@ -6,7 +6,6 @@ import ( "image" "image/color" "io" - "runtime" "testing" "github.com/disintegration/imaging" @@ -61,8 +60,8 @@ func AssertIsTestImage(t *testing.T, i io.Reader) { } func DockerHostAddress() string { - if runtime.GOOS == "linux" { - return "172.17.0.1" // XXX: This is bad - } + //if runtime.GOOS == "linux" { + // return "172.17.0.1" // XXX: This is bad + //} return "host.docker.internal" } From 547cc1a882f7a6abe8620122f7f4d5ade4550cce Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 2 Jul 2024 15:44:05 -0600 Subject: [PATCH 48/51] Hardcode `host.docker.internal` again --- test/msc3916_downloads_suite_test.go | 14 +++++++------- test/test_internals/util.go | 7 ------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index 15b56b2a..bfa75712 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -155,8 +155,8 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { defer testServer.Close() u, _ := url.Parse(testServer.URL) - origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) - config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup + origin = fmt.Sprintf("%s:%s", "host.docker.internal", u.Port()) + config.AddDomainForTesting("host.docker.internal", nil) // no port for config lookup raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) @@ -183,7 +183,7 @@ func (s *MSC3916DownloadsSuite) TestFederationFollowsRedirects() { defer testServer2.Close() u, _ := url.Parse(testServer2.URL) //goland:noinspection HttpUrlsUsage - redirectUrl := fmt.Sprintf("http://%s:%s/cdn/file", test_internals.DockerHostAddress(), u.Port()) + redirectUrl := fmt.Sprintf("http://%s:%s/cdn/file", "host.docker.internal", u.Port()) // Mock homeserver (1st hop) testServer1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -197,8 +197,8 @@ func (s *MSC3916DownloadsSuite) TestFederationFollowsRedirects() { defer testServer1.Close() u, _ = url.Parse(testServer1.URL) - origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) - config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup + origin = fmt.Sprintf("%s:%s", "host.docker.internal", u.Port()) + config.AddDomainForTesting("host.docker.internal", nil) // no port for config lookup raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) @@ -239,8 +239,8 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloadsAndFallsBack() defer testServer.Close() u, _ := url.Parse(testServer.URL) - origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) - config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup + origin = fmt.Sprintf("%s:%s", "host.docker.internal", u.Port()) + config.AddDomainForTesting("host.docker.internal", nil) // no port for config lookup raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) diff --git a/test/test_internals/util.go b/test/test_internals/util.go index d507c029..fe0744b9 100644 --- a/test/test_internals/util.go +++ b/test/test_internals/util.go @@ -58,10 +58,3 @@ func AssertIsTestImage(t *testing.T, i io.Reader) { } } } - -func DockerHostAddress() string { - //if runtime.GOOS == "linux" { - // return "172.17.0.1" // XXX: This is bad - //} - return "host.docker.internal" -} From 3039539f318a8cda8cbd83cfce6fddf94624c131 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 2 Jul 2024 16:15:34 -0600 Subject: [PATCH 49/51] Fix redirect behaviour on federation --- api/unstable/msc3916_download.go | 17 +++++++++++-- api/unstable/msc3916_thumbnail.go | 17 +++++++++++-- matrix/xmatrix.go | 1 + test/msc3916_downloads_suite_test.go | 36 ++++++++++++++++++++++++++++ test/test_internals/deps_mmr.go | 1 + util/readers/multipart_reader.go | 6 +++++ 6 files changed, 74 insertions(+), 4 deletions(-) diff --git a/api/unstable/msc3916_download.go b/api/unstable/msc3916_download.go index c55ee1b0..ff770809 100644 --- a/api/unstable/msc3916_download.go +++ b/api/unstable/msc3916_download.go @@ -19,8 +19,10 @@ func ClientDownloadMedia(r *http.Request, rctx rcontext.RequestContext, user _ap } func FederationDownloadMedia(r *http.Request, rctx rcontext.RequestContext, server _apimeta.ServerInfo) interface{} { - r.URL.Query().Set("allow_remote", "false") - r.URL.Query().Set("allow_redirect", "false") // not supported at the top level + query := r.URL.Query() + query.Set("allow_remote", "false") + query.Set("allow_redirect", "true") // we override how redirects work in the response + r.URL.RawQuery = query.Encode() r = _routers.ForceSetParam("server", r.Host, r) res := r0.DownloadMedia(r, rctx, _apimeta.UserInfo{}) @@ -35,6 +37,17 @@ func FederationDownloadMedia(r *http.Request, rctx rcontext.RequestContext, serv ), TargetDisposition: "attachment", } + } else if rd, ok := res.(*_responses.RedirectResponse); ok { + return &_responses.DownloadResponse{ + ContentType: "multipart/mixed", + Filename: "", + SizeBytes: 0, + Data: readers.NewMultipartReader( + &readers.MultipartPart{ContentType: "application/json", Reader: readers.MakeCloser(bytes.NewReader([]byte("{}")))}, + &readers.MultipartPart{Location: rd.ToUrl}, + ), + TargetDisposition: "attachment", + } } else { return res } diff --git a/api/unstable/msc3916_thumbnail.go b/api/unstable/msc3916_thumbnail.go index 3d1db0d2..9fa5c4df 100644 --- a/api/unstable/msc3916_thumbnail.go +++ b/api/unstable/msc3916_thumbnail.go @@ -19,8 +19,10 @@ func ClientThumbnailMedia(r *http.Request, rctx rcontext.RequestContext, user _a } func FederationThumbnailMedia(r *http.Request, rctx rcontext.RequestContext, server _apimeta.ServerInfo) interface{} { - r.URL.Query().Set("allow_remote", "false") - r.URL.Query().Set("allow_redirect", "false") // not supported at the top level + query := r.URL.Query() + query.Set("allow_remote", "false") + query.Set("allow_redirect", "true") // we override how redirects work in the response + r.URL.RawQuery = query.Encode() r = _routers.ForceSetParam("server", r.Host, r) res := r0.ThumbnailMedia(r, rctx, _apimeta.UserInfo{}) @@ -35,6 +37,17 @@ func FederationThumbnailMedia(r *http.Request, rctx rcontext.RequestContext, ser ), TargetDisposition: "attachment", } + } else if rd, ok := res.(*_responses.RedirectResponse); ok { + return &_responses.DownloadResponse{ + ContentType: "multipart/mixed", + Filename: "", + SizeBytes: 0, + Data: readers.NewMultipartReader( + &readers.MultipartPart{ContentType: "application/json", Reader: readers.MakeCloser(bytes.NewReader([]byte("{}")))}, + &readers.MultipartPart{Location: rd.ToUrl}, + ), + TargetDisposition: "attachment", + } } else { return res } diff --git a/matrix/xmatrix.go b/matrix/xmatrix.go index 6b9e890e..49f9342a 100644 --- a/matrix/xmatrix.go +++ b/matrix/xmatrix.go @@ -15,6 +15,7 @@ var ErrNoXMatrixAuth = errors.New("no X-Matrix auth headers") var ErrWrongDestination = errors.New("wrong destination") func ValidateXMatrixAuth(request *http.Request, expectNoContent bool) (string, error) { + return "localhost", nil if !expectNoContent { panic("development error: X-Matrix auth validation can only be done with an empty body for now") } diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index bfa75712..f2f968f2 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -209,6 +209,42 @@ func (s *MSC3916DownloadsSuite) TestFederationFollowsRedirects() { assert.Equal(t, fileContents, string(b)) } +func (s *MSC3916DownloadsSuite) TestFederationProducesRedirects() { + t := s.T() + + client1 := s.deps.Homeservers[0].UnprivilegedUsers[0].WithCsUrl(s.deps.Machines[0].HttpUrl) + remoteClient := &test_internals.MatrixClient{ + ClientServerUrl: s.deps.Machines[0].HttpUrl, + ServerName: s.deps.Homeservers[0].ServerName, + AccessToken: "", // this client isn't authed over the CS API + UserId: "", // this client isn't authed over the CS API + } + + contentType, img, err := test_internals.MakeTestImage(512, 512) + assert.NoError(t, err) + fname := "image" + util.ExtensionForContentType(contentType) + + res, err := client1.Upload(fname, contentType, img) + assert.NoError(t, err) + assert.NotEmpty(t, res.MxcUri) + + origin, mediaId, err := util.SplitMxc(res.MxcUri) + assert.NoError(t, err) + assert.Equal(t, client1.ServerName, origin) + assert.NotEmpty(t, mediaId) + + // Verify the federation download *fails* when lacking auth + uri := fmt.Sprintf("/_matrix/federation/v1/media/download/%s", mediaId) + header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", uri, &database.AnonymousJson{}, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion) + assert.NoError(t, err) + remoteClient.AuthHeaderOverride = header + raw, err := remoteClient.DoRaw("GET", uri, nil, "", nil) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, raw.StatusCode) + + // TODO: Need to actually test that redirects are properly formed, and set up the test suite to produce them +} + func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloadsAndFallsBack() { t := s.T() diff --git a/test/test_internals/deps_mmr.go b/test/test_internals/deps_mmr.go index 07308030..866a8790 100644 --- a/test/test_internals/deps_mmr.go +++ b/test/test_internals/deps_mmr.go @@ -21,6 +21,7 @@ type mmrHomeserverTmplArgs struct { ServerName string ClientServerApiUrl string SigningKeyPath string + PublicBaseUrl string } type mmrTmplArgs struct { diff --git a/util/readers/multipart_reader.go b/util/readers/multipart_reader.go index ea3c901d..ca8ba1ad 100644 --- a/util/readers/multipart_reader.go +++ b/util/readers/multipart_reader.go @@ -1,6 +1,7 @@ package readers import ( + "bytes" "io" "mime/multipart" "net/textproto" @@ -12,6 +13,7 @@ import ( type MultipartPart struct { ContentType string FileName string + Location string Reader io.ReadCloser } @@ -32,6 +34,10 @@ func NewMultipartReader(parts ...*MultipartPart) io.ReadCloser { headers.Set("Content-Disposition", "attachment; filename*=utf-8''"+url.QueryEscape(part.FileName)) } } + if part.Location != "" { + headers.Set("Location", part.Location) + part.Reader = io.NopCloser(bytes.NewReader(make([]byte, 0))) + } partW, err := mpw.CreatePart(headers) if err != nil { From 2e92cacd2950186cb6753e26b72b5016eac787af Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 2 Jul 2024 16:16:40 -0600 Subject: [PATCH 50/51] Move endpoints to correct package --- api/routes.go | 8 ++++---- api/{unstable/msc3916_download.go => v1/download.go} | 2 +- api/{unstable/msc3916_thumbnail.go => v1/thumbnail.go} | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) rename api/{unstable/msc3916_download.go => v1/download.go} (99%) rename api/{unstable/msc3916_thumbnail.go => v1/thumbnail.go} (99%) diff --git a/api/routes.go b/api/routes.go index 036a5f51..5bbdbe63 100644 --- a/api/routes.go +++ b/api/routes.go @@ -48,12 +48,12 @@ func buildRoutes() http.Handler { register([]string{"GET"}, PrefixClient, "versions", mxNoVersion, router, makeRoute(_routers.OptionalAccessToken(r0.ClientVersions), "client_versions", counter)) register([]string{"GET"}, PrefixClient, "media/preview_url", mxV1, router, previewUrlRoute) register([]string{"GET"}, PrefixClient, "media/config", mxV1, router, configRoute) - authedDownloadRoute := makeRoute(_routers.RequireAccessToken(unstable.ClientDownloadMedia), "download", counter) + authedDownloadRoute := makeRoute(_routers.RequireAccessToken(v1.ClientDownloadMedia), "download", counter) register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId/:filename", mxV1, router, authedDownloadRoute) register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId", mxV1, router, authedDownloadRoute) - register([]string{"GET"}, PrefixClient, "media/thumbnail/:server/:mediaId", mxV1, router, makeRoute(_routers.RequireAccessToken(unstable.ClientThumbnailMedia), "thumbnail", counter)) - register([]string{"GET"}, PrefixFederation, "media/download/:mediaId", mxV1, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) - register([]string{"GET"}, PrefixFederation, "media/thumbnail/:mediaId", mxV1, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) + register([]string{"GET"}, PrefixClient, "media/thumbnail/:server/:mediaId", mxV1, router, makeRoute(_routers.RequireAccessToken(v1.ClientThumbnailMedia), "thumbnail", counter)) + register([]string{"GET"}, PrefixFederation, "media/download/:mediaId", mxV1, router, makeRoute(_routers.RequireServerAuth(v1.FederationDownloadMedia), "download", counter)) + register([]string{"GET"}, PrefixFederation, "media/thumbnail/:mediaId", mxV1, router, makeRoute(_routers.RequireServerAuth(v1.FederationThumbnailMedia), "thumbnail", counter)) // Custom features register([]string{"GET"}, PrefixMedia, "local_copy/:server/:mediaId", mxUnstable, router, makeRoute(_routers.RequireAccessToken(unstable.LocalCopy), "local_copy", counter)) diff --git a/api/unstable/msc3916_download.go b/api/v1/download.go similarity index 99% rename from api/unstable/msc3916_download.go rename to api/v1/download.go index ff770809..6fd439ec 100644 --- a/api/unstable/msc3916_download.go +++ b/api/v1/download.go @@ -1,4 +1,4 @@ -package unstable +package v1 import ( "bytes" diff --git a/api/unstable/msc3916_thumbnail.go b/api/v1/thumbnail.go similarity index 99% rename from api/unstable/msc3916_thumbnail.go rename to api/v1/thumbnail.go index 9fa5c4df..438a6fcf 100644 --- a/api/unstable/msc3916_thumbnail.go +++ b/api/v1/thumbnail.go @@ -1,4 +1,4 @@ -package unstable +package v1 import ( "bytes" From 55742ccdd8e98744cd13e1e81951e3cfbe497c01 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 2 Jul 2024 16:56:09 -0600 Subject: [PATCH 51/51] Maybe remove the dev code --- matrix/xmatrix.go | 1 - 1 file changed, 1 deletion(-) diff --git a/matrix/xmatrix.go b/matrix/xmatrix.go index 49f9342a..6b9e890e 100644 --- a/matrix/xmatrix.go +++ b/matrix/xmatrix.go @@ -15,7 +15,6 @@ var ErrNoXMatrixAuth = errors.New("no X-Matrix auth headers") var ErrWrongDestination = errors.New("wrong destination") func ValidateXMatrixAuth(request *http.Request, expectNoContent bool) (string, error) { - return "localhost", nil if !expectNoContent { panic("development error: X-Matrix auth validation can only be done with an empty body for now") }