Siyao Meng created HDDS-16914:
---------------------------------
Summary: SCM leader counts root CA rotation prepare acks per
certificate, so one SCM acking twice lets it commit before every SCM prepared
Key: HDDS-16914
URL: https://issues.apache.org/jira/browse/HDDS-16914
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: MC-1-count-root-ca-rotation-prepare-acks-per-scm.patch,
TestBugMC1DuplicatePrepareAck.java
h3. Mechanism
{{RootCARotationHandlerImpl.rotationPrepareAck}} receives the new root
certificate ID, the sender's new sub-CA certificate ID and the sender's SCM ID.
It adds the certificate ID to a set and uses the SCM ID only in a log line.
{{RootCARotationManager.WaitSubCARotationPrepareAckTask}} submits
{{rotationCommit}} as soon as the size of that set equals the number of peers
in the Raft configuration.
An SCM that applies {{rotationPrepare}} a second time runs
{{SubCARotationPrepareTask}} again, which deletes its {{scm/sub-ca-next}}
directory, has a new sub-CA certificate signed with a new serial and sends
another ack. {{shouldSkipRootCert}} does not stop the second apply, because the
root certificate in memory only changes at {{rotationCommitted}}. An SCM
records a request as applied only when its transaction buffer is flushed
({{ozone.scm.ha.dbtransactionbuffer.flush.interval}}, default 60s), so an SCM
that is killed before the flush that follows {{rotationPrepare}} applies the
request again when it starts. An SCM that is killed in that interval after its
ack and is started again within {{hdds.x509.ca.rotation.ack.timeout}} (default
15 minutes) therefore acks twice. The leader of a three SCM ring then holds
three certificate IDs from two SCMs and commits although the third SCM has not
prepared.
h3. Trigger
Secure SCM HA with {{hdds.x509.ca.rotation.enabled=true}} (off by default),
three SCMs.
# One follower is down when the leader starts the rotation, so it does not ack.
# The other follower acks, is killed before its next transaction buffer flush
(default interval 60s) and is started again before the ack timeout (default 15
minutes). It applies {{rotationPrepare}} again and acks with a new certificate.
# The leader counts three acks and submits {{rotationCommit}} and
{{rotationCommitted}}.
Two overlapping faults are needed. Without the kill the same cluster behaves as
designed: the leader gets two acks, logs "Failed to receive all acks of
rotation prepare" at the timeout and does not commit.
h3. Impact
The rotation is committed while one SCM has no prepared sub-CA. On an
{{ozonesecure-ha}} compose cluster built from unmodified source (two runs) that
SCM, once started, applied {{rotationPrepare}} and {{rotationCommit}} within a
few milliseconds, before its prepare task had created {{scm/sub-ca-next}}: it
moved {{scm/sub-ca}} to {{scm/sub-ca-previous}}, failed the second move with
{{NoSuchFileException}}, and after a restart failed to start because
{{scm/sub-ca/certs}} was missing. The other two SCMs kept quorum and the leader
finished the rotation.
h3. Reproduction
PASS, deterministic, unmodified source, about 4 seconds.
[^TestBugMC1DuplicatePrepareAck.java] ({{hadoop-hdds/server-scm}}) runs one
SCM's rotation code in the test JVM: the real {{RootCARotationManager}} with
its tasks, {{RootCARotationHandlerImpl}}, {{SCMCertificateClient}} and the
certificate directories on disk. {{StorageContainerManager}} is a mock, the
Raft configuration is a mock that reports three peers, and a replicated request
is applied by calling the handler method the generated invoker calls. The first
test rebuilds the SCM objects from disk as after a restart, applies
{{rotationPrepare}} again and shows a second ack with the same SCM ID and a
different certificate ID. The second test lets the leader ack once, delivers
two acks from one follower and none from the third SCM, and shows the leader
logging "rotation is finished successfully" with the new root certificate in
memory. A passing test means the defect is present.
The kill and the Raft log replay are not part of this test. They were run on
the compose cluster described under Impact.
h3. Patch
[^MC-1-count-root-ca-rotation-prepare-acks-per-scm.patch], against
b9b33851b20fa11baf4b07982de9afb8ea44f85a. It also applies to master at
5108acff480.
The handler counts the SCM IDs of the senders instead of the certificate IDs.
The ack already carries the SCM ID, the tally only lives in the leader's
memory, and nothing changes in the replicated request, the generated invoker or
what is persisted. Counting per SCM means that every SCM has acked once, not
that every SCM is prepared at the moment of the commit: an SCM that is killed
after its ack and prepares again when it starts is still counted by its first
ack, which is a separate defect that this patch does not change (HDDS-16913).
Covered by {{testPrepareAckCountedOncePerScm}} in the existing
{{TestRootCARotationManager}}, which fails without the change (expected 2 acks,
was 3). With it {{TestRootCARotationManager}}, {{TestScmInvokerCodeGenerator}}
and {{TestScmCodecFactoryReplicateCoverage}} (16 tests) pass and checkstyle is
clean. With the patch the first test of the reproduction still passes, because
a second certificate is still signed, and the second fails because the leader
ends the rotation with "Failed to receive all acks of rotation prepare ...
received 2 acks". The patched build was not run on the compose cluster.
A stricter condition would be that the IDs of the acking SCMs equal the peer
IDs of the current Raft configuration (an SCM's Raft peer ID is its SCM ID). It
only differs when an SCM outside the configuration acks, and it needs a change
of the handler interface and its generated invoker, so the patch does not do 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]
