From b9e05e9b4718f5c667822b73d18e5586d5367b2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 22 Jul 2020 11:43:33 +0200 Subject: [PATCH] use same signing parameters as oc10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/command/server.go | 52 +++++++-------- pkg/middleware/account_uuid.go | 34 ++++++---- pkg/middleware/account_uuid_test.go | 4 +- pkg/middleware/presigned_url.go | 99 +++++++++++------------------ 4 files changed, 86 insertions(+), 103 deletions(-) diff --git a/pkg/command/server.go b/pkg/command/server.go index 10ce5ce4ec..fcec85dd4a 100644 --- a/pkg/command/server.go +++ b/pkg/command/server.go @@ -253,6 +253,31 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic middleware.Store(storepb.NewStoreService("com.owncloud.api.store", grpc.NewClient())), ) + // TODO this won't work with a registry other than mdns. Look into Micro's client initialization. + // https://github.com/owncloud/ocis-proxy/issues/38 + accounts := acc.NewAccountsService("com.owncloud.api.accounts", mclient.DefaultClient) + + uuidMW := middleware.AccountUUID( + middleware.Logger(l), + middleware.TokenManagerConfig(cfg.TokenManager), + middleware.AccountsClient(accounts), + ) + + // the connection will be established in a non blocking fashion + sc, err := cs3.GetGatewayServiceClient(cfg.Reva.Address) + if err != nil { + l.Error().Err(err). + Str("gateway", cfg.Reva.Address). + Msg("Failed to create reva gateway service client") + } + + chMW := middleware.CreateHome( + middleware.Logger(l), + middleware.RevaGatewayClient(sc), + middleware.AccountsClient(accounts), + middleware.TokenManagerConfig(cfg.TokenManager), + ) + if cfg.OIDC.Issuer != "" { l.Info().Msg("Loading OIDC-Middleware") l.Debug().Interface("oidc_config", cfg.OIDC).Msg("OIDC-Config") @@ -282,33 +307,8 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic middleware.OIDCProviderFunc(provider), ) - // TODO this won't work with a registry other than mdns. Look into Micro's client initialization. - // https://github.com/owncloud/ocis-proxy/issues/38 - accounts := acc.NewAccountsService("com.owncloud.api.accounts", mclient.DefaultClient) - - uuidMW := middleware.AccountUUID( - middleware.Logger(l), - middleware.TokenManagerConfig(cfg.TokenManager), - middleware.AccountsClient(accounts), - ) - - // the connection will be established in a non blocking fashion - sc, err := cs3.GetGatewayServiceClient(cfg.Reva.Address) - if err != nil { - l.Error().Err(err). - Str("gateway", cfg.Reva.Address). - Msg("Failed to create reva gateway service client") - } - - chMW := middleware.CreateHome( - middleware.Logger(l), - middleware.RevaGatewayClient(sc), - middleware.AccountsClient(accounts), - middleware.TokenManagerConfig(cfg.TokenManager), - ) - return alice.New(middleware.RedirectToHTTPS, oidcMW, psMW, uuidMW, chMW) } - return alice.New(middleware.RedirectToHTTPS, psMW) + return alice.New(middleware.RedirectToHTTPS, psMW, uuidMW, chMW) } diff --git a/pkg/middleware/account_uuid.go b/pkg/middleware/account_uuid.go index 529dfc67af..d7969f9dda 100644 --- a/pkg/middleware/account_uuid.go +++ b/pkg/middleware/account_uuid.go @@ -13,23 +13,23 @@ import ( oidc "github.com/owncloud/ocis-pkg/v2/oidc" ) -func getAccount(l log.Logger, claims *oidc.StandardClaims, ac acc.AccountsService) (account *acc.Account, status int) { - entry, err := svcCache.Get(AccountsKey, claims.Email) +func getAccount(l log.Logger, ac acc.AccountsService, query string) (account *acc.Account, status int) { + entry, err := svcCache.Get(AccountsKey, query) if err != nil { - l.Debug().Msgf("No cache entry for %v", claims.Email) + l.Debug().Msgf("No cache entry for %s", query) resp, err := ac.ListAccounts(context.Background(), &acc.ListAccountsRequest{ - Query: fmt.Sprintf("mail eq '%s'", strings.ReplaceAll(claims.Email, "'", "''")), + Query: query, PageSize: 2, }) if err != nil { - l.Error().Err(err).Str("email", claims.Email).Msgf("Error fetching from accounts-service") + l.Error().Err(err).Str("query", query).Msgf("Error fetching from accounts-service") status = http.StatusInternalServerError return } if len(resp.Accounts) <= 0 { - l.Error().Str("email", claims.Email).Msgf("Account not found") + l.Error().Str("query", query).Msgf("Account not found") status = http.StatusNotFound return } @@ -37,20 +37,21 @@ func getAccount(l log.Logger, claims *oidc.StandardClaims, ac acc.AccountsServic // TODO provision account if len(resp.Accounts) > 1 { - l.Error().Str("email", claims.Email).Msgf("More than one account with this email found. Not logging user in.") + l.Error().Str("query", query).Msgf("More than one account found. Not logging user in.") status = http.StatusForbidden return } - err = svcCache.Set(AccountsKey, claims.Email, *resp.Accounts[0]) + err = svcCache.Set(AccountsKey, query, *resp.Accounts[0]) if err != nil { - l.Err(err).Str("email", claims.Email).Msgf("Could not cache user") + l.Err(err).Str("query", query).Msgf("Could not cache user") status = http.StatusInternalServerError return } account = resp.Accounts[0] } else { + l.Debug().Msgf("using cache entry for %s", query) a, ok := entry.V.(acc.Account) // TODO how can we directly point to the cached account? if !ok { status = http.StatusInternalServerError @@ -104,10 +105,17 @@ func AccountUUID(opts ...Option) func(next http.Handler) http.Handler { return } - // TODO allow lookup by username? - // TODO allow lookup by custom claim, eg an id - - account, status := getAccount(l, claims, opt.AccountsClient) + var account *acc.Account + var status int + if claims.Email != "" { + account, status = getAccount(l, opt.AccountsClient, fmt.Sprintf("mail eq '%s'", strings.ReplaceAll(claims.Email, "'", "''"))) + } else if claims.PreferredUsername != "" { + account, status = getAccount(l, opt.AccountsClient, fmt.Sprintf("preferred_name eq '%s'", strings.ReplaceAll(claims.PreferredUsername, "'", "''"))) + } else { + // TODO allow lookup by custom claim, eg an id ... or sub + l.Error().Err(err).Msgf("Could not lookup account, no mail or preferred_username claim set") + w.WriteHeader(http.StatusInternalServerError) + } if status != 0 { if status == http.StatusNotFound { account, status = createAccount(l, claims, opt.AccountsClient) diff --git a/pkg/middleware/account_uuid_test.go b/pkg/middleware/account_uuid_test.go index abf2e8515d..7241fa901c 100644 --- a/pkg/middleware/account_uuid_test.go +++ b/pkg/middleware/account_uuid_test.go @@ -17,13 +17,13 @@ import ( // TODO testing the getAccount method should inject a cache func TestGetAccountSuccess(t *testing.T) { svcCache.Invalidate(AccountsKey, "success") - if _, status := getAccount(log.NewLogger(), &oidc.StandardClaims{Email: "success"}, mockAccountUUIDMiddlewareAccSvc(false, true)); status != 0 { + if _, status := getAccount(log.NewLogger(), mockAccountUUIDMiddlewareAccSvc(false, true), "mail eq 'success'"); status != 0 { t.Errorf("expected an account") } } func TestGetAccountInternalError(t *testing.T) { svcCache.Invalidate(AccountsKey, "failure") - if _, status := getAccount(log.NewLogger(), &oidc.StandardClaims{Email: "failure"}, mockAccountUUIDMiddlewareAccSvc(true, false)); status != http.StatusInternalServerError { + if _, status := getAccount(log.NewLogger(), mockAccountUUIDMiddlewareAccSvc(true, false), "mail eq 'failure'"); status != http.StatusInternalServerError { t.Errorf("expected an internal server error") } } diff --git a/pkg/middleware/presigned_url.go b/pkg/middleware/presigned_url.go index 905b2c73ee..7ac9e35050 100644 --- a/pkg/middleware/presigned_url.go +++ b/pkg/middleware/presigned_url.go @@ -8,6 +8,8 @@ import ( "strings" "time" + "github.com/owncloud/ocis-pkg/v2/log" + ocisoidc "github.com/owncloud/ocis-pkg/v2/oidc" storepb "github.com/owncloud/ocis-store/pkg/proto/v0" "golang.org/x/crypto/pbkdf2" ) @@ -15,38 +17,24 @@ import ( // PresignedURL provides a middleware to check access secured by a presigned URL. func PresignedURL(opts ...Option) func(next http.Handler) http.Handler { opt := newOptions(opts...) - - /*tokenManager, err := jwt.New(map[string]interface{}{ - "secret": opt.TokenManagerConfig.JWTSecret, - "expires": int64(60), - }) - if err != nil { - opt.Logger.Fatal().Err(err).Msgf("Could not initialize token-manager") - } - */ + l := opt.Logger return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - /* commented, because moving the ocs specific unmarshaling and statuscode mangling here seems wrong - if isGetSigningKeyRequest(r) { - claims := oidc.FromContext(r.Context()) - if claims == nil { - http.Error(w, "No claims in context", http.StatusUnauthorized) - return - } - - signingKey, _ := getSigningKey(r.Context(), opt.Store, claims.Email) - if len(signingKey) == 0 { - http.Error(w, "No signing key", http.StatusInternalServerError) - return - } - // TODO render as json or xml? - return - } - */ if isSignedRequest(r) { - if signedRequestIsValid(r, opt.Store) { - // TODO store user in context, let account middleware lookup the id? + if signedRequestIsValid(l, r, opt.Store) { + + l.Debug().Str("credential", r.URL.Query().Get("OC-Credential")).Msgf("valid signed request") + + // use openid claims to let the account_uuid middleware do a lookup by username + claims := ocisoidc.StandardClaims{ + PreferredUsername: r.URL.Query().Get("OC-Credential"), + } + + // inject claims to the request context for the account_uuid middleware + ctxWithClaims := ocisoidc.NewContext(r.Context(), &claims) + r = r.WithContext(ctxWithClaims) + next.ServeHTTP(w, r) } else { http.Error(w, "Invalid url signature", http.StatusUnauthorized) @@ -58,19 +46,13 @@ func PresignedURL(opts ...Option) func(next http.Handler) http.Handler { } } -/* -func isGetSigningKeyRequest(r *http.Request) bool { - return r.URL.Path == "/ocs/v1.php/cloud/user/signing-key" || r.URL.Path == "/ocs/v2.php/cloud/user/signing-key" -} -*/ - func isSignedRequest(r *http.Request) bool { return r.URL.Query().Get("OC-Signature") != "" } -func signedRequestIsValid(r *http.Request, s storepb.StoreService) bool { +func signedRequestIsValid(l log.Logger, r *http.Request, s storepb.StoreService) bool { // cheap checks first - // TODO OC-Algorythm - defined the used algo (e.g. sha256 or sha512 - we should agree on one default algo and make this parameter optional) + // TODO OC-Algorithm - defined the used algo (e.g. sha256 or sha512 - we should agree on one default algo and make this parameter optional) // OC-Credential - defines the user scope (shall we use the owncloud user id here - this might leak internal data ....) REQUIRED // OC-Date - defined the date the url was signed (ISO 8601 UTC) REQUIRED // OC-Expires - defines the expiry interval in seconds (between 1 and 604800 = 7 days) REQUIRED @@ -91,19 +73,34 @@ func signedRequestIsValid(r *http.Request, s storepb.StoreService) bool { } else { t.Add(expires) if t.After(time.Now()) { // TODO now client time and server time must be in sync + l.Debug().Msgf("signed url expired") return false } } - signingKey, _ := getSigningKey(r.Context(), s, r.URL.Query().Get("OC-Credential")) + signingKey, err := getSigningKey(r.Context(), s, r.URL.Query().Get("OC-Credential")) if len(signingKey) == 0 { + l.Debug().Err(err).Msgf("signing key empty") return false } - signature := r.URL.Query().Get("OC-Signature") - r.URL.Query().Del("OC-Signature") + q := r.URL.Query() + signature := q.Get("OC-Signature") + q.Del("OC-Signature") + r.URL.RawQuery = q.Encode() url := r.URL.String() - hash := pbkdf2.Key([]byte(url), signingKey, 10000, sha512.Size, sha512.New) + if !r.URL.IsAbs() { + url = "https://" + r.Host + url // TODO where do we get the scheme from + } + // the oc10 signature check: $hash = \hash_pbkdf2("sha512", $url, $signingKey, 10000, 64, false); + // - sets the length of the output string to 64 + // - sets raw output to false -> if raw_output is FALSE length corresponds to twice the byte-length of the derived key (as every byte of the key is returned as two hexits). + // TODO change to length 128 in oc10? + // fo golangs pbkdf2.Key we need to use 32 because it will be encoded into 64 hexits later + hash := pbkdf2.Key([]byte(url), signingKey, 10000, 32, sha512.New) + + l.Debug().Interface("request", r).Str("url", url).Str("signature", signature).Bytes("signingkey", signingKey).Bytes("hash", hash).Str("hexencodedhash", hex.EncodeToString(hash)).Msgf("signature check") + if hex.EncodeToString(hash) != signature { return false } @@ -120,28 +117,6 @@ func getSigningKey(ctx context.Context, s storepb.StoreService, credential strin }) if err != nil || len(res.Records) < 1 { return []byte{}, err - /* no need to create the key if that is handlead by ocs / a dedicated url-signer service - key := make([]byte, 64) - _, err := rand.Read(key[:]) - if err != nil { - return []byte{}, err - } - _, err = s.Write(ctx, &storepb.WriteRequest{ - Options: &storepb.WriteOptions{ - Database: "proxy", - Table: "signing-keys", - }, - Record: &storepb.Record{ - Key: credential, // TODO username or id? - Value: key, - // TODO Expiry? - }, - }) - if err != nil { - return []byte{}, err - } - return key, nil - */ } return res.Records[0].Value, nil