diff --git a/src/daemon/ca/certauth.rs b/src/daemon/ca/certauth.rs index b814be29..740670d1 100644 --- a/src/daemon/ca/certauth.rs +++ b/src/daemon/ca/certauth.rs @@ -414,7 +414,7 @@ impl Aggregate for CertAuth { // Key rolls CmdDet::KeyRollInitiate(duration, signer) => self.keyroll_initiate(duration, signer), - CmdDet::KeyRollActivate(duration, signer) => self.keyroll_activate(duration, signer), + CmdDet::KeyRollActivate(duration, config, signer) => self.keyroll_activate(duration, config, signer), CmdDet::KeyRollFinish(rcn, response) => self.keyroll_finish(rcn, response), // Route Authorizations @@ -1234,7 +1234,12 @@ impl CertAuth { Ok(res) } - fn keyroll_activate(&self, staging_time: Duration, signer: Arc) -> KrillResult> { + fn keyroll_activate( + &self, + staging_time: Duration, + config: Arc, + signer: Arc, + ) -> KrillResult> { if self.is_ta() { return Ok(vec![]); } @@ -1245,7 +1250,10 @@ impl CertAuth { for (rcn, rc) in self.resources.iter() { let mut activated = false; - for details in rc.keyroll_activate(staging_time, signer.deref())?.into_iter() { + for details in rc + .keyroll_activate(staging_time, &config.issuance_timing, signer.deref())? + .into_iter() + { activated = true; res.push(StoredEvent::new(self.handle(), version, details)); version += 1; diff --git a/src/daemon/ca/commands.rs b/src/daemon/ca/commands.rs index 8d9deebb..3632fdde 100644 --- a/src/daemon/ca/commands.rs +++ b/src/daemon/ca/commands.rs @@ -106,7 +106,7 @@ pub enum CmdDet { // // RFC6489 dictates that 24 hours MUST be observed. However, shorter time frames can // be used for testing, and in case of emergency rolls. - KeyRollActivate(Duration, Arc), + KeyRollActivate(Duration, Arc, Arc), // Finish the keyroll after the parent confirmed that a key for a parent and resource // class has been revoked. I.e. remove the old key, and withdraw the crl and mft for it. @@ -233,7 +233,7 @@ impl From for StorableCaCommand { CmdDet::KeyRollInitiate(older_than, _) => StorableCaCommand::KeyRollInitiate { older_than_seconds: older_than.num_seconds(), }, - CmdDet::KeyRollActivate(staged_for, _) => StorableCaCommand::KeyRollActivate { + CmdDet::KeyRollActivate(staged_for, _, _) => StorableCaCommand::KeyRollActivate { staged_for_seconds: staged_for.num_seconds(), }, CmdDet::KeyRollFinish(resource_class_name, _) => StorableCaCommand::KeyRollFinish { resource_class_name }, @@ -391,8 +391,14 @@ impl CmdDet { eventsourcing::SentCommand::new(handle, None, CmdDet::KeyRollInitiate(duration, signer), actor) } - pub fn key_roll_activate(handle: &Handle, staging: Duration, signer: Arc, actor: &Actor) -> Cmd { - eventsourcing::SentCommand::new(handle, None, CmdDet::KeyRollActivate(staging, signer), actor) + pub fn key_roll_activate( + handle: &Handle, + staging: Duration, + config: Arc, + signer: Arc, + actor: &Actor, + ) -> Cmd { + eventsourcing::SentCommand::new(handle, None, CmdDet::KeyRollActivate(staging, config, signer), actor) } pub fn key_roll_finish(handle: &Handle, rcn: ResourceClassName, res: RevocationResponse, actor: &Actor) -> Cmd { diff --git a/src/daemon/ca/keys.rs b/src/daemon/ca/keys.rs index 128d837b..2a7e529e 100644 --- a/src/daemon/ca/keys.rs +++ b/src/daemon/ca/keys.rs @@ -524,9 +524,12 @@ impl KeyState { } } - /// Returns true if there is a new key - pub fn has_new_key(&self) -> bool { - matches!(self, KeyState::RollNew(_, _)) + /// Returns the new key, iff there is a key roll in progress and there is a new key. + pub fn new_key(&self) -> Option<&CertifiedKey> { + match self { + KeyState::RollNew(new, _) => Some(new), + _ => None, + } } fn knows_key(&self, key_id: KeyIdentifier) -> bool { diff --git a/src/daemon/ca/manager.rs b/src/daemon/ca/manager.rs index 575395c1..be892fbd 100644 --- a/src/daemon/ca/manager.rs +++ b/src/daemon/ca/manager.rs @@ -1571,7 +1571,7 @@ impl CaManager { /// a staging period of 24 hours, but we may use a shorter period for testing and/or emergency /// manual key rolls. pub async fn ca_keyroll_activate(&self, handle: Handle, staging: Duration, actor: &Actor) -> KrillResult<()> { - let activate_cmd = CmdDet::key_roll_activate(&handle, staging, self.signer.clone(), actor); + let activate_cmd = CmdDet::key_roll_activate(&handle, staging, self.config.clone(), self.signer.clone(), actor); self.send_command(activate_cmd).await?; Ok(()) } diff --git a/src/daemon/ca/rc.rs b/src/daemon/ca/rc.rs index b783c9d2..2eac5226 100644 --- a/src/daemon/ca/rc.rs +++ b/src/daemon/ca/rc.rs @@ -450,18 +450,42 @@ impl ResourceClass { } /// Activate a new key, if it's been longer than the staging period. - pub fn keyroll_activate(&self, staging_time: Duration, signer: &KrillSigner) -> KrillResult> { - if !self.key_state.has_new_key() - || (staging_time > Duration::seconds(0) && self.last_key_change + staging_time > Time::now()) - { - return Ok(vec![]); - } + pub fn keyroll_activate( + &self, + staging_time: Duration, + issuance_timing: &IssuanceTimingConfig, + signer: &KrillSigner, + ) -> KrillResult> { + if let Some(new_key) = self.key_state.new_key() { + if staging_time > Duration::seconds(0) && self.last_key_change + staging_time > Time::now() { + Ok(vec![]) + } else { + let key_activated = + self.key_state + .keyroll_activate(self.name.clone(), self.parent_rc_name.clone(), signer)?; - Ok(vec![self.key_state.keyroll_activate( - self.name.clone(), - self.parent_rc_name.clone(), - signer, - )?]) + let roa_updates = self.roas.activate_key(new_key, issuance_timing, signer)?; + let roas_updated = CaEvtDet::RoasUpdated { + resource_class_name: self.name.clone(), + updates: roa_updates, + }; + + let mut cert_updates = ChildCertificateUpdates::default(); + for issued in self.certificates.iter() { + // re-issue + let re_issued = self.re_issue(issued, None, new_key, None, issuance_timing, signer)?; + cert_updates.issue(re_issued); + } + let certs_updated = CaEvtDet::ChildCertificatesUpdated { + resource_class_name: self.name.clone(), + updates: cert_updates, + }; + + Ok(vec![key_activated, roas_updated, certs_updated]) + } + } else { + Ok(vec![]) + } } /// Finish a key roll, withdraw the old key diff --git a/tests/functional.rs b/tests/functional.rs index 206bc052..7a4b0b3e 100644 --- a/tests/functional.rs +++ b/tests/functional.rs @@ -71,6 +71,13 @@ async fn expected_mft_and_crl(ca: &Handle, rcn: &ResourceClassName) -> Vec Vec { + let rc_key = ca_new_key_for_rcn(ca, rcn).await; + let mft_file = rc_key.incoming_cert().mft_name().to_string(); + let crl_file = rc_key.incoming_cert().crl_name().to_string(); + vec![mft_file, crl_file] +} + async fn expected_issued_cer(ca: &Handle, rcn: &ResourceClassName) -> String { let rc_key = ca_key_for_rcn(ca, rcn).await; ObjectName::from(rc_key.incoming_cert().cert()).to_string() @@ -78,7 +85,8 @@ async fn expected_issued_cer(ca: &Handle, rcn: &ResourceClassName) -> String { async fn will_publish(test_msg: &str, publisher: &PublisherHandle, files: &[String]) -> bool { let objects: Vec<_> = files.iter().map(|s| s.as_str()).collect(); - for _ in 0..6000 { + // for _ in 0..6000 { + for _ in 0..100 { let details = publisher_details(publisher).await; let current_files = details.current_files(); @@ -240,6 +248,7 @@ async fn functional() { let ca1 = handle_for("CA1"); let ca1_res = resources("10.0.0.0/16"); let ca1_res_reduced = resources("10.0.0.0/24"); + let ca1_route_definition = RoaDefinition::from_str("10.0.0.0/16-16 => 65000").unwrap(); let ca2 = handle_for("CA2"); let ca2_res = resources("10.1.0.0/16"); @@ -346,6 +355,31 @@ async fn functional() { assert!(will_publish("CA1 should publish the certificate for CA3", &ca1, &expected_files).await); } + { + info("##################################################################"); + info("# #"); + info("# Let CA1 publish a ROA (covering CA3 resources) #"); + info("# #"); + info("##################################################################"); + info(""); + + let mut updates = RoaDefinitionUpdates::empty(); + updates.add(ca1_route_definition); + ca_route_authorizations_update(&ca1, updates).await; + + let mut expected_files = expected_mft_and_crl(&ca1, &rcn_0).await; + expected_files.push(expected_issued_cer(&ca3, &rcn_0).await); + expected_files.push(ObjectName::from(&ca1_route_definition).to_string()); + assert!( + will_publish( + "CA1 should publish the certificate for CA3 and a ROA", + &ca1, + &expected_files + ) + .await + ); + } + { info("##################################################################"); info("# #"); @@ -431,8 +465,34 @@ async fn functional() { info(""); ca_roll_init(&ca1).await; assert!(state_becomes_new_key(&ca1).await); + + let mut expected_files = expected_mft_and_crl(&ca1, &rcn_0).await; + expected_files.push(expected_issued_cer(&ca3, &rcn_0).await); + expected_files.push(ObjectName::from(&ca1_route_definition).to_string()); + expected_files.append(&mut expected_new_key_mft_and_crl(&ca1, &rcn_0).await); + assert!( + will_publish( + "CA1 should publish MFT and CRL for both keys and the certificate for CA3 and a ROA", + &ca1, + &expected_files + ) + .await + ); + ca_roll_activate(&ca1).await; assert!(state_becomes_active(&ca1).await); + + let mut expected_files = expected_mft_and_crl(&ca1, &rcn_0).await; + expected_files.push(expected_issued_cer(&ca3, &rcn_0).await); + expected_files.push(ObjectName::from(&ca1_route_definition).to_string()); + assert!( + will_publish( + "CA1 should now publish MFT and CRL for the activated key only, and the certificate for CA3 and a ROA", + &ca1, + &expected_files + ) + .await + ); } //------------------------------------------------------------------------------------------ @@ -507,7 +567,7 @@ async fn functional() { { info("##################################################################"); info("# #"); - info("# Remove ROAs below the deaggregation threshold and we get #"); + info("# Remove ROAs below the de-aggregation threshold and we get #"); info("# separate files again #"); info("# #"); info("##################################################################"); @@ -542,7 +602,7 @@ async fn functional() { refresh_all().await; // if we skip this, then CA4 will not find out that it's resources were reduced expect_roas_for_ca4( - "CA4 resources are schrunk and we expect only one remaining roa", + "CA4 resources are shrunk and we expect only one remaining roa", &[route_rc1_1], ) .await; @@ -672,11 +732,13 @@ async fn functional() { // Expect that CA1 no longer publishes the certificate for CA3 // i.e. CA3 requested its revocation. { + let mut expected_files = expected_mft_and_crl(&ca1, &rcn_0).await; + expected_files.push(ObjectName::from(&ca1_route_definition).to_string()); assert!( will_publish( "CA1 should no longer publish the cer for CA3 after CA3 has been deleted", &ca1, - &expected_mft_and_crl(&ca1, &rcn_0).await + &expected_files ) .await ); diff --git a/tests/migrate_repository.rs b/tests/migrate_repository.rs index 7b927cfb..420c63b5 100644 --- a/tests/migrate_repository.rs +++ b/tests/migrate_repository.rs @@ -8,10 +8,9 @@ use tokio::time::delay_for; use rpki::uri::Rsync; -use krill::cli::report::ApiResponse; use krill::commons::api::{ Handle, ObjectName, ParentCaReq, ParentHandle, PublisherHandle, ResourceClassKeysInfo, ResourceClassName, - ResourceSet, + ResourceSet, RoaDefinitionUpdates, }; use krill::commons::remote::rfc8183; use krill::daemon::ca::ta_handle; @@ -20,6 +19,7 @@ use krill::{ cli::options::{CaCommand, Command, PublishersCommand}, commons::api::RepositoryContact, }; +use krill::{cli::report::ApiResponse, commons::api::RoaDefinition}; fn handle_for(s: &str) -> Handle { Handle::from_str(s).unwrap() @@ -249,6 +249,7 @@ async fn migrate_repository() { let ca1 = handle_for("CA1"); let ca1_res = resources("10.0.0.0/16"); + let ca1_route_definition = RoaDefinition::from_str("10.0.0.0/16-16 => 65000").unwrap(); let rcn_0 = ResourceClassName::from(0); @@ -287,6 +288,18 @@ async fn migrate_repository() { set_up_ca_under_parent_with_resources(&ca1, &testbed, &ca1_res).await; } + { + info("##################################################################"); + info("# #"); + info("# Create a ROA for CA1 #"); + info("# #"); + info("##################################################################"); + info(""); + let mut updates = RoaDefinitionUpdates::empty(); + updates.add(ca1_route_definition); + ca_route_authorizations_update(&ca1, updates).await; + } + { info("##################################################################"); info("# #"); @@ -313,7 +326,9 @@ async fn migrate_repository() { info("# #"); info("##################################################################"); info(""); - let expected_files = expected_mft_and_crl(&ca1, &rcn_0).await; + let mut expected_files = expected_mft_and_crl(&ca1, &rcn_0).await; + expected_files.push(ObjectName::from(&ca1_route_definition).to_string()); + assert!(will_publish_embedded("CA1 should publish the certificate for CA3", &ca1, &expected_files).await); } @@ -356,7 +371,9 @@ async fn migrate_repository() { // Expect that CA1 still publishes two current keys in the embedded repo { - let expected_files = expected_mft_and_crl(&ca1, &rcn_0).await; + let mut expected_files = expected_mft_and_crl(&ca1, &rcn_0).await; + expected_files.push(ObjectName::from(&ca1_route_definition).to_string()); + assert!( will_publish_embedded( "CA1 should publish the MFT and CRL for both current keys in the embedded repo", @@ -384,16 +401,11 @@ async fn migrate_repository() { ca_roll_activate(&ca1).await; assert!(state_becomes_active(&ca1).await); - // Expect that CA3 publishes nothing in the embedded repo - { - assert!( - will_publish_embedded("CA1 should no longer publish anything in the embedded repo", &ca1, &[]).await - ); - } - // Expect that CA1 publishes two current keys in the dedicated repo { - let expected_files = expected_mft_and_crl(&ca1, &rcn_0).await; + let mut expected_files = expected_mft_and_crl(&ca1, &rcn_0).await; + expected_files.push(ObjectName::from(&ca1_route_definition).to_string()); + assert!( will_publish_dedicated( "CA1 should publish the MFT and CRL for both current keys in the dedicated repo", @@ -403,6 +415,13 @@ async fn migrate_repository() { .await ); } + + // Expect that CA1 publishes nothing in the embedded repo + { + assert!( + will_publish_embedded("CA1 should no longer publish anything in the embedded repo", &ca1, &[]).await + ); + } } let _ = fs::remove_dir_all(krill_dir);