[ 
https://issues.apache.org/jira/browse/HDDS-17549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-17549:
------------------------------
    Affects Version/s: 2.2.1
                       2.0.0
                       1.4.0
                       2.1.2
                       2.1.0
                       1.4.1
                       2.1.1
                       1.3.0
                       2.2.0

> Without security, an OM follower runs the preExecute of S3 writes before 
> rejecting them, so a lagging follower can return an error, not NotLeader
> -------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-17549
>                 URL: https://issues.apache.org/jira/browse/HDDS-17549
>             Project: Apache Ozone
>          Issue Type: Bug
>    Affects Versions: 1.3.0, 1.4.0, 2.0.0, 1.4.1, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 
> 2.2.1
>            Reporter: Siyao Meng
>            Priority: Minor
>         Attachments: CR-4-check-leader-for-s3-writes-without-security.patch, 
> TestBugCR4S3WriteOnFollowerSkipsLeaderCheck.java
>
>
> h3. Mechanism
> {{OzoneManagerProtocolServerSideTranslatorPB.internalProcessRequest}} calls 
> {{OzoneManagerRatisUtils.checkLeaderStatus}} for a write only when the 
> request has no {{S3Authentication}}, with the comment that the leader status 
> was already verified while validating the credentials. That holds only when 
> security is enabled: {{S3SecurityUtil.validateS3Credential}} checks the 
> leader on both of its paths (the STS path directly, the other one through 
> {{OzoneDelegationTokenSecretManager.retrievePassword}}), but it does nothing 
> at all when security is disabled. The S3 Gateway sets {{S3Authentication}} on 
> every request ({{EndpointBase.initialization}}), so on a cluster without 
> security a write from the S3 Gateway that reaches a follower goes straight to 
> {{OMExecutionFlow.submit}}. The follower creates the client request and runs 
> its {{preExecute}} against its own state, and only the Ratis submit that 
> follows rejects it as not leader.
> HDDS-5885 skipped the second check because the translator itself then called 
> {{checkLeaderStatus}} for every request with {{S3Authentication}}. HDDS-6686 
> moved that check into {{retrievePassword}}, which only runs with security 
> enabled, and left the skip in place.
> h3. Trigger
> A cluster without security, OM HA, and an S3 Gateway write that is the first 
> call of the S3 Gateway's OM client to reach a follower. Most S3 requests do 
> not get there: every S3 Gateway write endpoint first sends 
> {{GetS3VolumeContext}} and {{InfoBucket}} through the same OM proxy, and with 
> the default configuration (no follower reads) a follower rejects those reads 
> with NotLeader, so the client is redirected before it writes. A write reaches 
> a follower first when the leader changes between those reads and the write of 
> one S3 request, or when an upload spans a leader change ({{AllocateBlock}} 
> and {{CommitKey}} are sent mid stream with no read before them). Both the 
> default Hadoop RPC transport and the gRPC transport reach the same translator 
> method. Secure clusters are not affected.
> h3. Impact
> * If {{preExecute}} succeeds, Ratis on the follower rejects the request with 
> NotLeader, the client fails over and the leader runs the request again. The 
> cost is the wasted work on the follower, and for {{CreateKey}} (with data 
> size) and {{AllocateBlock}} a block ID allocated in SCM that is never 
> written. SCM keeps no state for it ({{BlockManagerImpl.newBlock}} only takes 
> a local ID from the sequence generator in an open container and bumps a 
> pipeline metric, and container usage comes from datanode reports), so nothing 
> needs cleaning up; the cost is the wasted call.
> * If {{preExecute}} fails because the follower has not applied recent changes 
> yet, the follower returns an ordinary error response, not NotLeader, so the 
> client does not retry on another OM and the error reaches the S3 client. In 
> the reproduction a {{CreateKey}} for a bucket the leader had already created 
> returned {{BUCKET_NOT_FOUND}}. Through the S3 Gateway this needs both a write 
> that reaches the follower first (see Trigger) and a bucket created moments 
> before, which the follower has not applied yet. Other write requests whose 
> {{preExecute}} looks up the volume or bucket (for example {{AllocateBlock}}, 
> or bucket link resolution in {{resolveBucketAndCheckKeyAcls}}) can fail the 
> same way (argued, not run).
> * No data loss and no state divergence: {{preExecute}} does not write to the 
> OM DB, and the request is never applied unless it is submitted through the 
> leader. Without security the request identity is not authenticated anyway, so 
> no access check is weakened.
> h3. Reproduction
> PASS, deterministic, unmodified source. 
> [^TestBugCR4S3WriteOnFollowerSkipsLeaderCheck.java] 
> ({{hadoop-ozone/integration-test}}) starts a mini cluster without security, 
> with three OMs and one datanode, and hands {{CreateKey}} requests to a 
> follower's {{OzoneManagerProtocolServerSideTranslatorPB}}, the method both 
> RPC servers call. It shows, on the same follower:
> # a {{CreateKey}} without {{S3Authentication}} is rejected with 
> {{OMNotLeaderException}} and the SCM block allocation count does not change;
> # the same request with {{S3Authentication}} also ends in 
> {{OMNotLeaderException}}, but only after the SCM block allocation count went 
> up;
> # with the follower's apply of one {{CreateBucket}} held (its state machine 
> handler is wrapped for this, the only step that is not a request), a 
> {{CreateKey}} with {{S3Authentication}} for that bucket returns 
> {{BUCKET_NOT_FOUND}} although the leader has the bucket.
> A passing test means the defect is present. The S3 Gateway itself and the 
> client's retry were not run; the client side behaviour above follows from the 
> response types.
> h3. Patch
> [^CR-4-check-leader-for-s3-writes-without-security.patch], against 
> efe0c7eba1527530d1001f6517b733576fdc6d61 (current master).
> The translator now skips its own leader check only when the request has 
> {{S3Authentication}} and security is enabled, which is exactly when 
> {{validateS3Credential}} has done the check. Secure clusters still check 
> once, and reads are unchanged, so follower reads for S3 requests on clusters 
> without security keep working. The simpler alternative is to drop the skip 
> and always check the leader for writes, at the cost of a second check on 
> secure S3 writes.
> The fix uses {{isSecurityEnabled()}} as the signal that the leader was 
> already checked. Related: HDDS-15617 has the same root 
> ({{validateS3Credential}} does nothing without security) and proposes 
> validating S3 signatures on clusters without security; if that is done 
> through {{retrievePassword}}, it adds a leader check there, and this 
> condition should be revisited with it (it would also force S3 reads on such 
> clusters to the leader).
> Covered by two new parameterized tests in {{TestS3DerivedKey}}, without 
> security and with security for both an STS request and an ordinary S3 secret: 
> a write on a follower is rejected with NotLeader before the retry cache 
> lookup and the submit, and on the leader the leader is checked exactly once. 
> For the ordinary secret the token manager is a mock that checks the leader 
> first, as {{retrievePassword}} does. Without the change both tests fail in 
> the cases without security; a variant that always checks fails the "exactly 
> once" test in both secure cases. With the patch the second step of the 
> reproduction fails, because the follower rejects the request before 
> allocating a block.
> Found by code review of the OM follower read and read consistency routing 
> paths, as part of the TLA+ verification effort under HDDS-15926, on commit 
> efe0c7eba1527530d1001f6517b733576fdc6d61. 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
(v10.3.26#10030026)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to