HDDS-13668. Support S3 signed single chunk payload verification

[hevinhsu]

What changes were proposed in this pull request?

Add S3 request body SHA-256 verification to support single-chunk payload validation.

Please describe your PR in detail:

• Compute the SHA-256 hash of the request body and verify it matches the x-amz-content-sha256 header.
• Validate x-amz-content-sha256 for GET requests when the header is present.
• Add unit tests, integration tests, and end-to-end tests.
  • Introduce a new Python helper function to generate PUT presigned URLs, since the AWS CLI only supports generating GET presigned URLs.

What is the link to the Apache JIRA

HDDS-13668

How was this patch tested?

https://github.com/hevinhsu/ozone/actions/runs/19321794081

This patch also tested using the MinIO Mint S3 compatibility suite, focusing only on the test cases that previously failed in HDDS-12871.

After applying this patch:

• aws-sdk-go: Now passes after this update. The previous unmarshalling xml failed message (introduced on 2025/Oct/03) no longer appears.
• aws-sdk-ruby: This test case still fails. See comment below for detailed analysis.

[hevinhsu]

Investigation: why aws-sdk-ruby test still fails

In the AWS documentation — for example, IAM, S3 query parameter and S3 Signature Calculations for the Authorization Header — it is stated that all x-amz-* headers must be included in the signature calculation.

However, in the Mint Ruby testcase,it appears that the x-amz-acl header is not added to the signature calculation. As a result, the test currently fails.

In the MinIO implementation, only the x-amz-meta-* headers are validated to exist in the signedHeaders. Therefore, the Ruby test — which does not include x-amz-acl in the signed headers — can still pass successfully.

[hevinhsu]

During the implementation process, I noticed that Ozone doesn’t seem to validate mismatched values between signed headers and signed query parameters. According to the AWS spec:

If you add a signed header that is also a signed query parameter, and they differ in value, you will receive an InvalidRequest error as the input is conflicting.

To clarify my understanding: Ozone currently relies only on the query parameters for signature validation and does not compare them against the corresponding request headers. Because of this, a mismatch would never trigger an InvalidRequest error in the current implementation.

Please let me know if I’m missing something.

[ivandika3]

During the implementation process, I noticed that Ozone doesn’t seem to validate mismatched values between signed headers and signed query parameters.

Could you give some examples here? Currently I think we only check that all the headers in X-Amz-SignedHeaders should exist in the request headers. Regarding the actual HTTP header value, I think it should be calculated based on the AWS4 signature?

[hevinhsu]

Could you give some examples here? Currently I think we only check that all the headers in X-Amz-SignedHeaders should exist in the request headers. Regarding the actual HTTP header value, I think it should be calculated based on the AWS4 signature?

Hi @ivandika3, I added a test (link) to check if requests with conflicting X-Amz-Security-Token in both header and query are properly rejected. According to the S3 spec, this should fail, but the request currently succeeds. Please let me know if you have any suggestions!

[swamirishi]

@yandrey321 Do you wanna review this change?

[hevinhsu]

Hi @ivandika3,

I tried a new implementation that introduces a runnable preCommit interface to perform validation before commitKey.
This approach avoids adding S3-specific logic into client.io, so the validation can be set from ObjectEndpoint via setPreCommit.

However, the current implementation feels suboptimal. To avoid changing existing method signatures, I throw IllegalArgumentException when validation fails, and then transform that exception into an O3SException in ObjectEndpoint if the error message matches.

I have a couple of questions:

• Is this design acceptable (i.e., introducing preCommit in KeyOutputStream and exposing it via a setter)?
• Converting IllegalArgumentException into O3SException feels like an awkward way to handle the error — is there a better approach you’d recommend?

PTAL when you have time, thanks.

[hevinhsu]

Thanks @ivandika3 for the feedback. I’ll switch to using a list of preCommit handlers instead and address the remaining follow-up items over the weekend, as I’m a bit busy recently. Sorry for the delay.

we might be able to use Ratis CheckedRunnable<IOException> as the preCommit (similar to the Validator interface in XceiverClientSpi)? Afterwards, we can simply throw OS3Exception that will be automatically handled by OS3ExceptionMapper?

Sorry, I may have misunderstood this point.

I did try a similar approach while exploring the solution, but I ran into a limitation: OS3Exception does not extend IOException, so it cannot be thrown directly from a CheckedRunnable<IOException>. It’s possible that I’m not applying this pattern in the intended way.

I also considered introducing a new IOException wrapper to carry an OS3Exception, but that felt somewhat over-engineered, so I decided not to go down that path for now.

I’d appreciate any clarification on how you envisioned using CheckedRunnable<IOException> here.

[ivandika3]

@hevinhsu Yes, you are correct. Since OS3Exception doesn't extend IOException, it's not feasible to use it without doing any extra logic (e.g. wrapping logic you mentioned). Let me think about it.

I wonder whether there will be any effects if we simply change OS3Exception to extends IOException instead, we can then simply throw OS3Exception in preCommit.

[hevinhsu]

@ivandika3 Got it — I’ll give this approach a try and evaluate the impact.

This was actually my initial thought when exploring possible solutions. However, after some consideration, I hesitated because I wasn’t fully convinced that OS3Exception semantically fits as an IOException. It represents an S3-level error rather than a pure I/O failure, so extending IOException felt slightly questionable from a modeling perspective, which is why I initially dropped this approach.

That said, given the benefit of simplifying the preCommit flow and avoiding extra wrapping logic, I’m happy to revisit this direction and see how it behaves in practice.

[hevinhsu]

Hi @ivandika3,

I’ve updated this PR based on your feedback.

Separately, I also experimented with making O3SException extend IOException, so it can be thrown directly from preCommit.
Initially, this change caused some tests to fail, because there are existing error-handling paths that broadly catch IOException, for example:
https://github.com/apache/ozone/blob/41dd1509c76ff0a16fe6bda00fdce16cea217996/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java#L508

After adding this additional handling logic:
https://github.com/hevinhsu/ozone/blob/3d749523500d076ec9f794eb8f57697eabefb62c/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java#L509-L511
the tests passed again.(CI)

Due to this behavioral impact and the need to adjust existing exception-handling logic, I did not include the O3SException extends IOException change in this PR.
I think this direction is still possible, but it likely requires a more careful review of the current exception-handling paths before proceeding.