From 508ca429e8f9419e4c0c8dd5a2ef25403e1de561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 22 Jan 2024 17:14:16 +0100 Subject: [PATCH] Reduce the need to lock repeatidly the object cache --- pkcs11/src/backend/key.rs | 46 ++++++++++++++++++++++++----------- pkcs11/src/backend/session.rs | 26 ++++++++++++++------ 2 files changed, 50 insertions(+), 22 deletions(-) diff --git a/pkcs11/src/backend/key.rs b/pkcs11/src/backend/key.rs index da329edf..6b698923 100644 --- a/pkcs11/src/backend/key.rs +++ b/pkcs11/src/backend/key.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, sync::Mutex}; use super::{ - db::{self, attr::CkRawAttrTemplate, Db, Object}, + db::{self, attr::CkRawAttrTemplate, Object}, login::{self, LoginCtx}, Error, }; @@ -459,13 +459,11 @@ pub fn generate_key_from_template( fetch_key(&id, parsed.raw_id, login_ctx, db) } -// we need the raw id when the CKA_KEY_ID doesn't parse to an alphanumeric string -pub fn fetch_key( +fn fetch_one_key( key_id: &str, raw_id: Option>, mut login_ctx: LoginCtx, - db: &Mutex, -) -> Result, Error> { +) -> Result, Error> { if !login_ctx.can_run_mode(super::login::UserMode::OperatorOrAdministrator) { return Err(Error::NotLoggedIn( super::login::UserMode::OperatorOrAdministrator, @@ -491,17 +489,28 @@ pub fn fetch_key( let objects = db::object::from_key_data(key_data, key_id, raw_id)?; + Ok(objects) +} + +// we need the raw id when the CKA_KEY_ID doesn't parse to an alphanumeric string +pub fn fetch_key( + key_id: &str, + raw_id: Option>, + login_ctx: LoginCtx, + db: &Mutex, +) -> Result, Error> { + let objects = fetch_one_key(key_id, raw_id, login_ctx)?; + let mut db = db.lock()?; Ok(objects.into_iter().map(|o| db.add_object(o)).collect()) } -pub fn fetch_certificate( +fn fetch_one_certificate( key_id: &str, raw_id: Option>, mut login_ctx: LoginCtx, - db: &Mutex, -) -> Result, Error> { +) -> Result { if !login_ctx.can_run_mode(super::login::UserMode::OperatorOrAdministrator) { return Err(Error::NotLoggedIn( super::login::UserMode::OperatorOrAdministrator, @@ -515,9 +524,19 @@ pub fn fetch_certificate( let object = db::object::from_cert_data(cert_data.entity, key_id, raw_id)?; + Ok(object) +} + +pub fn fetch_certificate( + key_id: &str, + raw_id: Option>, + login_ctx: LoginCtx, + db: &Mutex, +) -> Result<(CK_OBJECT_HANDLE, Object), Error> { + let object = fetch_one_certificate(key_id, raw_id, login_ctx)?; let r = db.lock()?.add_object(object); - Ok(vec![r]) + Ok(r) } // get the id from the logation header value : @@ -537,10 +556,9 @@ fn extract_key_id_location_header(headers: HashMap) -> Result, login_ctx: &LoginCtx, kind: Option, -) -> Result, Error> { +) -> Result, Error> { let mut acc = Vec::new(); if matches!( @@ -551,13 +569,13 @@ pub fn fetch_one( | Some(ObjectKind::SecretKey) ) { let login_ctx = login_ctx.clone(); - acc = fetch_key(&key.id, None, login_ctx, db)?; + acc = fetch_one_key(&key.id, None, login_ctx)?; } if matches!(kind, None | Some(ObjectKind::Certificate)) { let login_ctx = login_ctx.clone(); - match fetch_certificate(&key.id, None, login_ctx, db) { - Ok(mut vec) => acc.append(&mut vec), + match fetch_one_certificate(&key.id, None, login_ctx) { + Ok(cert) => acc.push(cert), Err(err) => { debug!("Failed to fetch certificate: {:?}", err); } diff --git a/pkcs11/src/backend/session.rs b/pkcs11/src/backend/session.rs index 6b1a56ff..55de1c24 100644 --- a/pkcs11/src/backend/session.rs +++ b/pkcs11/src/backend/session.rs @@ -459,9 +459,9 @@ impl Session { self.login_ctx.clone(), &self.db.0, ) { - Ok(ref mut vec) => { - trace!("Fetched certificate: {:?}", vec); - results.append(vec); + Ok(cert) => { + trace!("Fetched certificate: {:?}", cert); + results.push(cert); } Err(err) => { debug!("Failed to fetch certificate: {:?}", err); @@ -557,20 +557,25 @@ impl Session { )? .entity; - let results: Result, _> = if THREADS_ALLOWED.load(Ordering::Relaxed) { + let results: Result>, _> = if THREADS_ALLOWED.load(Ordering::Relaxed) { use rayon::prelude::*; keys.par_iter() - .map(|k| super::key::fetch_one(k, &self.db.0, &self.login_ctx, kind)) + .map(|k| super::key::fetch_one(k, &self.login_ctx, kind)) .collect() } else { keys.iter() - .map(|k| super::key::fetch_one(k, &self.db.0, &self.login_ctx, kind)) + .map(|k| super::key::fetch_one(k, &self.login_ctx, kind)) .collect() }; - let handles = results?.into_iter().flatten().collect(); + let results = results?; let mut db = self.db.0.lock()?; + let handles = results + .into_iter() + .flatten() + .map(|o| db.add_object(o)) + .collect(); db.set_fetched_all_keys(true); Ok(handles) @@ -595,7 +600,12 @@ impl Session { let db = self.db.clone(); match key_info.1 { - ObjectKind::Certificate => fetch_certificate(&key_info.0, None, login_ctx, &db.0), + ObjectKind::Certificate => Ok(vec![fetch_certificate( + &key_info.0, + None, + login_ctx, + &db.0, + )?]), _ => fetch_key(&key_info.0, None, login_ctx, &db.0), } }