Siyao Meng created HDDS-16913:
---------------------------------
Summary: SCM that applies a root CA rotation commit after a
restart or without a staged sub CA certificate breaks its sub CA directory and
cannot start
Key: HDDS-16913
URL: https://issues.apache.org/jira/browse/HDDS-16913
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: MC-2-commit-reads-staged-sub-ca-certificate.patch,
TestBugMC2CommitAfterRestartNullCertId.java,
TestBugMC5PrepareAndCommitBackToBack.java
h3. Mechanism
{{RootCARotationHandlerImpl.rotationCommit}} moves {{scm/sub-ca}} to
{{scm/sub-ca-previous}}, moves {{scm/sub-ca-next}} to {{scm/sub-ca}} and then
writes the new certificate ID to the VERSION file. It takes that ID from the in
memory field {{newSubCACertId}}, which only {{SubCARotationPrepareTask}} sets
in the same process, and it does not check that {{scm/sub-ca-next}} exists
before the first move. Two cases follow.
# The SCM was restarted after it staged its certificate and sent the prepare
ack. The field is null, both moves succeed and
{{SCMStorageConfig.setScmCertSerialId(null)}} throws a
{{NullPointerException}}, which {{SCMStateMachine.applyTransaction}} turns into
a terminate. {{scm/sub-ca}} now holds the new certificate while the VERSION
file names the old one, which is only left in {{scm/sub-ca-previous}}.
# Nothing is staged when the commit is applied. {{rotationPrepare}} only
schedules the prepare task, and the task deletes {{scm/sub-ca-next}} before it
asks the leader for a new certificate. When the SCM applies prepare and commit
back to back from the RAFT log, the commit runs first: the first move succeeds,
the second fails with {{NoSuchFileException}} and the handler calls
{{scm.shutDown}}. {{scm/sub-ca}} is gone.
HDDS-8983 (fixed in 1.4.0) is not this issue. It fixed the same null ID for the
order of the ack and the commit inside one running process. The field is still
only in memory, so a restart loses it.
The code is the same 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. Trigger
Secure SCM HA with {{hdds.x509.ca.rotation.enabled=true}} (default false) while
a root CA rotation runs. With the default {{hdds.x509.max.duration}} of P1865D
that is once in about five years.
* Case 1: an SCM is restarted after its prepare ack and before it applies the
commit, with its applied index already past the prepare. A graceful stop always
gives that, because it takes a snapshot. The leader polls the acks every
second, so the window is one to two seconds when all SCMs answer, and up to
{{hdds.x509.ca.rotation.ack.timeout}} (PT15M) while the leader waits for
another SCM.
* Case 2, replay: an SCM is killed in the same window before its applied index
is persisted (the SCM transaction buffer is flushed every 60s by default, or by
a snapshot). On the next start Ratis applies prepare again and then commit.
* Case 2, never prepared: an SCM applies a commit for a rotation it has not
prepared for, for example one that was stopped during the whole rotation while
the leader counted two acks of a restarted SCM. A new SCM whose own certificate
chain still ends in the older root certificate, for example one bootstrapped
before the rotation and started after it, takes the same path when it replays
these entries (argued, not run). An SCM whose chain already ends in the new
root certificate skips them.
h3. Impact
The SCM is out of service and fails on every later start until an operator
repairs the directories by hand. The other SCMs finish the rotation, so the
ring loses one member, and quorum if two SCMs are hit.
* Case 1: the SCM terminates while it applies the commit. On later starts the
certificate client finds no certificate for the ID in the VERSION file. With
{{hdds.grpc.tls.enabled=true}} the start fails with a {{NullPointerException}}
when the Ratis TLS configuration is built. Without it (the default) a primary
SCM fails with a {{NullPointerException}} in
{{StorageContainerManager.persistPrimarySCMCerts}} (run with a real
{{StorageContainerManager}} constructor on the same on disk state, not part of
the attached reproductions). Any other SCM is expected to fail when the commit
is applied again, in {{RootCARotationManager.shouldSkipRootCert}} on the
missing trust chain (argued, not run).
* Case 2: the handler calls {{scm.shutDown}} on the thread that applies the log
entry, which leaves the process half stopped instead of exiting (HDDS-16911).
On later starts {{DefaultCAServer.init}} throws {{IllegalStateException:
INTERMEDIARY_CA Should not be in Initialize State during startup.}}
Keys and certificates are not lost: the old pair is in {{scm/sub-ca-previous}},
and in case 1 the new pair is in {{scm/sub-ca}}. {{ozone repair scm cert
recover}} does not handle these directories.
h3. Reproduction
PASS, deterministic, unmodified source.
[^TestBugMC2CommitAfterRestartNullCertId.java] and
[^TestBugMC5PrepareAndCommitBackToBack.java] ({{hadoop-hdds/server-scm}}) run
the real {{RootCARotationManager}} with its prepare task,
{{RootCARotationHandlerImpl}}, {{SCMCertificateClient}}, {{SCMStorageConfig}}
and CA servers on a temporary directory. The {{StorageContainerManager}}
object, Ratis and the security RPC are mocks: the tests call the handler where
the state machine applies a log entry and rebuild the objects from disk where
the process restarts. The second class holds the certificate request of the
prepare task with a latch, as an RPC that has not returned yet. No state is
written by hand. A passing test means the defect is present.
Both cases were also seen on a three SCM compose cluster built from unmodified
source, driven only by docker pause, stop, kill and start (logs read for this
report, not run again). Case 1, with {{hdds.grpc.tls.enabled=true}}: one SCM
sends its ack, is stopped gracefully and takes a snapshot, is started again,
logs {{Persistent new scm certificate null}} and {{Terminating with exit status
1}} when it applies the commit, and two further starts fail with the
{{NullPointerException}} from the Ratis TLS configuration. Case 2, never
prepared: an SCM that was stopped during the whole rotation applies prepare and
commit 7 ms apart and logs {{Failed to move /data/metadata/scm/sub-ca-next to
/data/metadata/scm/sub-ca}}. Its process keeps running, and its prepare task
starts only afterwards.
h3. Patch
[^MC-2-commit-reads-staged-sub-ca-certificate.patch], against
b9b33851b20fa11baf4b07982de9afb8ea44f85a. It also applies to master at
5108acff480. The patch is made against that commit alone, other fixes proposed
for the same methods (HDDS-16911 among them) will need a manual merge.
* {{rotationCommit}} reads the new certificate ID from the certificate staged
in {{scm/sub-ca-next}}, before the first move. It accepts the staged
certificate only when its chain ends in the root certificate being committed
and that root certificate issued it. A certificate signed by the sub CA of a
leader that no longer has the new root CA server is not accepted. The in memory
field is removed.
* {{rotationPrepare}} keeps a certificate that is already staged and acceptable
for the same root certificate and sends the ack again with the same ID, so a
replayed prepare no longer deletes it. Anything else in {{scm/sub-ca-next}} is
replaced by the prepare task as before.
* Without an acceptable staged certificate the commit logs an error and moves
nothing, the committed step does nothing, and the pending prepare task of that
rotation is cancelled: when the leader answers its request, with a certificate
or with a rejection, the task discards the result, stages nothing and does not
stop the SCM. The SCM keeps running on its current certificate.
With the patch an SCM that takes the last path is not rotated and needs
operator attention. Bringing it to the new root certificate is not part of this
patch. What follows from that state (all read from the code, not run):
* The only sign is one ERROR line from {{rotationCommit}} when the commit is
applied. There is no metric, the leader still counts the rotation as
successful, and nothing is logged on a later start.
* The old root certificate, and with it the chain of this SCM, expires at most
2 x {{hdds.x509.renew.grace.duration}} (56 days by default) after the rotation
started.
* If this SCM is elected leader while the post rotation record is still stored
({{hdds.x509.rootca.certificate.polling.interval}}, PT2h by default),
{{RootCARotationManager.checkAndHandlePostProcessing}} finds a stored root
certificate newer than its own and throws a {{RuntimeException}} out of
{{notifyStatusChanged}}.
* If it is elected leader later, its monitor task (run every
{{hdds.x509.ca.rotation.check.interval}}, P1D) finds its own root certificate
inside the grace period and schedules another rotation, which rotates the whole
ring again, this SCM included.
* It loads the new root certificate only on its next start, when
{{StorageContainerManager.start}} refreshes the CA certificates from the
leader. Until then, with {{hdds.grpc.tls.enabled=true}}, it cannot verify the
certificates of the rotated SCMs.
{{RootCARotationHandler.setSubCACertId}} now only logs. It is kept because
removing it changes the handler interface and the generated invoker.
Covered by six cases in the existing {{TestRootCARotationManager}}: commit
after a restart, prepare applied again before the commit, commit without a
staged certificate while the request of the prepare task is still open
(answered with a certificate and with a rejection), commit while a certificate
for another root certificate is staged, and commit while a certificate signed
by a sub CA is staged. Without the change all six fail with the
{{NullPointerException}} from {{rotationCommit}}. With it
{{TestRootCARotationManager}} (10 tests) and the SCM HA and certificate unit
suites next to it pass (29 tests in total) and checkstyle is clean. With the
patch the three tests of the two reproduction classes fail, because the commit
no longer fails.
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]
