From 9e7d570c794cb1f133ffbfba3b23e95f118695b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 13 Aug 2024 15:04:19 +0200 Subject: [PATCH] fix: verify file id in URL matches the one inside the access token We're using the file id from the access token, so changing the file id in the URL shouldn't matter. However, the file id must represent a single file. It will also help to detect unwanted changes in the URL. --- .../pkg/connector/fileconnector.go | 8 ++------ services/collaboration/pkg/helpers/cs3.go | 13 +++++++++++++ .../collaboration/pkg/middleware/wopicontext.go | 17 +++++++++++++++-- .../pkg/service/grpc/v0/service.go | 8 ++------ 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/services/collaboration/pkg/connector/fileconnector.go b/services/collaboration/pkg/connector/fileconnector.go index 316e3d1364..0f9d42f67d 100644 --- a/services/collaboration/pkg/connector/fileconnector.go +++ b/services/collaboration/pkg/connector/fileconnector.go @@ -3,7 +3,6 @@ package connector import ( "bytes" "context" - "crypto/sha256" "encoding/base64" "encoding/binary" "encoding/hex" @@ -20,11 +19,11 @@ import ( rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" - "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" "github.com/google/uuid" "github.com/owncloud/ocis/v2/services/collaboration/pkg/config" "github.com/owncloud/ocis/v2/services/collaboration/pkg/connector/fileinfo" + "github.com/owncloud/ocis/v2/services/collaboration/pkg/helpers" "github.com/owncloud/ocis/v2/services/collaboration/pkg/middleware" "github.com/rs/zerolog" ) @@ -1172,10 +1171,7 @@ func (f *FileConnector) generateWOPISrc(ctx context.Context, wopiContext middlew } // get the reference - resourceId := wopiContext.FileReference.GetResourceId() - c := sha256.New() - c.Write([]byte(storagespace.FormatResourceID(resourceId))) - fileRef := hex.EncodeToString(c.Sum(nil)) + fileRef := helpers.HashResourceId(wopiContext.FileReference.GetResourceId()) // generate the URL for the WOPI app to access the new created file wopiSrcURL, err := url.Parse(f.cfg.Wopi.WopiSrc) diff --git a/services/collaboration/pkg/helpers/cs3.go b/services/collaboration/pkg/helpers/cs3.go index 8b3f21f469..0d5cfbc9aa 100644 --- a/services/collaboration/pkg/helpers/cs3.go +++ b/services/collaboration/pkg/helpers/cs3.go @@ -1,8 +1,13 @@ package helpers import ( + "crypto/sha256" + "encoding/hex" + gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" + "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/owncloud/ocis/v2/services/collaboration/pkg/config" ) @@ -25,3 +30,11 @@ func GetCS3apiClient(cfg *config.Config, forceNew bool) (gatewayv1beta1.GatewayA } return client, err } + +// HashResourceId builds a urlsafe and stable file reference that can be used for proxy routing, +// so that all sessions on one file end on the same office server +func HashResourceId(resourceId *providerv1beta1.ResourceId) string { + c := sha256.New() + c.Write([]byte(storagespace.FormatResourceID(resourceId))) + return hex.EncodeToString(c.Sum(nil)) +} diff --git a/services/collaboration/pkg/middleware/wopicontext.go b/services/collaboration/pkg/middleware/wopicontext.go index a6dbd13ef1..1232da0317 100644 --- a/services/collaboration/pkg/middleware/wopicontext.go +++ b/services/collaboration/pkg/middleware/wopicontext.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "regexp" appproviderv1beta1 "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -12,6 +13,7 @@ import ( ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/golang-jwt/jwt/v4" "github.com/owncloud/ocis/v2/services/collaboration/pkg/config" + "github.com/owncloud/ocis/v2/services/collaboration/pkg/helpers" "github.com/rs/zerolog" "google.golang.org/grpc/metadata" ) @@ -45,6 +47,8 @@ type WopiContext struct { // * A contextual zerologger containing information about the request // and the WopiContext func WopiContextAuthMiddleware(cfg *config.Config, next http.Handler) http.Handler { + // compile a regexp here to extract the fileid from the URL + fileIDregexp := regexp.MustCompile(`^/wopi/files/([0-9a-f]{64})(/.*)?$`) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { accessToken := r.URL.Query().Get("access_token") if accessToken == "" { @@ -89,7 +93,7 @@ func WopiContextAuthMiddleware(cfg *config.Config, next http.Handler) http.Handl // we might need to check https://learn.microsoft.com/en-us/microsoft-365/cloud-storage-partner-program/rest/common-headers // although some headers might not be sent depending on the client. logger := zerolog.Ctx(ctx) - ctx = logger.With(). + wopiLogger := logger.With(). Str("WopiSessionId", r.Header.Get("X-WOPI-SessionId")). Str("WopiOverride", r.Header.Get("X-WOPI-Override")). Str("WopiProof", r.Header.Get("X-WOPI-Proof")). @@ -98,7 +102,16 @@ func WopiContextAuthMiddleware(cfg *config.Config, next http.Handler) http.Handl Str("FileReference", claims.WopiContext.FileReference.String()). Str("ViewMode", claims.WopiContext.ViewMode.String()). Str("Requester", claims.WopiContext.User.GetId().String()). - Logger().WithContext(ctx) + Logger() + ctx = wopiLogger.WithContext(ctx) + + hashedRef := helpers.HashResourceId(claims.WopiContext.FileReference.GetResourceId()) + matches := fileIDregexp.FindStringSubmatch(r.URL.Path) + if len(matches) < 2 || matches[1] != hashedRef { + wopiLogger.Error().Msg("file reference in the URL doesn't match the one inside the access token") + http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + return + } next.ServeHTTP(w, r.WithContext(ctx)) }) diff --git a/services/collaboration/pkg/service/grpc/v0/service.go b/services/collaboration/pkg/service/grpc/v0/service.go index 94f76e2ce4..1568efe0ad 100644 --- a/services/collaboration/pkg/service/grpc/v0/service.go +++ b/services/collaboration/pkg/service/grpc/v0/service.go @@ -2,8 +2,6 @@ package service import ( "context" - "crypto/sha256" - "encoding/hex" "fmt" "net/url" "path" @@ -19,6 +17,7 @@ import ( "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/collaboration/pkg/config" + "github.com/owncloud/ocis/v2/services/collaboration/pkg/helpers" "github.com/owncloud/ocis/v2/services/collaboration/pkg/middleware" ) @@ -83,10 +82,7 @@ func (s *Service) OpenInApp( // build a urlsafe and stable file reference that can be used for proxy routing, // so that all sessions on one file end on the same office server - - c := sha256.New() - c.Write([]byte(req.GetResourceInfo().GetId().GetStorageId() + "$" + req.GetResourceInfo().GetId().GetSpaceId() + "!" + req.GetResourceInfo().GetId().GetOpaqueId())) - fileRef := hex.EncodeToString(c.Sum(nil)) + fileRef := helpers.HashResourceId(req.GetResourceInfo().GetId()) // get the file extension to use the right wopi app url fileExt := path.Ext(req.GetResourceInfo().GetPath())