From f32efd5429a62bd1fe8a159e9c75e364c27e0f67 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 13 Jun 2023 13:52:33 -0700 Subject: [PATCH] more compliance related fixes (#17408) - lifecycle must return InvalidArgument for rule errors - do not return `null` versionId in HTTP header - reject mixed SSE uploads with correct error message --- cmd/api-errors.go | 16 ++++++++-------- cmd/api-headers.go | 2 +- cmd/bucket-handlers.go | 12 +++++++++++- cmd/bucket-lifecycle-handlers_test.go | 4 ++-- cmd/object-handlers-common.go | 2 +- cmd/object-handlers.go | 16 +++++++++++++--- cmd/object-multipart-handlers.go | 15 +++++++++++++++ internal/bucket/lifecycle/lifecycle.go | 2 +- 8 files changed, 52 insertions(+), 17 deletions(-) diff --git a/cmd/api-errors.go b/cmd/api-errors.go index 68a64700c..ac5f56977 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -1121,8 +1121,13 @@ var errorCodes = errorCodeMap{ HTTPStatusCode: http.StatusBadRequest, }, ErrInvalidEncryptionMethod: { - Code: "InvalidRequest", - Description: "The encryption method specified is not supported", + Code: "InvalidArgument", + Description: "Server Side Encryption with AWS KMS managed key requires HTTP header x-amz-server-side-encryption : aws:kms", + HTTPStatusCode: http.StatusBadRequest, + }, + ErrIncompatibleEncryptionMethod: { + Code: "InvalidArgument", + Description: "Server Side Encryption with Customer provided key is incompatible with the encryption method specified", HTTPStatusCode: http.StatusBadRequest, }, ErrInvalidEncryptionKeyID: { @@ -1185,11 +1190,6 @@ var errorCodes = errorCodeMap{ Description: "The provided encryption parameters did not match the ones used originally.", HTTPStatusCode: http.StatusBadRequest, }, - ErrIncompatibleEncryptionMethod: { - Code: "InvalidArgument", - Description: "Server side encryption specified with both SSE-C and SSE-S3 headers", - HTTPStatusCode: http.StatusBadRequest, - }, ErrKMSNotConfigured: { Code: "NotImplemented", Description: "Server side encryption specified but KMS is not configured", @@ -2392,7 +2392,7 @@ func toAPIError(ctx context.Context, err error) APIError { } case lifecycle.Error: apiErr = APIError{ - Code: "InvalidRequest", + Code: "InvalidArgument", Description: e.Error(), HTTPStatusCode: http.StatusBadRequest, } diff --git a/cmd/api-headers.go b/cmd/api-headers.go index 91e571b3b..539d393c7 100644 --- a/cmd/api-headers.go +++ b/cmd/api-headers.go @@ -203,7 +203,7 @@ func setObjectHeaders(w http.ResponseWriter, objInfo ObjectInfo, rs *HTTPRangeSp } // Set the relevant version ID as part of the response header. - if objInfo.VersionID != "" { + if objInfo.VersionID != "" && objInfo.VersionID != nullVersionID { w.Header()[xhttp.AmzVersionID] = []string{objInfo.VersionID} } diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 4e61e08c2..d5f789821 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -1199,6 +1199,16 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h return } + if crypto.SSEC.IsRequested(r.Header) && crypto.S3.IsRequested(r.Header) { + writeErrorResponse(ctx, w, toAPIError(ctx, crypto.ErrIncompatibleEncryptionMethod), r.URL) + return + } + + if crypto.SSEC.IsRequested(r.Header) && crypto.S3KMS.IsRequested(r.Header) { + writeErrorResponse(ctx, w, toAPIError(ctx, crypto.ErrIncompatibleEncryptionMethod), r.URL) + return + } + if crypto.SSEC.IsRequested(r.Header) && isReplicationEnabled(ctx, bucket) { writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidEncryptionParametersSSEC), r.URL) return @@ -1370,7 +1380,7 @@ func (api objectAPIHandlers) PostPolicyBucketHandler(w http.ResponseWriter, r *h w.Header()[xhttp.ETag] = []string{`"` + objInfo.ETag + `"`} // Set the relevant version ID as part of the response header. - if objInfo.VersionID != "" { + if objInfo.VersionID != "" && objInfo.VersionID != nullVersionID { w.Header()[xhttp.AmzVersionID] = []string{objInfo.VersionID} } diff --git a/cmd/bucket-lifecycle-handlers_test.go b/cmd/bucket-lifecycle-handlers_test.go index b0a1cc497..f101f2ef6 100644 --- a/cmd/bucket-lifecycle-handlers_test.go +++ b/cmd/bucket-lifecycle-handlers_test.go @@ -179,7 +179,7 @@ func testBucketLifecycleHandlers(obj ObjectLayer, instanceType, bucketName strin lifecycleResponse: []byte(``), errorResponse: APIErrorResponse{ Resource: SlashSeparator + bucketName + SlashSeparator, - Code: "InvalidRequest", + Code: "InvalidArgument", Message: "Filter must have exactly one of Prefix, Tag, or And specified", }, @@ -196,7 +196,7 @@ func testBucketLifecycleHandlers(obj ObjectLayer, instanceType, bucketName strin lifecycleResponse: []byte(``), errorResponse: APIErrorResponse{ Resource: SlashSeparator + bucketName + SlashSeparator, - Code: "InvalidRequest", + Code: "InvalidArgument", Message: "Date must be provided in ISO 8601 format", }, diff --git a/cmd/object-handlers-common.go b/cmd/object-handlers-common.go index ad03156d8..861518dae 100644 --- a/cmd/object-handlers-common.go +++ b/cmd/object-handlers-common.go @@ -328,7 +328,7 @@ func setPutObjHeaders(w http.ResponseWriter, objInfo ObjectInfo, delete bool) { } // Set the relevant version ID as part of the response header. - if objInfo.VersionID != "" { + if objInfo.VersionID != "" && objInfo.VersionID != nullVersionID { w.Header()[xhttp.AmzVersionID] = []string{objInfo.VersionID} // If version is a deleted marker, set this header as well if objInfo.DeleteMarker && delete { // only returned during delete object diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 9054760ee..a9f6dc5e6 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -1831,6 +1831,16 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req return } + if crypto.SSEC.IsRequested(r.Header) && crypto.S3.IsRequested(r.Header) { + writeErrorResponse(ctx, w, toAPIError(ctx, crypto.ErrIncompatibleEncryptionMethod), r.URL) + return + } + + if crypto.SSEC.IsRequested(r.Header) && crypto.S3KMS.IsRequested(r.Header) { + writeErrorResponse(ctx, w, toAPIError(ctx, crypto.ErrIncompatibleEncryptionMethod), r.URL) + return + } + if crypto.SSEC.IsRequested(r.Header) && isReplicationEnabled(ctx, bucket) { writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidEncryptionParametersSSEC), r.URL) return @@ -2849,7 +2859,7 @@ func (api objectAPIHandlers) GetObjectTaggingHandler(w http.ResponseWriter, r *h return } - if opts.VersionID != "" { + if opts.VersionID != "" && opts.VersionID != nullVersionID { w.Header()[xhttp.AmzVersionID] = []string{opts.VersionID} } @@ -2942,7 +2952,7 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h scheduleReplication(ctx, objInfo.Clone(), objAPI, dsc, replication.MetadataReplicationType) } - if objInfo.VersionID != "" { + if objInfo.VersionID != "" && objInfo.VersionID != nullVersionID { w.Header()[xhttp.AmzVersionID] = []string{objInfo.VersionID} } @@ -3018,7 +3028,7 @@ func (api objectAPIHandlers) DeleteObjectTaggingHandler(w http.ResponseWriter, r scheduleReplication(ctx, oi.Clone(), objAPI, dsc, replication.MetadataReplicationType) } - if oi.VersionID != "" { + if oi.VersionID != "" && oi.VersionID != nullVersionID { w.Header()[xhttp.AmzVersionID] = []string{oi.VersionID} } writeSuccessNoContent(w) diff --git a/cmd/object-multipart-handlers.go b/cmd/object-multipart-handlers.go index 4653d4e06..ce36c6e35 100644 --- a/cmd/object-multipart-handlers.go +++ b/cmd/object-multipart-handlers.go @@ -102,6 +102,21 @@ func (api objectAPIHandlers) NewMultipartUploadHandler(w http.ResponseWriter, r encMetadata := map[string]string{} if crypto.Requested(r.Header) { + if crypto.SSECopy.IsRequested(r.Header) { + writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidEncryptionParameters), r.URL) + return + } + + if crypto.SSEC.IsRequested(r.Header) && crypto.S3.IsRequested(r.Header) { + writeErrorResponse(ctx, w, toAPIError(ctx, crypto.ErrIncompatibleEncryptionMethod), r.URL) + return + } + + if crypto.SSEC.IsRequested(r.Header) && crypto.S3KMS.IsRequested(r.Header) { + writeErrorResponse(ctx, w, toAPIError(ctx, crypto.ErrIncompatibleEncryptionMethod), r.URL) + return + } + if crypto.SSEC.IsRequested(r.Header) && isReplicationEnabled(ctx, bucket) { writeErrorResponse(ctx, w, toAPIError(ctx, errInvalidEncryptionParametersSSEC), r.URL) return diff --git a/internal/bucket/lifecycle/lifecycle.go b/internal/bucket/lifecycle/lifecycle.go index 9a2543e28..d3891ae66 100644 --- a/internal/bucket/lifecycle/lifecycle.go +++ b/internal/bucket/lifecycle/lifecycle.go @@ -33,7 +33,7 @@ import ( var ( errLifecycleTooManyRules = Errorf("Lifecycle configuration allows a maximum of 1000 rules") errLifecycleNoRule = Errorf("Lifecycle configuration should have at least one rule") - errLifecycleDuplicateID = Errorf("Lifecycle configuration has rule with the same ID. Rule ID must be unique.") + errLifecycleDuplicateID = Errorf("Rule ID must be unique. Found same ID for more than one rule") errXMLNotWellFormed = Errorf("The XML you provided was not well-formed or did not validate against our published schema") )