You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smengcl (via GitHub)" <gi...@apache.org> on 2026/10/20 19:28:51 UTC
[PR] HDDS-17177. OM renew delegation token apply can write a canceled token back and can lower a token's stored expiry [ozone]
smengcl opened a new pull request, #11642:
URL: https://github.com/apache/ozone/pull/11642
Generated-by: Claude Code (Opus 5.5)
## What changes were proposed in this pull request?
`OMRenewDelegationTokenRequest.preExecute` validates the token on the leader, but `validateAndUpdateCache` applies the renew on every OM without checking the state again: `updateRenewToken` puts the token back into the in memory map and the renew writes a `dTokenTable` row with the request's expiry. So:
- A renew applied after a cancel of the same token writes the canceled token back into the map, the cache and RocksDB. `retrievePassword` accepts it again, also after an OM restart, until its stored expiry, and its renewer can keep renewing it up to its max date.
- A renew carrying an earlier expiry applied after one carrying a later expiry lowers the stored expiry. This fails closed: the token is rejected earlier than the expiry returned to the client.
This PR:
- Records, in `OzoneDelegationTokenSecretManager`, the identifiers removed by applied cancels (`removeToken`, whose only caller is the cancel apply), and drops each one in the remover scan once the token's max date has passed. The renew apply rejects such a token with `TOKEN_ERROR_OTHER`. The check does not look at `dTokenTable` or the in memory map, since `ExpiredTokenRemover` deletes from both outside Ratis on each OM's own clock (HDDS-13457), so a check there could make OMs diverge.
- Stores `min(maxDate, max(new expiry, stored expiry))` in the renew apply when a row exists, otherwise the request's expiry as before.
The record of applied cancels is in memory only, so an OM that restarts or installs a checkpoint between applying a cancel and a later renew of the same token still writes the token back, as today. A persisted cancel tombstone would close that gap but changes what a cancel writes, so it is left out of this PR.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-17177
## How was this patch tested?
Added four cases to `TestOzoneDelegationTokenSecretManager`, which drive the real secret manager and the real cancel and renew `validateAndUpdateCache` against an `OmMetadataManagerImpl` with RocksDB:
- `testRenewAppliedAfterCancelDoesNotRestoreToken`
- `testRenewAppliedWithEarlierExpiryKeepsStoredExpiry`
- `testRenewAppliedAfterRemovalOutsideRatisIsApplied` (a renew applied after the remover deleted the token is still applied, as on the other OMs)
- `testRenewAppliedCapsExpiryAtMaxDate`
Without the fix the first two fail (`expected: TOKEN_ERROR_OTHER but was: OK`, and the earlier expiry is stored instead of the later one).
The issue was found by TLA+ model checking of the OM delegation token paths under HDDS-16566.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-17177. OM renew delegation token apply can write a canceled token back and can lower a token's stored expiry [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #11642:
URL: https://github.com/apache/ozone/pull/11642#issuecomment-6304471058
@jojochuang That sounds like a bigger undertaking involving a OM layout feature bump if done properly, outside the scope of this PR. Would you like to open a follow-up one?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-17177. OM renew delegation token apply can write a canceled token back and can lower a token's stored expiry [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl merged PR #11642:
URL: https://github.com/apache/ozone/pull/11642
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-17177. OM renew delegation token apply can write a canceled token back and can lower a token's stored expiry [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #11642:
URL: https://github.com/apache/ozone/pull/11642#discussion_r4339795569
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/security/OMRenewDelegationTokenRequest.java:
##########
@@ -156,6 +158,15 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
ozoneTokenIdentifier.getRenewer() == null ? "" :
ozoneTokenIdentifier.getRenewer().toString());
+ // The token was validated in preExecute, but a cancel or another renew may have been applied since then.
+ if (ozoneManager.getDelegationTokenMgr().isCanceled(ozoneTokenIdentifier)) {
+ throw new OMException("Delegation token has been canceled: " + ozoneTokenIdentifier, TOKEN_ERROR_OTHER);
+ }
+ Long storedRenewTime = omMetadataManager.getDelegationTokenTable().get(ozoneTokenIdentifier);
+ if (storedRenewTime != null) {
+ renewTime = Math.min(ozoneTokenIdentifier.getMaxDate(), Math.max(renewTime, storedRenewTime));
+ }
Review Comment:
Since we won't support ZDU to the release that this goes out in this change is ok, but under ZDU this type of change could cause divergence. If the token was cancelled before processing, the new OMs would fail the request but old OMs would still apply it.
If we needed ZDU support we would wrap this in a version check. Component versions are going to be much more common in the time frame between ZDU support and the leader execution project. That's ok since they are enum entries/feature flags which are cheap.
HDDS-17252 will be able to add comments like this to PRs automatically to make it less likely to slip through.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-17177. OM renew delegation token apply can write a canceled token back and can lower a token's stored expiry [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #11642:
URL: https://github.com/apache/ozone/pull/11642#issuecomment-6328952843
Thanks @jojochuang @errose28 for the reviews.
Filed follow-up HDDS-17289 for https://github.com/apache/ozone/pull/11642#pullrequestreview-5592347674
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org