fix(guest-agent): reject a certificate validity bound it cannot represent

dstack/guest-agent/src/rpc_service.rs

@@ -602,6 +602,16 @@ pub(crate) fn validate_cert_validity(
     not_before: Option<u64>,
     not_after: Option<u64>,
 ) -> Result<()> {
+    for (name, value) in [("not_before", not_before), ("not_after", not_after)] {
+        if let Some(value) = value {
+            if value > ra_tls::cert::MAX_CERT_VALIDITY_SECS {
+                anyhow::bail!(
+                    "{name} must not be later than {}",
+                    ra_tls::cert::MAX_CERT_VALIDITY_SECS
+                );
+            }
+        }
+    }
     if let (Some(not_before), Some(not_after)) = (not_before, not_after) {
         if not_before >= not_after {
             anyhow::bail!("not_before must be earlier than not_after");
@@ -1789,7 +1799,8 @@ pNs85uhOZE8z2jr8Pg==
     }
 
     #[test]
-    fn test_tls_certificate_validity_order() {
+    fn test_tls_certificate_validity() {
+        let maximum = ra_tls::cert::MAX_CERT_VALIDITY_SECS;
         assert!(validate_cert_validity(None, None).is_ok());
         assert!(validate_cert_validity(Some(10), Some(11)).is_ok());
         assert_eq!(
@@ -1799,6 +1810,10 @@ pNs85uhOZE8z2jr8Pg==
             "not_before must be earlier than not_after"
         );
         assert!(validate_cert_validity(Some(12), Some(11)).is_err());
+        assert!(validate_cert_validity(None, Some(maximum)).is_ok());
+        assert!(validate_cert_validity(Some(0), Some(maximum)).is_ok());
+        assert!(validate_cert_validity(None, Some(maximum + 1)).is_err());
+        assert!(validate_cert_validity(Some(maximum + 1), None).is_err());
     }
 
     #[tokio::test]

dstack/ra-tls/src/cert.rs

@@ -113,8 +113,16 @@ impl CaCert {
             .maybe_app_id(app_id)
             .maybe_app_info(app_info.as_ref())
             .special_usage(usage)
-            .maybe_not_before(cfg.not_before.map(unix_time_to_system_time))
-            .maybe_not_after(cfg.not_after.map(unix_time_to_system_time))
+            .maybe_not_before(
+                cfg.not_before
+                    .map(|secs| unix_time_to_system_time(secs, "not_before"))
+                    .transpose()?,
+            )
+            .maybe_not_after(
+                cfg.not_after
+                    .map(|secs| unix_time_to_system_time(secs, "not_after"))
+                    .transpose()?,
+            )
             .build();
         self.sign(req).context("Failed to sign certificate")
     }
@@ -427,8 +435,16 @@ fn add_ext(params: &mut CertificateParams, oid: &[u64], content: impl AsRef<[u8]
         .push(CustomExtension::from_oid_content(oid, content));
 }
 
-fn unix_time_to_system_time(secs: u64) -> SystemTime {
-    UNIX_EPOCH + Duration::from_secs(secs)
+/// The last timestamp representable by an RFC 5280 certificate.
+pub const MAX_CERT_VALIDITY_SECS: u64 = 253_402_300_799;
+
+fn unix_time_to_system_time(secs: u64, field: &str) -> Result<SystemTime> {
+    if secs > MAX_CERT_VALIDITY_SECS {
+        bail!("{field} {secs} is past the last representable certificate time");
+    }
+    UNIX_EPOCH
+        .checked_add(Duration::from_secs(secs))
+        .with_context(|| format!("{field} {secs} overflows the system clock range"))
 }
 
 impl CertRequest<'_, KeyPair> {
@@ -697,6 +713,59 @@ mod tests {
         assert_eq!(decompressed, large_data);
     }
 
+    #[test]
+    fn validates_certificate_timestamps() {
+        assert!(unix_time_to_system_time(MAX_CERT_VALIDITY_SECS, "not_after").is_ok());
+        assert!(unix_time_to_system_time(MAX_CERT_VALIDITY_SECS + 1, "not_after").is_err());
+        assert!(unix_time_to_system_time(u64::MAX, "not_after").is_err());
+    }
+
+    #[test]
+    fn signing_a_csr_with_an_unrepresentable_not_after_fails_without_aborting() {
+        let ca_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).unwrap();
+        let mut ca_params = CertificateParams::new(vec![]).unwrap();
+        ca_params.is_ca = IsCa::Ca(BasicConstraints::Constrained(1));
+        let ca_cert = ca_params.self_signed(&ca_key).unwrap();
+        let ca = CaCert::from_parts(KeyPair::from_pem(&ca_key.serialize_pem()).unwrap(), ca_cert);
+
+        let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).unwrap();
+        let csr = CertSigningRequestV2 {
+            confirm: "please sign cert:".to_string(),
+            pubkey: key.public_key_der(),
+            config: CertConfigV2 {
+                org_name: None,
+                subject: "test.example.com".to_string(),
+                subject_alt_names: vec![],
+                usage_server_auth: true,
+                usage_client_auth: false,
+                ext_quote: false,
+                ext_app_info: false,
+                not_before: None,
+                not_after: Some(u64::MAX),
+            },
+            attestation: Attestation {
+                quote: AttestationQuote::DstackTdx(TdxQuote {
+                    quote: vec![],
+                    event_log: vec![],
+                }),
+                runtime_events: vec![],
+                report_data: [0u8; 64],
+                config: "".into(),
+                report: (),
+            }
+            .into_versioned(),
+        };
+
+        let error = match ca.sign_csr(&csr, None, "app:custom") {
+            Ok(_) => panic!("an unrepresentable not_after must be rejected"),
+            Err(error) => error,
+        };
+        assert!(
+            format!("{error:#}").contains("not_after"),
+            "the error must name the field: {error:#}"
+        );
+    }
+
     #[test]
     fn test_csr_v2_scale_encoding_stable() {
         let csr = CertSigningRequestV2 {