From 4bdb3bf70ff99086dcd9635a6441066da23d4d44 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 8 Jul 2025 16:33:52 +0200 Subject: [PATCH] proxy(sign_url_auth): Allow to verify server signed URLs With the ocdav service being able to provided signed download URLs we need the proxy to be able to verify the signatures. This should also be a first step towards phasing out the weird ocs based client side signed urls. Related Tickets: #1104 --- services/ocdav/pkg/command/server.go | 1 + services/ocdav/pkg/config/config.go | 5 +- services/proxy/pkg/command/server.go | 12 ++++ services/proxy/pkg/config/config.go | 7 +- .../proxy/pkg/middleware/signed_url_auth.go | 67 ++++++++++++++++++- .../pkg/middleware/signed_url_auth_test.go | 36 +++++++++- 6 files changed, 119 insertions(+), 9 deletions(-) diff --git a/services/ocdav/pkg/command/server.go b/services/ocdav/pkg/command/server.go index 00608a609..6201ddfa9 100644 --- a/services/ocdav/pkg/command/server.go +++ b/services/ocdav/pkg/command/server.go @@ -89,6 +89,7 @@ func Server(cfg *config.Config) *cli.Command { ocdav.WithTraceProvider(traceProvider), ocdav.RegisterTTL(registry.GetRegisterTTL()), ocdav.RegisterInterval(registry.GetRegisterInterval()), + ocdav.URLSigningSharedSecret(cfg.URLSigningSharedSecret), } s, err := ocdav.Service(opts...) diff --git a/services/ocdav/pkg/config/config.go b/services/ocdav/pkg/config/config.go index 627a6530d..08737ef33 100644 --- a/services/ocdav/pkg/config/config.go +++ b/services/ocdav/pkg/config/config.go @@ -34,8 +34,9 @@ type Config struct { MachineAuthAPIKey string `yaml:"machine_auth_api_key" env:"OC_MACHINE_AUTH_API_KEY;OCDAV_MACHINE_AUTH_API_KEY" desc:"Machine auth API key used to validate internal requests necessary for the access to resources from other services." introductionVersion:"1.0.0"` - Context context.Context `yaml:"-"` - Status Status `yaml:"-"` + URLSigningSharedSecret string `yaml:"url_signing_shared_secret" env:"OC_URL_SIGNING_SHARED_SECRET" desc:"The shared secret used to sign URLs." introductionVersion:"4.0.0"` + Context context.Context `yaml:"-"` + Status Status `yaml:"-"` AllowPropfindDepthInfinity bool `yaml:"allow_propfind_depth_infinity" env:"OCDAV_ALLOW_PROPFIND_DEPTH_INFINITY" desc:"Allow the use of depth infinity in PROPFINDS. When enabled, a propfind will traverse through all subfolders. If many subfolders are expected, depth infinity can cause heavy server load and/or delayed response times." introductionVersion:"1.0.0"` } diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 8b5a9a48d..f15d5e90c 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -36,6 +36,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/events" "github.com/opencloud-eu/reva/v2/pkg/events/stream" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + "github.com/opencloud-eu/reva/v2/pkg/signedurl" "github.com/opencloud-eu/reva/v2/pkg/store" "github.com/urfave/cli/v2" "go-micro.dev/v4/selector" @@ -316,6 +317,16 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, Logger: logger, RevaGatewaySelector: gatewaySelector, }) + + var signURLVerifier signedurl.Verifier + + if cfg.PreSignedURL.JWTSigningSharedSecret != "" { + var err error + signURLVerifier, err = signedurl.NewJWTSignedURL(signedurl.WithSecret(cfg.PreSignedURL.JWTSigningSharedSecret)) + if err != nil { + logger.Fatal().Err(err).Msg("Failed to initialize signed URL configuration.") + } + } authenticators = append(authenticators, middleware.SignedURLAuthenticator{ Logger: logger, PreSignedURLConfig: cfg.PreSignedURL, @@ -323,6 +334,7 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, UserRoleAssigner: roleAssigner, Store: signingKeyStore, Now: time.Now, + URLVerifier: signURLVerifier, }) cspConfig, err := middleware.LoadCSPConfig(cfg) diff --git a/services/proxy/pkg/config/config.go b/services/proxy/pkg/config/config.go index 452f38497..db96a28a4 100644 --- a/services/proxy/pkg/config/config.go +++ b/services/proxy/pkg/config/config.go @@ -180,9 +180,10 @@ type StaticSelectorConf struct { // PreSignedURL is the config for the pre-signed url middleware type PreSignedURL struct { - AllowedHTTPMethods []string `yaml:"allowed_http_methods"` - Enabled bool `yaml:"enabled" env:"PROXY_ENABLE_PRESIGNEDURLS" desc:"Allow OCS to get a signing key to sign requests." introductionVersion:"1.0.0"` - SigningKeys *SigningKeys `yaml:"signing_keys"` + AllowedHTTPMethods []string `yaml:"allowed_http_methods"` + Enabled bool `yaml:"enabled" env:"PROXY_ENABLE_PRESIGNEDURLS" desc:"Allow OCS to get a signing key to sign requests." introductionVersion:"1.0.0"` + SigningKeys *SigningKeys `yaml:"signing_keys"` + JWTSigningSharedSecret string `yaml:"url_signing_shared_secret" env:"OC_URL_SIGNING_SHARED_SECRET" desc:"The shared secret used to sign URLs." introductionVersion:"4.0.0"` } // SigningKeys is a store configuration. diff --git a/services/proxy/pkg/middleware/signed_url_auth.go b/services/proxy/pkg/middleware/signed_url_auth.go index b2fb7c02f..4381027e9 100644 --- a/services/proxy/pkg/middleware/signed_url_auth.go +++ b/services/proxy/pkg/middleware/signed_url_auth.go @@ -15,6 +15,7 @@ import ( "github.com/opencloud-eu/opencloud/services/proxy/pkg/user/backend" "github.com/opencloud-eu/opencloud/services/proxy/pkg/userroles" revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" + "github.com/opencloud-eu/reva/v2/pkg/signedurl" microstore "go-micro.dev/v4/store" "golang.org/x/crypto/pbkdf2" ) @@ -26,6 +27,7 @@ const ( _paramOCExpires = "OC-Expires" _paramOCVerb = "OC-Verb" _paramOCAlgo = "OC-Algo" + _paramOCJWTSig = "oc-jwt-sig" ) var ( @@ -46,15 +48,23 @@ type SignedURLAuthenticator struct { UserRoleAssigner userroles.UserRoleAssigner Store microstore.Store Now func() time.Time + URLVerifier signedurl.Verifier } -func (m SignedURLAuthenticator) shouldServe(req *http.Request) bool { +func (m SignedURLAuthenticator) shouldServeLegacy(req *http.Request) bool { if !m.PreSignedURLConfig.Enabled { return false } return req.URL.Query().Get(_paramOCSignature) != "" } +func (m SignedURLAuthenticator) shouldServe(req *http.Request) bool { + if m.URLVerifier == nil { + return false + } + return req.URL.Query().Get(_paramOCJWTSig) != "" +} + func (m SignedURLAuthenticator) validate(req *http.Request) (err error) { query := req.URL.Query() @@ -216,10 +226,61 @@ func (m SignedURLAuthenticator) createSignature(url string, signingKey []byte) s // Authenticate implements the authenticator interface to authenticate requests via signed URL auth. func (m SignedURLAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) { - if !m.shouldServe(r) { - return nil, false + switch { + case m.shouldServeLegacy(r): + return m.authenticateLegacy(r) + case m.shouldServe(r): + return m.authenticate(r) + } + return nil, false +} + +func (m SignedURLAuthenticator) authenticate(r *http.Request) (*http.Request, bool) { + u := r.URL.String() + if !r.URL.IsAbs() { + u = "https://" + r.Host + u } + userid, err := m.URLVerifier.Verify(u) + if err != nil { + m.Logger.Error(). + Err(err). + Str("authenticator", "signed_url_jwt"). + Str("path", r.URL.Path). + Str("url", u). + Msg("Could not verify JWT signature") + return nil, false + } + user, _, err := m.UserProvider.GetUserByClaims(r.Context(), "userid", userid) + if err != nil { + m.Logger.Error(). + Err(err). + Str("authenticator", "signed_url_jwt"). + Str("path", r.URL.Path). + Msg("Could not get user by claim") + return nil, false + } + user, err = m.UserRoleAssigner.ApplyUserRole(r.Context(), user) + if err != nil { + m.Logger.Error(). + Err(err). + Str("authenticator", "signed_url"). + Str("path", r.URL.Path). + Msg("Could not get user by claim") + return nil, false + } + ctx := revactx.ContextSetUser(r.Context(), user) + r = r.WithContext(ctx) + m.Logger.Debug(). + Str("authenticator", "signed_url"). + Str("path", r.URL.Path). + Msg("successfully authenticated request") + return r, true +} + +// authenticateLegacy is a helper function to authenticate requests that use the legacy +// client side signed URLs +func (m SignedURLAuthenticator) authenticateLegacy(r *http.Request) (*http.Request, bool) { user, _, err := m.UserProvider.GetUserByClaims(r.Context(), "username", r.URL.Query().Get(_paramOCCredential)) if err != nil { m.Logger.Error(). diff --git a/services/proxy/pkg/middleware/signed_url_auth_test.go b/services/proxy/pkg/middleware/signed_url_auth_test.go index 3e27c6794..c507fa787 100644 --- a/services/proxy/pkg/middleware/signed_url_auth_test.go +++ b/services/proxy/pkg/middleware/signed_url_auth_test.go @@ -9,11 +9,12 @@ import ( userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" "github.com/opencloud-eu/opencloud/services/proxy/pkg/config" revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" + "github.com/opencloud-eu/reva/v2/pkg/signedurl" "github.com/stretchr/testify/assert" "go-micro.dev/v4/store" ) -func TestSignedURLAuth_shouldServe(t *testing.T) { +func TestSignedURLAuthLegacy_shouldServe(t *testing.T) { pua := SignedURLAuthenticator{} tests := []struct { url string @@ -29,6 +30,39 @@ func TestSignedURLAuth_shouldServe(t *testing.T) { for _, tt := range tests { pua.PreSignedURLConfig.Enabled = tt.enabled r := httptest.NewRequest("", tt.url, nil) + result := pua.shouldServeLegacy(r) + + if result != tt.expected { + t.Errorf("with %s expected %t got %t", tt.url, tt.expected, result) + } + } +} + +func TestSignedURLAuth_shouldServe(t *testing.T) { + tests := []struct { + url string + secret string + enabled bool + expected bool + }{ + {"https://example.com/example.jpg", "", true, false}, + {"https://example.com/example.jpg", "", false, false}, + {"https://example.com/example.jpg?oc-jwt-sig=something1", "secret", true, true}, + {"https://example.com/example.jpg?oc-jwt-sig=something2", "", true, false}, + {"https://example.com/example.jpg?oc-jwt-sig=something3", "secret", false, true}, + } + + for _, tt := range tests { + pua := SignedURLAuthenticator{} + pua.PreSignedURLConfig.Enabled = tt.enabled + if tt.secret != "" { + signURLVerifier, err := signedurl.NewJWTSignedURL(signedurl.WithSecret(tt.secret)) + if err != nil { + t.Fatalf("failed to create signed URL verifier: %v", err) + } + pua.URLVerifier = signURLVerifier + } + r := httptest.NewRequest("", tt.url, nil) result := pua.shouldServe(r) if result != tt.expected {