[jira] [Updated] (HDDS-17175) Expired delegation token remover can drop a renew applied during its scan, so the OM rejects the token before its renewed expiry

["Siyao Meng (Jira)" <ji...@apache.org>]

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

Siyao Meng updated HDDS-17175:
------------------------------
    Attachment: MC-3-remove-only-checked-entry-in-expired-token-remover.patch

> Expired delegation token remover can drop a renew applied during its scan, so the OM rejects the token before its renewed expiry
> --------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-17175
>                 URL: https://issues.apache.org/jira/browse/HDDS-17175
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Minor
>         Attachments: MC-3-remove-only-checked-entry-in-expired-token-remover.patch, TestBugMC3RemoverDropsConcurrentRenew.java
>
>
> h3. Mechanism
> {{OzoneDelegationTokenSecretManager.removeExpiredToken}} runs on its own thread on every OM. For each entry of the in-memory token map it reads the renew date, and if that date has passed it calls {{Iterator.remove()}} and then deletes the row from {{delegationTokenTable}} directly in RocksDB, outside Ratis and the double buffer. On a {{ConcurrentHashMap}} the iterator's remove deletes the key whatever value it holds by then. The renew apply ({{OMRenewDelegationTokenRequest.validateAndUpdateCache}}, which calls {{updateRenewToken}}) shares no lock with the remover, so a renew applied between the check and the remove is dropped from memory. Its DB row survives only if the renew's flush lands after the remover's delete.
> h3. Trigger
> # A client renews a token shortly before its renew date. The leader's {{renewToken}} check passes and the client gets a new expiry.
> # The renew is applied after the old renew date, while that OM's expired token remover is checking this token.
> No fault is needed, only timing. The window is the gap between reading the entry and removing it within one scan (microseconds per token per scan; the scan runs every {{ozone.manager.delegation.remover.scan.interval}}, 1 hour by default). Only a renew applied after the token's old renew date is exposed, so renewals made well ahead of the renew date never hit it. I did not measure how often it happens in a real cluster.
> h3. Impact
> * On that OM, before the acknowledged expiry, {{retrievePassword}} (token authentication) and {{renewToken}} fail with "token ... can't be found in cache", and {{cancelToken}} fails with "Token not found in-memory map of tokens". The in-memory map is only rebuilt when the secret manager is created (OM start, or installing a checkpoint from the leader), and the client cannot recover by renewing again.
> * If the renew's flush landed after the remover's delete, the DB row holds the new expiry and a restart of that OM brings the token back. If it landed before, the row is gone too and the token stays lost on that OM until it would have expired, or until that OM installs a checkpoint from an OM that kept the row.
> * Each OM runs its own remover, so the OMs can disagree about the token. A client can get a new token; no other data is affected.
> h3. Reproduction
> PASS, deterministic, unmodified source. [^TestBugMC3RemoverDropsConcurrentRenew.java] ({{hadoop-ozone/ozone-manager}}) drives one {{OzoneDelegationTokenSecretManager}} with its real remover thread and calls the methods the request path calls for preExecute ({{createToken}}, {{renewToken}}), apply ({{updateToken}}, {{updateRenewToken}}) and flush (a {{delegationTokenTable}} put). The renew is accepted 300 ms before the old renew date. A test {{TokenInfo}} in the token map holds the remover thread inside {{getRenewDate()}}, on its first read after the old renew date, until the renew has been applied. The first test flushes the renew before releasing the remover: {{retrievePassword}} is rejected about 1.2 s before the acknowledged expiry, the DB row is gone and a restart does not help. The second flushes it after the remover finished: the token is rejected the same way, the DB row holds the new expiry and a restart brings the token back. A passing test means the defect is present. Ratis and the double buffer are not running in the test; their calls are made directly, and no multi OM cluster was run.
> h3. Patch
> [^MC-3-remove-only-checked-entry-in-expired-token-remover.patch], against b94df213f20ec0d675f079b57a4566e4e64e4060. It also applies to master at df42f7f60a2.
> The remover now removes an expired entry with {{currentTokens.computeIfPresent}}: if the map still holds the {{TokenInfo}} whose renew date it checked, it deletes the DB row and removes the entry inside that call, otherwise it keeps the entry. A renew applied after the check has replaced the value, so the token and its row are left to the renew. A renew applied while the remover is deleting the row waits for the map entry ({{updateRenewToken}} puts the same key), so its flush always lands after the delete and the row is kept. Behaviour without a concurrent renew is unchanged. The cost is one RocksDB delete per expired token done while holding that map bin; reads of the map do not wait, and only writes to keys in the same bin wait for it.
> Covered by two tests in the existing {{TestOzoneDelegationTokenSecretManager}}: {{testExpiredTokenRemoverKeepsConcurrentRenew}} (renew applied between the check and the removal; also checks that an expired token that is not renewed is still removed from memory and DB) and {{testExpiredTokenRemoverKeepsRenewAppliedDuringRowDelete}} (renew applied and flushed while the remover is inside its row delete). Without the change the first fails with "can't be found in cache" and the second finds the row deleted. With it {{TestOzoneDelegationTokenSecretManager}}, {{TestOMGetDelegationTokenRequest}}, {{TestOMGetDelegationTokenResponse}} and {{TestS3SecurityUtil}} (38 tests) pass and checkstyle is clean. With the patch both tests of the reproduction fail, because the token stays valid. The secure mini cluster suites were not run.
> The remover still deletes rows outside Ratis, so the OMs still remove expired tokens at different times; moving that to the leader through Ratis is HDDS-13457 and is not needed for this fix.
> Found by TLA+ model checking and code review of the OM delegation token secret manager and request paths under HDDS-15926, on commit b94df213f20ec0d675f079b57a4566e4e64e4060. Checked against HDDS issues and apache/ozone pull requests for duplicates before filing (HDDS-13457, and its duplicate HDDS-13450, cover the uncoordinated removal, not a lost renew). 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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org