Check `X-Amz-Content-Sha256` header

alexvanin commented

2023-09-28 11:55:52 +00:00

Owner

It is possible to pass invalid X-Amz-Content-Sha256 as in Mint's test here.

Describe the solution you'd like

Support signed payloads as it described in https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html, look for Signed payload option.

## Is your feature request related to a problem? Please describe. It is possible to pass invalid `X-Amz-Content-Sha256` as in Mint's test [here](https://github.com/minio/mint/blob/2da412344b07863771e7e2f0aeceaedb835bf9b8/run/core/aws-sdk-go/main.go#L205). ## Describe the solution you'd like Support signed payloads as it described in https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html, look for `Signed payload option`.

alexvanin added the

discussion

label 2023-09-28 11:55:52 +00:00

r.loginov self-assigned this 2023-10-31 16:01:37 +00:00

r.loginov commented

2023-11-02 06:52:47 +00:00

Member

If we want to support signed payloads, we must take into account that this can reduce performance, especially if the file size is quite large. This may happen due to the fact that we will have to read the payload twice (once to calculate the checksum of the payload for signing and subsequent verification, and once for transmission) or buffer them in memory.

fyrchik commented

2023-11-02 07:36:53 +00:00

Owner

frostfs-node already validates payload hash, which is also sha256, so we will receive an error.
2 things to think about:

For big objects the error will be received after the last part of the object is put. It may be ok, but we need to have this in mind (we cannot validate until we have received all payload anyway).
For small objects (and, possibly, big ones too) we can set this hash in the object transformer from SDK to avoid hash calculations on the client: this can provide performance improvements for the client-split scenario.

frostfs-node already validates payload hash, which is also sha256, so we will receive an error. 2 things to think about: 1. For big objects the error will be received after the last part of the object is put. It may be ok, but we need to have this in mind (we cannot validate until we have received all payload anyway). 2. For small objects (and, possibly, big ones too) we can set this hash in the object transformer from SDK to avoid hash calculations on the client: this can provide performance improvements for the client-split scenario.

r.loginov commented

2023-11-08 09:14:13 +00:00

Member

As mentioned above, frost's-node already contains validation of the payload hash. However, if we want to use this, then there are several problems:

Client cut:
When using client cut, the payload hash is calculated in transformer from SDK . If we receive a request with a pre-signed URL, where the signed payload will be invalid (for example, as in mint test). Then when requesting a gate to put an object (with an invalid hash to the payload), the invalid hash will be overwritten in the SDK transformer and as a result we will not receive a hash validation error from frostfs-node.

Server cut:
When using server cut, we also won't be able to get an error from the frosts-node hash validator, since it won't execute.
@alexvanin @dkirillov

As mentioned above, frost's-node already contains validation of the payload hash. However, if we want to use this, then there are several problems: **Client cut**: When using client cut, the payload hash is calculated in [transformer from SDK](https://git.frostfs.info/TrueCloudLab/frostfs-sdk-go/src/branch/master/object/transformer/hasher.go#L26) . If we receive a request with a pre-signed URL, where the signed payload will be invalid (for example, as in [mint test](https://github.com/minio/mint/blob/2da412344b07863771e7e2f0aeceaedb835bf9b8/run/core/aws-sdk-go/main.go#L205)). Then when requesting a gate to put an object (with an invalid hash to the payload), the invalid hash will be overwritten in the SDK transformer and as a result we will not receive a hash validation error from frostfs-node. **Server cut:** When using server cut, we also won't be able to get an error from the frosts-node hash validator, since it won't execute. @alexvanin @dkirillov

alexvanin added this to the v0.29.0 milestone 2023-11-09 10:24:53 +00:00

alexvanin commented

2023-11-09 10:30:20 +00:00

Author

Owner

As we discussed with @r.loginov , @fyrchik , @dkirillov, in this issue we won't use any optimizations from storage side, because they require some fixes in transformer (see previous comment) and maybe some tweaks on storage side for server-side cutting. Also, these optimization work for non-split objects only (with the size of < than MaxObjectSize in the network configuration).

In this issue we will not cover all use-cases for X-Amz-Content-Sha256 checking. It requires different approaches for encrypted and non-encrypted payload. So in this issue:

check X-Amz-Content-Sha256 format before accepting the request
check value of X-Amz-Content-Sha256 for non-encrypted payload. Hash is calculated during object put operation. If value is not the same, then do a rollback by removing upload object. See md5 hashsum check rollback here.

As we discussed with @r.loginov , @fyrchik , @dkirillov, in this issue we won't use any optimizations from storage side, because they require some fixes in transformer (see previous comment) and maybe some tweaks on storage side for server-side cutting. Also, these optimization work for non-split objects only (with the size of < than MaxObjectSize in the network configuration). In this issue we *will not* cover all use-cases for `X-Amz-Content-Sha256` checking. It requires different approaches for encrypted and non-encrypted payload. So in this issue: 1) check `X-Amz-Content-Sha256` format before accepting the request 2) check value of `X-Amz-Content-Sha256` for non-encrypted payload. Hash is calculated during object put operation. If value is not the same, then do a rollback by removing upload object. See md5 hashsum check rollback [here](https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/src/commit/5ee73fad6ab799d936c5f0fa1faf00d3cb0cc63d/api/layer/object.go#L311).

alexvanin removed the

discussion

label 2023-11-09 10:30:51 +00:00

~~r.loginov referenced this issue 2023-11-13 08:17:39 +00:00~~

[#218] Add check content sha256 header #264

r.loginov referenced this issue from a pull request that will close it,

2023-11-14 06:53:38 +00:00

[#218] Add check content sha256 header #264

alexvanin closed this issue

2023-11-22 11:33:55 +00:00

alexvanin referenced this issue from a commit

2023-11-22 11:33:56 +00:00