[jira] [Created] (HDDS-16914) SCM leader counts root CA rotation prepare acks per certificate, so one SCM acking twice lets it commit before every SCM prepared

["Siyao Meng (Jira)" <ji...@apache.org>]

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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org