From daed4d4d86e08f312e8103ede7af410286d3dde8 Mon Sep 17 00:00:00 2001 From: Martin Hoffmann Date: Mon, 25 Mar 2024 17:49:32 +0100 Subject: [PATCH] Consider manifest cert for chain validity and note when points go stale. (#945) This PR changes the code that calculates the validity of the full validation chain to also consider the validity of the EE certificates of the publication points along the chain. It also adds a new field in the jsonext output format that provides the time when any of the publication points along the validation chain become stale. --- doc/manual/source/advanced-features.rst | 22 +++++---- doc/manual/source/manual-page.rst | 6 ++- doc/manual/source/output-formats.rst | 20 ++++---- src/engine.rs | 62 +++++++++++++++---------- src/output.rs | 4 +- src/payload/info.rs | 24 +++++++--- src/payload/validation.rs | 35 ++++++++++---- 7 files changed, 114 insertions(+), 59 deletions(-) diff --git a/doc/manual/source/advanced-features.rst b/doc/manual/source/advanced-features.rst index a6a5120..1519e71 100644 --- a/doc/manual/source/advanced-features.rst +++ b/doc/manual/source/advanced-features.rst @@ -54,8 +54,9 @@ in the :term:`json` and :term:`jsonext` output formats, e.g.: }, "chainValidity": { "notBefore": "2023-04-18T14:32:13Z", - "notAfter": "2024-07-01T00:00:00Z" - } + "notAfter": "2023-04-20T00:00:00Z" + }, + "stale": "2023-04-20T00:00:00Z" }] }], "routerKeys": [], @@ -73,8 +74,9 @@ in the :term:`json` and :term:`jsonext` output formats, e.g.: }, "chainValidity": { "notBefore": "2023-04-18T14:32:13Z", - "notAfter": "2024-04-11T07:26:24Z" - } + "notAfter": "2023-04-20T00:00:00Z" + }, + "stale": "2023-04-20T00:00:00Z" }] }] } @@ -125,8 +127,9 @@ in the :term:`SLURM`, :term:`json` and :term:`jsonext` output formats, e.g.: }, "chainValidity": { "notBefore": "2021-05-06T12:51:30Z", - "notAfter": "2022-07-01T00:00:00Z" - } + "notAfter": "2021-05-08T00:00:00Z" + }, + "stale": "2021-05-08T00:00:00Z" }] }], "routerKeys": [{ @@ -142,8 +145,9 @@ in the :term:`SLURM`, :term:`json` and :term:`jsonext` output formats, e.g.: }, "chainValidity": { "notBefore": "2022-01-16T14:45:51Z", - "notAfter": "2022-08-06T00:00:00Z" - } + "notAfter": "2021-01-18T00:00:00Z" + }, + "stale": "2021-01-18T00:00:00Z" }] }], "aspas": [] @@ -205,4 +209,4 @@ Routinator will report the resources used to sign the object: - `A proof-of-concept for constructing and validating RTAs `_ -.. versionadded:: 0.8.0 \ No newline at end of file +.. versionadded:: 0.8.0 diff --git a/doc/manual/source/manual-page.rst b/doc/manual/source/manual-page.rst index f7f97a0..fdb751b 100644 --- a/doc/manual/source/manual-page.rst +++ b/doc/manual/source/manual-page.rst @@ -542,9 +542,11 @@ These can be requested by providing different commands on the command line. For RPKI objects, *tal* provides the name of the trust anchor locator the object was published under, *uri* provides the rsync URI of the ROA or router certificate, - *validity* provides the validity of the ROA itself, and + *validity* provides the validity of the ROA itself, *chainValidity* the validity considering the validity of - the certificates along the validation chain. + the certificates along the validation chain, and + *stale* the time when any of the publication points along + the validation chain becomes stale. For assertions from local exceptions, *path* will provide the path of the local exceptions file and, optionally, diff --git a/doc/manual/source/output-formats.rst b/doc/manual/source/output-formats.rst index 1509091..d1fb3a8 100644 --- a/doc/manual/source/output-formats.rst +++ b/doc/manual/source/output-formats.rst @@ -206,9 +206,10 @@ generated in a wide range of output formats for various use cases. For RPKI objects, *tal* provides the name of the trust anchor locator the object was published under, *uri* provides the rsync URI of the ROA or router certificate, *validity* provides the - validity of the ROA itself, and *chainValidity* the validity + validity of the ROA itself, *chainValidity* the validity considering the validity of the certificates along the validation - chain. + chain, and *stale* the time when any of the publication points along + the validation chain becomes stale. For assertions from local exceptions, *path* will provide the path of the local exceptions file and, optionally, *comment* will @@ -244,8 +245,9 @@ generated in a wide range of output formats for various use cases. }, "chainValidity": { "notBefore": "2022-07-25T20:47:37Z", - "notAfter": "2023-02-24T12:31:01Z" - } + "notAfter": "2022-07-26T00:00:00Z" + }, + "stale": "2022-07-26T00:00:00Z" }] } ], @@ -263,8 +265,9 @@ generated in a wide range of output formats for various use cases. }, "chainValidity": { "notBefore": "2022-07-25T20:47:37Z", - "notAfter": "2023-02-24T12:31:01Z" - } + "notAfter": "2022-07-26T00:00:00Z" + }, + "stale": "2022-07-26T00:00:00Z" }] }], "aspas": [{ @@ -281,8 +284,9 @@ generated in a wide range of output formats for various use cases. }, "chainValidity": { "notBefore": "2023-04-18T14:32:13Z", - "notAfter": "2024-04-11T07:26:24Z" - } + "notAfter": "2023-04-20T00:00:00Z" + }, + "stale": "2022-07-26T00:00:00Z" }] }] } diff --git a/src/engine.rs b/src/engine.rs index b5a86d2..9c141e1 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -18,7 +18,7 @@ /// the accompanying trait [`ProcessPubPoint`] dealing with individual /// publication points. -use std::{fmt, fs, thread}; +use std::{cmp, fmt, fs, thread}; use std::borrow::Cow; use std::collections::HashMap; use std::fs::File; @@ -705,7 +705,12 @@ impl<'a, P: ProcessRun> PubPoint<'a, P> { } }; - // The manifest is fine, so we can now look at the objects. The + // The manifest is fine, so we can continue. + // + // First, report its validity to the processor. + collected.point_validity(&mut self.processor); + + // We can look at the objects now. The // objects are fine if they are present and match the hash. If they // don’t we have to cancel the update. We also validate them while we // are at it. This also collects all the child CAs that need @@ -713,7 +718,7 @@ impl<'a, P: ProcessRun> PubPoint<'a, P> { // // However, the processor can decide it doesn’t like the publication // point at all. This is not an error -- the publication point is - // correct from a store perspective --, but we must not process te + // correct from a store perspective --, but we must not process the // collected `ca_tasks`. We keep track of this through `point_ok` and, // if that happens to end up being `false` return an empty list to // signal that the publication point was processed successfully but @@ -1029,6 +1034,8 @@ impl<'a, P: ProcessRun> PubPoint<'a, P> { } }; + manifest.point_validity(&mut self.processor); + let mut ca_tasks = Vec::new(); for object in &mut store { let object = match object { @@ -1332,13 +1339,12 @@ impl<'a, P: ProcessRun> PubPoint<'a, P> { manifest.metrics.valid_ca_certs += 1; - let mut processor = match self.processor.process_ca( + let processor = match self.processor.process_ca( uri, &cert )? { Some(processor) => processor, None => return Ok(()) }; - processor.update_refresh(cert.cert().validity().not_after()); // Defer operation if we need to update the repository part where // the CA lives. @@ -1543,6 +1549,17 @@ impl ValidPointManifest { Ok(()) } + + /// Reports the validity to the given processor. + fn point_validity(&self, processor: &mut impl ProcessPubPoint) { + processor.point_validity( + self.ee_cert.validity(), + cmp::min( + self.content.next_update(), + self.crl.next_update(), + ) + ) + } } @@ -1610,6 +1627,9 @@ struct CaTask

{ //------------ CaCert -------------------------------------------------------- /// A CA certificate plus references to all its parents. +/// +/// Note that this does not represent the full publication point, only the +/// certificate itself. #[derive(Clone, Debug)] pub struct CaCert { /// The CA certificate of this CA. @@ -1640,12 +1660,6 @@ pub struct CaCert { /// The index of the TAL in the metrics. pub(crate) // XXX tal: usize, - - /// The combined validity of the certificate. - /// - /// This is derived from the validity of all the parents and the - /// certificate itself. - combined_validity: Validity, } impl CaCert { @@ -1695,10 +1709,6 @@ impl CaCert { chain_len: usize, tal: usize, ) -> Result, Failed> { - let combined_validity = match parent.as_ref() { - Some(ca) => cert.validity().trim(ca.combined_validity()), - None => cert.validity() - }; let ca_repository = match cert.ca_repository() { Some(uri) => uri.clone(), None => { @@ -1727,8 +1737,7 @@ impl CaCert { } }; Ok(Arc::new(CaCert { - cert, uri, ca_repository, rpki_manifest, parent, chain_len, tal, - combined_validity, + cert, uri, ca_repository, rpki_manifest, parent, chain_len, tal })) } @@ -1783,11 +1792,6 @@ impl CaCert { self.cert.rpki_notify() } - /// Returns the combined validaty of the whole CA. - pub fn combined_validity(&self) -> Validity { - self.combined_validity - } - /// Returns whether the CA is in a different repository from its parent. /// /// This is just a quick check and may report a switch when in fact there @@ -1947,9 +1951,17 @@ pub trait ProcessPubPoint: Sized + Send + Sync { let _ = repository_index; } - /// Updates the refresh time for this publication poont. - fn update_refresh(&mut self, not_after: Time) { - let _ = not_after; + /// Process the publication points’s validity. + /// + /// The validity of the manifest’s EE certificate is provided in + /// `manifest_ee`. The smaller of the manifest’s and CRL’s next update + /// time is given in `stale`. + fn point_validity( + &mut self, + manifest_ee: Validity, + stale: Time, + ) { + let _ = (manifest_ee, stale); } /// Determines whether an object with the given URI should be processed. diff --git a/src/output.rs b/src/output.rs index d682dc1..d97449c 100644 --- a/src/output.rs +++ b/src/output.rs @@ -1020,13 +1020,15 @@ impl ExtendedJson { \"validity\": {{ \"notBefore\": \"{}\", \ \"notAfter\": \"{}\" }}, \ \"chainValidity\": {{ \"notBefore\": \"{}\", \ - \"notAfter\": \"{}\" }} \ + \"notAfter\": \"{}\" }}, \ + \"stale\": \"{}\" \ }}", json_str(roa.tal.name()), format_iso_date(roa.roa_validity.not_before().into()), format_iso_date(roa.roa_validity.not_after().into()), format_iso_date(roa.chain_validity.not_before().into()), format_iso_date(roa.chain_validity.not_after().into()), + format_iso_date(roa.point_stale.into()), )?; } if let Some(exc) = item.exception_info() { diff --git a/src/payload/info.rs b/src/payload/info.rs index 9189af1..54f3301 100644 --- a/src/payload/info.rs +++ b/src/payload/info.rs @@ -6,8 +6,7 @@ use std::sync::Arc; use rpki::uri; use rpki::repository::cert::{Cert, ResourceCert}; use rpki::repository::tal::TalInfo; -use rpki::repository::x509::Validity; -use crate::engine::CaCert; +use rpki::repository::x509::{Validity, Time}; use crate::slurm::ExceptionInfo; //------------ PayloadInfo --------------------------------------------------- @@ -144,11 +143,18 @@ pub struct PublishInfo { /// The validity of the validation chain. pub chain_validity: Validity, + + /// When will the object’s publication point become stale? + pub point_stale: Time, } impl PublishInfo { /// Creates a new origin info from the EE certificate of a ROA. - pub fn signed_object(cert: &ResourceCert, ca_validity: Validity) -> Self { + pub fn signed_object( + cert: &ResourceCert, + ca_validity: Validity, + point_stale: Time, + ) -> Self { PublishInfo { tal: cert.tal().clone(), uri: cert.signed_object().cloned().map(|mut uri| { @@ -156,17 +162,23 @@ impl PublishInfo { }), roa_validity: cert.validity(), chain_validity: cert.validity().trim(ca_validity), + point_stale, } } pub fn router_cert( - cert: &Cert, uri: &uri::Rsync, ca_cert: &CaCert + cert: &Cert, + uri: &uri::Rsync, + tal: Arc, + ca_validity: Validity, + point_stale: Time, ) -> Self { PublishInfo { - tal: ca_cert.cert().tal().clone(), + tal, uri: Some(uri.clone()), roa_validity: cert.validity(), - chain_validity: cert.validity().trim(ca_cert.combined_validity()) + chain_validity: cert.validity().trim(ca_validity), + point_stale, } } diff --git a/src/payload/validation.rs b/src/payload/validation.rs index c6fa567..e752f4c 100644 --- a/src/payload/validation.rs +++ b/src/payload/validation.rs @@ -133,7 +133,8 @@ impl<'a> ProcessRun for &'a ValidationReport { PubPointProcessor { report: self, pub_point: PubPoint::new_ta(cert, tal_index), - validity: cert.combined_validity(), + validity: cert.cert().validity(), + point_stale: cert.cert().validity().not_after(), } )) } @@ -155,8 +156,11 @@ pub struct PubPointProcessor<'a> { /// The data being collected. pub_point: PubPoint, - /// The (combined) validity of the CA certificate. + /// The (combined) validity of the publication point. validity: Validity, + + /// When will this publication point become stale. + point_stale: Time, } impl<'a> ProcessPubPoint for PubPointProcessor<'a> { @@ -164,10 +168,13 @@ impl<'a> ProcessPubPoint for PubPointProcessor<'a> { self.pub_point.repository_index = Some(repository_index) } - fn update_refresh(&mut self, not_after: Time) { + fn point_validity(&mut self, manifest: Validity, stale: Time) { self.pub_point.refresh = cmp::min( - self.pub_point.refresh, not_after + self.pub_point.refresh, + cmp::min(manifest.not_after(), stale) ); + self.validity = self.validity.trim(manifest); + self.point_stale = cmp::min(self.point_stale, stale); } fn want(&self, _uri: &uri::Rsync) -> Result { @@ -183,7 +190,10 @@ impl<'a> ProcessPubPoint for PubPointProcessor<'a> { PubPointProcessor { report: self.report, pub_point: PubPoint::new_ca(&self.pub_point, cert), - validity: cert.combined_validity(), + validity: self.validity.trim(cert.cert().validity()), + point_stale: cmp::min( + self.point_stale, cert.cert().validity().not_after() + ), } )) } @@ -233,7 +243,10 @@ impl<'a> ProcessPubPoint for PubPointProcessor<'a> { self.pub_point.update_refresh(cert.validity().not_after()); self.pub_point.add_router_key( asns, id, key, - Arc::new(PublishInfo::router_cert(&cert, uri, ca_cert)), + Arc::new(PublishInfo::router_cert( + &cert, uri, ca_cert.cert().tal().clone(), + self.validity, self.point_stale, + )), ); Ok(()) } @@ -245,7 +258,10 @@ impl<'a> ProcessPubPoint for PubPointProcessor<'a> { route: RouteOriginAttestation ) -> Result<(), Failed> { if self.pub_point.add_roa( - route, Arc::new(PublishInfo::signed_object(&cert, self.validity)), + route, + Arc::new(PublishInfo::signed_object( + &cert, self.validity, self.point_stale + )), self.report.limit_v4_len, self.report.limit_v6_len, ) { self.pub_point.update_refresh(cert.validity().not_after()); @@ -264,7 +280,10 @@ impl<'a> ProcessPubPoint for PubPointProcessor<'a> { } self.pub_point.update_refresh(cert.validity().not_after()); self.pub_point.add_aspa( - aspa, Arc::new(PublishInfo::signed_object(&cert, self.validity)) + aspa, + Arc::new(PublishInfo::signed_object( + &cert, self.validity, self.point_stale + )) ); Ok(()) }