From 6651c655cb954657a2858539b928c752004547df Mon Sep 17 00:00:00 2001 From: Poorna Date: Mon, 29 Jul 2024 01:02:16 -0700 Subject: [PATCH] fix replication of checksum when encryption is enabled (#20161) - Adding functional tests - Return checksum header on GET/HEAD, previously this was returning InvalidPartNumber error --- cmd/bucket-replication.go | 31 ++----- cmd/encryption-v1.go | 8 ++ cmd/erasure-metadata.go | 1 + cmd/object-handlers-common.go | 17 +++- .../run-replication-with-checksum-header.sh | 89 ++++++++++++++++++- 5 files changed, 115 insertions(+), 31 deletions(-) diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go index fecf8c9e7..6a211762a 100644 --- a/cmd/bucket-replication.go +++ b/cmd/bucket-replication.go @@ -774,21 +774,6 @@ func (m caseInsensitiveMap) Lookup(key string) (string, bool) { return "", false } -func getCRCMeta(oi ObjectInfo, partNum int, h http.Header) map[string]string { - meta := make(map[string]string) - cs := oi.decryptChecksums(partNum, h) - for k, v := range cs { - cksum := hash.NewChecksumString(k, v) - if cksum == nil { - continue - } - if cksum.Valid() { - meta[cksum.Type.Key()] = v - } - } - return meta -} - func putReplicationOpts(ctx context.Context, sc string, objInfo ObjectInfo, partNum int) (putOpts minio.PutObjectOptions, err error) { meta := make(map[string]string) isSSEC := crypto.SSEC.IsEncrypted(objInfo.UserDefined) @@ -797,11 +782,6 @@ func putReplicationOpts(ctx context.Context, sc string, objInfo ObjectInfo, part // In case of SSE-C objects copy the allowed internal headers as well if !isSSEC || !slices.Contains(maps.Keys(validSSEReplicationHeaders), k) { if stringsHasPrefixFold(k, ReservedMetadataPrefixLower) { - if strings.EqualFold(k, ReservedMetadataPrefixLower+"crc") { - for k, v := range getCRCMeta(objInfo, partNum, nil) { - meta[k] = v - } - } continue } if isStandardHeader(k) { @@ -820,8 +800,12 @@ func putReplicationOpts(ctx context.Context, sc string, objInfo ObjectInfo, part if isSSEC { meta[ReplicationSsecChecksumHeader] = base64.StdEncoding.EncodeToString(objInfo.Checksum) } else { - for k, v := range getCRCMeta(objInfo, 0, nil) { - meta[k] = v + for _, pi := range objInfo.Parts { + if pi.Number == partNum { + for k, v := range pi.Checksums { + meta[k] = v + } + } } } } @@ -1675,8 +1659,7 @@ func replicateObjectWithMultipart(ctx context.Context, c *minio.Core, bucket, ob cHeader := http.Header{} cHeader.Add(xhttp.MinIOSourceReplicationRequest, "true") if !isSSEC { - crc := getCRCMeta(objInfo, partInfo.Number, nil) // No SSE-C keys here. - for k, v := range crc { + for k, v := range partInfo.Checksums { cHeader.Add(k, v) } } diff --git a/cmd/encryption-v1.go b/cmd/encryption-v1.go index e4801ca48..6c3ea3e48 100644 --- a/cmd/encryption-v1.go +++ b/cmd/encryption-v1.go @@ -1172,6 +1172,14 @@ func (o *ObjectInfo) decryptChecksums(part int, h http.Header) map[string]string if len(data) == 0 { return nil } + if part > 0 && !crypto.SSEC.IsEncrypted(o.UserDefined) { + // already decrypted in ToObjectInfo for multipart objects + for _, pi := range o.Parts { + if pi.Number == part { + return pi.Checksums + } + } + } if _, encrypted := crypto.IsEncrypted(o.UserDefined); encrypted { decrypted, err := o.metadataDecrypter(h)("object-checksum", data) if err != nil { diff --git a/cmd/erasure-metadata.go b/cmd/erasure-metadata.go index 4b025ae4b..2dce8587e 100644 --- a/cmd/erasure-metadata.go +++ b/cmd/erasure-metadata.go @@ -174,6 +174,7 @@ func (fi FileInfo) ToObjectInfo(bucket, object string, versioned bool) ObjectInf } } objInfo.Checksum = fi.Checksum + objInfo.decryptPartsChecksums(nil) objInfo.Inlined = fi.InlineData() // Success. return objInfo diff --git a/cmd/object-handlers-common.go b/cmd/object-handlers-common.go index ef7446504..fba7ddbda 100644 --- a/cmd/object-handlers-common.go +++ b/cmd/object-handlers-common.go @@ -248,10 +248,19 @@ func checkPreconditions(ctx context.Context, w http.ResponseWriter, r *http.Requ } // Check if the part number is correct. - if opts.PartNumber > 1 && opts.PartNumber > len(objInfo.Parts) { - // According to S3 we don't need to set any object information here. - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidPartNumber), r.URL) - return true + if opts.PartNumber > 1 { + partFound := false + for _, pi := range objInfo.Parts { + if pi.Number == opts.PartNumber { + partFound = true + break + } + } + if !partFound { + // According to S3 we don't need to set any object information here. + writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidPartNumber), r.URL) + return true + } } // If-None-Match : Return the object only if its entity tag (ETag) is different from the diff --git a/docs/site-replication/run-replication-with-checksum-header.sh b/docs/site-replication/run-replication-with-checksum-header.sh index 6f20a1efa..9e8040843 100755 --- a/docs/site-replication/run-replication-with-checksum-header.sh +++ b/docs/site-replication/run-replication-with-checksum-header.sh @@ -65,8 +65,8 @@ echo "done" # Start MinIO instances echo -n "Starting MinIO instances ..." -CI=on MINIO_ROOT_USER=minio MINIO_ROOT_PASSWORD=minio123 minio server --certs-dir /tmp/certs --address ":9001" --console-address ":10000" /tmp/minio1/{1...4}/disk{1...4} /tmp/minio1/{5...8}/disk{1...4} >/tmp/minio1_1.log 2>&1 & -CI=on MINIO_ROOT_USER=minio MINIO_ROOT_PASSWORD=minio123 minio server --certs-dir /tmp/certs --address ":9002" --console-address ":11000" /tmp/minio2/{1...4}/disk{1...4} /tmp/minio2/{5...8}/disk{1...4} >/tmp/minio2_1.log 2>&1 & +CI=on MINIO_KMS_SECRET_KEY=minio-default-key:IyqsU3kMFloCNup4BsZtf/rmfHVcTgznO2F25CkEH1g= MINIO_ROOT_USER=minio MINIO_ROOT_PASSWORD=minio123 minio server --certs-dir /tmp/certs --address ":9001" --console-address ":10000" /tmp/minio1/{1...4}/disk{1...4} /tmp/minio1/{5...8}/disk{1...4} >/tmp/minio1_1.log 2>&1 & +CI=on MINIO_KMS_SECRET_KEY=minio-default-key:IyqsU3kMFloCNup4BsZtf/rmfHVcTgznO2F25CkEH1g= MINIO_ROOT_USER=minio MINIO_ROOT_PASSWORD=minio123 minio server --certs-dir /tmp/certs --address ":9002" --console-address ":11000" /tmp/minio2/{1...4}/disk{1...4} /tmp/minio2/{5...8}/disk{1...4} >/tmp/minio2_1.log 2>&1 & echo "done" if [ ! -f ./mc ]; then @@ -99,7 +99,7 @@ sleep 30 echo "Create bucket in source MinIO instance" ./mc mb minio1/test-bucket --insecure -# Load objects to source site +# Load objects to source site with checksum header echo "Loading objects to source MinIO instance" OBJ_CHKSUM=$(openssl dgst -sha256 -binary fileparts.json +jq