From bb10f00d0e6d34d859e9da2de182a44bb4ca8fbc Mon Sep 17 00:00:00 2001 From: Tim Bruijnzeels Date: Tue, 2 Nov 2021 16:23:45 +0100 Subject: [PATCH] Force re-issuance of ROAs on upgrade from before 0.9.3-rc2 (#704) --- src/commons/api/history.rs | 5 +++++ src/commons/error.rs | 12 ++++++++++++ src/daemon/ca/certauth.rs | 28 +++++++++++++++++++++------- src/daemon/ca/commands.rs | 4 ++++ src/daemon/ca/events.rs | 4 ++-- src/daemon/ca/manager.rs | 18 ++++++++++++++++++ src/daemon/ca/rc.rs | 9 +++++++-- src/daemon/ca/routes.rs | 5 +++-- src/daemon/http/server.rs | 12 +++++++----- src/daemon/krillserver.rs | 5 +++++ src/upgrades/mod.rs | 31 ++++++++++++++++++++++--------- 11 files changed, 106 insertions(+), 27 deletions(-) diff --git a/src/commons/api/history.rs b/src/commons/api/history.rs index 89056007..2ebb6e38 100644 --- a/src/commons/api/history.rs +++ b/src/commons/api/history.rs @@ -469,6 +469,7 @@ pub enum StorableCaCommand { updates: RoaDefinitionUpdates, }, ReissueBeforeExpiring, + ForceReissue, RepoUpdate { service_uri: ServiceUri, }, @@ -574,6 +575,7 @@ impl WithStorableDetails for StorableCaCommand { } StorableCaCommand::ReissueBeforeExpiring => CommandSummary::new("cmd-ca-reissue-before-expiring", &self), + StorableCaCommand::ForceReissue => CommandSummary::new("cmd-ca-force-reissue", &self), // RTA StorableCaCommand::RtaPrepare { name } => { @@ -719,6 +721,9 @@ impl fmt::Display for StorableCaCommand { StorableCaCommand::ReissueBeforeExpiring => { write!(f, "Automatically re-issue objects before they would expire") } + StorableCaCommand::ForceReissue => { + write!(f, "Force re-issuance of objects") + } // ------------------------------------------------------------ // Publishing diff --git a/src/commons/error.rs b/src/commons/error.rs index 29af3f3e..00304d9e 100644 --- a/src/commons/error.rs +++ b/src/commons/error.rs @@ -23,6 +23,7 @@ use crate::{ util::{httpclient, softsigner::SignerError}, }, daemon::{ca::RouteAuthorization, http::tls_keys}, + upgrades::UpgradeError, }; #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] @@ -168,6 +169,7 @@ pub enum Error { HttpsSetup(String), HttpClientError(httpclient::Error), ConfigError(String), + UpgradeError(UpgradeError), //----------------------------------------------------------------- // General API Client Issues @@ -315,6 +317,7 @@ impl fmt::Display for Error { Error::HttpsSetup(e) => write!(f, "Cannot set up HTTPS: {}", e), Error::HttpClientError(e) => write!(f, "HTTP client error: {}", e), Error::ConfigError(e) => write!(f, "Configuration error: {}", e), + Error::UpgradeError(e) => write!(f, "Could not upgrade Krill: {}", e), //----------------------------------------------------------------- // General API Client Issues @@ -542,6 +545,12 @@ impl From for Error { } } +impl From for Error { + fn from(e: UpgradeError) -> Self { + Error::UpgradeError(e) + } +} + impl Error { pub fn signer(e: impl Display) -> Self { Error::SignerError(e.to_string()) @@ -622,6 +631,9 @@ impl Error { // internal configuration error Error::ConfigError(e) => ErrorResponse::new("sys-config", &self).with_cause(e), + // upgrade error + Error::UpgradeError(e) => ErrorResponse::new("sys-upgrade", &self).with_cause(e), + //----------------------------------------------------------------- // General API Client Issues (label: api-*) //----------------------------------------------------------------- diff --git a/src/daemon/ca/certauth.rs b/src/daemon/ca/certauth.rs index eac91841..d1a0f0c1 100644 --- a/src/daemon/ca/certauth.rs +++ b/src/daemon/ca/certauth.rs @@ -442,7 +442,12 @@ impl Aggregate for CertAuth { CmdDet::RouteAuthorizationsUpdate(updates, config, signer) => { self.route_authorizations_update(updates, &config, signer) } - CmdDet::RouteAuthorizationsRenew(config, signer) => self.route_authorizations_renew(&config, &signer), + CmdDet::RouteAuthorizationsRenew(config, signer) => { + self.route_authorizations_renew(false, &config, &signer) + } + CmdDet::RouteAuthorizationsForceRenew(config, signer) => { + self.route_authorizations_renew(true, &config, &signer) + } // Republish CmdDet::RepoUpdate(contact, signer) => self.update_repo(contact, &signer), @@ -1622,16 +1627,25 @@ impl CertAuth { } /// Renew existing ROA objects if needed. - pub fn route_authorizations_renew(&self, config: &Config, signer: &KrillSigner) -> KrillResult> { + pub fn route_authorizations_renew( + &self, + force: bool, + config: &Config, + signer: &KrillSigner, + ) -> KrillResult> { let mut evt_dets = vec![]; for (rcn, rc) in self.resources.iter() { - let updates = rc.renew_roas(&config.issuance_timing, signer)?; + let updates = rc.renew_roas(force, &config.issuance_timing, signer)?; if updates.contains_changes() { - info!( - "CA '{}' reissued ROAs under RC '{}' before they would expire: {}", - self.handle, rcn, updates - ); + if force { + info!("CA '{}' reissued all ROAs under RC '{}'", self.handle, rcn); + } else { + info!( + "CA '{}' reissued ROAs under RC '{}' before they would expire: {}", + self.handle, rcn, updates + ); + } evt_dets.push(CaEvtDet::RoasUpdated { resource_class_name: rcn.clone(), diff --git a/src/daemon/ca/commands.rs b/src/daemon/ca/commands.rs index 861c680a..0d9fdf71 100644 --- a/src/daemon/ca/commands.rs +++ b/src/daemon/ca/commands.rs @@ -145,6 +145,9 @@ pub enum CmdDet { // will only be stored if there any updates to be done. RouteAuthorizationsRenew(Arc, Arc), + // Re-issue all ROA objects regardless of their expiration time. + RouteAuthorizationsForceRenew(Arc, Arc), + // ------------------------------------------------------------ // Publishing // ------------------------------------------------------------ @@ -269,6 +272,7 @@ impl From for StorableCaCommand { updates: updates.into(), }, CmdDet::RouteAuthorizationsRenew(_, _) => StorableCaCommand::ReissueBeforeExpiring, + CmdDet::RouteAuthorizationsForceRenew(_, _) => StorableCaCommand::ForceReissue, // ------------------------------------------------------------ // Publishing diff --git a/src/daemon/ca/events.rs b/src/daemon/ca/events.rs index 2aac5112..ad2fe7f1 100644 --- a/src/daemon/ca/events.rs +++ b/src/daemon/ca/events.rs @@ -341,7 +341,7 @@ impl RoaUpdates { impl fmt::Display for RoaUpdates { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { if !self.updated.is_empty() { - write!(f, "Added single VRP ROAs: ")?; + write!(f, "Updated single VRP ROAs: ")?; for roa in self.updated.keys() { write!(f, "{} ", ObjectName::from(roa))?; } @@ -353,7 +353,7 @@ impl fmt::Display for RoaUpdates { } } if !self.aggregate_updated.is_empty() { - write!(f, "Added ASN aggregated ROAs: ")?; + write!(f, "Updated ASN aggregated ROAs: ")?; for roa in self.aggregate_updated.keys() { write!(f, "{} ", ObjectName::from(roa))?; } diff --git a/src/daemon/ca/manager.rs b/src/daemon/ca/manager.rs index a7b938a8..f7d4649b 100644 --- a/src/daemon/ca/manager.rs +++ b/src/daemon/ca/manager.rs @@ -1809,6 +1809,24 @@ impl CaManager { } Ok(()) } + + /// Force the reissuance of all ROAs in all CAs. This function was added + /// because we need to re-issue ROAs in Krill 0.9.3 to force that a short + /// subject CN is used for the EE certificate: i.e. the SKI rather than the + /// full public key. But there may also be other cases in future where + /// forcing to re-issue ROAs may be useful. + pub async fn force_renew_roas_all(&self, actor: &Actor) -> KrillResult<()> { + for ca in self.ca_store.list()? { + let cmd = Cmd::new( + &ca, + None, + CmdDet::RouteAuthorizationsForceRenew(self.config.clone(), self.signer.clone()), + actor, + ); + self.send_command(cmd).await?; + } + Ok(()) + } } /// # Resource Tagged Attestation functions diff --git a/src/daemon/ca/rc.rs b/src/daemon/ca/rc.rs index 96088268..75f94ca7 100644 --- a/src/daemon/ca/rc.rs +++ b/src/daemon/ca/rc.rs @@ -647,9 +647,14 @@ impl ResourceClass { impl ResourceClass { /// Renew all ROAs under the current for which the not-after time closer /// than the given number of weeks - pub fn renew_roas(&self, issuance_timing: &IssuanceTimingConfig, signer: &KrillSigner) -> KrillResult { + pub fn renew_roas( + &self, + force: bool, + issuance_timing: &IssuanceTimingConfig, + signer: &KrillSigner, + ) -> KrillResult { let key = self.get_current_key()?; - self.roas.renew(key, issuance_timing, signer) + self.roas.renew(force, key, issuance_timing, signer) } /// Publish all ROAs under the new key diff --git a/src/daemon/ca/routes.rs b/src/daemon/ca/routes.rs index a1697d68..6723e0a4 100644 --- a/src/daemon/ca/routes.rs +++ b/src/daemon/ca/routes.rs @@ -657,6 +657,7 @@ impl Roas { /// Re-new ROAs before they would expire pub fn renew( &self, + force: bool, certified_key: &CertifiedKey, issuance_timing: &IssuanceTimingConfig, signer: &KrillSigner, @@ -667,7 +668,7 @@ impl Roas { for (auth, roa_info) in self.simple.iter() { let name = ObjectName::from(auth); - if roa_info.expires() < renew_threshold { + if force || roa_info.expires() < renew_threshold { let roa = Self::make_roa( &[*auth], &name, @@ -684,7 +685,7 @@ impl Roas { for (roa_key, aggregate) in self.aggregate.iter() { let roa_info = aggregate.roa_info(); - if roa_info.expires() < renew_threshold { + if force || roa_info.expires() < renew_threshold { let authorizations = aggregate.authorizations().clone(); let name = ObjectName::from(roa_key); let new_roa = Self::make_roa( diff --git a/src/daemon/http/server.rs b/src/daemon/http/server.rs index e7a6e6d3..d8236487 100644 --- a/src/daemon/http/server.rs +++ b/src/daemon/http/server.rs @@ -45,7 +45,7 @@ use crate::{ }, krillserver::KrillServer, }, - upgrades::{pre_start_upgrade, update_storage_version}, + upgrades::{post_start_upgrade, pre_start_upgrade, update_storage_version}, }; use hyper::{ header::HeaderName, @@ -115,15 +115,17 @@ pub async fn start_krill_daemon(config: Arc) -> Result<(), Error> { test_data_dirs_or_die(&config); // Call upgrade, this will only do actual work if needed. - pre_start_upgrade(config.clone()).map_err(|e| Error::Custom(format!("Could not upgrade Krill: {}", e)))?; + pre_start_upgrade(config.clone())?; // Create the server, this will create the necessary data sub-directories if needed let krill = KrillServer::build(config.clone()).await?; + // Call post-start upgrades to trigger any upgrade related runtime actions, such as + // re-issuing ROAs because subject name strategy has changed. + post_start_upgrade(&config, &krill).await?; + // Update the version identifiers for the storage dirs - update_storage_version(&config.data_dir) - .map_err(|e| Error::Custom(format!("Could not upgrade Krill: {}", e))) - .await?; + update_storage_version(&config.data_dir).await?; // If the operator wanted to do the upgrade only, now is a good time to report success and stop if env::var(KRILL_ENV_UPGRADE_ONLY).is_ok() { diff --git a/src/daemon/krillserver.rs b/src/daemon/krillserver.rs index 39f00b95..eba5bf95 100644 --- a/src/daemon/krillserver.rs +++ b/src/daemon/krillserver.rs @@ -699,6 +699,11 @@ impl KrillServer { .suggest(definitions.as_slice(), &resources_held, limit) .await) } + + /// Re-issue ROA objects so that they will use short subjects (see issue #700) + pub async fn force_renew_roas(&self) -> KrillResult<()> { + self.ca_manager.force_renew_roas_all(self.system_actor()).await + } } /// # Handle publication requests diff --git a/src/upgrades/mod.rs b/src/upgrades/mod.rs index 8b6b3d97..7cc60350 100644 --- a/src/upgrades/mod.rs +++ b/src/upgrades/mod.rs @@ -14,7 +14,7 @@ use crate::{ eventsourcing::{AggregateStoreError, CommandKey, KeyStoreKey, KeyValueError, KeyValueStore}, util::{file, KrillVersion}, }, - daemon::config::Config, + daemon::{config::Config, krillserver::KrillServer}, pubd::RepositoryManager, upgrades::v0_9_0::{CaObjectsMigration, PubdObjectsMigration}, }; @@ -34,7 +34,6 @@ pub enum UpgradeError { IoError(KrillIoError), Unrecognised(String), CannotLoadAggregate(Handle), - KrillError(crate::commons::error::Error), Custom(String), } @@ -46,7 +45,6 @@ impl fmt::Display for UpgradeError { UpgradeError::IoError(e) => e.fmt(f), UpgradeError::Unrecognised(s) => write!(f, "Unrecognised command summary: {}", s), UpgradeError::CannotLoadAggregate(handle) => write!(f, "Cannot load: {}", handle), - UpgradeError::KrillError(e) => e.fmt(f), UpgradeError::Custom(s) => s.fmt(f), } } @@ -81,7 +79,7 @@ impl From for UpgradeError { impl From for UpgradeError { fn from(e: crate::commons::error::Error) -> Self { - UpgradeError::KrillError(e) + UpgradeError::Custom(e.to_string()) } } @@ -160,7 +158,18 @@ pub trait UpgradeStore { /// Should be called when Krill starts, before the KrillServer is initiated pub fn pre_start_upgrade(config: Arc) -> Result<(), UpgradeError> { - upgrade_0_9_0(config) + upgrade_data_to_0_9_0(config) +} + +/// Should be called when the KrillServer is initiated, before the webserver is started +/// and operators can make changes. +pub async fn post_start_upgrade(config: &Config, server: &KrillServer) -> Result<(), UpgradeError> { + if needs_upgrade(&config.data_dir, "cas", KrillVersion::candidate(0, 9, 3, 2)) { + info!("Reissue ROAs on upgrade to force short EE certificate subjects in the objects"); + server.force_renew_roas().await.map_err(|e| e.into()) + } else { + Ok(()) + } } pub async fn update_storage_version(work_dir: &Path) -> Result<(), UpgradeError> { @@ -179,7 +188,7 @@ pub async fn update_storage_version(work_dir: &Path) -> Result<(), UpgradeError> Ok(()) } -fn upgrade_0_9_0(config: Arc) -> Result<(), UpgradeError> { +fn upgrade_data_to_0_9_0(config: Arc) -> Result<(), UpgradeError> { let work_dir = &config.data_dir; if needs_v0_9_0_upgrade(work_dir, "pubd") { PubdObjectsMigration::migrate(config.clone())?; @@ -196,11 +205,15 @@ fn upgrade_0_9_0(config: Arc) -> Result<(), UpgradeError> { } fn needs_v0_9_0_upgrade(work_dir: &Path, ns: &str) -> bool { + needs_upgrade(work_dir, ns, KrillVersion::release(0, 9, 0)) +} + +fn needs_upgrade(work_dir: &Path, ns: &str, before: KrillVersion) -> bool { let keystore_path = work_dir.join(ns); if keystore_path.exists() { let version_path = keystore_path.join("version"); let version_found = file::load_json(&version_path).unwrap_or_else(|_| KrillVersion::v0_5_0_or_before()); - version_found < KrillVersion::release(0, 9, 0) + version_found < before } else { false } @@ -227,7 +240,7 @@ mod tests { let config = Arc::new(Config::test(&work_dir, false, false, false)); let _ = config.init_logging(); - upgrade_0_9_0(config).unwrap(); + upgrade_data_to_0_9_0(config).unwrap(); let _ = fs::remove_dir_all(work_dir); } @@ -241,7 +254,7 @@ mod tests { let config = Arc::new(Config::test(&work_dir, false, false, false)); let _ = config.init_logging(); - upgrade_0_9_0(config).unwrap(); + upgrade_data_to_0_9_0(config).unwrap(); let _ = fs::remove_dir_all(work_dir); }