Skip to content

Commit 8dfbdb0

Browse files
authored
Merge pull request #952 from Dstack-TEE/codex/fix-kms-ca-restart
fix(kms): preserve CA certificates across restart
2 parents d50b75a + 17650d3 commit 8dfbdb0

1 file changed

Lines changed: 73 additions & 5 deletions

File tree

dstack/kms/src/onboard_service.rs

Lines changed: 73 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
//
33
// SPDX-License-Identifier: Apache-2.0
44

5-
use std::sync::{Arc, Mutex, OnceLock};
5+
use std::{
6+
sync::{Arc, Mutex, OnceLock},
7+
time::{Duration, SystemTime, UNIX_EPOCH},
8+
};
69

710
use anyhow::{bail, Context, Result};
811
use dstack_kms_rpc::{
@@ -412,6 +415,30 @@ mod tests {
412415
assert!(validate_onboarding_domain(domain).is_err(), "{domain:?}");
413416
}
414417
}
418+
419+
fn ca_cert_expiring_at(not_after: SystemTime) -> Vec<u8> {
420+
let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).unwrap();
421+
CertRequest::builder()
422+
.subject("Test CA")
423+
.ca_level(0)
424+
.not_after(not_after)
425+
.key(&key)
426+
.build()
427+
.self_signed()
428+
.unwrap()
429+
.pem()
430+
.into_bytes()
431+
}
432+
433+
#[test]
434+
fn ca_certificate_is_renewed_only_within_the_renewal_window() {
435+
let now = UNIX_EPOCH + Duration::from_secs(2_000_000_000);
436+
let inside_window = ca_cert_expiring_at(now + CA_RENEWAL_WINDOW - Duration::from_secs(1));
437+
let outside_window = ca_cert_expiring_at(now + CA_RENEWAL_WINDOW + Duration::from_secs(1));
438+
439+
assert!(ca_cert_expires_within(&inside_window, now, CA_RENEWAL_WINDOW).unwrap());
440+
assert!(!ca_cert_expires_within(&outside_window, now, CA_RENEWAL_WINDOW).unwrap());
441+
}
415442
}
416443

417444
struct Keys {
@@ -638,15 +665,56 @@ pub(crate) async fn update_certs(cfg: &KmsConfig) -> Result<()> {
638665
.await
639666
.context("Failed to regenerate certificates")?;
640667

641-
// Write the new certificates to files. This runs on every start, so a
642-
// hand-placed certificate is replaced -- say so, because the old silence
643-
// made that look like the file had survived.
644-
keys.store_certs(cfg)?;
668+
renew_ca_cert_if_expiring(
669+
cfg.root_ca_cert(),
670+
keys.ca_cert.pem(),
671+
"KMS root CA certificate",
672+
)?;
673+
renew_ca_cert_if_expiring(
674+
cfg.tmp_ca_cert(),
675+
keys.tmp_ca_cert.pem(),
676+
"temporary client CA certificate",
677+
)?;
678+
679+
// The RPC leaf depends on the refreshed domain and platform attestation, so
680+
// it is reissued on every startup.
681+
safe_write(cfg.rpc_cert(), keys.rpc_cert.pem())?;
645682
info!("Reissued the KMS RPC certificate for {domain}");
646683

647684
Ok(())
648685
}
649686

687+
const CA_RENEWAL_WINDOW: Duration = Duration::from_secs(365 * 24 * 60 * 60);
688+
689+
fn renew_ca_cert_if_expiring(
690+
path: impl AsRef<std::path::Path>,
691+
renewed_pem: String,
692+
description: &str,
693+
) -> Result<()> {
694+
let path = path.as_ref();
695+
let current_pem = fs::read(path)
696+
.with_context(|| format!("Failed to read {description} from {}", path.display()))?;
697+
if !ca_cert_expires_within(&current_pem, SystemTime::now(), CA_RENEWAL_WINDOW)? {
698+
return Ok(());
699+
}
700+
safe_write(path, renewed_pem)?;
701+
info!("Renewed {description}");
702+
Ok(())
703+
}
704+
705+
fn ca_cert_expires_within(cert_pem: &[u8], now: SystemTime, window: Duration) -> Result<bool> {
706+
let (_, pem) =
707+
x509_parser::pem::parse_x509_pem(cert_pem).context("Failed to parse CA certificate PEM")?;
708+
let cert = pem.parse_x509().context("Failed to parse CA certificate")?;
709+
let now = now
710+
.duration_since(UNIX_EPOCH)
711+
.context("System time is before the Unix epoch")?
712+
.as_secs();
713+
let renewal_deadline = now.saturating_add(window.as_secs());
714+
let not_after = u64::try_from(cert.validity().not_after.timestamp()).unwrap_or(0);
715+
Ok(not_after <= renewal_deadline)
716+
}
717+
650718
pub(crate) async fn bootstrap_keys(cfg: &KmsConfig, verifier: &AttestationVerifier) -> Result<()> {
651719
validate_onboarding_domain(&cfg.onboard.auto_bootstrap_domain)?;
652720
ensure_self_kms_allowed(cfg, verifier)

0 commit comments

Comments
 (0)