diff --git a/Cargo.lock b/Cargo.lock index fcde1d0c8..bc5e0c9ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -133,8 +133,9 @@ dependencies = [ [[package]] name = "bcder" -version = "0.6.1-dev" -source = "git+https://github.com/NLnetLabs/bcder#0e0ef26d23afe18603768b7f6757df997b87568b" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "52e7c3bae57b01ec5b1b028af1f77b7d41dd2bd97a41bab7968739b5329c0e39" dependencies = [ "backtrace", "bytes", @@ -1628,8 +1629,9 @@ dependencies = [ [[package]] name = "rpki" -version = "0.12.3" -source = "git+https://github.com/ximon18/rpki-rs?branch=0.12.3-unsigned-from-slice#643e9e14dcf19de136e280989ae96c00144f5be5" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ee4c0626a16c808bb6b557298def266e8cd1078515b5ea07b382f2a0b75375c" dependencies = [ "base64 0.13.0", "bcder", diff --git a/Cargo.toml b/Cargo.toml index 525fa0643..fc616b357 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ build = "build.rs" backoff = { version = "0.3.0", optional = true } base64 = "^0.13" basic-cookies = { version = "^0.1", optional = true } -bcder = "0.6.1-dev" +bcder = "0.6.1" bytes = "1" chrono = { version = "^0.4", features = ["serde"] } clap = "^2.33" @@ -47,7 +47,7 @@ rand = "^0.8" regex = { version = "^1.4", optional = true, default_features = false, features = ["std"] } reqwest = { version = "0.11", features = ["json"] } rpassword = { version = "^5.0", optional = true } -rpki = { version = "0.12.3", features = [ "repository", "rrdp", "serde" ] } +rpki = { version = "0.13.0", features = [ "repository", "rrdp", "serde" ] } scrypt = { version = "^0.6", optional = true, default-features = false } serde = { version = "^1.0", features = ["derive"] } serde_json = "^1.0" @@ -189,7 +189,3 @@ shadow-utils = "*" # END RPM PACKAGING # ------------------------------------------------------------------------------ - -[patch.crates-io] -bcder = { git = 'https://github.com/NLnetLabs/bcder' } -rpki = { git = 'https://github.com/ximon18/rpki-rs', branch = '0.12.3-unsigned-from-slice' } \ No newline at end of file diff --git a/src/commons/api/ca.rs b/src/commons/api/ca.rs index 810dfe612..e9b0e8f17 100644 --- a/src/commons/api/ca.rs +++ b/src/commons/api/ca.rs @@ -2585,10 +2585,10 @@ mod test { fn mft_uri() { test::test_under_tmp(|d| { #[cfg(not(feature = "hsm"))] - let mut signer = OpenSslSigner::build(&d).unwrap(); + let signer = OpenSslSigner::build(&d).unwrap(); #[cfg(feature = "hsm")] - let mut signer = OpenSslSigner::build(&d, "dummy", None).unwrap(); + let signer = OpenSslSigner::build(&d, "dummy", None).unwrap(); let key_id = signer.create_key(PublicKeyFormat::Rsa).unwrap(); let pub_key = signer.get_key_info(&key_id).unwrap(); diff --git a/src/commons/crypto/signing/dispatch/krillsigner.rs b/src/commons/crypto/signing/dispatch/krillsigner.rs index 83ed69247..8ba6ee439 100644 --- a/src/commons/crypto/signing/dispatch/krillsigner.rs +++ b/src/commons/crypto/signing/dispatch/krillsigner.rs @@ -50,14 +50,12 @@ impl KrillSigner { pub fn create_key(&self) -> CryptoResult { self.router - .create_key_minimally_locking(PublicKeyFormat::Rsa) + .create_key(PublicKeyFormat::Rsa) .map_err(crypto::Error::signer) } pub fn destroy_key(&self, key_id: &KeyIdentifier) -> CryptoResult<()> { - self.router - .destroy_key_minimally_locking(key_id) - .map_err(crypto::Error::key_error) + self.router.destroy_key(key_id).map_err(crypto::Error::key_error) } pub fn get_key_info(&self, key_id: &KeyIdentifier) -> CryptoResult { diff --git a/src/commons/crypto/signing/dispatch/signerprovider.rs b/src/commons/crypto/signing/dispatch/signerprovider.rs index e8808d21a..103336bc0 100644 --- a/src/commons/crypto/signing/dispatch/signerprovider.rs +++ b/src/commons/crypto/signing/dispatch/signerprovider.rs @@ -13,7 +13,7 @@ use crate::commons::{api::Handle, crypto::signers::kmip::KmipSigner}; /// /// Named and modelled after the similar AuthProvider concept that already exists in Krill. #[allow(dead_code)] // Needed as we currently only ever construct one variant -#[derive(Clone, Debug)] +#[derive(Debug)] pub enum SignerProvider { OpenSsl(OpenSslSigner), @@ -31,7 +31,7 @@ impl SignerProvider { } #[cfg(feature = "hsm")] - pub fn create_registration_key(&mut self) -> Result<(PublicKey, String), SignerError> { + pub fn create_registration_key(&self) -> Result<(PublicKey, String), SignerError> { match self { SignerProvider::OpenSsl(signer) => signer.create_registration_key(), #[cfg(feature = "hsm")] @@ -53,7 +53,7 @@ impl SignerProvider { } #[cfg(feature = "hsm")] - pub fn set_handle(&mut self, handle: Handle) { + pub fn set_handle(&self, handle: Handle) { match self { SignerProvider::OpenSsl(signer) => signer.set_handle(handle), #[cfg(feature = "hsm")] @@ -85,7 +85,7 @@ impl Signer for SignerProvider { type Error = SignerError; - fn create_key(&mut self, algorithm: PublicKeyFormat) -> Result { + fn create_key(&self, algorithm: PublicKeyFormat) -> Result { match self { SignerProvider::OpenSsl(signer) => signer.create_key(algorithm), #[cfg(feature = "hsm")] @@ -101,7 +101,7 @@ impl Signer for SignerProvider { } } - fn destroy_key(&mut self, key: &Self::KeyId) -> Result<(), KeyError> { + fn destroy_key(&self, key: &Self::KeyId) -> Result<(), KeyError> { match self { SignerProvider::OpenSsl(signer) => signer.destroy_key(key), #[cfg(feature = "hsm")] diff --git a/src/commons/crypto/signing/dispatch/signerrouter.rs b/src/commons/crypto/signing/dispatch/signerrouter.rs index 8f5d97ea1..42b33f480 100644 --- a/src/commons/crypto/signing/dispatch/signerrouter.rs +++ b/src/commons/crypto/signing/dispatch/signerrouter.rs @@ -1,7 +1,4 @@ -use std::{ - path::Path, - sync::{Arc, RwLock}, -}; +use std::{path::Path, sync::Arc}; use rpki::repository::crypto::{ signer::KeyError, KeyIdentifier, PublicKey, PublicKeyFormat, Signature, SignatureAlgorithm, Signer, SigningError, @@ -16,7 +13,7 @@ use crate::commons::{ }; #[cfg(feature = "hsm")] -use std::{collections::HashMap, str::FromStr}; +use std::{collections::HashMap, str::FromStr, sync::RwLock}; #[cfg(feature = "hsm")] use crate::commons::{ @@ -71,17 +68,17 @@ pub struct SignerRouter { /// - One-off signing keys are NOT created by the default signer. See `one_off_signer` below. /// - Random numbers can only be generated by the default signer if it supports it. See `rand_fallback_signer` /// below. - default_signer: Arc>, + default_signer: Arc, /// The signer to create, sign with and destroy a one-off key. /// /// As the security of a HSM isn't needed for one-off keys, and HSMs are slow, by default this should be an instance /// of [OpenSslSigner]. However, if users think the perceived extra security is warranted let them use a different /// signer for one-off keys if that's what they want. - one_off_signer: Arc>, + one_off_signer: Arc, /// The signer to use when a configured signer doesn't support generation of random numbers. - rand_fallback_signer: Arc>, + rand_fallback_signer: Arc, /// A mechanism for identifying the signer [Handle] that owns the key with a particular [KeyIdentifier]. /// @@ -127,20 +124,20 @@ pub struct SignerRouter { /// we can connect to them and can identify the correct signer [Handle] used by the [SignerMapper] to associate with /// keys created by that signer. #[cfg(feature = "hsm")] - active_signers: RwLock>>>, + active_signers: RwLock>>, /// The set of [SignerProvider] instances that are configured but not yet confirmed to be usable. All signers start /// off in this set and are moved to the `active_signers` set as soon as we are able to confirm them. See /// `active_signers` above. #[cfg(feature = "hsm")] - pending_signers: RwLock>>>, + pending_signers: RwLock>>, } #[cfg(feature = "hsm")] struct SignerRoleAssignments { - default_signer: Arc>, - one_off_signer: Arc>, - rand_fallback_signer: Arc>, + default_signer: Arc, + one_off_signer: Arc, + rand_fallback_signer: Arc, } /// Before HSM support was added we used a single OpenSSL based "soft" signer for all key management and signing @@ -148,7 +145,7 @@ struct SignerRoleAssignments { #[cfg(not(feature = "hsm"))] impl SignerRouter { pub fn build(work_dir: &Path) -> KrillResult { - let openssl_signer = Arc::new(RwLock::new(SignerProvider::OpenSsl(OpenSslSigner::build(work_dir)?))); + let openssl_signer = Arc::new(SignerProvider::OpenSsl(OpenSslSigner::build(work_dir)?)); Ok(SignerRouter { default_signer: openssl_signer.clone(), @@ -157,7 +154,7 @@ impl SignerRouter { }) } - fn get_signer_for_key(&self, _key_id: &KeyIdentifier) -> Result>, SignerError> { + fn get_signer_for_key(&self, _key_id: &KeyIdentifier) -> Result, SignerError> { Ok(self.default_signer.clone()) } } @@ -194,8 +191,8 @@ impl SignerRouter { roles.rand_fallback_signer.clone(), ]; - unique_signers.sort_by_key(|k| k.read().unwrap().get_name().to_string()); - unique_signers.dedup_by_key(|k| k.read().unwrap().get_name().to_string()); + unique_signers.sort_by_key(|k| k.get_name().to_string()); + unique_signers.dedup_by_key(|k| k.get_name().to_string()); let unique_signers = RwLock::new(unique_signers); @@ -219,16 +216,16 @@ impl SignerRouter { // - Use the HSM for key creation, signing, deletion, except for one-off keys. // - Use the HSM for random number generation, if supported, else use the OpenSSL signer. // - Use the OpenSSL signer for one-off keys. - let openssl_signer = Arc::new(RwLock::new(SignerProvider::OpenSsl(OpenSslSigner::build( + let openssl_signer = Arc::new(SignerProvider::OpenSsl(OpenSslSigner::build( work_dir, "OpenSslSigner - No config file name available yet", Some(signer_mapper.clone()), - )?))); + )?)); - let kmip_signer = Arc::new(RwLock::new(SignerProvider::Kmip(KmipSigner::build( + let kmip_signer = Arc::new(SignerProvider::Kmip(KmipSigner::build( "KmipSigner - No config file name available yet", signer_mapper, - )?))); + )?)); Ok(SignerRoleAssignments { default_signer: kmip_signer.clone(), @@ -243,16 +240,16 @@ impl SignerRouter { // When the HSM feature is activated AND test mode is activated: // - Use the HSM for as much as possible to depend on it as broadly as possible in the Krill test suite.. // - Fallback to OpenSSL for random number generation if the HSM doesn't support it. - let openssl_signer = Arc::new(RwLock::new(SignerProvider::OpenSsl(OpenSslSigner::build( + let openssl_signer = Arc::new(SignerProvider::OpenSsl(OpenSslSigner::build( work_dir, "OpenSslSigner - No config file name available yet", Some(signer_mapper.clone()), - )?))); + )?)); - let kmip_signer = Arc::new(RwLock::new(SignerProvider::Kmip(KmipSigner::build( + let kmip_signer = Arc::new(SignerProvider::Kmip(KmipSigner::build( "KmipSigner - No config file name available yet", signer_mapper, - )?))); + )?)); Ok(SignerRoleAssignments { default_signer: kmip_signer.clone(), @@ -266,7 +263,7 @@ impl SignerRouter { /// 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> { + fn get_signer_for_key(&self, key_id: &KeyIdentifier) -> Result, SignerError> { // Get the signer handle for the key let signer_handle = self .signer_mapper @@ -280,23 +277,6 @@ impl SignerRouter { } } -/// Variants of the `Signer` trait functions that take `&mut` arguments and so must be locked to use them, but for which -/// we don't want the caller to have to lock the entire `SignerRouter`, only the single `Signer` being used. Ideally the -/// `Signer` trait wouldn't use `&mut` at all and rather require the implementation to use the interior mutability -/// pattern with as much or as little locking internally at the finest level of granularity possible. -/// Update: https://github.com/NLnetLabs/rpki-rs/pull/162 removes the &mut. -impl SignerRouter { - pub fn create_key_minimally_locking(&self, algorithm: PublicKeyFormat) -> Result { - self.bind_ready_signers(); - self.default_signer.write().unwrap().create_key(algorithm) - } - - pub fn destroy_key_minimally_locking(&self, key_id: &KeyIdentifier) -> Result<(), KeyError> { - self.bind_ready_signers(); - self.get_signer_for_key(key_id)?.write().unwrap().destroy_key(key_id) - } -} - /// When the "hsm" feature is enabled we can no longer assume that signers are immediately and always available as was /// the case without the "hsm" feature when only the OpenSslSigner was supported. We therefore keep created signers on /// standby in a "pending" set until we can verify that they are reachable and usable and can determine which @@ -374,7 +354,7 @@ impl SignerRouter { return true; } - let signer_name = signer_provider.read().unwrap().get_name().to_string(); + let signer_name = signer_provider.get_name().to_string(); // See if this is a known signer that whose signature matches the public key stored in the // [SignerMapper] for the signer. @@ -455,10 +435,10 @@ impl SignerRouter { fn identify_signer( &self, - signer_provider: &Arc>, + signer_provider: &Arc, candidate_handles: &[Handle], ) -> Result { - let config_signer_name = signer_provider.read().unwrap().get_name().to_string(); + let config_signer_name = signer_provider.get_name().to_string(); // First try any candidate handle whose signer name matches the name of the signer provider then fall back to // trying other candidate handles, as perhaps the signer was renamed in the config file and no longer matches by @@ -501,7 +481,7 @@ impl SignerRouter { fn is_signer_identified_by_handle( &self, - signer_provider: &Arc>, + signer_provider: &Arc, candidate_handle: &Handle, ) -> Result { let (key_identifier, signer_private_key_id) = match Self::decode_signer_handle(candidate_handle) { @@ -516,8 +496,7 @@ impl SignerRouter { }; let handle_name = self.signer_mapper.get_signer_name(candidate_handle)?; - let sp_read = signer_provider.read().unwrap(); - let signer_name = sp_read.get_name().to_string(); + let signer_name = signer_provider.get_name().to_string(); trace!( "Attempting to identify signer '{}' using identity key stored for signer '{}'", signer_name, @@ -544,7 +523,7 @@ impl SignerRouter { } let challenge = "Krill signer verification challenge".as_bytes(); - let signature = match sp_read.sign_registration_challenge(&signer_private_key_id, challenge) { + let signature = match signer_provider.sign_registration_challenge(&signer_private_key_id, challenge) { Err(SignerError::SignerUnavailable) => { debug!("Signer '{}' could not be contacted", signer_name); return Ok(IdentifyResult::Unavailable); @@ -565,12 +544,12 @@ impl SignerRouter { if public_key.verify(challenge, &signature).is_ok() { debug!("Signer '{}' is ready and known, binding", signer_name); - let signer_info = sp_read.get_info().unwrap_or("No signer info".to_string()); + let signer_info = signer_provider.get_info().unwrap_or("No signer info".to_string()); // Drop the read lock so that we can acquire a write lock - std::mem::drop(sp_read); + std::mem::drop(signer_provider); - signer_provider.write().unwrap().set_handle(candidate_handle.clone()); + signer_provider.set_handle(candidate_handle.clone()); if let Err(err) = self.signer_mapper.change_signer_name(candidate_handle, &signer_name) { // This is unexpected and perhaps indicative of a deeper problem but log and keep going. @@ -598,21 +577,17 @@ impl SignerRouter { Ok(IdentifyResult::Identified(candidate_handle.clone())) } - fn register_new_signer( - &self, - signer_provider: &Arc>, - ) -> Result { - let mut sp_write = signer_provider.write().unwrap(); - let signer_name = sp_write.get_name().to_string(); + fn register_new_signer(&self, signer_provider: &Arc) -> Result { + let signer_name = signer_provider.get_name().to_string(); - let (public_key, signer_private_key_id) = match sp_write.create_registration_key() { + let (public_key, signer_private_key_id) = match signer_provider.create_registration_key() { Err(SignerError::SignerUnavailable) => return Ok(RegisterResult::NotReady), Err(_) => return Ok(RegisterResult::ReadyUnusable), Ok(res) => res, }; let challenge = "Krill signer verification challenge".as_bytes(); - let signature = match sp_write.sign_registration_challenge(&signer_private_key_id, challenge) { + let signature = match signer_provider.sign_registration_challenge(&signer_private_key_id, challenge) { Err(SignerError::SignerUnavailable) => return Ok(RegisterResult::NotReady), Err(_) => return Ok(RegisterResult::ReadyUnusable), Ok(res) => res, @@ -626,9 +601,9 @@ impl SignerRouter { debug!("Signer '{}' is ready and new, binding", signer_name); let new_signer_handle = Self::encode_signer_handle(public_key.key_identifier(), &signer_private_key_id)?; - let signer_info = sp_write.get_info().unwrap_or("No signer info".to_string()); + let signer_info = signer_provider.get_info().unwrap_or("No signer info".to_string()); - sp_write.set_handle(new_signer_handle.clone()); + signer_provider.set_handle(new_signer_handle.clone()); self.signer_mapper .add_signer(&new_signer_handle, &signer_name, &signer_info, &public_key) .unwrap(); // TODO: handle me @@ -656,19 +631,19 @@ impl Signer for SignerRouter { type KeyId = KeyIdentifier; type Error = SignerError; - fn create_key(&mut self, algorithm: PublicKeyFormat) -> Result { + fn create_key(&self, algorithm: PublicKeyFormat) -> Result { self.bind_ready_signers(); - self.default_signer.write().unwrap().create_key(algorithm) + 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)?.read().unwrap().get_key_info(key_id) + self.get_signer_for_key(key_id)?.get_key_info(key_id) } - fn destroy_key(&mut self, key_id: &KeyIdentifier) -> Result<(), KeyError> { + fn destroy_key(&self, key_id: &KeyIdentifier) -> Result<(), KeyError> { self.bind_ready_signers(); - self.get_signer_for_key(key_id)?.write().unwrap().destroy_key(key_id) + self.get_signer_for_key(key_id)?.destroy_key(key_id) } fn sign + ?Sized>( @@ -678,10 +653,7 @@ impl Signer for SignerRouter { data: &D, ) -> Result> { self.bind_ready_signers(); - self.get_signer_for_key(key_id)? - .read() - .unwrap() - .sign(key_id, algorithm, data) + self.get_signer_for_key(key_id)?.sign(key_id, algorithm, data) } fn sign_one_off + ?Sized>( @@ -690,16 +662,15 @@ impl Signer for SignerRouter { data: &D, ) -> Result<(Signature, PublicKey), Self::Error> { self.bind_ready_signers(); - self.one_off_signer.read().unwrap().sign_one_off(algorithm, data) + self.one_off_signer.sign_one_off(algorithm, data) } fn rand(&self, target: &mut [u8]) -> Result<(), Self::Error> { self.bind_ready_signers(); - let signer = self.default_signer.read().unwrap(); - if signer.supports_random() { - signer.rand(target) + if self.default_signer.supports_random() { + self.default_signer.rand(target) } else { - self.rand_fallback_signer.read().unwrap().rand(target) + self.rand_fallback_signer.rand(target) } } } diff --git a/src/commons/crypto/signing/signers/kmip/internal.rs b/src/commons/crypto/signing/signers/kmip/internal.rs index a54f83570..561048f0b 100644 --- a/src/commons/crypto/signing/signers/kmip/internal.rs +++ b/src/commons/crypto/signing/signers/kmip/internal.rs @@ -59,11 +59,11 @@ pub type KmipTlsClient = Client>; //------------ The KMIP signer management interface ------------------------------------------------------------------- -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct KmipSigner { name: String, - handle: Option, + handle: RwLock>, mapper: Arc, @@ -79,7 +79,7 @@ impl KmipSigner { let s = KmipSigner { name: name.to_string(), - handle: None, + handle: RwLock::new(None), mapper: mapper.clone(), server, }; @@ -91,11 +91,12 @@ impl KmipSigner { &self.name } - pub fn set_handle(&mut self, handle: Handle) { - if self.handle.is_some() { + pub fn set_handle(&self, handle: Handle) { + let mut writable_handle = self.handle.write().unwrap(); + if writable_handle.is_some() { panic!("Cannot set signer handle as handle is already set"); } - self.handle = Some(handle); + *writable_handle = Some(handle); } pub fn get_info(&self) -> Option { @@ -113,7 +114,7 @@ impl KmipSigner { self.sign_with_key(signer_private_key_id, SignatureAlgorithm::default(), challenge.as_ref()) } - pub fn create_registration_key(&mut self) -> Result<(PublicKey, String), SignerError> { + pub fn create_registration_key(&self) -> Result<(PublicKey, String), SignerError> { let (public_key, kmip_key_pair_ids) = self.build_key(PublicKeyFormat::Rsa)?; let internal_key_id = kmip_key_pair_ids.private_key_id.to_string(); Ok((public_key, internal_key_id)) @@ -504,8 +505,12 @@ impl KmipSigner { // TODO: Don't assume colons cannot appear in HSM key ids. let internal_key_id = format!("{}:{}", kmip_key_ids.public_key_id, kmip_key_ids.private_key_id); + let readable_handle = self.handle.read().unwrap(); + let signer_handle = readable_handle.as_ref().ok_or(SignerError::Other( + "Failed to record signer key: Signer handle not set".to_string(), + ))?; self.mapper - .add_key(self.handle.as_ref().unwrap(), key_id, &internal_key_id) + .add_key(signer_handle, key_id, &internal_key_id) .map_err(|err| SignerError::KmipError(format!("Failed to record signer key: {}", err)))?; Ok(()) @@ -516,7 +521,8 @@ impl KmipSigner { &self, key_id: &KeyIdentifier, ) -> Result::Error>> { - let signer_handle = self.handle.as_ref().ok_or(KeyError::KeyNotFound)?; + let readable_handle = self.handle.read().unwrap(); + let signer_handle = readable_handle.as_ref().ok_or(KeyError::KeyNotFound)?; let internal_key_id = self .mapper diff --git a/src/commons/crypto/signing/signers/kmip/signer.rs b/src/commons/crypto/signing/signers/kmip/signer.rs index 9db0ef419..3ae87072a 100644 --- a/src/commons/crypto/signing/signers/kmip/signer.rs +++ b/src/commons/crypto/signing/signers/kmip/signer.rs @@ -11,7 +11,7 @@ impl Signer for KmipSigner { type KeyId = KeyIdentifier; type Error = SignerError; - fn create_key(&mut self, algorithm: PublicKeyFormat) -> Result { + fn create_key(&self, algorithm: PublicKeyFormat) -> Result { let (key, kmip_key_pair_ids) = self.build_key(algorithm)?; let key_id = key.key_identifier(); self.remember_kmip_key_ids(&key_id, kmip_key_pair_ids)?; @@ -24,7 +24,7 @@ impl Signer for KmipSigner { .map_err(|err| KeyError::Signer(err)) } - fn destroy_key(&mut self, key_id: &Self::KeyId) -> Result<(), KeyError> { + fn destroy_key(&self, key_id: &Self::KeyId) -> Result<(), KeyError> { let kmip_key_pair_ids = self.lookup_kmip_key_ids(key_id)?; match self.destroy_key_pair(&kmip_key_pair_ids, KeyStatus::Active)? { true => Ok(()), diff --git a/src/commons/crypto/signing/signers/softsigner.rs b/src/commons/crypto/signing/signers/softsigner.rs index d5c283479..7c6d19880 100644 --- a/src/commons/crypto/signing/signers/softsigner.rs +++ b/src/commons/crypto/signing/signers/softsigner.rs @@ -26,13 +26,16 @@ use crate::{ constants::KEYS_DIR, }; +#[cfg(feature = "hsm")] +use std::sync::RwLock; + #[cfg(feature = "hsm")] use crate::commons::{api::Handle, crypto::dispatch::signerinfo::SignerMapper}; //------------ OpenSslSigner ------------------------------------------------- /// An openssl based signer. -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct OpenSslSigner { keys_dir: Arc, @@ -40,7 +43,7 @@ pub struct OpenSslSigner { name: String, #[cfg(feature = "hsm")] - handle: Option, + handle: RwLock>, #[cfg(feature = "hsm")] info: Option, @@ -74,7 +77,7 @@ impl OpenSslSigner { openssl::version::version(), keys_dir.as_path().display() )), - handle: None, // will be set later + handle: RwLock::new(None), // will be set later mapper: mapper.clone(), keys_dir: keys_dir.into(), }; @@ -88,11 +91,12 @@ impl OpenSslSigner { } #[cfg(feature = "hsm")] - pub fn set_handle(&mut self, handle: Handle) { - if self.handle.is_some() { + pub fn set_handle(&self, handle: Handle) { + let mut writable_handle = self.handle.write().unwrap(); + if writable_handle.is_some() { panic!("Cannot set signer handle as handle is already set"); } - self.handle = Some(handle); + *writable_handle = Some(handle); } #[cfg(feature = "hsm")] @@ -110,7 +114,7 @@ impl OpenSslSigner { } #[cfg(feature = "hsm")] - pub fn create_registration_key(&mut self) -> Result<(PublicKey, String), SignerError> { + pub fn create_registration_key(&self) -> Result<(PublicKey, String), SignerError> { // For the OpenSslSigner we use the KeyIdentifier as the internal key id so the two are the same. let key_id = self.build_key()?; let internal_key_id = key_id.to_string(); @@ -159,7 +163,7 @@ impl OpenSslSigner { } } - fn build_key(&mut self) -> Result { + fn build_key(&self) -> Result { let kp = OpenSslKeyPair::build()?; let pk = &kp.subject_public_key_info()?; @@ -215,8 +219,12 @@ impl OpenSslSigner { // doesn't need a mapper to map from KeyIdentifier to internal key id as the internal key id IS the // KeyIdentifier. if let Some(mapper) = &self.mapper { + let readable_handle = self.handle.read().unwrap(); + let signer_handle = readable_handle.as_ref().ok_or(SignerError::Other( + "Failed to record signer key: Signer handle not set".to_string(), + ))?; mapper - .add_key(self.handle.as_ref().unwrap(), key_id, &format!("{}", key_id)) + .add_key(signer_handle, key_id, &format!("{}", key_id)) .map_err(|err| SignerError::Other(format!("Failed to record signer key: {}", err))) } else { Ok(()) @@ -228,7 +236,7 @@ impl Signer for OpenSslSigner { type KeyId = KeyIdentifier; type Error = SignerError; - fn create_key(&mut self, _algorithm: PublicKeyFormat) -> Result { + fn create_key(&self, _algorithm: PublicKeyFormat) -> Result { let key_id = self.build_key()?; self.remember_key_id(&key_id)?; Ok(key_id) @@ -239,7 +247,7 @@ impl Signer for OpenSslSigner { Ok(key_pair.subject_public_key_info()?) } - fn destroy_key(&mut self, key_id: &Self::KeyId) -> Result<(), KeyError> { + fn destroy_key(&self, key_id: &Self::KeyId) -> Result<(), KeyError> { let path = self.key_path(key_id); if path.exists() { fs::remove_file(&path).map_err(|e| { @@ -346,10 +354,10 @@ pub mod tests { fn should_return_subject_public_key_info() { test::test_under_tmp(|d| { #[cfg(not(feature = "hsm"))] - let mut s = OpenSslSigner::build(&d).unwrap(); + let s = OpenSslSigner::build(&d).unwrap(); #[cfg(feature = "hsm")] - let mut s = OpenSslSigner::build(&d, "dummy", None).unwrap(); + let s = OpenSslSigner::build(&d, "dummy", None).unwrap(); let ki = s.create_key(PublicKeyFormat::Rsa).unwrap(); s.get_key_info(&ki).unwrap();