Skip to content

Commit

Permalink
Reduce the need to lock repeatidly the object cache
Browse files Browse the repository at this point in the history
  • Loading branch information
sosthene-nitrokey committed Feb 1, 2024
1 parent 8213f64 commit 4619618
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 22 deletions.
46 changes: 32 additions & 14 deletions pkcs11/src/backend/key.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -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<Vec<u8>>,
mut login_ctx: LoginCtx,
db: &Mutex<db::Db>,
) -> Result<Vec<(CK_OBJECT_HANDLE, Object)>, Error> {
) -> Result<Vec<Object>, Error> {
if !login_ctx.can_run_mode(super::login::UserMode::OperatorOrAdministrator) {
return Err(Error::NotLoggedIn(
super::login::UserMode::OperatorOrAdministrator,
Expand All @@ -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<Vec<u8>>,
login_ctx: LoginCtx,
db: &Mutex<db::Db>,
) -> Result<Vec<(CK_OBJECT_HANDLE, Object)>, 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<Vec<u8>>,
mut login_ctx: LoginCtx,
db: &Mutex<db::Db>,
) -> Result<Vec<(CK_OBJECT_HANDLE, Object)>, Error> {
) -> Result<Object, Error> {
if !login_ctx.can_run_mode(super::login::UserMode::OperatorOrAdministrator) {
return Err(Error::NotLoggedIn(
super::login::UserMode::OperatorOrAdministrator,
Expand All @@ -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<Vec<u8>>,
login_ctx: LoginCtx,
db: &Mutex<db::Db>,
) -> 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 :
Expand All @@ -537,10 +556,9 @@ fn extract_key_id_location_header(headers: HashMap<String, String>) -> Result<St

pub fn fetch_one(
key: &KeyItem,
db: &Mutex<Db>,
login_ctx: &LoginCtx,
kind: Option<ObjectKind>,
) -> Result<Vec<(CK_OBJECT_HANDLE, Object)>, Error> {
) -> Result<Vec<Object>, Error> {
let mut acc = Vec::new();

if matches!(
Expand All @@ -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),

Check warning on line 578 in pkcs11/src/backend/key.rs

View check run for this annotation

Codecov / codecov/patch

pkcs11/src/backend/key.rs#L578

Added line #L578 was not covered by tests
Err(err) => {
debug!("Failed to fetch certificate: {:?}", err);
}
Expand Down
26 changes: 18 additions & 8 deletions pkcs11/src/backend/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -554,20 +554,25 @@ impl Session {
)?
.entity;

let results: Result<Vec<_>, _> = if THREADS_ALLOWED.load(Ordering::Relaxed) {
let results: Result<Vec<Vec<Object>>, _> = 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, None))
.map(|k| super::key::fetch_one(k, &self.login_ctx, None))
.collect()
} else {
keys.iter()
.map(|k| super::key::fetch_one(k, &self.db.0, &self.login_ctx, None))
.map(|k| super::key::fetch_one(k, &self.login_ctx, None))

Check warning on line 564 in pkcs11/src/backend/session.rs

View check run for this annotation

Codecov / codecov/patch

pkcs11/src/backend/session.rs#L564

Added line #L564 was not covered by tests
.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)
Expand All @@ -592,7 +597,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),
}
}
Expand Down

0 comments on commit 4619618

Please sign in to comment.