Siyao Meng created HDDS-16912:
---------------------------------
Summary: 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
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]
