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

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)
This commit is contained in:
Zettat123 2024-10-31 23:28:25 +08:00 committed by GitHub
parent 8107823026
commit 0690cb076b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 30 additions and 24 deletions

View File

@ -37,8 +37,8 @@ func (s *ContentStore) ShouldServeDirect() bool {
return setting.Packages.Storage.ServeDirect() return setting.Packages.Storage.ServeDirect()
} }
func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename string) (*url.URL, error) { func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename string, reqParams url.Values) (*url.URL, error) {
return s.store.URL(KeyToRelativePath(key), filename) return s.store.URL(KeyToRelativePath(key), filename, reqParams)
} }
// FIXME: Workaround to be removed in v1.20 // FIXME: Workaround to be removed in v1.20

View File

@ -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. // 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) blobClient := a.getBlobClient(path)
startTime := time.Now() startTime := time.Now()

View File

@ -30,7 +30,7 @@ func (s discardStorage) Delete(_ string) error {
return fmt.Errorf("%s", s) 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) return nil, fmt.Errorf("%s", s)
} }

View File

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

View File

@ -114,7 +114,7 @@ func (l *LocalStorage) Delete(path string) error {
} }
// URL gets the redirect URL to a file // 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 return nil, ErrURLNotSupported
} }

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. // 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) { func (m *MinioStorage) URL(path, name string, serveDirectReqParams url.Values) (*url.URL, error) {
reqParams := make(url.Values) // 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? // 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)+"\"") 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) u, err := m.client.PresignedGetObject(m.ctx, m.bucket, m.buildMinioPath(path), 5*time.Minute, reqParams)

View File

@ -63,7 +63,7 @@ type ObjectStorage interface {
Save(path string, r io.Reader, size int64) (int64, error) Save(path string, r io.Reader, size int64) (int64, error)
Stat(path string) (os.FileInfo, error) Stat(path string) (os.FileInfo, error)
Delete(path string) 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 IterateObjects(path string, iterator func(path string, obj Object) error) error
} }

View File

@ -425,7 +425,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) {
for _, artifact := range artifacts { for _, artifact := range artifacts {
var downloadURL string var downloadURL string
if setting.Actions.ArtifactStorage.ServeDirect() { 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) { if err != nil && !errors.Is(err, storage.ErrURLNotSupported) {
log.Error("Error getting serve direct url: %v", err) log.Error("Error getting serve direct url: %v", err)
} }

View File

@ -517,7 +517,7 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) {
respData := GetSignedArtifactURLResponse{} respData := GetSignedArtifactURLResponse{}
if setting.Actions.ArtifactStorage.ServeDirect() { 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 { if u != nil && err == nil {
respData.SignedUrl = u.String() respData.SignedUrl = u.String()
} }

View File

@ -703,7 +703,9 @@ func DeleteManifest(ctx *context.Context) {
} }
func serveBlob(ctx *context.Context, pfd *packages_model.PackageFileDescriptor) { 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 { if err != nil {
apiError(ctx, http.StatusInternalServerError, err) apiError(ctx, http.StatusInternalServerError, err)
return return

View File

@ -215,7 +215,7 @@ func servePackageFile(ctx *context.Context, params parameters, serveContent bool
return return
} }
s, u, _, err := packages_service.GetPackageBlobStream(ctx, pf, pb) s, u, _, err := packages_service.GetPackageBlobStream(ctx, pf, pb, nil)
if err != nil { if err != nil {
apiError(ctx, http.StatusInternalServerError, err) apiError(ctx, http.StatusInternalServerError, err)
return return

View File

@ -209,7 +209,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) {
if setting.LFS.Storage.ServeDirect() { if setting.LFS.Storage.ServeDirect() {
// If we have a signed url (S3, object storage), redirect to this directly. // 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 { if u != nil && err == nil {
ctx.Redirect(u.String()) ctx.Redirect(u.String())
return return
@ -334,7 +334,7 @@ func download(ctx *context.APIContext, archiveName string, archiver *repo_model.
rPath := archiver.RelativePath() rPath := archiver.RelativePath()
if setting.RepoArchive.Storage.ServeDirect() { if setting.RepoArchive.Storage.ServeDirect() {
// If we have a signed url (S3, object storage), redirect to this directly. // 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 u != nil && err == nil {
ctx.Redirect(u.String()) ctx.Redirect(u.String())
return return

View File

@ -39,7 +39,7 @@ func storageHandler(storageSetting *setting.Storage, prefix string, objStore sto
rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/") rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/")
rPath = util.PathJoinRelX(rPath) 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 err != nil {
if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) { if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) {
log.Warn("Unable to find %s %s", prefix, rPath) log.Warn("Unable to find %s %s", prefix, rPath)

View File

@ -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" { if len(artifacts) == 1 && artifacts[0].ArtifactName+".zip" == artifacts[0].ArtifactPath && artifacts[0].ContentEncoding == "application/zip" {
art := artifacts[0] art := artifacts[0]
if setting.Actions.ArtifactStorage.ServeDirect() { 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 { if u != nil && err == nil {
ctx.Redirect(u.String()) ctx.Redirect(u.String())
return return

View File

@ -129,7 +129,7 @@ func ServeAttachment(ctx *context.Context, uuid string) {
if setting.Attachment.Storage.ServeDirect() { if setting.Attachment.Storage.ServeDirect() {
// If we have a signed url (S3, object storage), redirect to this directly. // 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 { if u != nil && err == nil {
ctx.Redirect(u.String()) ctx.Redirect(u.String())

View File

@ -55,7 +55,7 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified *time.Tim
if setting.LFS.Storage.ServeDirect() { if setting.LFS.Storage.ServeDirect() {
// If we have a signed url (S3, object storage, blob storage), redirect to this directly. // 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 { if u != nil && err == nil {
ctx.Redirect(u.String()) ctx.Redirect(u.String())
return nil return nil

View File

@ -494,7 +494,7 @@ func download(ctx *context.Context, archiveName string, archiver *repo_model.Rep
rPath := archiver.RelativePath() rPath := archiver.RelativePath()
if setting.RepoArchive.Storage.ServeDirect() { if setting.RepoArchive.Storage.ServeDirect() {
// If we have a signed url (S3, object storage), redirect to this directly. // 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 u != nil && err == nil {
ctx.Redirect(u.String()) ctx.Redirect(u.String())
return return

View File

@ -460,7 +460,7 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa
var link *lfs_module.Link var link *lfs_module.Link
if setting.LFS.Storage.ServeDirect() { if setting.LFS.Storage.ServeDirect() {
// If we have a signed url (S3, object storage), redirect to this directly. // 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 { if u != nil && err == nil {
// Presigned url does not need the Authorization header // Presigned url does not need the Authorization header
// https://github.com/go-gitea/gitea/issues/21525 // https://github.com/go-gitea/gitea/issues/21525

View File

@ -596,12 +596,12 @@ func GetPackageFileStream(ctx context.Context, pf *packages_model.PackageFile) (
return nil, nil, nil, err 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 // 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. // 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) key := packages_module.BlobHash256Key(pb.HashSHA256)
cs := packages_module.NewContentStore() cs := packages_module.NewContentStore()
@ -611,7 +611,7 @@ func GetPackageBlobStream(ctx context.Context, pf *packages_model.PackageFile, p
var err error var err error
if cs.ShouldServeDirect() { 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) { if err != nil && !errors.Is(err, storage.ErrURLNotSupported) {
log.Error("Error getting serve direct url: %v", err) log.Error("Error getting serve direct url: %v", err)
} }