Fix part md5 calculating with sse-c #543

Open
opened 2024-11-07 11:15:41 +00:00 by dkirillov · 0 comments
Member

Expected Behavior

Calculating md5 of original pyaload

Current Behavior

Calculating md5 of encrypted payload

Possible Solution

Something like that:

diff --git a/api/layer/multipart_upload.go b/api/layer/multipart_upload.go
index ed7612c9..e86598b0 100644
--- a/api/layer/multipart_upload.go
+++ b/api/layer/multipart_upload.go
@@ -224,8 +224,12 @@ func (n *Layer) uploadPart(ctx context.Context, multipartInfo *data.MultipartInf
        }
 
        decSize := p.Size
+       md5Hash := md5.New()
        if p.Info.Encryption.Enabled() {
-               r, encSize, err := encryptionReader(p.Reader, p.Size, p.Info.Encryption.Key())
+               rr := wrapReader(p.Reader, 64*1024, func(buf []byte) {
+                       md5Hash.Write(buf)
+               })
+               r, encSize, err := encryptionReader(rr, p.Size, p.Info.Encryption.Key())
                if err != nil {
                        return nil, fmt.Errorf("failed to create ecnrypted reader: %w", err)
                }
@@ -246,7 +250,13 @@ func (n *Layer) uploadPart(ctx context.Context, multipartInfo *data.MultipartInf
                if err != nil {
                        return nil, apierr.GetAPIError(apierr.ErrInvalidDigest)
                }
-               if hex.EncodeToString(hashBytes) != hex.EncodeToString(createdObj.MD5Sum) {
+
+               match := hex.EncodeToString(hashBytes) == hex.EncodeToString(createdObj.MD5Sum)
+               if p.Info.Encryption.Enabled() {
+                       match = hex.EncodeToString(hashBytes) == hex.EncodeToString(md5Hash.Sum(nil))
+               }
+
+               if !match {
                        prm := frostfs.PrmObjectDelete{
                                Object:    createdObj.ID,
                                Container: bktInfo.CID,

Steps to Reproduce (for bugs)

Run the following tests:

Additional

In order to these tests pass we also need return correct size of multipart object. We store encrypted size to tree

multipartObjetSize = encMultipartObjectSize


Size: &multipartObjetSize,


newVersion.Size = *p.Size

I'm not sure which approach we should use:

  • Store to tree object user size
  • Set correct size during listing

Your Environment

  • Version used: v0.31.0-rc.5
<!--- Provide a general summary of the issue in the Title above --> ## Expected Behavior Calculating md5 of original pyaload ## Current Behavior Calculating md5 of encrypted payload ## Possible Solution Something like that: ```diff diff --git a/api/layer/multipart_upload.go b/api/layer/multipart_upload.go index ed7612c9..e86598b0 100644 --- a/api/layer/multipart_upload.go +++ b/api/layer/multipart_upload.go @@ -224,8 +224,12 @@ func (n *Layer) uploadPart(ctx context.Context, multipartInfo *data.MultipartInf } decSize := p.Size + md5Hash := md5.New() if p.Info.Encryption.Enabled() { - r, encSize, err := encryptionReader(p.Reader, p.Size, p.Info.Encryption.Key()) + rr := wrapReader(p.Reader, 64*1024, func(buf []byte) { + md5Hash.Write(buf) + }) + r, encSize, err := encryptionReader(rr, p.Size, p.Info.Encryption.Key()) if err != nil { return nil, fmt.Errorf("failed to create ecnrypted reader: %w", err) } @@ -246,7 +250,13 @@ func (n *Layer) uploadPart(ctx context.Context, multipartInfo *data.MultipartInf if err != nil { return nil, apierr.GetAPIError(apierr.ErrInvalidDigest) } - if hex.EncodeToString(hashBytes) != hex.EncodeToString(createdObj.MD5Sum) { + + match := hex.EncodeToString(hashBytes) == hex.EncodeToString(createdObj.MD5Sum) + if p.Info.Encryption.Enabled() { + match = hex.EncodeToString(hashBytes) == hex.EncodeToString(md5Hash.Sum(nil)) + } + + if !match { prm := frostfs.PrmObjectDelete{ Object: createdObj.ID, Container: bktInfo.CID, ``` ## Steps to Reproduce (for bugs) Run the following tests: * [test_encryption_sse_c_multipart_upload](https://git.frostfs.info/TrueCloudLab/s3-tests/src/commit/d245097771698774d36a0283e9ef69bf67f3ba76/s3tests_boto3/functional/test_s3.py#L9958) * [test_encryption_sse_c_unaligned_multipart_upload](https://git.frostfs.info/TrueCloudLab/s3-tests/src/commit/d245097771698774d36a0283e9ef69bf67f3ba76/s3tests_boto3/functional/test_s3.py#L10004) * [test_encryption_sse_c_multipart_bad_download](https://git.frostfs.info/TrueCloudLab/s3-tests/src/commit/d245097771698774d36a0283e9ef69bf67f3ba76/s3tests_boto3/functional/test_s3.py#L10105) ## Additional In order to these tests pass we also need return correct size of multipart object. We store encrypted size to tree https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/src/commit/17d40245de5c1035867266e089f6d9e54234ad79/api/layer/multipart_upload.go#L452 https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/src/commit/17d40245de5c1035867266e089f6d9e54234ad79/api/layer/multipart_upload.go#L465 https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/src/commit/17d40245de5c1035867266e089f6d9e54234ad79/api/layer/object.go#L339 I'm not sure which approach we should use: * Store to tree object user size * Set correct size during listing ## Your Environment <!--- Include as many relevant details about the environment you experienced the bug in --> * Version used: v0.31.0-rc.5
dkirillov added the
bug
label 2024-11-07 11:15:41 +00:00
pogpp self-assigned this 2024-11-13 12:16:54 +00:00
alexvanin added this to the v0.31.0 milestone 2024-11-20 08:13:51 +00:00
alexvanin modified the milestone from v0.31.0 to v0.31.1 2024-11-20 12:48:08 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-s3-gw#543
No description provided.