[jira] [Created] (HDDS-17177) OM renew delegation token apply can write a canceled token back and can lower a token's stored expiry

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

Siyao Meng created HDDS-17177:
---------------------------------

             Summary: OM renew delegation token apply can write a canceled token back and can lower a token's stored expiry
                 Key: HDDS-17177
                 URL: https://issues.apache.org/jira/browse/HDDS-17177
             Project: Apache Ozone
          Issue Type: Bug
            Reporter: Siyao Meng
         Attachments: MC-1-reject-renew-of-removed-token-and-keep-later-expiry.patch, TestBugMC1RenewAfterCancel.java, TestBugMC2RenewLowersExpiry.java

h3. Mechanism
{{OMRenewDelegationTokenRequest.preExecute}} validates the token on the leader ({{OzoneDelegationTokenSecretManager.renewToken}}: the token is in the in memory token map, is not expired, and the caller is its renewer) and carries the computed expiry in the request. {{OMRenewDelegationTokenRequest.validateAndUpdateCache}} then, on every OM, calls {{OzoneDelegationTokenSecretManager.updateRenewToken}}, which puts the token into the in memory map unconditionally, then adds a {{dTokenTable}} cache entry with that expiry, and {{OMRenewDelegationTokenResponse}} writes the row to RocksDB. The apply neither checks that the token is still stored nor compares the new expiry with the stored one. Two consequences:
* If a cancel of the token was applied before the renew, the renew apply writes the token back into the map, the cache and RocksDB. {{OMCancelDelegationTokenRequest}} had removed it from all three and returned success.
* If a renew carrying an earlier expiry is applied after a renew carrying a later one, the stored expiry goes down to the earlier value, below what the first applied renew returned.

h3. Trigger
A renew whose validation on the leader passed is applied when the stored state is no longer the one it was validated against: the token's cancel has been applied since, or a renew with a later expiry has been applied since. Validation in preExecute and the apply are separate steps, and nothing at apply time checks the validated state again. The expiry in the request is computed by the leader, clients cannot set it.

h3. Impact
* *Canceled token written back.* {{retrievePassword}}, which the OM RPC server uses to authenticate token connections, accepts the canceled token again. The row is in RocksDB, and a secret manager started on the same DB, as after an OM restart, loads the token and accepts it. Every OM applies the same entries through the same code, so all of them hold it (argued, one OM was run). The token stays valid until its stored expiry and its renewer can renew it again up to its max date (7 days by default). The client that canceled it was told that the cancel succeeded. [^TestBugMC1RenewAfterCancel.java]
* *Stored expiry lowered.* Once the earlier expiry passes, {{retrievePassword}} rejects the token and {{renewToken}} refuses to renew it, although a later expiry was returned for it, and {{ExpiredTokenRemover}} deletes it on its next scan. This fails closed. The shortfall equals the gap between the leader's validations of the two renews. [^TestBugMC2RenewLowersExpiry.java]

h3. Reproduction
The two attached classes run in {{hadoop-ozone/ozone-manager}} (target path and Maven command are in each class comment) and pass on unmodified source while the bug is present. They use the real {{OzoneDelegationTokenSecretManager}}, the real {{validateAndUpdateCache}} of the cancel and renew requests and a real {{OmMetadataManagerImpl}} with RocksDB, and call them in sequence: the leader side validation ({{renewToken}}, {{cancelToken}}), then the applies in the stated sequence, each response committed to RocksDB as the double buffer does. No Ratis server and no second OM are involved.

h3. Patch
[^MC-1-reject-renew-of-removed-token-and-keep-later-expiry.patch], against b94df213f20ec0d675f079b57a4566e4e64e4060. It also applies to master at df42f7f60a2.
* {{OzoneDelegationTokenSecretManager}} records the identifiers removed by applied cancels ({{removeToken}}, whose only caller is the cancel apply) and drops each one once the token's max date has passed, since no renew can extend a token beyond it. The renew apply rejects a token found there with {{TOKEN_ERROR_OTHER}}, so no row is written and the map and cache are left alone. The check deliberately does not look at {{dTokenTable}} or the in memory map: {{ExpiredTokenRemover}} deletes from both outside Ratis on each OM's own clock (HDDS-13457), on a restarted OM even before it replays its log, so a check there could accept a renew on one OM and reject it on another, and lose a valid token on the OM that catches up.
* The renew apply stores {{min(maxDate, max(new expiry, stored expiry))}} when a row exists, otherwise the request's expiry as before. The cap keeps today's result when the stored expiry is beyond max date (renew interval longer than the max lifetime).

The record of applied cancels is kept in memory only. An OM that restarts, or installs a checkpoint, between applying a cancel and applying a later renew of the same token still writes that token back, as every OM does today. A stricter alternative is a persisted cancel tombstone in {{dTokenTable}} that is rebuilt into the record when tokens are loaded; it changes what a cancel writes, may need a layout feature, and still meets rows that {{ExpiredTokenRemover}} deletes outside Ratis.

Covered by four cases in the existing {{TestOzoneDelegationTokenSecretManager}}: {{testRenewAppliedAfterCancelDoesNotRestoreToken}}, {{testRenewAppliedWithEarlierExpiryKeepsStoredExpiry}}, {{testRenewAppliedAfterRemovalOutsideRatisIsApplied}} (a renew applied after the remover deleted the token must still be applied, as on the other OMs) and {{testRenewAppliedCapsExpiryAtMaxDate}}. Without the change the first two fail ("expected: TOKEN_ERROR_OTHER but was: OK", and the earlier expiry stored instead of the later one). With it, 146 tests in the delegation token, request handler, state machine and double buffer suites pass, checkstyle is clean, and the two attached classes no longer show the bug. The Kerberos integration suites were not run with the change.

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. Related but not duplicates: HDDS-13457 (the expired token remover runs on every OM outside Ratis), HDDS-1973 and HDDS-1974 (moved renew and cancel to the cache and double buffer). 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