[
https://issues.apache.org/jira/browse/HDDS-16957?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16957:
------------------------------
Attachment: TestBugMC3VerifierCachesAbsentKey.java
> 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
> Priority: Major
> 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]
