[
https://issues.apache.org/jira/browse/HDDS-16912?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16912:
------------------------------
Attachment: MC-3-complete-root-ca-rotation-at-commit.patch
> Leader loss between root CA rotation commit and committed leaves the rotation
> unfinished, and the next rotation stops every SCM
> -------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16912
> URL: https://issues.apache.org/jira/browse/HDDS-16912
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: MC-3-complete-root-ca-rotation-at-commit.patch,
> TestBugMC3CommitWithoutCommitted.java, TestBugMC3LeaderLossOnRing.java
>
>
> h3. Mechanism
> {{RootCARotationManager.WaitSubCARotationPrepareAckTask}} finishes a rotation
> with two Ratis requests in a row, {{rotationCommit}} and
> {{rotationCommitted}}. {{RootCARotationHandlerImpl.rotationCommit}} moves
> {{scm/sub-ca}} to {{scm/sub-ca-previous}} and {{scm/sub-ca-next}} to
> {{scm/sub-ca}} and persists the new certificate ID. Only
> {{rotationCommitted}} reloads the key and certificates in memory and deletes
> {{scm/sub-ca-previous}}.
> If the leader loses leadership after the first request is committed and
> before the second one is replicated, its task ends in {{catch (Throwable)}},
> which only cleans up the leader's own state. Nothing records that the second
> request is owed: {{notifyStatusChanged}} on the new leader only checks the
> post-processing state, which is saved after {{rotationCommitted}}. Every SCM
> is left with the new sub-CA on disk, the old root certificate in memory and a
> non-empty {{scm/sub-ca-previous}}.
> {{MonitorTask}} on the new leader reads the root certificate from memory,
> finds it still inside the rotation window and starts another rotation. The
> first step of that rotation's {{rotationCommit}} is a {{Files.move}} of
> {{scm/sub-ca}} onto {{scm/sub-ca-previous}} with {{ATOMIC_MOVE}} and
> {{REPLACE_EXISTING}}, which cannot replace a directory that is not empty. It
> fails with "Directory not empty" on every SCM and each SCM calls
> {{scm.shutDown}}.
> h3. Trigger
> Secure SCM HA with {{hdds.x509.ca.rotation.enabled=true}} (off by default).
> # The leader collects all prepare acks and submits {{rotationCommit}}, which
> is committed.
> # The leader loses leadership before {{rotationCommitted}} is replicated
> (run: leader cut off from its followers; a leader crash leaves the followers
> with the same log). The window is the replication of {{rotationCommit}}, the
> leader applying it, and the replication of {{rotationCommitted}}.
> # The new leader starts the next rotation at its next check
> ({{hdds.x509.ca.rotation.check.interval}}, default 1 day, then
> {{hdds.x509.ca.rotation.time-of-day}}).
> An admin leadership transfer is not a trigger,
> {{SCMClientProtocolServer.transferLeadership}} is rejected while a rotation
> is in progress (read, not run).
> The same two requests in {{WaitSubCARotationPrepareAckTask}} and the same
> handler code are in the release tags ozone-1.4.0, 1.4.1, 2.0.0, 2.1.0, 2.1.1,
> 2.1.2, 2.2.0 and 2.2.1 (read, not run).
> h3. Impact
> * Between the two rotations every SCM signs with a sub-CA of the new root
> while it serves only the old root from {{getAllRootCaCertificates}}. On the
> mini cluster the new leader's sub-CA certificate does not verify under the
> only root it publishes. No datanode or OM was run against it.
> * The next rotation stops all SCMs of the ring. On the mini cluster all three
> failed {{rotationCommit}} within 20 seconds of the leader loss (with a 5
> second check interval). The processes did not exit: {{scm.shutDown}} is
> called on the Ratis apply thread (HDDS-16911), the last thing each of them
> logged was the close of the Raft server, and 10 seconds later all three apply
> threads were still inside {{rotationCommit}}.
> * A restart does not repair it. The SCM loads the new root from disk but
> {{scm/sub-ca-previous}} stays, so the same {{rotationCommit}} fails again
> (run on one SCM's objects rebuilt from disk; that the entry is applied again
> after a restart is argued from {{SCMStateMachine.applyTransaction}}, which
> records the applied index only after the handler returns, not run).
> * The same leftover directory, on one SCM only, results when that SCM is
> stopped after it applied {{rotationCommit}} and before it applied
> {{rotationCommitted}}: after the restart both entries are skipped by
> {{shouldSkipRootCert}} (argued, not run).
> h3. Reproduction
> PASS, unmodified source, two runs. [^TestBugMC3LeaderLossOnRing.java]
> ({{hadoop-ozone/integration-test}}, about 150 seconds) starts a secure mini
> cluster with three SCMs and root certificates that live eight minutes, so the
> leader rotates by itself. The only intervention is the Ratis test hook at
> {{RaftServerImpl.appendEntries}}: from the first AppendEntries that carries
> the {{rotationCommitted}} entry on, the followers refuse every AppendEntries
> of that leader. The test then shows the old leader's "Execution error" with
> {{NotLeaderException}}, on all three SCMs the new root on disk, the old root
> in memory and a non-empty {{scm/sub-ca-previous}}, no {{rotationCommitted}}
> applied anywhere, the second rotation started by the new leader, "Directory
> not empty" on all three and all three stopped.
> [^TestBugMC3CommitWithoutCommitted.java] ({{hadoop-hdds/server-scm}}, 2
> seconds, PASS) covers what the first test does not: it runs one SCM's
> handler, manager and certificate client on real certificate material, starts
> from a commit request without its committed request, and shows that a
> certificate issued in that state chains to the unpublished root, that the
> commit request of the next rotation fails, and that it fails again after the
> SCM objects are rebuilt from disk.
> A passing test means the defect is present.
> h3. Patch
> [^MC-3-complete-root-ca-rotation-at-commit.patch], against
> b9b33851b20fa11baf4b07982de9afb8ea44f85a. It also applies to master at
> 5108acff480.
> Three changes in {{RootCARotationHandlerImpl}}, none to the requests, the
> interface or what is persisted:
> * {{rotationCommit}} finishes the rotation itself: after the directories are
> switched and the certificate ID is persisted it reloads the key and
> certificates and deletes {{scm/sub-ca-previous}}, which is what
> {{rotationCommitted}} did. A {{rotationCommitted}} request that arrives
> afterwards finds the new root in memory and is skipped by the existing
> {{shouldSkipRootCert}} check. The leader only learns that its own SCM applied
> {{rotationCommit}} before it sends the second request, so the split did not
> make the switch any more atomic across SCMs.
> * {{rotationCommit}} deletes a leftover {{scm/sub-ca-previous}} before the
> first move, so a directory left behind by an earlier rotation cannot fail it.
> * {{rotationCommit}} returns after a failed move or a failed write of the
> certificate ID. Until now it went on to the next step whenever
> {{scm.shutDown}} returned, and with the first change that would reach the
> reload and the removal of {{scm/sub-ca-previous}}.
> Covered by {{testCommitCompletesRotationWithoutCommittedRequest}},
> {{testCommitReplacesBackupOfEarlierRotation}} and
> {{testFailedCommitKeepsBackup}} in the existing
> {{TestRootCARotationManager}}. Without the change the first two fail (no
> reload, and {{scm.shutDown}} on the leftover directory). The third fails
> without the added returns, because a failed second move is then followed by
> the removal of {{scm/sub-ca-previous}}. With it
> {{TestRootCARotationManager}}, {{TestScmInvokerCodeGenerator}} and
> {{TestScmCodecFactoryReplicateCoverage}} (18 tests) pass and checkstyle is
> clean. On the mini cluster with the patch and the same leader cut, all three
> SCMs had the new root in memory and no {{scm/sub-ca-previous}} right after
> the leader loss, without any {{rotationCommitted}} applied, and the new
> leader published the new root. The following rotation, started by the new
> leader two minutes later when the new root entered its own rotation window,
> completed on all three SCMs. With the patch both reproductions fail at their
> first assertion on the root in memory, which is then the new root.
> Not changed by the patch: when the leader is lost after {{rotationCommit}},
> the steps its task runs after the two requests are still skipped (storing the
> new root in the certificate store, the post-processing window, saving the new
> root in the stateful configuration). So after such an interrupted rotation
> the new root gets no certificate store entry from that rotation, and
> certificate signing is not held back for one
> {{hdds.x509.rootca.certificate.polling.interval}} as it is after a completed
> rotation (both seen in the log of the patched mini cluster run). The effect
> on datanodes and OMs was not run.
> The patch is against the commit above alone. Other fixes proposed for
> {{rotationCommit}} will need a manual merge with it.
> Found by TLA+ model checking and code review of the SCM root CA rotation
> paths under HDDS-15926, on commit b9b33851b20fa11baf4b07982de9afb8ea44f85a.
> 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]
