Fix `missing signature key` error when pulling Docker images with `SERVE_DIRECT`...

Fix #28121

I did some tests and found that the `missing signature key` error is
caused by an incorrect `Content-Type` header. Gitea correctly sets the
`Content-Type` header when serving files.

https://github.com/go-gitea/gitea/blob/348d1d0f322ca57c459acd902f54821d687ca804/routers/api/packages/container/container.go#L712-L717
However, when `SERVE_DIRECT` is enabled, the `Content-Type` header may
be set to an incorrect value by the storage service. To fix this issue,
we can use query parameters to override response header values.

https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html
<img width="600px"
src="https://github.com/user-attachments/assets/f2ff90f0-f1df-46f9-9680-b8120222c555"
/>

In this PR, I introduced a new parameter to the `URL` method to support
additional parameters.

```
URL(path, name string, reqParams url.Values) (*url.URL, error)
```

---

Most S3-like services support specifying the content type when storing
objects. However, Gitea always use `application/octet-stream`.
Therefore, I believe we also need to improve the `Save` method to
support storing objects with the correct content type.

https://github.com/go-gitea/gitea/blob/b7fb20e73e63b8edc9b90c52073e248bef428fcc/modules/storage/minio.go#L214-L221

diff --git a/modules/packages/content_store.go b/modules/packages/content_store.go
index 2108be64d25fb4640a63cd733645b9b04257bb42..37612556d7f6ef96ad4054c4bb321422d4aaf875 100644 (file)
--- a/modules/packages/content_store.go
+++ b/modules/packages/content_store.go
@@ -37,8 +37,8 @@ func (s *ContentStore) ShouldServeDirect() bool {
        return setting.Packages.Storage.ServeDirect()
 }
 
-func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename string) (*url.URL, error) {
-       return s.store.URL(KeyToRelativePath(key), filename)
+func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename string, reqParams url.Values) (*url.URL, error) {
+       return s.store.URL(KeyToRelativePath(key), filename, reqParams)
 }
 
 // FIXME: Workaround to be removed in v1.20

diff --git a/modules/storage/azureblob.go b/modules/storage/azureblob.go
index 568227ca47800644816fca4468b794580ef99183..96c2525b29b0595cf0edceb43122d8ea77dc31bf 100644 (file)
--- a/modules/storage/azureblob.go
+++ b/modules/storage/azureblob.go
@@ -247,7 +247,7 @@ func (a *AzureBlobStorage) Delete(path string) error {
 }
 
 // URL gets the redirect URL to a file. The presigned link is valid for 5 minutes.
-func (a *AzureBlobStorage) URL(path, name string) (*url.URL, error) {
+func (a *AzureBlobStorage) URL(path, name string, reqParams url.Values) (*url.URL, error) {
        blobClient := a.getBlobClient(path)
 
        startTime := time.Now()

diff --git a/modules/storage/helper.go b/modules/storage/helper.go
index f8dff9e64d912a1b122c75d2396eaa7dac0c8762..9e6cceb537da794093de7d9ae205ad4069a50eeb 100644 (file)
--- a/modules/storage/helper.go
+++ b/modules/storage/helper.go
@@ -30,7 +30,7 @@ func (s discardStorage) Delete(_ string) error {
        return fmt.Errorf("%s", s)
 }
 
-func (s discardStorage) URL(_, _ string) (*url.URL, error) {
+func (s discardStorage) URL(_, _ string, _ url.Values) (*url.URL, error) {
        return nil, fmt.Errorf("%s", s)
 }
 

diff --git a/modules/storage/helper_test.go b/modules/storage/helper_test.go
index f4c2d0467f7e73bc3a7722d26116e2d5088bc795..62ebd8753c89b55e82298560b221f658bbea9cec 100644 (file)
--- a/modules/storage/helper_test.go
+++ b/modules/storage/helper_test.go
@@ -37,7 +37,7 @@ func Test_discardStorage(t *testing.T) {
                                assert.Error(t, err, string(tt))
                        }
                        {
-                               got, err := tt.URL("path", "name")
+                               got, err := tt.URL("path", "name", nil)
                                assert.Nil(t, got)
                                assert.Errorf(t, err, string(tt))
                        }

diff --git a/modules/storage/local.go b/modules/storage/local.go
index 9bb532f1df812544fa5d59844037b5aac257c9d9..00c7f668aa2c301f26eb3003f70df0a5a6e348b1 100644 (file)
--- a/modules/storage/local.go
+++ b/modules/storage/local.go
@@ -114,7 +114,7 @@ func (l *LocalStorage) Delete(path string) error {
 }
 
 // URL gets the redirect URL to a file
-func (l *LocalStorage) URL(path, name string) (*url.URL, error) {
+func (l *LocalStorage) URL(path, name string, reqParams url.Values) (*url.URL, error) {
        return nil, ErrURLNotSupported
 }
 

diff --git a/modules/storage/minio.go b/modules/storage/minio.go
index 1b32b2f54f0bd60e8dc70e3f25e20da66ee860d8..8acb7b0354c2ebbdf8a7700e97b0df47ff324f73 100644 (file)
--- a/modules/storage/minio.go
+++ b/modules/storage/minio.go
@@ -276,8 +276,12 @@ func (m *MinioStorage) Delete(path string) error {
 }
 
 // URL gets the redirect URL to a file. The presigned link is valid for 5 minutes.
-func (m *MinioStorage) URL(path, name string) (*url.URL, error) {
-       reqParams := make(url.Values)
+func (m *MinioStorage) URL(path, name string, serveDirectReqParams url.Values) (*url.URL, error) {
+       // copy serveDirectReqParams
+       reqParams, err := url.ParseQuery(serveDirectReqParams.Encode())
+       if err != nil {
+               return nil, err
+       }
        // TODO it may be good to embed images with 'inline' like ServeData does, but we don't want to have to read the file, do we?
        reqParams.Set("response-content-disposition", "attachment; filename=\""+quoteEscaper.Replace(name)+"\"")
        u, err := m.client.PresignedGetObject(m.ctx, m.bucket, m.buildMinioPath(path), 5*time.Minute, reqParams)

diff --git a/modules/storage/storage.go b/modules/storage/storage.go
index 8f970b5dfccc978ce4838397d97c311616b1a92e..52a250080cd4a50484e55e472e54a11698e0c21d 100644 (file)
--- a/modules/storage/storage.go
+++ b/modules/storage/storage.go
@@ -63,7 +63,7 @@ type ObjectStorage interface {
        Save(path string, r io.Reader, size int64) (int64, error)
        Stat(path string) (os.FileInfo, error)
        Delete(path string) error
-       URL(path, name string) (*url.URL, error)
+       URL(path, name string, reqParams url.Values) (*url.URL, error)
        IterateObjects(path string, iterator func(path string, obj Object) error) error
 }
 

diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go
index da5850589f2e6badf65a72e744ea8b6b3e1c86a2..0a7f92ac40a7b5992aa07e72e0df3bd1340d1c72 100644 (file)
--- a/routers/api/actions/artifacts.go
+++ b/routers/api/actions/artifacts.go
@@ -425,7 +425,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) {
        for _, artifact := range artifacts {
                var downloadURL string
                if setting.Actions.ArtifactStorage.ServeDirect() {
-                       u, err := ar.fs.URL(artifact.StoragePath, artifact.ArtifactName)
+                       u, err := ar.fs.URL(artifact.StoragePath, artifact.ArtifactName, nil)
                        if err != nil && !errors.Is(err, storage.ErrURLNotSupported) {
                                log.Error("Error getting serve direct url: %v", err)
                        }

diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go
index 9e463cceebc194b42a7b7b73d6dc3c2aa7625567..6dd36888d2429d883f8210fedd37fb58d5800ebb 100644 (file)
--- a/routers/api/actions/artifactsv4.go
+++ b/routers/api/actions/artifactsv4.go
@@ -517,7 +517,7 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) {
        respData := GetSignedArtifactURLResponse{}
 
        if setting.Actions.ArtifactStorage.ServeDirect() {
-               u, err := storage.ActionsArtifacts.URL(artifact.StoragePath, artifact.ArtifactPath)
+               u, err := storage.ActionsArtifacts.URL(artifact.StoragePath, artifact.ArtifactPath, nil)
                if u != nil && err == nil {
                        respData.SignedUrl = u.String()
                }

diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go
index d495d199d9d6608d1a11a9f9186b514394a9725b..3a470ad68523ad747ff54bd157a2a4676c1b2975 100644 (file)
--- a/routers/api/packages/container/container.go
+++ b/routers/api/packages/container/container.go
@@ -703,7 +703,9 @@ func DeleteManifest(ctx *context.Context) {
 }
 
 func serveBlob(ctx *context.Context, pfd *packages_model.PackageFileDescriptor) {
-       s, u, _, err := packages_service.GetPackageBlobStream(ctx, pfd.File, pfd.Blob)
+       serveDirectReqParams := make(url.Values)
+       serveDirectReqParams.Set("response-content-type", pfd.Properties.GetByName(container_module.PropertyMediaType))
+       s, u, _, err := packages_service.GetPackageBlobStream(ctx, pfd.File, pfd.Blob, serveDirectReqParams)
        if err != nil {
                apiError(ctx, http.StatusInternalServerError, err)
                return

diff --git a/routers/api/packages/maven/maven.go b/routers/api/packages/maven/maven.go
index 343705990a0712a44c11abf3fd463b136dbbde4e..9474b17bc79d184ab1b21d6dd92966f15b41be8f 100644 (file)
--- a/routers/api/packages/maven/maven.go
+++ b/routers/api/packages/maven/maven.go
@@ -215,7 +215,7 @@ func servePackageFile(ctx *context.Context, params parameters, serveContent bool
                return
        }
 
-       s, u, _, err := packages_service.GetPackageBlobStream(ctx, pf, pb)
+       s, u, _, err := packages_service.GetPackageBlobStream(ctx, pf, pb, nil)
        if err != nil {
                apiError(ctx, http.StatusInternalServerError, err)
                return

diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go
index e335c29c7065a6c2d2486ba644ed53a55577af43..97f7a49390543cfba9c359a0535bd4ecf3062d68 100644 (file)
--- a/routers/api/v1/repo/file.go
+++ b/routers/api/v1/repo/file.go
@@ -209,7 +209,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) {
 
        if setting.LFS.Storage.ServeDirect() {
                // If we have a signed url (S3, object storage), redirect to this directly.
-               u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name())
+               u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name(), nil)
                if u != nil && err == nil {
                        ctx.Redirect(u.String())
                        return
@@ -334,7 +334,7 @@ func download(ctx *context.APIContext, archiveName string, archiver *repo_model.
        rPath := archiver.RelativePath()
        if setting.RepoArchive.Storage.ServeDirect() {
                // If we have a signed url (S3, object storage), redirect to this directly.
-               u, err := storage.RepoArchives.URL(rPath, downloadName)
+               u, err := storage.RepoArchives.URL(rPath, downloadName, nil)
                if u != nil && err == nil {
                        ctx.Redirect(u.String())
                        return

diff --git a/routers/web/base.go b/routers/web/base.go
index c44233f9575065cc4db7df85e94e2ab0663359cb..aa0b43c16a1f050a5fbc560f59264e55b891034c 100644 (file)
--- a/routers/web/base.go
+++ b/routers/web/base.go
@@ -39,7 +39,7 @@ func storageHandler(storageSetting *setting.Storage, prefix string, objStore sto
                        rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/")
                        rPath = util.PathJoinRelX(rPath)
 
-                       u, err := objStore.URL(rPath, path.Base(rPath))
+                       u, err := objStore.URL(rPath, path.Base(rPath), nil)
                        if err != nil {
                                if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) {
                                        log.Warn("Unable to find %s %s", prefix, rPath)

diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go
index 20e29425a39c438278fce181a36f737d94799397..f86d4c61775d101f0b0c0f31763582b2f260a1c5 100644 (file)
--- a/routers/web/repo/actions/view.go
+++ b/routers/web/repo/actions/view.go
@@ -663,7 +663,7 @@ func ArtifactsDownloadView(ctx *context_module.Context) {
        if len(artifacts) == 1 && artifacts[0].ArtifactName+".zip" == artifacts[0].ArtifactPath && artifacts[0].ContentEncoding == "application/zip" {
                art := artifacts[0]
                if setting.Actions.ArtifactStorage.ServeDirect() {
-                       u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath)
+                       u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, nil)
                        if u != nil && err == nil {
                                ctx.Redirect(u.String())
                                return

diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go
index 0df2efeac7d59c04cdd953b552e6b028ff7dd889..04f480f611013431403967c45499ecc1aa0b740b 100644 (file)
--- a/routers/web/repo/attachment.go
+++ b/routers/web/repo/attachment.go
@@ -129,7 +129,7 @@ func ServeAttachment(ctx *context.Context, uuid string) {
 
        if setting.Attachment.Storage.ServeDirect() {
                // If we have a signed url (S3, object storage), redirect to this directly.
-               u, err := storage.Attachments.URL(attach.RelativePath(), attach.Name)
+               u, err := storage.Attachments.URL(attach.RelativePath(), attach.Name, nil)
 
                if u != nil && err == nil {
                        ctx.Redirect(u.String())

diff --git a/routers/web/repo/download.go b/routers/web/repo/download.go
index 8c4da3406045370fa1b114f534446f75ac8ec4bd..1ed907b2f97c2d01e9d54d72b31e035a79e183ec 100644 (file)
--- a/routers/web/repo/download.go
+++ b/routers/web/repo/download.go
@@ -55,7 +55,7 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified *time.Tim
 
                if setting.LFS.Storage.ServeDirect() {
                        // If we have a signed url (S3, object storage, blob storage), redirect to this directly.
-                       u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name())
+                       u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name(), nil)
                        if u != nil && err == nil {
                                ctx.Redirect(u.String())
                                return nil

diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go
index f7a7cb5d389423c3442b63a65d3c2f37cca599db..be6f2d483ff31a22ca712813132cf42808012bbf 100644 (file)
--- a/routers/web/repo/repo.go
+++ b/routers/web/repo/repo.go
@@ -494,7 +494,7 @@ func download(ctx *context.Context, archiveName string, archiver *repo_model.Rep
        rPath := archiver.RelativePath()
        if setting.RepoArchive.Storage.ServeDirect() {
                // If we have a signed url (S3, object storage), redirect to this directly.
-               u, err := storage.RepoArchives.URL(rPath, downloadName)
+               u, err := storage.RepoArchives.URL(rPath, downloadName, nil)
                if u != nil && err == nil {
                        ctx.Redirect(u.String())
                        return

diff --git a/services/lfs/server.go b/services/lfs/server.go
index 6932f839c7c43786f2059fff457f35c9520a2d53..f8ef177387026aa996bf9814ae957b6bb19e60a4 100644 (file)
--- a/services/lfs/server.go
+++ b/services/lfs/server.go
@@ -460,7 +460,7 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa
                        var link *lfs_module.Link
                        if setting.LFS.Storage.ServeDirect() {
                                // If we have a signed url (S3, object storage), redirect to this directly.
-                               u, err := storage.LFS.URL(pointer.RelativePath(), pointer.Oid)
+                               u, err := storage.LFS.URL(pointer.RelativePath(), pointer.Oid, nil)
                                if u != nil && err == nil {
                                        // Presigned url does not need the Authorization header
                                        // https://github.com/go-gitea/gitea/issues/21525

diff --git a/services/packages/packages.go b/services/packages/packages.go
index 64b1ddd869632714bbd33fae8fb571b425b507e1..95579be34be5b0703cf74ded32573f514925f8ae 100644 (file)
--- a/services/packages/packages.go
+++ b/services/packages/packages.go
@@ -596,12 +596,12 @@ func GetPackageFileStream(ctx context.Context, pf *packages_model.PackageFile) (
                return nil, nil, nil, err
        }
 
-       return GetPackageBlobStream(ctx, pf, pb)
+       return GetPackageBlobStream(ctx, pf, pb, nil)
 }
 
 // GetPackageBlobStream returns the content of the specific package blob
 // If the storage supports direct serving and it's enabled, only the direct serving url is returned.
-func GetPackageBlobStream(ctx context.Context, pf *packages_model.PackageFile, pb *packages_model.PackageBlob) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) {
+func GetPackageBlobStream(ctx context.Context, pf *packages_model.PackageFile, pb *packages_model.PackageBlob, serveDirectReqParams url.Values) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) {
        key := packages_module.BlobHash256Key(pb.HashSHA256)
 
        cs := packages_module.NewContentStore()
@@ -611,7 +611,7 @@ func GetPackageBlobStream(ctx context.Context, pf *packages_model.PackageFile, p
        var err error
 
        if cs.ShouldServeDirect() {
-               u, err = cs.GetServeDirectURL(key, pf.Name)
+               u, err = cs.GetServeDirectURL(key, pf.Name, serveDirectReqParams)
                if err != nil && !errors.Is(err, storage.ErrURLNotSupported) {
                        log.Error("Error getting serve direct url: %v", err)
                }