Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
smklein wants to merge 9 commits into
base: main
Choose a base branch
from
+2,821 −105
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5007ba6
fm: add certificate diagnosis engine for expiring silo TLS certificates
smklein Sep 4, 2026
e8c3674
fm: name only the absolute expiration time in certificate fact comments
smklein Sep 4, 2026
590fba9
fm: only observe certificates Nexus can actually serve
smklein Sep 4, 2026
4d7489d
fm: fix wrapped warning string in the certificate loader
smklein Sep 4, 2026
9e449d0
fm: break best-certificate ties deterministically
smklein Sep 4, 2026
391d604
nexus: pick the best certificate by its converted not_after
smklein Sep 4, 2026
2e1ca73
fm: make certificate fact payload columns NOT NULL
smklein Sep 4, 2026
de88ede
fm: use silo_list_all_batched in the certificate loader
smklein Sep 4, 2026
4a64593
fm: make the parsed certificate case's fact non-optional
smklein Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions certificates/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ license = "MPL-2.0"
workspace = true

[dependencies]
chrono.workspace = true
display-error-chain.workspace = true
foreign-types.workspace = true
openssl.workspace = true
Expand Down
91 changes: 90 additions & 1 deletion certificates/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@

//! Utilities for validating X509 certificates, used by both nexus and wicketd.

use chrono::DateTime;
use chrono::Utc;
use display_error_chain::DisplayErrorChain;
use omicron_common::api::external::Error;
use openssl::asn1::Asn1Time;
use openssl::asn1::Asn1TimeRef;
use openssl::pkey::PKey;
use openssl::x509::X509;
use openssl::x509::X509Ref;
use std::borrow::Borrow;
use std::ffi::CString;

Expand Down Expand Up @@ -47,6 +51,9 @@ pub enum CertificateError {
#[error("Unsupported certificate purpose (not usable for server auth)")]
UnsupportedPurpose,

#[error("Certificate validity time is out of the representable range")]
TimeOutOfRange,

#[error("Unexpected error")]
Unexpected(#[source] openssl::error::ErrorStack),
}
Expand All @@ -62,7 +69,8 @@ impl From<CertificateError> for Error {
| InvalidValidationHostname(_)
| ErrorValidatingHostname(_)
| NoDnsNameMatchingHostname { .. }
| UnsupportedPurpose => Error::invalid_value(
| UnsupportedPurpose
| TimeOutOfRange => Error::invalid_value(
"certificate",
DisplayErrorChain::new(&error).to_string(),
),
Expand All @@ -77,6 +85,47 @@ impl From<CertificateError> for Error {
}
}

/// The validity window of an X509 certificate.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct CertificateValidity {
/// The certificate is not valid before this time.
pub not_before: DateTime<Utc>,
/// The certificate is not valid after this time.
pub not_after: DateTime<Utc>,
}

/// Returns the validity window of an X509 certificate.
///
/// When called on the leaf certificate of a chain (the first certificate in
/// the chain, which is the one presented to TLS clients), the returned window
/// is the one clients will check.
pub fn validity(
cert: &X509Ref,
) -> Result<CertificateValidity, CertificateError> {
Ok(CertificateValidity {
not_before: asn1_time_to_chrono(cert.not_before())?,
not_after: asn1_time_to_chrono(cert.not_after())?,
})
}

/// Converts an ASN.1 time to a `chrono` timestamp by measuring its offset
/// from the Unix epoch.
///
/// `Asn1TimeRef` offers no direct conversion to a Unix timestamp, but
/// `ASN1_TIME_diff` can compute the (days, seconds) difference between two

@hawkw hawkw Sep 4, 2026

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.

what is ASN1_TIME_diff?

@hawkw hawkw Sep 4, 2026

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.

oh i guess it's the name of the C function that Asn1Time::diff is a binding to.

/// ASN.1 times.
fn asn1_time_to_chrono(
time: &Asn1TimeRef,
) -> Result<DateTime<Utc>, CertificateError> {
const SECS_PER_DAY: i64 = 24 * 60 * 60;
let epoch = Asn1Time::from_unix(0).map_err(CertificateError::Unexpected)?;
// `diff` computes `time - epoch`, split into whole days and the
// remaining seconds.
let diff = epoch.diff(time).map_err(CertificateError::Unexpected)?;

@hawkw hawkw Sep 4, 2026

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.

hm, do we know the errors that diff would return? is it possible that any of these would be time out of range-y?

let secs = i64::from(diff.days) * SECS_PER_DAY + i64::from(diff.secs);

@hawkw hawkw Sep 4, 2026

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.

should we use checked/saturating arithmetic here, since the cert is user controlled and might be overflowy?

DateTime::from_timestamp(secs, 0).ok_or(CertificateError::TimeOutOfRange)
}

pub struct CertificateValidator {
validate_expiration: bool,
}
Expand Down Expand Up @@ -444,4 +493,44 @@ mod tests {
);
}
}

#[test]
fn test_validity_converts_asn1_times() {
// Pin the leaf's validity window to exact second offsets from the
// Unix epoch. The root and intermediate certificates in the chain
// keep rcgen's default (much wider) window, so a correct result
// proves we read the certificate we were given and not some other
// link in the chain.
const NOT_BEFORE_SECS: u64 = 1_000_000_000;
const NOT_AFTER_SECS: u64 = 2_000_000_000;
let mut params = CertificateParams::new(vec![
"fake.test.oxide.computer".to_string(),
]);
params.not_before = (std::time::SystemTime::UNIX_EPOCH
+ std::time::Duration::from_secs(NOT_BEFORE_SECS))
.into();
params.not_after = (std::time::SystemTime::UNIX_EPOCH
+ std::time::Duration::from_secs(NOT_AFTER_SECS))
.into();
let chain = CertificateChain::with_params(params);
let certs = X509::stack_from_pem(chain.cert_chain_as_pem().as_bytes())
.expect("chain should parse");

let leaf_validity =
validity(&certs[0]).expect("leaf validity should convert");
assert_eq!(
leaf_validity,
CertificateValidity {
not_before: DateTime::from_timestamp(NOT_BEFORE_SECS as i64, 0)
.unwrap(),
not_after: DateTime::from_timestamp(NOT_AFTER_SECS as i64, 0)
.unwrap(),
}
);
// The intermediate certificate keeps rcgen's default window, which
// differs from the leaf's.
let intermediate_validity =
validity(&certs[1]).expect("intermediate validity should convert");
assert_ne!(leaf_validity, intermediate_validity);
}
}
17 changes: 17 additions & 0 deletions dev-tools/omdb/src/bin/omdb/nexus/fm_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,20 @@ struct ConfigOpts {
/// revert this setting to the default value.
#[clap(long, action = ArgAction::Set)]
analysis_enabled: Option<Setting<settings::AnalysisEnabled>>,

/// Sets how many days before a silo's external TLS certificate expires
/// the certificate diagnosis engine opens a case and requests an alert.
Comment on lines +141 to +142

@hawkw hawkw Sep 4, 2026

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 is sort of a weirdly structured sentence; I get what you're trying to say, but i wonder if it could be re-structured a bit.

///
/// The window applies to the certificate Nexus serves for the silo (the
/// one with the latest expiration time), so a case opens only when no
/// later-expiring replacement is installed.
///
/// This must be a non-zero integer no greater than 3650, or 'default'. If
/// it is set to 'default', any previous override will be removed, and the
/// system will revert this setting to the default value.
#[clap(long, action = ArgAction::Set)]
certificate_expiry_warning_days:
Option<Setting<settings::CertificateExpiryWarningDays>>,
Comment on lines +152 to +153

@hawkw hawkw Sep 4, 2026

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.

good call to put this here rather than in the TOML.

}

impl ConfigOpts {
Expand All @@ -151,6 +165,9 @@ impl ConfigOpts {
analysis_enabled: self
.analysis_enabled
.unwrap_or(current.analysis_enabled),
certificate_expiry_warning_days: self
.certificate_expiry_warning_days
.unwrap_or(current.certificate_expiry_warning_days),
}
}

Expand Down
98 changes: 56 additions & 42 deletions dev-tools/omdb/tests/successes.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,10 +731,11 @@ task: "fm_config_loader"
config last loaded at: <REDACTED_TIMESTAMP>
loaded by this activation: false
current config:
source: default
analysis enabled: true (default)
sitrep limit: 2500 (default)
history pruning threshold: 2000 (default)
source: default
analysis enabled: true (default)
sitrep limit: 2500 (default)
history pruning threshold: 2000 (default)
certificate expiry warning days: 30 (default)

task: "fm_rendezvous"
configured period: every <REDACTED_DURATION>m
Expand Down Expand Up @@ -788,6 +789,8 @@ task: "fm_sitrep_gc"
batches: 1
orphaned fm_ereport_in_case rows deleted: 0
batches: 1
orphaned fm_fact_certificate rows deleted: 0
batches: 1
orphaned fm_fact_physical_disk rows deleted: 0
batches: 1
orphaned fm_fact_saga rows deleted: 0
Expand All @@ -808,9 +811,10 @@ task: "fm_sitrep_history_pruner"
last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
configuration:
analysis enabled: true (default)
sitrep limit: 2500 (default)
history pruning threshold: 2000 (default)
analysis enabled: true (default)
sitrep limit: 2500 (default)
history pruning threshold: 2000 (default)
certificate expiry warning days: 30 (default)
deletion batch size: 1000
status: within limit (nothing was deleted)
current count: 1
Expand Down Expand Up @@ -1459,10 +1463,11 @@ task: "fm_config_loader"
config last loaded at: <REDACTED_TIMESTAMP>
loaded by this activation: false
current config:
source: default
analysis enabled: true (default)
sitrep limit: 2500 (default)
history pruning threshold: 2000 (default)
source: default
analysis enabled: true (default)
sitrep limit: 2500 (default)
history pruning threshold: 2000 (default)
certificate expiry warning days: 30 (default)

task: "fm_rendezvous"
configured period: every <REDACTED_DURATION>m
Expand Down Expand Up @@ -1516,6 +1521,8 @@ task: "fm_sitrep_gc"
batches: 1
orphaned fm_ereport_in_case rows deleted: 0
batches: 1
orphaned fm_fact_certificate rows deleted: 0
batches: 1
orphaned fm_fact_physical_disk rows deleted: 0
batches: 1
orphaned fm_fact_saga rows deleted: 0
Expand All @@ -1536,9 +1543,10 @@ task: "fm_sitrep_history_pruner"
last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
configuration:
analysis enabled: true (default)
sitrep limit: 2500 (default)
history pruning threshold: 2000 (default)
analysis enabled: true (default)
sitrep limit: 2500 (default)
history pruning threshold: 2000 (default)
certificate expiry warning days: 30 (default)
deletion batch size: 1000
status: within limit (nothing was deleted)
current count: 1
Expand Down Expand Up @@ -2425,10 +2433,11 @@ termination: Exited(0)
---------------------------------------------
stdout:
current fault management configuration:
source: default
analysis enabled: true (default)
sitrep limit: 2500 (default)
history pruning threshold: 2000 (default)
source: default
analysis enabled: true (default)
sitrep limit: 2500 (default)
history pruning threshold: 2000 (default)
certificate expiry warning days: 30 (default)

---------------------------------------------
stderr:
Expand All @@ -2439,9 +2448,10 @@ termination: Exited(0)
---------------------------------------------
stdout:
fault management config updated to version 1:
analysis enabled: true (default)
sitrep limit: 3000 (overriden)
history pruning threshold: 2000 (default)
analysis enabled: true (default)
sitrep limit: 3000 (overriden)
history pruning threshold: 2000 (default)
certificate expiry warning days: 30 (default)

---------------------------------------------
stderr:
Expand All @@ -2452,13 +2462,14 @@ termination: Exited(0)
---------------------------------------------
stdout:
current fault management configuration:
source: version 1
modified at: <REDACTED_TIMESTAMP>
comment:
source: version 1
modified at: <REDACTED_TIMESTAMP>
comment:
I am altering the config. Pray I do not alter it further.
analysis enabled: true (default)
sitrep limit: 3000 (overriden)
history pruning threshold: 2000 (default)
analysis enabled: true (default)
sitrep limit: 3000 (overriden)
history pruning threshold: 2000 (default)
certificate expiry warning days: 30 (default)

---------------------------------------------
stderr:
Expand All @@ -2469,9 +2480,10 @@ termination: Exited(0)
---------------------------------------------
stdout:
fault management config updated to version 2:
analysis enabled: false (overriden)
sitrep limit: 3000 (overriden)
history pruning threshold: 2000 (default)
analysis enabled: false (overriden)
sitrep limit: 3000 (overriden)
history pruning threshold: 2000 (default)
certificate expiry warning days: 30 (default)

---------------------------------------------
stderr:
Expand All @@ -2482,13 +2494,14 @@ termination: Exited(0)
---------------------------------------------
stdout:
current fault management configuration:
source: version 2
modified at: <REDACTED_TIMESTAMP>
comment:
source: version 2
modified at: <REDACTED_TIMESTAMP>
comment:
oops i altered it further
analysis enabled: false (overriden)
sitrep limit: 3000 (overriden)
history pruning threshold: 2000 (default)
analysis enabled: false (overriden)
sitrep limit: 3000 (overriden)
history pruning threshold: 2000 (default)
certificate expiry warning days: 30 (default)

---------------------------------------------
stderr:
Expand All @@ -2499,13 +2512,14 @@ termination: Exited(0)
---------------------------------------------
stdout:
fault management configuration v1:
source: version 1
modified at: <REDACTED_TIMESTAMP>
comment:
source: version 1
modified at: <REDACTED_TIMESTAMP>
comment:
I am altering the config. Pray I do not alter it further.
analysis enabled: true (default)
sitrep limit: 3000 (overriden)
history pruning threshold: 2000 (default)
analysis enabled: true (default)
sitrep limit: 3000 (overriden)
history pruning threshold: 2000 (default)
certificate expiry warning days: 30 (default)

---------------------------------------------
stderr:
Expand Down
Loading
Loading