From 8625a9dbb340ae1fde9843a84426431755372fe1 Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Thu, 6 Apr 2023 07:52:35 -0700 Subject: [PATCH] Make listing metadata permissions stricter (#16974) --- cmd/api-response.go | 61 +++++++++++++++-------- cmd/bucket-listobjects-handlers.go | 80 +++++++----------------------- 2 files changed, 58 insertions(+), 83 deletions(-) diff --git a/cmd/api-response.go b/cmd/api-response.go index c1cbea3c7..183c7b1d0 100644 --- a/cmd/api-response.go +++ b/cmd/api-response.go @@ -35,6 +35,7 @@ import ( "github.com/minio/minio/internal/hash" xhttp "github.com/minio/minio/internal/http" "github.com/minio/minio/internal/logger" + "github.com/minio/pkg/bucket/policy" xxml "github.com/minio/xxml" ) @@ -493,7 +494,7 @@ func generateListBucketsResponse(buckets []BucketInfo) ListBucketsResponse { } // generates an ListBucketVersions response for the said bucket with other enumerated options. -func generateListVersionsResponse(bucket, prefix, marker, versionIDMarker, delimiter, encodingType string, maxKeys int, resp ListObjectVersionsInfo, metadata bool) ListVersionsResponse { +func generateListVersionsResponse(bucket, prefix, marker, versionIDMarker, delimiter, encodingType string, maxKeys int, resp ListObjectVersionsInfo, metadata metaCheckFn) ListVersionsResponse { versions := make([]ObjectVersion, 0, len(resp.Objects)) owner := Owner{ @@ -501,11 +502,19 @@ func generateListVersionsResponse(bucket, prefix, marker, versionIDMarker, delim DisplayName: "minio", } data := ListVersionsResponse{} + var lastObjMetaName string + var tagErr, metaErr APIErrorCode = -1, -1 for _, object := range resp.Objects { if object.Name == "" { continue } + // Cache checks for the same object + if metadata != nil && lastObjMetaName != object.Name { + tagErr = metadata(object.Name, policy.GetObjectTaggingAction) + metaErr = metadata(object.Name, policy.GetObjectAction) + lastObjMetaName = object.Name + } content := ObjectVersion{} content.Key = s3EncodeName(object.Name, encodingType) content.LastModified = amztime.ISO8601Format(object.ModTime.UTC()) @@ -518,8 +527,10 @@ func generateListVersionsResponse(bucket, prefix, marker, versionIDMarker, delim } else { content.StorageClass = globalMinioDefaultStorageClass } - if metadata { + if tagErr == ErrNone { content.UserTags = object.UserTags + } + if metaErr == ErrNone { content.UserMetadata = &Metadata{} switch kind, _ := crypto.IsEncrypted(object.UserDefined); kind { case crypto.S3: @@ -625,7 +636,7 @@ func generateListObjectsV1Response(bucket, prefix, marker, delimiter, encodingTy } // generates an ListObjectsV2 response for the said bucket with other enumerated options. -func generateListObjectsV2Response(bucket, prefix, token, nextToken, startAfter, delimiter, encodingType string, fetchOwner, isTruncated bool, maxKeys int, objects []ObjectInfo, prefixes []string, metadata bool) ListObjectsV2Response { +func generateListObjectsV2Response(bucket, prefix, token, nextToken, startAfter, delimiter, encodingType string, fetchOwner, isTruncated bool, maxKeys int, objects []ObjectInfo, prefixes []string, metadata metaCheckFn) ListObjectsV2Response { contents := make([]Object, 0, len(objects)) owner := Owner{ ID: globalMinioDefaultOwnerID, @@ -650,28 +661,32 @@ func generateListObjectsV2Response(bucket, prefix, token, nextToken, startAfter, content.StorageClass = globalMinioDefaultStorageClass } content.Owner = owner - if metadata { - content.UserTags = object.UserTags - content.UserMetadata = &Metadata{} - switch kind, _ := crypto.IsEncrypted(object.UserDefined); kind { - case crypto.S3: - content.UserMetadata.Set(xhttp.AmzServerSideEncryption, xhttp.AmzEncryptionAES) - case crypto.S3KMS: - content.UserMetadata.Set(xhttp.AmzServerSideEncryption, xhttp.AmzEncryptionKMS) - case crypto.SSEC: - content.UserMetadata.Set(xhttp.AmzServerSideEncryptionCustomerAlgorithm, xhttp.AmzEncryptionAES) + if metadata != nil { + if metadata(object.Name, policy.GetObjectTaggingAction) == ErrNone { + content.UserTags = object.UserTags } - for k, v := range cleanMinioInternalMetadataKeys(object.UserDefined) { - if strings.HasPrefix(strings.ToLower(k), ReservedMetadataPrefixLower) { - // Do not need to send any internal metadata - // values to client. - continue + if metadata(object.Name, policy.GetObjectAction) == ErrNone { + content.UserMetadata = &Metadata{} + switch kind, _ := crypto.IsEncrypted(object.UserDefined); kind { + case crypto.S3: + content.UserMetadata.Set(xhttp.AmzServerSideEncryption, xhttp.AmzEncryptionAES) + case crypto.S3KMS: + content.UserMetadata.Set(xhttp.AmzServerSideEncryption, xhttp.AmzEncryptionKMS) + case crypto.SSEC: + content.UserMetadata.Set(xhttp.AmzServerSideEncryptionCustomerAlgorithm, xhttp.AmzEncryptionAES) } - // https://github.com/google/security-research/security/advisories/GHSA-76wf-9vgp-pj7w - if equals(k, xhttp.AmzMetaUnencryptedContentLength, xhttp.AmzMetaUnencryptedContentMD5) { - continue + for k, v := range cleanMinioInternalMetadataKeys(object.UserDefined) { + if strings.HasPrefix(strings.ToLower(k), ReservedMetadataPrefixLower) { + // Do not need to send any internal metadata + // values to client. + continue + } + // https://github.com/google/security-research/security/advisories/GHSA-76wf-9vgp-pj7w + if equals(k, xhttp.AmzMetaUnencryptedContentLength, xhttp.AmzMetaUnencryptedContentMD5) { + continue + } + content.UserMetadata.Set(k, v) } - content.UserMetadata.Set(k, v) } } contents = append(contents, content) @@ -699,6 +714,8 @@ func generateListObjectsV2Response(bucket, prefix, token, nextToken, startAfter, return data } +type metaCheckFn = func(name string, action policy.Action) (s3Err APIErrorCode) + // generates CopyObjectResponse from etag and lastModified time. func generateCopyObjectResponse(etag string, lastModified time.Time) CopyObjectResponse { return CopyObjectResponse{ diff --git a/cmd/bucket-listobjects-handlers.go b/cmd/bucket-listobjects-handlers.go index c74dc7e46..56af2b9d9 100644 --- a/cmd/bucket-listobjects-handlers.go +++ b/cmd/bucket-listobjects-handlers.go @@ -88,7 +88,12 @@ func (api objectAPIHandlers) listObjectVersionsHandler(w http.ResponseWriter, r writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL) return } - + var checkObjMeta metaCheckFn + if metadata { + checkObjMeta = func(name string, action policy.Action) (s3Err APIErrorCode) { + return checkRequestAuthType(ctx, r, action, bucket, name) + } + } urlValues := r.Form // Extract all the listBucketVersions query params to their native values. @@ -119,7 +124,7 @@ func (api objectAPIHandlers) listObjectVersionsHandler(w http.ResponseWriter, r writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) return } - response := generateListVersionsResponse(bucket, prefix, marker, versionIDMarker, delimiter, encodingType, maxkeys, listObjectVersionsInfo, metadata) + response := generateListVersionsResponse(bucket, prefix, marker, versionIDMarker, delimiter, encodingType, maxkeys, listObjectVersionsInfo, checkObjMeta) // Write success response. writeSuccessResponseXML(w, encodeResponseList(response)) @@ -135,64 +140,7 @@ func (api objectAPIHandlers) listObjectVersionsHandler(w http.ResponseWriter, r // MinIO continues to support ListObjectsV1 and V2 for supporting legacy tools. func (api objectAPIHandlers) ListObjectsV2MHandler(w http.ResponseWriter, r *http.Request) { ctx := newContext(r, w, "ListObjectsV2M") - - defer logger.AuditLog(ctx, w, r, mustGetClaimsFromToken(r)) - - vars := mux.Vars(r) - bucket := vars["bucket"] - - objectAPI := api.ObjectAPI() - if objectAPI == nil { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL) - return - } - - if s3Error := checkRequestAuthType(ctx, r, policy.ListBucketAction, bucket, ""); s3Error != ErrNone { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL) - return - } - - urlValues := r.Form - - // Extract all the listObjectsV2 query params to their native values. - prefix, token, startAfter, delimiter, fetchOwner, maxKeys, encodingType, errCode := getListObjectsV2Args(urlValues) - if errCode != ErrNone { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(errCode), r.URL) - return - } - - // Validate the query params before beginning to serve the request. - // fetch-owner is not validated since it is a boolean - if s3Error := validateListObjectsArgs(prefix, token, delimiter, encodingType, maxKeys); s3Error != ErrNone { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL) - return - } - - listObjectsV2 := objectAPI.ListObjectsV2 - - // Inititate a list objects operation based on the input params. - // On success would return back ListObjectsInfo object to be - // marshaled into S3 compatible XML header. - listObjectsV2Info, err := listObjectsV2(ctx, bucket, prefix, token, delimiter, maxKeys, fetchOwner, startAfter) - if err != nil { - writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) - return - } - - if err = DecryptETags(ctx, GlobalKMS, listObjectsV2Info.Objects); err != nil { - writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL) - return - } - - // The next continuation token has id@node_index format to optimize paginated listing - nextContinuationToken := listObjectsV2Info.NextContinuationToken - - response := generateListObjectsV2Response(bucket, prefix, token, nextContinuationToken, startAfter, - delimiter, encodingType, fetchOwner, listObjectsV2Info.IsTruncated, - maxKeys, listObjectsV2Info.Objects, listObjectsV2Info.Prefixes, true) - - // Write success response. - writeSuccessResponseXML(w, encodeResponseList(response)) + api.listObjectsV2Handler(ctx, w, r, true) } // ListObjectsV2Handler - GET Bucket (List Objects) Version 2. @@ -205,7 +153,11 @@ func (api objectAPIHandlers) ListObjectsV2MHandler(w http.ResponseWriter, r *htt // MinIO continues to support ListObjectsV1 for supporting legacy tools. func (api objectAPIHandlers) ListObjectsV2Handler(w http.ResponseWriter, r *http.Request) { ctx := newContext(r, w, "ListObjectsV2") + api.listObjectsV2Handler(ctx, w, r, false) +} +// listObjectsV2Handler performs listing either with or without extra metadata. +func (api objectAPIHandlers) listObjectsV2Handler(ctx context.Context, w http.ResponseWriter, r *http.Request, metadata bool) { defer logger.AuditLog(ctx, w, r, mustGetClaimsFromToken(r)) vars := mux.Vars(r) @@ -222,6 +174,12 @@ func (api objectAPIHandlers) ListObjectsV2Handler(w http.ResponseWriter, r *http return } + var checkObjMeta metaCheckFn + if metadata { + checkObjMeta = func(name string, action policy.Action) (s3Err APIErrorCode) { + return checkRequestAuthType(ctx, r, action, bucket, name) + } + } urlValues := r.Form // Extract all the listObjectsV2 query params to their native values. @@ -264,7 +222,7 @@ func (api objectAPIHandlers) ListObjectsV2Handler(w http.ResponseWriter, r *http response := generateListObjectsV2Response(bucket, prefix, token, listObjectsV2Info.NextContinuationToken, startAfter, delimiter, encodingType, fetchOwner, listObjectsV2Info.IsTruncated, - maxKeys, listObjectsV2Info.Objects, listObjectsV2Info.Prefixes, false) + maxKeys, listObjectsV2Info.Objects, listObjectsV2Info.Prefixes, checkObjMeta) // Write success response. writeSuccessResponseXML(w, encodeResponseList(response))