From 365e3c01a2c8a99bc3d9be120583c18b7bfe1774 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 29 Aug 2021 18:46:59 +0000 Subject: [PATCH 1/2] Test if object is accessible. --- integrations/api_repo_lfs_test.go | 20 ++++++++--- services/lfs/server.go | 56 +++++++++++++++++++++++-------- 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/integrations/api_repo_lfs_test.go b/integrations/api_repo_lfs_test.go index 7aa172064e049..56a6fa81fd86f 100644 --- a/integrations/api_repo_lfs_test.go +++ b/integrations/api_repo_lfs_test.go @@ -253,6 +253,10 @@ func TestAPILFSBatch(t *testing.T) { assert.NoError(t, err) assert.True(t, exist) + repo2 := createLFSTestRepository(t, "batch2") + content := []byte("dummy0") + storeObjectInRepo(t, repo2.ID, &content) + meta, err := repo.GetLFSMetaObjectByOid(p.Oid) assert.Nil(t, meta) assert.Equal(t, models.ErrLFSObjectNotExist, err) @@ -358,13 +362,19 @@ func TestAPILFSUpload(t *testing.T) { assert.Nil(t, meta) assert.Equal(t, models.ErrLFSObjectNotExist, err) - req := newRequest(t, p, "") + t.Run("InvalidAccess", func(t *testing.T) { + req := newRequest(t, p, "invalid") + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + }) - session.MakeRequest(t, req, http.StatusOK) + t.Run("ValidAccess", func(t *testing.T) { + req := newRequest(t, p, "dummy5") - meta, err = repo.GetLFSMetaObjectByOid(p.Oid) - assert.NoError(t, err) - assert.NotNil(t, meta) + session.MakeRequest(t, req, http.StatusOK) + meta, err = repo.GetLFSMetaObjectByOid(p.Oid) + assert.NoError(t, err) + assert.NotNil(t, meta) + }) }) t.Run("MetaAlreadyExists", func(t *testing.T) { diff --git a/services/lfs/server.go b/services/lfs/server.go index 81d535beec421..4d99bbf321261 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -5,7 +5,9 @@ package lfs import ( + "crypto/sha256" "encoding/base64" + "encoding/hex" "errors" "fmt" "io" @@ -214,14 +216,22 @@ func BatchHandler(ctx *context.Context) { } } - if exists { - if meta == nil { + if exists && meta == nil { + accessible, err := models.LFSObjectAccessible(ctx.User, p.Oid) + if err != nil { + log.Error("Unable to check if LFS MetaObject [%s] is accessible. Error: %v", p.Oid, err) + writeStatus(ctx, http.StatusInternalServerError) + return + } + if accessible { _, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) if err != nil { log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err) writeStatus(ctx, http.StatusInternalServerError) return } + } else { + exists = false } } @@ -271,31 +281,49 @@ func UploadHandler(ctx *context.Context) { return } - meta, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) - if err != nil { - log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err) - writeStatus(ctx, http.StatusInternalServerError) - return - } - contentStore := lfs_module.NewContentStore() - exists, err := contentStore.Exists(p) if err != nil { log.Error("Unable to check if LFS OID[%s] exist. Error: %v", p.Oid, err) writeStatus(ctx, http.StatusInternalServerError) return } - if meta.Existing || exists { - ctx.Resp.WriteHeader(http.StatusOK) - return + + uploadOrVerify := func() error { + if exists { + accessible, err := models.LFSObjectAccessible(ctx.User, p.Oid) + if err != nil { + return err + } + if !accessible { + // The file exists but the user has no access to it. + // The upload gets verified by hashing and size comparison to prove access to it. + hash := sha256.New() + written, err := io.Copy(hash, ctx.Req.Body) + if err != nil { + return err + } + + if written != p.Size { + return lfs_module.ErrSizeMismatch + } + if hex.EncodeToString(hash.Sum(nil)) != p.Oid { + return lfs_module.ErrHashMismatch + } + } + } else if err := contentStore.Put(p, ctx.Req.Body); err != nil { + return err + } + _, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) + return err } defer ctx.Req.Body.Close() - if err := contentStore.Put(meta.Pointer, ctx.Req.Body); err != nil { + if err := uploadOrVerify(); err != nil { if errors.Is(err, lfs_module.ErrSizeMismatch) || errors.Is(err, lfs_module.ErrHashMismatch) { writeStatusMessage(ctx, http.StatusUnprocessableEntity, err.Error()) } else { + log.Error("Error whilst uploading LFS MetaObject [%s]. Error: %v", p.Oid, err) writeStatus(ctx, http.StatusInternalServerError) } if _, err = repository.RemoveLFSMetaObjectByOid(p.Oid); err != nil { From 6a6ce8a22b2be9ce3debff73344978b9530583fa Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sun, 29 Aug 2021 20:57:13 +0000 Subject: [PATCH 2/2] Added more logging. --- services/lfs/server.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index 4d99bbf321261..946437fb274df 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -293,6 +293,7 @@ func UploadHandler(ctx *context.Context) { if exists { accessible, err := models.LFSObjectAccessible(ctx.User, p.Oid) if err != nil { + log.Error("Unable to check if LFS MetaObject [%s] is accessible. Error: %v", p.Oid, err) return err } if !accessible { @@ -301,6 +302,7 @@ func UploadHandler(ctx *context.Context) { hash := sha256.New() written, err := io.Copy(hash, ctx.Req.Body) if err != nil { + log.Error("Error creating hash. Error: %v", err) return err } @@ -312,6 +314,7 @@ func UploadHandler(ctx *context.Context) { } } } else if err := contentStore.Put(p, ctx.Req.Body); err != nil { + log.Error("Error putting LFS MetaObject [%s] into content store. Error: %v", p.Oid, err) return err } _, err := models.NewLFSMetaObject(&models.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) @@ -321,9 +324,9 @@ func UploadHandler(ctx *context.Context) { defer ctx.Req.Body.Close() if err := uploadOrVerify(); err != nil { if errors.Is(err, lfs_module.ErrSizeMismatch) || errors.Is(err, lfs_module.ErrHashMismatch) { + log.Error("Upload does not match LFS MetaObject [%s]. Error: %v", p.Oid, err) writeStatusMessage(ctx, http.StatusUnprocessableEntity, err.Error()) } else { - log.Error("Error whilst uploading LFS MetaObject [%s]. Error: %v", p.Oid, err) writeStatus(ctx, http.StatusInternalServerError) } if _, err = repository.RemoveLFSMetaObjectByOid(p.Oid); err != nil {