[ 
https://issues.apache.org/jira/browse/HDDS-16923?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-16923:
------------------------------
    Attachment: MC-3-save-secret-keys-before-replacing-scm-db.patch

> SCM snapshot install replaces scm.db before saving the leader's secret keys, 
> so a failure in between later drops unexpired secret keys on all SCMs
> --------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-16923
>                 URL: https://issues.apache.org/jira/browse/HDDS-16923
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Major
>         Attachments: MC-3-save-secret-keys-before-replacing-scm-db.patch, 
> TestBugMC3SecretKeyFileLagsInstalledDb.java
>
>
> h3. Mechanism
> SCM keeps its symmetric secret keys only in memory and in a local file 
> ({{LocalSecretKeyStore}}). They are not in scm.db. When a follower installs a 
> Ratis snapshot, {{SCMStateMachine.reinitialize}} first calls 
> {{SCMHAManagerImpl.installCheckpoint}}, which moves the leader's checkpoint 
> into place as scm.db and reloads all managers from it. Only after that does 
> it call {{SecretKeyManager.reinitialize}} with the keys fetched from the 
> leader, which updates memory and saves the file.
> If the SCM stops between the two steps, scm.db is at the checkpoint index 
> while the key file is from before the install, or missing. The key rotations 
> in between are already purged from the leader's log, so they are never 
> replayed, and a follower never loads the key file at startup, so its in 
> memory key state stays empty. When this SCM later becomes leader, 
> {{SecretKeyManager.checkAndInitialize}} loads the stale file (or, if nothing 
> unexpired is left, generates one new key) and replicates it through the 
> {{@Replicate}} method {{SecretKeyState.updateKeys}}. Every SCM replaces its 
> key list with it and rewrites its own file, so keys that have not expired are 
> gone from the whole cluster. Later rotations build on that list, so they 
> never come back.
> h3. Trigger
> # A follower SCM is behind the leader's purged log (a newly bootstrapped SCM, 
> or one that was down while the leader rotated a key and moved on), so it 
> receives an install snapshot notification.
> # Between the DB swap and the key save, the SCM stops. This happens without a 
> crash if {{LocalSecretKeyStore.save}} throws, for example on an I/O error, or 
> on a full disk when the key file is on another volume than scm.db 
> ({{hdds.metadata.dir}} versus {{ozone.scm.db.dirs}}): {{reinitialize}} 
> throws, Ratis closes the state machine and SCM terminates. (If both share one 
> full volume, reopening the new DB is likely to fail first, and the install 
> rolls back to the old DB.) A process crash (kill -9, OOM kill, power loss) at 
> that point should leave the same files; this was argued from code, not run. A 
> graceful stop waits for {{reinitialize}} to finish and is not a trigger.
> # The SCM restarts and becomes leader (election or {{ozone admin scm 
> transfer}}) before a further key rotation is replicated to it 
> ({{hdds.secret.key.rotate.duration}}, default 1d).
> The window covers the reload of all SCM managers from the new DB, the 
> deletion of the old DB backup and the transaction buffer init. It was about 
> 60 ms on a small test DB and grows with the size of the SCM tables (not 
> measured on a large DB).
> h3. Impact
> * Unexpired secret keys are removed from every SCM. 
> {{SecretKeyProtocol.getSecretKey}} returns null for them, so datanodes reject 
> block and container tokens signed with them ({{ShortLivedTokenVerifier}}: 
> "Can't find the signing secret key"), and OM cannot verify delegation and STS 
> tokens signed with them. This was run on a secure mini cluster up to the 
> {{getSecretKey}} lookup. The token level consequences below are from code 
> reading.
> * OM keeps signing new tokens with its cached current key, which may be one 
> of the dropped keys. It switches only after that key passes the rotation 
> duration, or after {{ozone admin om fetch-key}}. Until then, retries that get 
> a new block token from OM fail the same way. A datanode or OM that had 
> already cached the key before it was dropped keeps accepting it.
> * Delegation tokens signed with a dropped key fail until they are replaced or 
> expire, which can take up to their 7 day maximum lifetime 
> ({{ozone.manager.delegation.token.max-lifetime}}); read from code, not run.
> * It fails closed: no token is accepted that should be rejected. It ends once 
> OM signs with the new key and the tokens signed with the dropped keys have 
> been replaced or have expired.
> h3. Reproduction
> PASS on an unmodified checkout, no hook or instrumentation. 
> [^TestBugMC3SecretKeyFileLagsInstalledDb.java] 
> ({{hadoop-ozone/integration-test}}) uses the setup of 
> {{TestSecretKeySnapshot}}: a secure mini cluster with 3 SCMs of which 2 
> start. The leader rotates from K1 to K2 and its log is purged past index 200. 
> A directory is created at the inactive SCM's key file path, so the save fails 
> with an I/O error, and the SCM is started. It installs the snapshot and 
> terminates, with scm.db at index 202 and no key file. The directory is 
> removed, the SCM is restarted and leadership is transferred to it. All three 
> SCMs then hold only one newly generated key, and {{getSecretKey(K2)}} returns 
> null although K2 expires hours later. A passing test means the defect is 
> present. The class Javadoc explains two harness steps needed only for the in 
> JVM restart.
> h3. Patch
> [^MC-3-save-secret-keys-before-replacing-scm-db.patch], against 
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at 
> 52812559dad.
> {{SCMStateMachine.reinitialize}} now saves the keys fetched from the leader 
> before it calls {{installCheckpoint}}. The keys are fetched after the 
> checkpoint download, so they include every unexpired key at the checkpoint 
> index. If the SCM stops after the save, it restarts on the old DB, installs 
> again and saves again. If the save fails, the DB is not touched.
> Covered by two tests in the existing {{TestSCMStateMachine}}: the keys are 
> saved before the DB is replaced, and a failing save means 
> {{installCheckpoint}} is never called. Both fail without the change. With it, 
> {{TestSCMStateMachine}} and the neighbouring SCM HA unit suites (27 tests), 
> {{TestSecretKeySnapshot}}, {{TestSecretKeysApi}}, {{TestSCMInstallSnapshot}} 
> and {{TestSCMInstallSnapshotWithHA}} pass, and checkstyle is clean. With the 
> patch, the reproduction fails at its first assertion because the follower's 
> DB is never replaced. With that assertion removed, the restarted SCM installs 
> again and keeps K1 and K2 after it becomes leader.
> A related patch for the same {{reinitialize}} method (a transaction buffer 
> init failure there is swallowed) is filed separately. The two conflict 
> textually in {{SCMStateMachine.reinitialize}} and {{TestSCMStateMachine}}. 
> This one is meant to go in first; the other will be rebased on it and reuse 
> its test helper.
> {{LocalSecretKeyStore.save}} still rewrites the file in place without fsync. 
> Writing to a temporary file and renaming it would close the remaining gap for 
> a crash during the save itself or a power loss; that is not part of this 
> patch.
> Found by TLA+ model checking and code review of the SCM Ratis snapshot and 
> checkpoint install paths under HDDS-15926, on commit 
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. 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]

Reply via email to