[
https://issues.apache.org/jira/browse/HDDS-17273?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17273:
------------------------------
Attachment: MC-5-commit-s3-write-only-after-transfer-complete.patch
> S3 PUT or UploadPart that fails with an Error after the body is copied still
> commits, skipping the Content-MD5 and x-amz-content-sha256 checks
> ----------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17273
> URL: https://issues.apache.org/jira/browse/HDDS-17273
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.2.0, 2.2.1
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: MC-5-commit-s3-write-only-after-transfer-complete.patch,
> TestBugMC5ErrorAfterBodyCopyCommits.java
>
>
> h3. Mechanism
> The S3 write paths commit the key in {{close()}} of a try-with-resources
> {{S3ObjectWriteGuard}}. The guard's pre-commit blocks the commit only if
> {{copyFrom}} recorded an {{IOException}} or {{RuntimeException}}, or if the
> length does not match. The Content-MD5 and x-amz-content-sha256 checks, the
> ETag and, for parts, {{requirePartETag}} are added only after {{copyFrom}}
> returns.
> So anything thrown after the last body byte was copied leaves the commit
> open: an {{Error}} from {{SignedChunksInputStream.verifyComplete}} (the final
> chunk check inside {{copyFrom}}), or any throwable in the endpoint code that
> follows (digest, ETag, Content-MD5 header lookup, hook registration).
> {{close()}} then commits the content without the digest checks, and without
> an ETag if the throwable came before the ETag was put. An Error during the
> copy loop itself is blocked by the length check. In the current code no
> {{RuntimeException}} can occur in that window, so the realistic trigger is
> {{OutOfMemoryError}}.
> The same pattern is in PutObject, UploadPart, UploadPartCopy and CopyObject
> in {{ObjectEndpoint}} and in the datastream put, copy and part upload in
> {{ObjectEndpointStreaming}}.
> h3. Trigger
> # A client PUTs an object (or part) whose body does not match its Content-MD5
> or x-amz-content-sha256.
> # The gateway hits an {{OutOfMemoryError}} after the whole body has been read
> and before the digest checks are added.
> The reproduction injects the Error through the mocked request; a real out of
> memory condition at that point was not produced.
> h3. Impact
> The client gets an error, but content that its own digest would have rejected
> replaces the previous object (or becomes the part), in the signed chunked
> case without an ETag. A retry with the correct body overwrites it, except
> with {{If-None-Match: *}}. With a body that matches its digest the effect is
> an error response for a write that was committed, in some paths without an
> ETag (the signed chunked PUT, and UploadPart, where the Content-MD5 lookup
> comes before the ETag is set). Committing unchecked content needs two
> independent faults at once, so it is unlikely.
> h3. Reproduction
> [^TestBugMC5ErrorAfterBodyCopyCommits.java] ({{hadoop-ozone/s3gateway}}) runs
> the real {{ObjectEndpoint}} and {{S3ObjectWriteGuard}} against the gateway's
> {{OzoneClientStub}}. Every request carries a digest that does not match the
> body. Without an Error such a request is rejected and the previous object
> stays. With an {{OutOfMemoryError}} from the Content-MD5 header lookup (done
> after the copy), or from the request body when the final chunk of a signed
> chunked upload is read, the PUT replaces the previous object with the
> unchecked content, the signed PUT leaves an object without ETag, and the
> UploadPart commits the part. On the unmodified source all four tests pass,
> which means the defect is present. An UploadPart without ETag against a real
> OM follows from the same code but was not run.
> h3. Patch
> [^MC-5-commit-s3-write-only-after-transfer-complete.patch], against
> b10a7d427bdf83807a7e48a1be507d3b02d5a503. It also applies to master at
> e0de543cbe9.
> The guard commits only after the request called the new
> {{S3ObjectWriteGuard.markTransferComplete}}, which each of the seven write
> blocks does as its last statement, after all checks are added. Its existing
> pre-commit refuses the commit otherwise, so any throwable before that point
> aborts the write. Successful requests and the existing error responses are
> unchanged.
> The patch applies together with the one for HDDS-17272 without conflict.
> Whichever of the two lands second needs {{guard.markTransferComplete();}}
> before {{guard.close();}} in
> {{TestS3ObjectWriteGuard.encryptedWriteCommitsAllContent}}, the test
> HDDS-17272 adds.
> Covered by {{testPutObjectNotCommittedOnErrorAfterBodyCopy}} in the existing
> {{TestObjectPut}}, {{testPartUploadNotCommittedOnErrorAfterBodyCopy}} in
> {{TestPartUpload}} (with and without datastream) and two contract tests in
> {{TestS3ObjectWriteGuard}}. Without the change the endpoint tests fail
> because the key or part is committed. With it all s3gateway unit tests (1623)
> and the AWS SDK suites {{TestS3SDK}} and {{TestS3SDKWithRatisStreaming}} on a
> mini cluster (490) pass, and checkstyle is clean. With the patch the three
> failure cases of the reproduction fail, because nothing is committed.
> Found by TLA+ model checking and code review of the S3 gateway object put and
> key commit paths under HDDS-15926, on commit
> b10a7d427bdf83807a7e48a1be507d3b02d5a503. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. The attached patch
> is a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
