From 426eb154633f4be3371b51afd88cea9e80bf50ab Mon Sep 17 00:00:00 2001 From: Tim Bruijnzeels Date: Wed, 25 Oct 2023 10:17:55 +0200 Subject: [PATCH 1/2] Fix clippy warnings. --- src/commons/api/import.rs | 6 +----- src/daemon/ca/certauth.rs | 1 + src/upgrades/mod.rs | 7 ++++++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/commons/api/import.rs b/src/commons/api/import.rs index 30e29b103..062e697d5 100644 --- a/src/commons/api/import.rs +++ b/src/commons/api/import.rs @@ -217,11 +217,7 @@ pub struct ImportChildCertificate { impl fmt::Display for ImportChild { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { writeln!(f, "Name: {}", self.name)?; - writeln!( - f, - "Id Key: {}", - self.id_cert.public_key().key_identifier().to_string() - )?; + writeln!(f, "Id Key: {}", self.id_cert.public_key().key_identifier())?; writeln!(f, "Resources: {}", self.resources)?; if let Some(class_name) = &self.issued_cert.class_name { writeln!(f, "Classname: {}", class_name)?; diff --git a/src/daemon/ca/certauth.rs b/src/daemon/ca/certauth.rs index fac304f74..ab07ef717 100644 --- a/src/daemon/ca/certauth.rs +++ b/src/daemon/ca/certauth.rs @@ -947,6 +947,7 @@ impl CertAuth { self.child_certify(child_handle, child.resources(), my_rcn, csr_info, limit, config, signer) } + #[allow(clippy::too_many_arguments)] fn child_certify( &self, child_handle: ChildHandle, diff --git a/src/upgrades/mod.rs b/src/upgrades/mod.rs index acd27f5de..05dc99112 100644 --- a/src/upgrades/mod.rs +++ b/src/upgrades/mod.rs @@ -78,8 +78,13 @@ impl AspaMigrationConfigs { pub fn is_empty(&self) -> bool { self.0.is_empty() } +} + +impl std::iter::IntoIterator for AspaMigrationConfigs { + type Item = (CaHandle, Vec); + type IntoIter = std::collections::hash_map::IntoIter>; - pub fn into_iter(self) -> impl Iterator)> { + fn into_iter(self) -> Self::IntoIter { self.0.into_iter() } } From 18337c85bd35df179df718a76f09c91f98356814 Mon Sep 17 00:00:00 2001 From: Tim Bruijnzeels Date: Thu, 26 Oct 2023 12:32:47 +0200 Subject: [PATCH 2/2] Push impl Signer down to SignerProviders. --- .../crypto/signing/dispatch/krillsigner.rs | 112 ++++++++++++--- .../crypto/signing/dispatch/signerprovider.rs | 49 ++++--- .../crypto/signing/dispatch/signerrouter.rs | 131 ++++++++---------- .../crypto/signing/signers/mocksigner.rs | 2 +- src/test.rs | 6 +- src/upgrades/mod.rs | 7 +- 6 files changed, 183 insertions(+), 124 deletions(-) diff --git a/src/commons/crypto/signing/dispatch/krillsigner.rs b/src/commons/crypto/signing/dispatch/krillsigner.rs index b799414d0..d0d0c3d28 100644 --- a/src/commons/crypto/signing/dispatch/krillsigner.rs +++ b/src/commons/crypto/signing/dispatch/krillsigner.rs @@ -198,6 +198,7 @@ impl KrillSigner { pub fn create_key(&self) -> CryptoResult { self.router + .get_default_signer() .create_key(PublicKeyFormat::Rsa) .map_err(crypto::Error::signer) } @@ -208,47 +209,71 @@ impl KrillSigner { /// Creates a new self-signed (TA) IdCert pub fn create_self_signed_id_cert(&self) -> CryptoResult { - let key = self.create_key()?; + let signer = self.router.get_default_signer(); + + let key = signer.create_key(PublicKeyFormat::Rsa).map_err(crypto::Error::signer)?; + let validity = Validity::new( Time::five_minutes_ago(), Time::years_from_now(ID_CERTIFICATE_VALIDITY_YEARS), ); - IdCert::new_ta(validity, &key, &self.router).map_err(crypto::Error::signer) + IdCert::new_ta(validity, &key, signer.as_ref()).map_err(crypto::Error::signer) } pub fn destroy_key(&self, key_id: &KeyIdentifier) -> CryptoResult<()> { - self.router.destroy_key(key_id).map_err(crypto::Error::key_error) + let signer = self + .router + .get_signer_for_key(key_id) + .map_err(crypto::Error::key_error)?; + signer.destroy_key(key_id).map_err(crypto::Error::key_error) } pub fn get_key_info(&self, key_id: &KeyIdentifier) -> CryptoResult { - self.router.get_key_info(key_id).map_err(crypto::Error::key_error) + let signer = self + .router + .get_signer_for_key(key_id) + .map_err(crypto::Error::key_error)?; + + signer.get_key_info(key_id).map_err(crypto::Error::key_error) } pub fn random_serial(&self) -> CryptoResult { - Serial::random(&self.router).map_err(crypto::Error::signer) + Serial::random(self.router.get_default_signer().as_ref()).map_err(crypto::Error::signer) } pub fn sign + ?Sized>(&self, key_id: &KeyIdentifier, data: &D) -> CryptoResult { - self.router + let signer = self + .router + .get_signer_for_key(key_id) + .map_err(crypto::Error::key_error)?; + + signer .sign(key_id, RpkiSignatureAlgorithm::default(), data) .map_err(crypto::Error::signing) } pub fn sign_one_off + ?Sized>(&self, data: &D) -> CryptoResult<(RpkiSignature, PublicKey)> { - self.router + let signer = self.router.get_one_off_signer(); + + signer .sign_one_off(RpkiSignatureAlgorithm::default(), data) .map_err(crypto::Error::signer) } - pub fn sign_csr(&self, base_repo: &RepoInfo, name_space: &str, key: &KeyIdentifier) -> CryptoResult { - let signing_key_id = self.router.get_key_info(key).map_err(crypto::Error::key_error)?; + pub fn sign_csr(&self, base_repo: &RepoInfo, name_space: &str, key_id: &KeyIdentifier) -> CryptoResult { + let signer = self + .router + .get_signer_for_key(key_id) + .map_err(crypto::Error::key_error)?; + + let signing_key_id = signer.get_key_info(key_id).map_err(crypto::Error::key_error)?; let mft_file_name = ObjectName::mft_for_key(&signing_key_id.key_identifier()); // The rpki-rs library returns a signed and encoded CSR for a CA certificate. let signed_and_encoded_csr = Csr::construct_rpki_ca( - &self.router, - key, + signer.as_ref(), + key_id, &base_repo.ca_repository(name_space), &base_repo.resolve(name_space, mft_file_name.as_ref()), base_repo.rpki_notify(), @@ -260,11 +285,21 @@ impl KrillSigner { } pub fn sign_cert(&self, tbs: TbsCert, key_id: &KeyIdentifier) -> CryptoResult { - tbs.into_cert(&self.router, key_id).map_err(crypto::Error::signing) + let signer = self + .router + .get_signer_for_key(key_id) + .map_err(crypto::Error::key_error)?; + + tbs.into_cert(signer.as_ref(), key_id).map_err(crypto::Error::signing) } pub fn sign_crl(&self, tbs: TbsCertList>, key_id: &KeyIdentifier) -> CryptoResult { - tbs.into_crl(&self.router, key_id).map_err(crypto::Error::signing) + let signer = self + .router + .get_signer_for_key(key_id) + .map_err(crypto::Error::key_error)?; + + tbs.into_crl(signer.as_ref(), key_id).map_err(crypto::Error::signing) } pub fn sign_manifest( @@ -273,8 +308,13 @@ impl KrillSigner { builder: SignedObjectBuilder, key_id: &KeyIdentifier, ) -> CryptoResult { + let signer = self + .router + .get_signer_for_key(key_id) + .map_err(crypto::Error::key_error)?; + content - .into_manifest(builder, &self.router, key_id) + .into_manifest(builder, signer.as_ref(), key_id) .map_err(crypto::Error::signing) } @@ -284,8 +324,13 @@ impl KrillSigner { object_builder: SignedObjectBuilder, key_id: &KeyIdentifier, ) -> CryptoResult { + let signer = self + .router + .get_signer_for_key(key_id) + .map_err(crypto::Error::key_error)?; + roa_builder - .finalize(object_builder, &self.router, key_id) + .finalize(object_builder, signer.as_ref(), key_id) .map_err(crypto::Error::signing) } @@ -295,16 +340,26 @@ impl KrillSigner { object_builder: SignedObjectBuilder, key_id: &KeyIdentifier, ) -> CryptoResult { + let signer = self + .router + .get_signer_for_key(key_id) + .map_err(crypto::Error::key_error)?; + aspa_builder - .finalize(object_builder, &self.router, key_id) + .finalize(object_builder, signer.as_ref(), key_id) .map_err(crypto::Error::signing) } pub fn sign_rta(&self, rta_builder: &mut rta::RtaBuilder, ee: Cert) -> CryptoResult<()> { - let key = ee.subject_key_identifier(); + let key_id = ee.subject_key_identifier(); + let signer = self + .router + .get_signer_for_key(&key_id) + .map_err(crypto::Error::key_error)?; + rta_builder.push_cert(ee); rta_builder - .sign(&self.router, &key, None, None) + .sign(signer.as_ref(), &key_id, None, None) .map_err(crypto::Error::signing) } @@ -313,7 +368,12 @@ impl KrillSigner { message: provisioning::Message, signing_key: &KeyIdentifier, ) -> CryptoResult { - provisioning::ProvisioningCms::create(message, signing_key, &self.router).map_err(crypto::Error::signing) + let signer = self + .router + .get_signer_for_key(signing_key) + .map_err(crypto::Error::key_error)?; + + provisioning::ProvisioningCms::create(message, signing_key, signer.as_ref()).map_err(crypto::Error::signing) } pub fn create_rfc8181_cms( @@ -321,7 +381,12 @@ impl KrillSigner { message: publication::Message, signing_key: &KeyIdentifier, ) -> CryptoResult { - publication::PublicationCms::create(message, signing_key, &self.router).map_err(crypto::Error::signing) + let signer = self + .router + .get_signer_for_key(signing_key) + .map_err(crypto::Error::key_error)?; + + publication::PublicationCms::create(message, signing_key, signer.as_ref()).map_err(crypto::Error::signing) } pub fn create_ta_signed_message( @@ -330,9 +395,14 @@ impl KrillSigner { validity_days: i64, signing_key: &KeyIdentifier, ) -> CryptoResult { + let signer = self + .router + .get_signer_for_key(signing_key) + .map_err(crypto::Error::key_error)?; + let validity = SignSupport::sign_validity_days(validity_days); - SignedMessage::create(data, validity, signing_key, &self.router).map_err(crypto::Error::signing) + SignedMessage::create(data, validity, signing_key, signer.as_ref()).map_err(crypto::Error::signing) } } diff --git a/src/commons/crypto/signing/dispatch/signerprovider.rs b/src/commons/crypto/signing/dispatch/signerprovider.rs index b8750840c..c746c6b1e 100644 --- a/src/commons/crypto/signing/dispatch/signerprovider.rs +++ b/src/commons/crypto/signing/dispatch/signerprovider.rs @@ -1,6 +1,6 @@ use rpki::crypto::{ signer::{KeyError, SigningAlgorithm}, - KeyIdentifier, PublicKey, PublicKeyFormat, RpkiSignature, Signature, SignatureAlgorithm, SigningError, + KeyIdentifier, PublicKey, PublicKeyFormat, RpkiSignature, Signature, SignatureAlgorithm, Signer, SigningError, }; use crate::commons::crypto::{ @@ -165,22 +165,6 @@ impl SignerProvider { signer.wipe_all_keys() } } -} - -// Implement the functions defined by the `Signer` trait because `SignerRouter` expects to invoke them, but as the -// dispatching is not trait based we don't actually have to implement the `Signer` trait. -impl SignerProvider { - pub fn create_key(&self, algorithm: PublicKeyFormat) -> Result { - match self { - SignerProvider::OpenSsl(_, signer) => signer.create_key(algorithm), - #[cfg(feature = "hsm")] - SignerProvider::Kmip(_, signer) => signer.create_key(algorithm), - #[cfg(feature = "hsm")] - SignerProvider::Pkcs11(_, signer) => signer.create_key(algorithm), - #[cfg(all(test, feature = "hsm"))] - SignerProvider::Mock(_, signer) => signer.create_key(algorithm), - } - } /// Import an existing private key. Only supported for OpenSslSigner. Other /// signers will return an error. @@ -195,8 +179,27 @@ impl SignerProvider { SignerProvider::Mock(_, _) => Err(SignerError::other("import key not supported for the mock signer")), } } +} + +// Implement the functions defined by the `Signer` trait because `SignerRouter` expects to invoke them, but as the +// dispatching is not trait based we don't actually have to implement the `Signer` trait. +impl Signer for SignerProvider { + type KeyId = KeyIdentifier; + type Error = SignerError; + + fn create_key(&self, algorithm: PublicKeyFormat) -> Result { + match self { + SignerProvider::OpenSsl(_, signer) => signer.create_key(algorithm), + #[cfg(feature = "hsm")] + SignerProvider::Kmip(_, signer) => signer.create_key(algorithm), + #[cfg(feature = "hsm")] + SignerProvider::Pkcs11(_, signer) => signer.create_key(algorithm), + #[cfg(all(test, feature = "hsm"))] + SignerProvider::Mock(_, signer) => signer.create_key(algorithm), + } + } - pub fn get_key_info(&self, key: &KeyIdentifier) -> Result> { + fn get_key_info(&self, key: &KeyIdentifier) -> Result> { match self { SignerProvider::OpenSsl(_, signer) => signer.get_key_info(key), #[cfg(feature = "hsm")] @@ -208,7 +211,7 @@ impl SignerProvider { } } - pub fn destroy_key(&self, key: &KeyIdentifier) -> Result<(), KeyError> { + fn destroy_key(&self, key: &KeyIdentifier) -> Result<(), KeyError> { match self { SignerProvider::OpenSsl(_, signer) => signer.destroy_key(key), #[cfg(feature = "hsm")] @@ -220,7 +223,7 @@ impl SignerProvider { } } - pub fn sign + ?Sized>( + fn sign + ?Sized>( &self, key: &KeyIdentifier, algorithm: Alg, @@ -242,7 +245,7 @@ impl SignerProvider { } } - pub fn sign_one_off + ?Sized>( + fn sign_one_off + ?Sized>( &self, algorithm: Alg, data: &D, @@ -262,4 +265,8 @@ impl SignerProvider { SignerProvider::Mock(_, signer) => signer.sign_one_off(algorithm, data), } } + + fn rand(&self, target: &mut [u8]) -> Result<(), Self::Error> { + openssl::rand::rand_bytes(target).map_err(SignerError::OpenSslError) + } } diff --git a/src/commons/crypto/signing/dispatch/signerrouter.rs b/src/commons/crypto/signing/dispatch/signerrouter.rs index 4cc5c913d..72d3ef21a 100644 --- a/src/commons/crypto/signing/dispatch/signerrouter.rs +++ b/src/commons/crypto/signing/dispatch/signerrouter.rs @@ -1,9 +1,7 @@ use std::sync::Arc; use std::{collections::HashMap, sync::RwLock}; -use rpki::crypto::{ - signer::KeyError, KeyIdentifier, PublicKey, PublicKeyFormat, Signature, SignatureAlgorithm, Signer, SigningError, -}; +use rpki::crypto::KeyIdentifier; use crate::commons::{ crypto::{ @@ -157,15 +155,28 @@ impl SignerRouter { } pub fn get_active_signers(&self) -> HashMap> { + self.bind_ready_signers(); self.active_signers.read().unwrap().clone() } + /// Get the default signer + pub fn get_default_signer(&self) -> &Arc { + self.bind_ready_signers(); + &self.default_signer + } + + /// Get the one-off signer (usually OpenSSL) + pub fn get_one_off_signer(&self) -> &Arc { + self.bind_ready_signers(); + &self.one_off_signer + } + /// Locate the [SignerProvider] that owns a given [KeyIdentifier], if the signer is active. /// /// If the signer that owns the key has not yet been promoted from the pending set to the active set or if no /// the key was not created by us or was not registered with the [SignerMapper] then this lookup will fail with /// [SignerError::KeyNotFound]. - fn get_signer_for_key(&self, key_id: &KeyIdentifier) -> Result, SignerError> { + pub fn get_signer_for_key(&self, key_id: &KeyIdentifier) -> Result, SignerError> { match &self.signer_mapper { None => Ok(self.default_signer.clone()), Some(mapper) => { @@ -584,52 +595,10 @@ impl SignerRouter { } } -impl Signer for SignerRouter { - type KeyId = KeyIdentifier; - type Error = SignerError; - - fn create_key(&self, algorithm: PublicKeyFormat) -> Result { - self.bind_ready_signers(); - self.default_signer.create_key(algorithm) - } - - fn get_key_info(&self, key_id: &KeyIdentifier) -> Result> { - self.bind_ready_signers(); - self.get_signer_for_key(key_id)?.get_key_info(key_id) - } - - fn destroy_key(&self, key_id: &KeyIdentifier) -> Result<(), KeyError> { - self.bind_ready_signers(); - self.get_signer_for_key(key_id)?.destroy_key(key_id) - } - - fn sign + ?Sized>( - &self, - key_id: &KeyIdentifier, - algorithm: Alg, - data: &D, - ) -> Result, SigningError> { - self.bind_ready_signers(); - self.get_signer_for_key(key_id)?.sign(key_id, algorithm, data) - } - - fn sign_one_off + ?Sized>( - &self, - algorithm: Alg, - data: &D, - ) -> Result<(Signature, PublicKey), Self::Error> { - self.bind_ready_signers(); - self.one_off_signer.sign_one_off(algorithm, data) - } - - fn rand(&self, target: &mut [u8]) -> Result<(), Self::Error> { - self.bind_ready_signers(); - openssl::rand::rand_bytes(target).map_err(SignerError::OpenSslError) - } -} - #[cfg(all(test, feature = "hsm"))] pub mod tests { + use rpki::crypto::signer::KeyError; + use rpki::crypto::PublicKeyFormat; use rpki::crypto::RpkiSignatureAlgorithm; use crate::{ @@ -639,6 +608,7 @@ pub mod tests { CreateRegistrationKeyErrorCb, FnIdx, MockSigner, MockSignerCallCounts, SignRegistrationChallengeErrorCb, }, }, + rpki::crypto::Signer, test, }; @@ -681,12 +651,11 @@ pub mod tests { assert_eq!(0, call_counts.get(FnIdx::Sign)); assert_eq!(0, call_counts.get(FnIdx::DestroyKey)); - // Try to use the SignerRouter to generate a random value. This should cause the SignerRouter to contact + // Try to use the SignerRouter to bind ready signers. This should cause the SignerRouter to contact // the mock signer, ask it to create a registration key, verify that it can sign correctly with that key, // assign a signer mapper handle to the signer, then check for random number generation support and finally // actually generate the random number. - let mut out_buf: [u8; 1] = [0; 1]; - router.rand(&mut out_buf).unwrap(); + router.bind_ready_signers(); assert_eq!(1, call_counts.get(FnIdx::CreateRegistrationKey)); assert_eq!(1, call_counts.get(FnIdx::SignRegistrationChallenge)); assert_eq!(1, call_counts.get(FnIdx::GetInfo)); @@ -695,20 +664,28 @@ pub mod tests { // One signer has been registered with the SignerMapper now assert_eq!(1, signer_mapper.get_signer_handles().unwrap().len()); - // Ask for another random number. This time none of the registration steps should be performed as the signer + // Ask to bind the signers again. This time none of the registration steps should be performed as the signer // is already registered and active. - router.rand(&mut out_buf).unwrap(); + router.bind_ready_signers(); // Check that we can create a new key with the mock signer via the SignerRouter and that the key gets // registered with the signer mapper. - let key_identifier = router.create_key(PublicKeyFormat::Rsa).unwrap(); + let key_identifier = router + .get_default_signer() + .create_key(rpki::crypto::PublicKeyFormat::Rsa) + .unwrap(); assert!(signer_mapper.get_signer_for_key(&key_identifier).is_ok()); assert_eq!(1, call_counts.get(FnIdx::CreateKey)); // Check that we can sign with the SignerRouter using the Krill key identifier. The SignerRouter should // discover from the SignerMapper that the key belongs to the mock signer and so dispatch the signing // request to the mock signer. - router.sign(&key_identifier, DEF_SIG_ALG, &out_buf).unwrap(); + let random_data = test::random_bytes(); + + router + .get_default_signer() + .sign(&key_identifier, DEF_SIG_ALG, &random_data) + .unwrap(); assert_eq!(1, call_counts.get(FnIdx::Sign)); // Throw the SignerRouter away and create a new one. This is like restarting Krill. Keep the mock signer as @@ -720,21 +697,22 @@ pub mod tests { // Try to use the SignerRouter to sign again. This time around the SignerMapper should find the existing // signer in its records and only ask the signer to sign the registration challenge, but not ask it to // create a registration key. - router.sign(&key_identifier, DEF_SIG_ALG, &out_buf).unwrap(); + router + .get_default_signer() + .sign(&key_identifier, DEF_SIG_ALG, &random_data) + .unwrap(); assert_eq!(1, call_counts.get(FnIdx::CreateRegistrationKey)); assert_eq!(2, call_counts.get(FnIdx::SignRegistrationChallenge)); assert_eq!(2, call_counts.get(FnIdx::GetInfo)); assert_eq!(2, call_counts.get(FnIdx::SetHandle)); assert_eq!(2, call_counts.get(FnIdx::Sign)); - // Now delete the key and verify that we can no longer sign with it. - router.destroy_key(&key_identifier).unwrap(); + // Now delete the key and verify that we no longer have it. + router.get_default_signer().destroy_key(&key_identifier).unwrap(); assert_eq!(1, call_counts.get(FnIdx::DestroyKey)); - let err = router.sign(&key_identifier, RpkiSignatureAlgorithm::default(), &out_buf); - // TODO: Should this error from the SignerRouter actually be SigningError::KeyNotFound instead of - // SigningError::Signer(SignerError::KeyNotFound)? - assert!(matches!(err, Err(SigningError::Signer(SignerError::KeyNotFound)))); + let err = router.get_default_signer().get_key_info(&key_identifier); + assert!(matches!(err, Err(KeyError::Signer(SignerError::KeyNotFound)))); // The Sign call count is still 2 because the SignerRouter fails to determine which signer owns the key // and fails. @@ -746,8 +724,11 @@ pub mod tests { // The mock signer still works for the moment because the SignerRouter doesn't do registration again as // it thinks it still has an active signer. - let key_identifier = router.create_key(PublicKeyFormat::Rsa).unwrap(); - router.sign(&key_identifier, DEF_SIG_ALG, &out_buf).unwrap(); + let key_identifier = router.get_default_signer().create_key(PublicKeyFormat::Rsa).unwrap(); + router + .get_default_signer() + .sign(&key_identifier, DEF_SIG_ALG, &random_data) + .unwrap(); assert_eq!(1, call_counts.get(FnIdx::CreateRegistrationKey)); assert_eq!(2, call_counts.get(FnIdx::SignRegistrationChallenge)); @@ -764,8 +745,8 @@ pub mod tests { // registered again (and then sign challenged again, hence the double increment). let router = create_signer_router(&[mock_signer], signer_mapper.clone()); - let err = router.sign(&key_identifier, DEF_SIG_ALG, &out_buf); - assert!(matches!(err, Err(SigningError::Signer(SignerError::KeyNotFound)))); + let err = router.get_default_signer().get_key_info(&key_identifier); + assert!(matches!(err, Err(KeyError::Signer(SignerError::KeyNotFound)))); assert_eq!(2, call_counts.get(FnIdx::CreateRegistrationKey)); assert_eq!(4, call_counts.get(FnIdx::SignRegistrationChallenge)); @@ -836,13 +817,11 @@ pub mod tests { // No signers have been registered with the SignerMapper yet assert_eq!(0, signer_mapper.get_signer_handles().unwrap().len()); - let mut rand_out: [u8; 1] = [0; 1]; - - // Try to use the SignerRouter to generate a random value. This should cause the SignerRouter to contact + // Try to use the SignerRouter to bind the ready signers. This should cause the SignerRouter to contact // all of the mock signers, asking them to create a registration key, and if that succeeds to then verify // that the signer can sign correctly with that key. None of the broken signers will succeed at these steps // and so the counter of registered signers will remain at zero. - router.rand(&mut rand_out).unwrap(); + router.bind_ready_signers(); // The number of attempts to register a signer should have increased by the number of signers. // Half of the signers should fail at the registration step, the other half at the challenge signing step. @@ -855,7 +834,7 @@ pub mod tests { // // Try again. // - router.rand(&mut rand_out).unwrap(); + router.bind_ready_signers(); // The signers that were permanently unusable at registration should not be tried again. assert_eq!(6 + 2, call_counts.get(FnIdx::CreateRegistrationKey)); @@ -901,14 +880,12 @@ pub mod tests { // No signers have been registered with the SignerMapper yet assert_eq!(0, signer_mapper.get_signer_handles().unwrap().len()); - let mut rand_out: [u8; 1] = [0; 1]; - - // Try to use the SignerRouter to generate a random value. This should cause the SignerRouter to contact + // Try to use the SignerRouter to bind ready signers. This should cause the SignerRouter to contact // the mock signer, ask it to create a registration key, verify that it can sign correctly with that key, // assign a signer mapper handle to the signer, then check for random number generation support and finally // actually generate the random number. This should fail the first time due to the logic imlpemented by the // temp_avail() function above. - router.rand(&mut rand_out).unwrap(); + router.bind_ready_signers(); // The number of attempts to register a signer should have increased by one. assert_eq!(1, call_counts.get(FnIdx::CreateRegistrationKey)); @@ -916,10 +893,10 @@ pub mod tests { assert_eq!(0, signer_mapper.get_signer_handles().unwrap().len()); // - // Try again. We should succeed the second time due to the logic imlpemented by the temp_avail() function + // Try again. We should succeed the second time due to the logic implemented by the temp_avail() function // above. // - router.rand(&mut rand_out).unwrap(); + router.bind_ready_signers(); // We should be all green now assert_eq!(2, call_counts.get(FnIdx::CreateRegistrationKey)); diff --git a/src/commons/crypto/signing/signers/mocksigner.rs b/src/commons/crypto/signing/signers/mocksigner.rs index 9f78c9b36..3e609a08a 100644 --- a/src/commons/crypto/signing/signers/mocksigner.rs +++ b/src/commons/crypto/signing/signers/mocksigner.rs @@ -230,7 +230,7 @@ impl MockSigner { pub fn get_key_info(&self, key_identifier: &KeyIdentifier) -> Result> { self.inc_fn_call_count(FnIdx::GetKeyInfo); - let internal_id = self.internal_id_from_key_identifier(key_identifier).unwrap(); + let internal_id = self.internal_id_from_key_identifier(key_identifier)?; let pkey = self.load_key(&internal_id).ok_or(KeyError::KeyNotFound)?; let public_key = Self::public_key_from_pkey(&pkey).unwrap(); Ok(public_key) diff --git a/src/test.rs b/src/test.rs index 57745c092..45277c125 100644 --- a/src/test.rs +++ b/src/test.rs @@ -889,9 +889,13 @@ pub fn tmp_dir() -> (PathBuf, impl FnOnce()) { } fn random_hex_string() -> String { + hex::encode(random_bytes()) +} + +pub fn random_bytes() -> [u8; 8] { let mut bytes = [0; 8]; openssl::rand::rand_bytes(&mut bytes).unwrap(); - hex::encode(bytes) + bytes } pub fn mem_storage() -> Url { diff --git a/src/upgrades/mod.rs b/src/upgrades/mod.rs index 05dc99112..ac603965c 100644 --- a/src/upgrades/mod.rs +++ b/src/upgrades/mod.rs @@ -36,7 +36,7 @@ use crate::{ use rpki::crypto::KeyIdentifier; #[cfg(feature = "hsm")] -use crate::commons::crypto::SignerHandle; +use crate::{commons::crypto::SignerHandle, rpki::crypto::Signer}; use self::{pre_0_13_0::OldRepositoryContent, pre_0_14_0::OldCommandKey}; @@ -1153,7 +1153,8 @@ mod tests { let _repo: pre_0_13_0::OldRepositoryContent = serde_json::from_str(json).unwrap(); } - #[cfg(all(feature = "hsm", not(any(feature = "hsm-tests-kmip", feature = "hsm-tests-pkcs11"))))] + // #[cfg(all(feature = "hsm", not(any(feature = "hsm-tests-kmip", feature = "hsm-tests-pkcs11"))))] + #[allow(dead_code)] // this only looks dead because of complex features. fn unmapped_keys_test_core(do_upgrade: bool) { let expected_key_id = KeyIdentifier::from_str("5CBCAB14B810C864F3EEA8FD102B79F4E53FCC70").unwrap(); @@ -1197,7 +1198,7 @@ mod tests { if do_upgrade { // Verify that the mapper has a record of the test key belonging to the signer - mapper.get_signer_for_key(&expected_key_id).unwrap(); + krill_signer.get_key_info(&expected_key_id).unwrap(); } else { // Verify that the mapper does NOT have a record of the test key belonging to the signer assert!(mapper.get_signer_for_key(&expected_key_id).is_err());