From 38637897ba427d5dd54b6b8d4661c561adab6a4a Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 15 Jan 2024 00:57:49 -0800 Subject: [PATCH] fix: listing SSE encrypted multipart objects (#18786) GetActualSize() was heavily relying on o.Parts() to be non-empty to figure out if the object is multipart or not, However, we have many indicators of whether an object is multipart or not. Blindly assuming that o.Parts == nil is not a multipart, is an incorrect expectation instead, multipart must be obtained via - Stored metadata value indicating this is a multipart encrypted object. - Rely on -actual-size metadata to get the object's actual size. This value is preserved for additional reasons such as these. - ETag != 32 length --- cmd/encryption-v1.go | 5 +- cmd/erasure-healing.go | 2 +- cmd/erasure-metadata.go | 50 ------------------- cmd/erasure-object.go | 1 - cmd/object-api-datatypes.go | 3 -- cmd/object-api-utils.go | 26 ++++++---- .../replication/setup_3site_replication.sh | 31 +++++++++++- 7 files changed, 47 insertions(+), 71 deletions(-) diff --git a/cmd/encryption-v1.go b/cmd/encryption-v1.go index da734f9af..81dd5e41d 100644 --- a/cmd/encryption-v1.go +++ b/cmd/encryption-v1.go @@ -209,9 +209,6 @@ func DecryptETags(ctx context.Context, k kms.KMS, objects []ObjectInfo) error { // uploaded by the user using multipart mechanism: // initiate new multipart, upload part, complete upload func (o *ObjectInfo) isMultipart() bool { - if len(o.Parts) == 0 { - return false - } _, encrypted := crypto.IsEncrypted(o.UserDefined) if encrypted { if !crypto.IsMultiPart(o.UserDefined) { @@ -228,7 +225,7 @@ func (o *ObjectInfo) isMultipart() bool { // Further check if this object is uploaded using multipart mechanism // by the user and it is not about Erasure internally splitting the // object into parts in PutObject() - return !(o.backendType == BackendErasure && len(o.ETag) == 32) + return len(o.ETag) != 32 } // ParseSSECopyCustomerRequest parses the SSE-C header fields of the provided request. diff --git a/cmd/erasure-healing.go b/cmd/erasure-healing.go index 9eaf03705..6006eafda 100644 --- a/cmd/erasure-healing.go +++ b/cmd/erasure-healing.go @@ -334,7 +334,7 @@ func (er *erasureObjects) healObject(ctx context.Context, bucket string, object erasure.ShardFileSize(latestMeta.Parts[0].ActualSize) < smallFileThreshold) } - result.ObjectSize, err = latestMeta.GetActualSize() + result.ObjectSize, err = latestMeta.ToObjectInfo(bucket, object, true).GetActualSize() if err != nil { return result, err } diff --git a/cmd/erasure-metadata.go b/cmd/erasure-metadata.go index 693addc27..ca14f17d3 100644 --- a/cmd/erasure-metadata.go +++ b/cmd/erasure-metadata.go @@ -22,18 +22,15 @@ import ( "encoding/hex" "fmt" "sort" - "strconv" "strings" "time" "github.com/minio/minio/internal/amztime" "github.com/minio/minio/internal/bucket/replication" - "github.com/minio/minio/internal/crypto" "github.com/minio/minio/internal/hash/sha256" xhttp "github.com/minio/minio/internal/http" "github.com/minio/minio/internal/logger" "github.com/minio/pkg/v2/sync/errgroup" - "github.com/minio/sio" ) // Object was stored with additional erasure codes due to degraded system at upload time @@ -88,52 +85,6 @@ func (fi FileInfo) IsValid() bool { correctIndexes) } -func (fi FileInfo) checkMultipart() (int64, bool) { - if len(fi.Parts) == 0 { - return 0, false - } - if !crypto.IsMultiPart(fi.Metadata) { - return 0, false - } - var size int64 - for _, part := range fi.Parts { - psize, err := sio.DecryptedSize(uint64(part.Size)) - if err != nil { - return 0, false - } - size += int64(psize) - } - - return size, len(extractETag(fi.Metadata)) != 32 -} - -// GetActualSize - returns the actual size of the stored object -func (fi FileInfo) GetActualSize() (int64, error) { - if _, ok := fi.Metadata[ReservedMetadataPrefix+"compression"]; ok { - sizeStr, ok := fi.Metadata[ReservedMetadataPrefix+"actual-size"] - if !ok { - return -1, errInvalidDecompressedSize - } - size, err := strconv.ParseInt(sizeStr, 10, 64) - if err != nil { - return -1, errInvalidDecompressedSize - } - return size, nil - } - if _, ok := crypto.IsEncrypted(fi.Metadata); ok { - size, ok := fi.checkMultipart() - if !ok { - size, err := sio.DecryptedSize(uint64(fi.Size)) - if err != nil { - err = errObjectTampered // assign correct error type - } - return int64(size), err - } - return size, nil - } - return fi.Size, nil -} - // ToObjectInfo - Converts metadata to object info. func (fi FileInfo) ToObjectInfo(bucket, object string, versioned bool) ObjectInfo { object = decodeDirObject(object) @@ -166,7 +117,6 @@ func (fi FileInfo) ToObjectInfo(bucket, object string, versioned bool) ObjectInf objInfo.Expires = t.UTC() } } - objInfo.backendType = BackendErasure // Extract etag from metadata. objInfo.ETag = extractETag(fi.Metadata) diff --git a/cmd/erasure-object.go b/cmd/erasure-object.go index b4c586e18..e22e2d2bb 100644 --- a/cmd/erasure-object.go +++ b/cmd/erasure-object.go @@ -1559,7 +1559,6 @@ func (er erasureObjects) putObject(ctx context.Context, bucket string, object st if errors.Is(err, errFileNotFound) { return ObjectInfo{}, toObjectErr(errErasureWriteQuorum, bucket, object) } - logger.LogOnceIf(ctx, err, "erasure-object-rename-"+bucket+"-"+object) return ObjectInfo{}, toObjectErr(err, bucket, object) } diff --git a/cmd/object-api-datatypes.go b/cmd/object-api-datatypes.go index 53d5f97f9..2ec4c45ef 100644 --- a/cmd/object-api-datatypes.go +++ b/cmd/object-api-datatypes.go @@ -200,8 +200,6 @@ type ObjectInfo struct { Legacy bool // indicates object on disk is in legacy data format - // backendType indicates which backend filled this structure - backendType BackendType // internal representation of version purge status VersionPurgeStatusInternal string VersionPurgeStatus VersionPurgeStatusType @@ -282,7 +280,6 @@ func (o *ObjectInfo) Clone() (cinfo ObjectInfo) { metadataOnly: o.metadataOnly, versionOnly: o.versionOnly, keyRotation: o.keyRotation, - backendType: o.backendType, AccTime: o.AccTime, Legacy: o.Legacy, VersionPurgeStatus: o.VersionPurgeStatus, diff --git a/cmd/object-api-utils.go b/cmd/object-api-utils.go index 42279ae66..d2c385520 100644 --- a/cmd/object-api-utils.go +++ b/cmd/object-api-utils.go @@ -533,6 +533,14 @@ func (o ObjectInfo) GetActualSize() (int64, error) { return size, nil } if _, ok := crypto.IsEncrypted(o.UserDefined); ok { + sizeStr, ok := o.UserDefined[ReservedMetadataPrefix+"actual-size"] + if ok { + size, err := strconv.ParseInt(sizeStr, 10, 64) + if err != nil { + return -1, errObjectTampered + } + return size, nil + } return o.DecryptedSize() } @@ -632,16 +640,14 @@ func getCompressedOffsets(oi ObjectInfo, offset int64, decrypt func([]byte) ([]b var skipLength int64 var cumulativeActualSize int64 var firstPartIdx int - if len(oi.Parts) > 0 { - for i, part := range oi.Parts { - cumulativeActualSize += part.ActualSize - if cumulativeActualSize <= offset { - compressedOffset += part.Size - } else { - firstPartIdx = i - skipLength = cumulativeActualSize - part.ActualSize - break - } + for i, part := range oi.Parts { + cumulativeActualSize += part.ActualSize + if cumulativeActualSize <= offset { + compressedOffset += part.Size + } else { + firstPartIdx = i + skipLength = cumulativeActualSize - part.ActualSize + break } } partSkip = offset - skipLength diff --git a/docs/bucket/replication/setup_3site_replication.sh b/docs/bucket/replication/setup_3site_replication.sh index fc75cc8d2..0789f5e8f 100755 --- a/docs/bucket/replication/setup_3site_replication.sh +++ b/docs/bucket/replication/setup_3site_replication.sh @@ -45,8 +45,11 @@ go install github.com/minio/minio/docs/debugging/s3-check-md5@latest wget -O mc https://dl.minio.io/client/mc/release/linux-amd64/mc && chmod +x mc -wget -O mc.RELEASE.2021-03-12T03-36-59Z https://dl.minio.io/client/mc/release/linux-amd64/archive/mc.RELEASE.2021-03-12T03-36-59Z && - chmod +x mc.RELEASE.2021-03-12T03-36-59Z + +if [ ! -f mc.RELEASE.2021-03-12T03-36-59Z ]; then + wget -O mc.RELEASE.2021-03-12T03-36-59Z https://dl.minio.io/client/mc/release/linux-amd64/archive/mc.RELEASE.2021-03-12T03-36-59Z && + chmod +x mc.RELEASE.2021-03-12T03-36-59Z +fi minio server --address 127.0.0.1:9001 "http://127.0.0.1:9001/tmp/multisitea/data/disterasure/xl{1...4}" \ "http://127.0.0.1:9002/tmp/multisitea/data/disterasure/xl{5...8}" >/tmp/sitea_1.log 2>&1 & @@ -209,4 +212,28 @@ s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://1 s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9005/ -bucket olockbucket s3-check-md5 -versions -access-key minio -secret-key minio123 -endpoint http://127.0.0.1:9006/ -bucket olockbucket +# additional tests for encryption object alignment +go install -v github.com/minio/multipart-debug@latest + +upload_id=$(multipart-debug --endpoint 127.0.0.1:9001 --accesskey minio --secretkey minio123 multipart new --bucket bucket --object new-test-encrypted-object --encrypt) + +dd if=/dev/urandom bs=1 count=7048531 of=/tmp/7048531.txt +dd if=/dev/urandom bs=1 count=2847391 of=/tmp/2847391.txt + +sudo apt install jq -y + +etag_1=$(multipart-debug --endpoint 127.0.0.1:9002 --accesskey minio --secretkey minio123 multipart upload --bucket bucket --object new-test-encrypted-object --uploadid ${upload_id} --file /tmp/7048531.txt --number 1 | jq -r .ETag) +etag_2=$(multipart-debug --endpoint 127.0.0.1:9001 --accesskey minio --secretkey minio123 multipart upload --bucket bucket --object new-test-encrypted-object --uploadid ${upload_id} --file /tmp/2847391.txt --number 2 | jq -r .ETag) +multipart-debug --endpoint 127.0.0.1:9002 --accesskey minio --secretkey minio123 multipart complete --bucket bucket --object new-test-encrypted-object --uploadid ${upload_id} 1.${etag_1} 2.${etag_2} + +sleep 10 + +./mc stat sitea/bucket/new-test-encrypted-object +./mc stat siteb/bucket/new-test-encrypted-object +./mc stat sitec/bucket/new-test-encrypted-object + +./mc ls -r sitea/bucket/ +./mc ls -r siteb/bucket/ +./mc ls -r sitec/bucket/ + catch