foster/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2024-11-21 08:31:27 -05:00
Code Issues Wiki Activity

Fix missing signature key error when pulling Docker images with SERVE_DIRECT enabled (#32365)

Browse source 
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.

348d1d0f32/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.

b7fb20e73e/modules/storage/minio.go (L214-L221)
(cherry picked from commit 0690cb076bf63f71988a709f62a9c04660b51a4f)

Conflicts:
	- modules/storage/azureblob.go
	  Dropped the change, as we do not support Azure blob storage.
	- modules/storage/helper.go
	  Resolved by adjusting their `discardStorage` to our
	  `DiscardStorage`
	- routers/api/actions/artifacts.go
	  routers/api/actions/artifactsv4.go
	  routers/web/repo/actions/view.go
	  routers/web/repo/download.go
	  Resolved the conflicts by manually adding the new `nil`
	  parameter to the `storage.Attachments.URL()` calls.

	  Originally conflicted due to differences in the if expression
	  above these calls.
This commit is contained in:
Zettat123 2024-10-31 23:28:25 +08:00 committed by Gergely Nagy
parent 4aa61601c3
commit 6b74043b85
No known key found for this signature in database
18 changed files with 31 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
modules/packages/content_store.go
View file

@ -37,8 +37,8 @@ func (s *ContentStore) ShouldServeDirect() bool {
return setting.Packages.Storage.MinioConfig.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

2
modules/storage/helper.go
View file

@ -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)
}

2
modules/storage/helper_test.go
View file

@ -38,7 +38,7 @@ func Test_discardStorage(t *testing.T) {
require.Error(t, err, string(tt))
}
{
got, err := tt.URL("path", "name")
got, err := tt.URL("path", "name", nil)
assert.Nil(t, got)
require.Errorf(t, err, string(tt))
}

2
modules/storage/local.go
View file

@ -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
}

8
modules/storage/minio.go
View file

@ -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)

2
modules/storage/storage.go
View file

@ -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
}

2
routers/api/actions/artifacts.go
View file

@ -437,7 +437,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) {
for _, artifact := range artifacts {
var downloadURL string
if setting.Actions.ArtifactStorage.MinioConfig.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)
}

2
routers/api/actions/artifactsv4.go
View file

@ -530,7 +530,7 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) {
respData := GetSignedArtifactURLResponse{}
if setting.Actions.ArtifactStorage.MinioConfig.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()
}

4
routers/api/packages/container/container.go
View file

@ -689,7 +689,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

2
routers/api/packages/maven/maven.go
View file

@ -217,7 +217,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

4
routers/api/v1/repo/file.go
View file

@ -214,7 +214,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) {
if setting.LFS.Storage.MinioConfig.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
@ -341,7 +341,7 @@ func download(ctx *context.APIContext, archiveName string, archiver *repo_model.
rPath := archiver.RelativePath()
if setting.RepoArchive.Storage.MinioConfig.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

2
routers/web/base.go
View file

@ -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)

3
routers/web/repo/actions/view.go
View file

@ -688,7 +688,8 @@ 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.MinioConfig.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

2
routers/web/repo/attachment.go
View file

@ -134,7 +134,7 @@ func ServeAttachment(ctx *context.Context, uuid string) {
if setting.Attachment.Storage.MinioConfig.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())

4
routers/web/repo/download.go
View file

@ -54,8 +54,8 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified *time.Tim
}
if setting.LFS.Storage.MinioConfig.ServeDirect {
// If we have a signed url (S3, object storage), redirect to this directly.
u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name())
// If we have a signed url (S3, object storage, blob storage), redirect to this directly.
u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name(), nil)
if u != nil && err == nil {
ctx.Redirect(u.String())
return nil

2
routers/web/repo/repo.go
View file

@ -505,7 +505,7 @@ func download(ctx *context.Context, archiveName string, archiver *repo_model.Rep
rPath := archiver.RelativePath()
if setting.RepoArchive.Storage.MinioConfig.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 {
if archiver.ReleaseID != 0 {
err = repo_model.CountArchiveDownload(ctx, ctx.Repo.Repository.ID, archiver.ReleaseID, archiver.Type)

2
services/lfs/server.go
View file

@ -485,7 +485,7 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa
var link *lfs_module.Link
if setting.LFS.Storage.MinioConfig.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

6
services/packages/packages.go
View file

@ -602,12 +602,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()
@ -617,7 +617,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)
}