Siyao Meng created HDDS-16957:
---------------------------------
Summary: Datanode caches a "no such secret key" answer from a
stale SCM leader and keeps rejecting tokens signed with that key
Key: HDDS-16957
URL: https://issues.apache.org/jira/browse/HDDS-16957
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-3-do-not-keep-unknown-secret-key-id-in-verifier-cache.patch,
TestBugMC3VerifierCachesAbsentKey.java
h3. Mechanism
{{DefaultSecretKeyVerifierClient}} loads secret keys by ID from SCM into a
Guava cache with {{expireAfterWrite}} of twice
{{hdds.secret.key.expiry.duration}} (18d at defaults). When SCM answers that it
has no key with that ID, the loader stores {{Optional.empty()}} like a key.
From then on {{getSecretKey}} returns null for that ID without asking SCM,
until the entry expires or the process restarts, and
{{ShortLivedTokenVerifier.verifyTokenPassword}} rejects every token signed with
that key with "Can't find the signing secret key". The comment on the cache
size explains the factor of two with recently expired keys and says nothing
about unknown IDs.
"No such key" is not a stable answer. SCM serves {{getSecretKey}} from its own
memory, and the only gate is {{StorageContainerManager.checkLeader}}, which is
the local Ratis {{isLeaderReady}} with no lease or majority check. An SCM
leader that has lost its followers keeps passing it until Ratis notices, which
happens only after the leader has seen no event for one maximum election
timeout (5.2s for SCM), and every append to its own log restarts that timer. In
the meantime a new leader can create a new key. A verifier whose SCM proxy is
still on the old leader asks it for that key, gets the empty answer, which is a
successful call and causes no failover, and keeps it.
OM uses the same client to verify delegation tokens and STS tokens
({{OzoneDelegationTokenSecretManager.verifySignature}},
{{STSSecurityUtil.getValidatedSecretKey}}), so an OM that cached the empty
answer rejects those in the same way. This is from reading the code, it was not
run.
h3. Trigger
# Secure cluster with SCM HA and {{hdds.block.token.enabled}} or
{{hdds.container.token.enabled}}.
# The SCM leader loses the Raft links to both peers but is still reachable for
datanodes, and requests that need a Raft write keep arriving at it, at least
one every 5.2s for the whole time until step 5. They cannot commit, but each
append to its own log keeps it from stepping down. Every such request blocks
its caller for {{ozone.scm.ha.ratis.request.timeout}} (30s), so the writes have
to come from several callers. The datanodes have their secret key proxy on this
SCM because it was the leader.
# The peers elect a new leader, and the new leader rotates the secret key (the
daily rotation falls into this time).
# A token signed with the new key reaches a datanode. By code reading the new
SCM leader is itself such a signer: it puts a container token signed with its
current key into the close container commands it sends to datanodes.
# The datanode asks the old leader for the key and caches the empty answer.
This needs a partition of the SCM leader that overlaps a key rotation, so it is
rare. When the writes on the old leader stop, it steps down 5.2s later, usually
before a successor exists: in one run with a single write the old leader
stepped down 88 ms before the successor was elected, and the verifier found the
key.
h3. Impact
* The datanode rejects every block and container token signed with that key,
although all SCMs have the key, and it keeps doing so after the partition is
gone. Every datanode that looked the key up during the window is affected. A
datanode that asks later is not.
* It lasts as long as tokens signed with that key are presented, which is about
one rotation period (1d) plus the token lifetime, or until the datanode is
restarted. The cache entry itself lives 18d.
* Fail closed: no token is accepted that should not be.
h3. Reproduction
PASS, unmodified source, about 60 seconds.
[^TestBugMC3VerifierCachesAbsentKey.java] ({{hadoop-ozone/integration-test}})
starts a secure mini cluster with three SCMs and default Ratis timeouts. It
holds back the Raft RPCs between the leader and its two peers with the Ratis
{{CodeInjectionForTesting}} hook and submits one request per second to the old
leader's Ratis server. When the peers have elected a new leader, the test
rotates the key on it, lets it sign a container token, and verifies the token
with a verifier that is built the way a datanode builds its own. In the run the
new key was created 8.78 s after the cut, {{checkLeader}} was still true on the
old leader, the verifier logged "Secret key fetched from SCM: null" and
rejected the token. After the links were restored all three SCMs had the key
and a second verifier accepted the same token, but the first verifier still
rejected it 14 s later. A passing test means the defect is present.
Not exercised: a datanode process rejecting a request over the wire, OM as the
signer of a block token, a partition made by anything other than the Ratis test
hook, and real write traffic on a cut off leader. The writes in the test are
synthetic (the test hands one request per second to the old leader's Ratis
server), so that a production SCM stays leader ready in this way is argued from
the Ratis code, not shown.
h3. Patch
[^MC-3-do-not-keep-unknown-secret-key-id-in-verifier-cache.patch], against
025266b52a6dcaf39508607cad736799c40907e8. It also applies to master at
a65a32e7c4b.
The cache loader no longer produces an entry for an ID that SCM does not know.
Such an ID is remembered for 10 seconds in a second bounded cache, then the
next token with that ID makes the verifier ask SCM again. The ID is remembered
briefly so that a key that SCM really does not have is not looked up again at
once. 10 seconds is longer than the 5.2s after which a cut off SCM leader
without writes steps down, so the next lookup normally reaches the new leader.
No new configuration and no protocol change. The server side is not changed, so
an SCM leader that has lost its followers can still give the stale answer; with
the patch the datanode recovers from it by itself.
Covered by the new {{TestDefaultSecretKeyVerifierClient}} (there was no unit
test for this class): {{keyUnknownToScmIsLookedUpAgainAfterExpiry}} and
{{failedLookupIsNotCached}}. Without the change the first one fails ("expected:
SecretKey(...) but was: null"), the second one passes and pins the neighbouring
behaviour. With it the secret key and token unit suites of
{{hadoop-hdds/framework}} (40 tests) and {{TestBlockTokens}} on a secure mini
cluster (5 tests) pass, and checkstyle is clean. With the patch the
reproduction fails at the assertion that the first verifier still rejects the
token, because that verifier fetches the key again (20 s after the empty answer
in the run) and accepts the token.
Found by TLA+ model checking and code review of the SCM secret key rotation and
distribution paths under HDDS-15926, on commit
025266b52a6dcaf39508607cad736799c40907e8. 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
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
