Skip to content

fm: add certificate diagnosis engine for expiring silo TLS certificates - #11238

Merged
smklein merged 15 commits into
mainfrom
fm-certificate-de
Oct 19, 2026
Merged

smklein merged 15 commits into
mainfrom
fm-certificate-de

Conversation

@smklein

@smklein smklein commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator

Nexus serves each silo's external API with the silo's certificate whose leaf not_after is latest (ExternalEndpoint::best_certificate). When that certificate is about to expire or has expired, nothing told the operator.

This adds a fault management diagnosis engine (nexus/fm/src/diagnosis/certificate.rs) that opens a case per silo when the best certificate is inside a configurable warning window or already expired. The case carries one fact, BestCertificateExpiring or BestCertificateExpired, and requests a silo.certificate.expiring or silo.certificate.expired alert whenever that fact changes: entering the window, passing not_after, or a different (still expiring or expired) certificate becoming the best one. Unchanged input requests nothing. The case closes once a later-expiring certificate is installed or the silo is removed.

The warning window is a new FmConfig setting, certificate_expiry_warning_days (default 30, at most 3650). This value can be modified with omdb nexus fm-config set --certificate-expiry-warning-days <N>.

Supporting changes:

  • omicron-certificates: leaf_validity parses a PEM chain's leaf not_before/not_after into chrono timestamps.
  • nexus-types: DiagnosisEngineKind::Certificate, the CertificateFact payloads, the ObservedSiloCertificates analysis input, the two alert classes with V0 payloads, the FmConfig setting, and an input report section listing observed silos and certificates.
  • Database changes (schema/crdb/fm-certificate-de): diagnosis_engine and alert_class enum values, the fm_fact_certificate table (typed columns, one table per engine, as for disks and sagas), and the fm_config column with its CHECK constraint.
  • nexus-fm: Input now carries the FmConfig and the observed silo certificates (both required inputs). The fm_analysis task loads them from the same silo and certificate rows the external_endpoints task reads.

Not in scope, as follow-ups: not_before handling (to be added to best_certificate and the engine together), alerting on silos with no certificates at all, and exposing not_after in the external_endpoints task status.

Nexus serves each silo's external API with the silo's certificate whose
leaf not_after is latest (ExternalEndpoint::best_certificate). When that
certificate is about to expire or has expired, no later-expiring
replacement is installed, and until now nothing told the operator.

Add a fault management diagnosis engine that predicts the same choice
and opens a case per silo when the best certificate is inside a
configurable warning window or already expired. The case carries one
fact (BestCertificateExpiring or BestCertificateExpired) and requests a
silo.certificate.expiring or silo.certificate.expired alert whenever
that fact changes. The case closes once a later-expiring certificate is
installed or the silo is removed. Silos with no certificates open no
case. Like best_certificate, the engine ignores not_before.

The warning window is a new FmConfig setting,
certificate_expiry_warning_days (default 30, at most 3650), settable via
omdb nexus fm-config set --certificate-expiry-warning-days.

Supporting changes:
- omicron-certificates: leaf_validity parses a PEM chain's leaf
  not_before/not_after into chrono timestamps.
- nexus-types: DiagnosisEngineKind::Certificate, CertificateFact
  payloads, the ObservedSiloCertificates analysis input, the two alert
  classes with V0 payloads, and the FmConfig setting.
- Schema version 300 (fm-certificate-de): diagnosis_engine and
  alert_class enum values, the fm_fact_certificate table, and the
  fm_config column with its CHECK constraint.
- nexus-fm: Input carries the FmConfig and observed silo certificates;
  the fm_analysis task loads them from the silo and certificate tables.
@smklein
smklein marked this pull request as draft September 4, 2026 17:19
pub comment: String,

/// The silo this fact is about.
pub silo_id: Uuid,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silos and certs don't have a strongly-typed UUID (yet), they probably should

A fact's comment is recorded when the fact is added and carried forward
unchanged, so a relative phrase like "in 10days" or "1day ago" goes
stale as sitreps advance. Record the absolute not_after only.
Ok(ParsedCertificateCase { silo_id, fact: kept, duplicate_facts })
}

pub(super) fn analyze(builder: &mut SitrepBuilder<'_>) -> anyhow::Result<()> {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of this function is coping with "observing parent cases, reconciliation of facts". I'll annotate where the more important "new case opened, alert created" stuff happens.

Comment on lines +214 to +215
for silo in silos.iter() {
let Some((desired, best)) = desired_fact(silo, reference_time, window)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where we flag certs which are "expired, or about to be"

Comment thread nexus/fm/src/diagnosis/certificate.rs Outdated

let carried = parent.and_then(|(_, p)| p.fact.as_ref());
if carried.map(|(_, fact)| fact) == Some(&desired) {
continue;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we bail here if the fact was the same as the parent, so below this, the fact is new - meaning we need a new alert.

name: best.name.clone(),
not_after: best.not_after,
};
let alert_result = match &desired {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and this is where we actually create alerts

Comment thread nexus/types/src/alert/certificate.rs
@smklein
smklein marked this pull request as ready for review September 4, 2026 18:43
@smklein smklein assigned augustuswm and unassigned augustuswm Sep 4, 2026
@smklein
smklein requested a review from augustuswm September 4, 2026 19:01
Comment thread certificates/src/lib.rs Outdated
Comment thread certificates/src/lib.rs Outdated
Comment thread certificates/src/lib.rs Outdated
Comment thread certificates/src/lib.rs Outdated
}

#[test]
fn test_leaf_validity_reads_leaf_certificate() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels like it might perhaps wanna be a proptest someday.

Comment thread certificates/src/lib.rs Outdated
Comment thread nexus/fm/src/analysis_input.rs
Comment on lines +107 to +109
// Expiry is judged against the input's deterministic "now"; see
// `Input::reference_time`.
let reference_time = input.reference_time();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for adding this, i had been thinking we were gonna want something like this for a while now. (i was just gonna call it input.now(), but maybe your name is better!)

Comment thread nexus/types/src/fm/analysis_reports.rs
Comment thread nexus/src/app/external_endpoints.rs Outdated
Comment thread nexus/fm/src/analysis_input.rs
@hawkw

hawkw commented Sep 4, 2026

Copy link
Copy Markdown
Member

(i should add that it's neat to see DEs for simple, but high-impact purely software/config issues starting to be written! thanks for doing this!)

The certificate diagnosis engine's loader accepted any certificate
whose PEM parsed, but ExternalEndpoints::new also drops certificates
whose key rustls cannot load. Such a certificate could be chosen as the
silo's best and hide an expiring one that is served. Run the loader's
rows through TlsCertificate::try_from so both sides agree.
The "silo not found" warning was wrapped without a trailing backslash,
leaving a run of interior whitespace in the text omdb prints.
ObservedSiloCertificates::best_certificate claimed callers depended
only on not_after, but the engine stores the chosen certificate's id
in its fact and re-alerts when it changes. Break ties toward the
greatest id, document why, and test that equal-expiry certificates
carry the fact unchanged across sitreps.
TlsCertificate now carries the leaf validity as a DateTime, so the
served-certificate rule can be the same max_by_key expression the
certificate diagnosis engine uses, instead of a hand-rolled loop over
Asn1Time partial_cmp. Drop the parsed X509 field, which had no other
readers left.
Both certificate fact kinds carry the same payload, so the nullable
columns plus two identical per-kind CHECK constraints were NOT NULL
spelled twice. Declare them NOT NULL, drop the Option fields and the
unreachable missing-column error path in the Diesel model.
The loader hand-rolled the same pagination loop the datastore helper
already provides, with an ad-hoc batch size. Use the helper for silos
and the standard SQL_BATCH_SIZE for certificates.
parse_case returns NoFacts when a case has no facts, so a successfully
parsed case always has one. Store it directly and derive the silo id
from it instead of threading an Option through the reconcile loop.
- Share certificate selection between Nexus and the engine: both
  ExternalEndpoint::best_certificate and
  ObservedSiloCertificates::best_certificate now call
  nexus_types::observed_certificate::best_certificate, and TlsCertificate
  carries the certificate id so the tie-break matches. The comments asking
  to keep the two in sync are gone, and a Nexus-side tie-break test is
  added.
- Record the analysis reference time in InputReport.
- omicron-certificates: map ASN1_TIME_diff failures to a new
  CertificateError::BadValidityTime (an invalid certificate, not an
  internal error), use checked arithmetic for the epoch offset, and
  clarify the docs and test comment.
- Reword the engine's comments on parsing parent cases: a corrupt case is
  quarantined by closing it, at the cost of one duplicate alert if its
  silo still needs attention.
- Restructure the omdb --certificate-expiry-warning-days help text.
Replace the generic helper with one that takes a slice of
CertificateCandidate (id and not_after) and returns the winning id. Both
Nexus and the certificate diagnosis engine build candidates from their own
certificate types and look the winner up by id, so the whole selection rule
lives in one non-generic function.
/// the diagnosis engines for this analysis pass. Keyed by silo ID.
#[serde(default)]
pub observed_silo_certificates: BTreeMap<Uuid, SiloCertificatesReport>,
// Reports are serialized to the database, so any new field here should

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need one more thing here, the current value of certificate_expiry_warning_days, which was input to the DE. Or some proxy for that value (e.g. maybe the config version that contained the value, although then it's no longer self contained)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in bbb0900 - now we're emitting the configuration options to FM as part of the InputReport

Comment on lines +193 to +196
None => case_mut.close(format!(
"silo {} ({silo_id}) has no certificates installed; that \
is outside this engine's scope",
silo.silo_name,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about if the operator responds to a "certificate expired" alert by just deleting the certificate, leaving no remaining certs installed for some given silo? The DE says "yup, case closed," but now Nexus can't serve TLS for that silo at all.

@hawkw hawkw added the fault-management Everything related to the fault-management initiative (RFD480 and others) label Sep 18, 2026
@smklein

smklein commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

This should be ready for a re-review

@hawkw

hawkw commented Oct 15, 2026

Copy link
Copy Markdown
Member

I should probably add silos to the list of impact-able resources over in #11029

@mergeconflict
mergeconflict self-requested a review October 16, 2026 22:36
@smklein
smklein merged commit 79112b0 into main Oct 19, 2026
21 checks passed
@smklein
smklein deleted the fm-certificate-de branch October 19, 2026 17:25
smklein added a commit that referenced this pull request Oct 19, 2026
Fixes the `nexus-fm` test build on main.

#11238 added a test that constructs `steno::SagaId(Uuid::new_v4())`, but
#11517 (steno 0.5) landed first and made `SagaId` a newtype-uuid type
alias, so the tuple-struct constructor no longer exists. Each PR passed
CI on its own; the break only shows up once both are merged. This
switches the test to `steno::SagaId::new_v4()`.
hawkw pushed a commit that referenced this pull request Oct 24, 2026
Fixes the `nexus-fm` test build on main.

#11238 added a test that constructs `steno::SagaId(Uuid::new_v4())`, but
#11517 (steno 0.5) landed first and made `SagaId` a newtype-uuid type
alias, so the tuple-struct constructor no longer exists. Each PR passed
CI on its own; the break only shows up once both are merged. This
switches the test to `steno::SagaId::new_v4()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fault-management Everything related to the fault-management initiative (RFD480 and others)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants