Siyao Meng created HDDS-17406:
---------------------------------
Summary: Revoked STS token purge deletes cutoffs without checking
their current value and leaves purged entries in the table cache
Key: HDDS-17406
URL: https://issues.apache.org/jira/browse/HDDS-17406
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: MC-1-CR-4-purge-only-unchanged-revocation-cutoffs.patch
h3. Mechanism
{{RevokedSTSTokenCleanupService}} picks the {{s3RevokedStsTokenTable}} entries
to purge from {{revokedStsTokenTable.iterator()}}, which reads RocksDB only
({{TypedTable.iterator}} does not consult the table cache), and submits
{{DeleteRevokedSTSTokens}} with the original access key IDs whose cutoff is
older than 12h. The request carries the key IDs only.
{{S3DeleteRevokedSTSTokensRequest.validateAndUpdateCache}} does not read the
current cutoff and writes nothing to the cache, and
{{S3DeleteRevokedSTSTokensResponse.addToDBBatch}} deletes every listed key from
the DB.
This has two effects on the table, which is a full cache table:
# A purge entry deletes whatever cutoff the key has in the DB when the entry is
applied, not the one the scan read. When a {{RevokeSTSToken}} for the same key
that writes a newer cutoff is ordered before the purge entry, the purge removes
that newer cutoff from the DB. The table cache keeps it, because the purge
writes no cache entry, so an OM that has not rebuilt its cache still reads it,
while an OM that rebuilds the cache from the DB (restart, checkpoint install)
has no cutoff for that key.
# Every purged entry stays in the table cache, since {{FullTableCache}} only
evicts entries whose cache value is null. Cache first reads ({{getIfExist}})
and the cache size keep reporting purged cutoffs on every OM until it rebuilds
the cache, while the DB has none.
{{S3SecurityUtil.isRevokedStsToken}} reads this table through the cache
({{getIfExist}}).
h3. Trigger
For the first effect, the leader's cleanup scan reads a cutoff older than 12h
for a key, and a {{RevokeSTSToken}} for the same key that writes a newer cutoff
is ordered before the purge entry. Two states lead there, both reproduced with
the real scan and apply code:
# The newer cutoff is applied on the leader but not yet flushed to RocksDB when
the scan creates its iterator.
# The newer {{RevokeSTSToken}} is applied after the scan, but is ordered before
the purge entry in the log.
The older cutoff must still be in the table (the first cleanup run after it
turns 12h old removes it; the default interval is 3h), so this is a race with
one cleanup run.
The second effect follows from every purge.
h3. Impact
* First effect: after the purge is flushed, the DB of every OM has no row for
the key, although the last applied {{RevokeSTSToken}} for it wrote a newer
cutoff. OMs that rebuild the table cache from the DB have no cutoff for the
key; OMs that did not still have the newer one in the cache. Revocation checks
for STS tokens of that key created before the newer cutoff read that state and
differ between the two groups until those tokens expire (at most 12h). Argued
from {{S3SecurityUtil.isRevokedStsToken}}, not run on a cluster.
* Second effect: purged cutoffs stay readable through the cache and counted in
the cache size until a cache rebuild, so OMs at the same applied index hold
different cache contents. A purged cutoff is at least 12h old and STS tokens
live at most 12h, so this does not change any revocation check.
h3. Reproduction
The regression tests in the attached patch cover both effects
({{hadoop-ozone/ozone-manager}}, real {{OmMetadataManagerImpl}} with a flush as
the double buffer does it). They need the new field to compile, so they were
checked with only the apply change in {{S3DeleteRevokedSTSTokensRequest}}
reverted, where the first two and the mismatch test fail:
* {{TestS3RevokeSTSTokenRequest.testPurgeKeepsNewerCutoffOfSameKey}}: older
cutoffs of three keys are flushed and one purge with those values is applied.
For the first key a newer cutoff is applied before the purge, for the second
one right after it (before the flush), and the third has none. The newer
cutoffs must remain in the cache and in the DB, and only the third key is
removed. Without the change the DB has no row for the first key.
*
{{TestS3RevokeSTSTokenRequest.testPurgeRemovesUnchangedCutoffFromCacheAndDB}}:
after a purge of an unchanged cutoff is flushed, neither the cache nor the DB
may return it and the cache must become empty. Without the change
{{getIfExist}} still returns the purged cutoff.
* {{TestS3RevokeSTSTokenRequest.testPurgeWithoutCutoffsDeletesListedEntries}}:
a request without the new list still deletes the listed entries.
* {{TestS3RevokeSTSTokenRequest.testPurgeWithMismatchedCutoffsDeletesNothing}}:
a cutoff list whose length differs from the key list is rejected and nothing is
deleted.
*
{{TestRevokedSTSTokenCleanupService.submitsCleanupRequestForOnlyExpiredTokens}}
now also checks that the scan sends the cutoff it read.
No standalone reproduction is attached.
h3. Patch
[^MC-1-CR-4-purge-only-unchanged-revocation-cutoffs.patch], against
5f7111a27ea07ce6b7bf5284a4213ebacb65f2ae. It also applies to master at
65320362775.
{{DeleteRevokedSTSTokensRequest}} gets {{repeated uint64 revocationTimeMillis =
2}}, the cutoff the scan read for each key, in key order.
{{RevokedSTSTokenCleanupService}} fills it and includes it in the request size
check. On apply, {{S3DeleteRevokedSTSTokensRequest}} reads each key's current
cutoff with {{getIfExist}}, which every OM answers the same way for the same
log prefix, and deletes only the keys whose cutoff still equals the one the
scan read. For each deleted key it writes a delete marker into the table cache,
as other full cache table deletes do, so the entry leaves the cache after the
flush. A newer cutoff written between the scan and the purge stays in the cache
and in the DB. A cutoff list whose length differs from the key list is rejected
with {{INTERNAL_ERROR}} and deletes nothing.
A request with no cutoff list, as written before this change, is applied as
before (all listed keys deleted, no cache entry), so log entries written
without the field apply the same way on patched OMs. An OM without the patch
ignores the field and deletes unconditionally, and there is no layout feature
for it, so the patch should land before the first release that ships STS;
{{DeleteRevokedSTSTokens}} is not in any release yet. Three byte limit tests in
{{TestRevokedSTSTokenCleanupService}} have new numbers because each entry is
now larger.
A narrower alternative without a wire change is to let the scan read each
cutoff through the table cache; that covers only the first of the two states
and leaves purged entries in the cache.
With this patch the purge also removes the cutoff from the table cache on every
OM, so the change relies on the purge threshold covering clock differences
between OMs, which a separate issue addresses.
With the patch {{TestS3RevokeSTSTokenRequest}},
{{TestRevokedSTSTokenCleanupService}}, {{TestOzoneManagerRatisRequest}},
{{TestS3SecurityUtil}}, {{TestS3AssumeRoleRequest}},
{{TestOzoneManagerDoubleBuffer}} and {{TestOmMetadataManager}} pass, and
checkstyle on {{hadoop-ozone/interface-client}} and
{{hadoop-ozone/ozone-manager}} is clean.
Found by TLA+ model checking and code review of the OM STS token revocation and
cleanup paths under HDDS-15926, on commit
5f7111a27ea07ce6b7bf5284a4213ebacb65f2ae. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. Related but not
duplicates: HDDS-14094 (introduced the cleanup service and the purge request),
HDDS-14373 (revocation writes the table cache at once), HDDS-16879 (cleanup
service after a checkpoint install). 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]
