Skip to content

Commit c35ce86

Browse files
ionut-armionut-arm
committed
Improve key metadata handling
This commit improves the handling of key metadata in the PKCS11 provider, allowing the provider to cache metadata in memory instead of always requesting it from the key info manager. Signed-off-by: Ionut Mihalcea <ionut.mihalcea@arm.com>
1 parent 879da27 commit c35ce86

File tree

7 files changed

+179
-186
lines changed

7 files changed

+179
-186
lines changed

‎src/key_info_managers/mod.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,16 @@ impl KeyTriple {
7777
pub fn belongs_to_provider(&self, provider_id: ProviderID) -> bool {
7878
self.provider_id == provider_id
7979
}
80+
81+
/// Get the key name
82+
pub fn key_name(&self) -> &str {
83+
&self.key_name
84+
}
85+
86+
/// Get the app name
87+
pub fn app_name(&self) -> &ApplicationName {
88+
&self.app_name
89+
}
8090
}
8191

8292
/// Converts the error string returned by the ManageKeyInfo methods to

‎src/providers/mbed_crypto_provider/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ impl MbedCryptoProvider {
105105
match store_handle.get_all(ProviderID::MbedCrypto) {
106106
Ok(key_triples) => {
107107
for key_triple in key_triples.iter().cloned() {
108-
let key_id = match key_management::get_key_id(key_triple, &*store_handle) {
108+
let key_id = match key_management::get_key_id(&key_triple, &*store_handle) {
109109
Ok(key_id) => key_id,
110110
Err(response_status) => {
111111
error!("Error getting the Key ID for triple:\n{}\n(error: {}), continuing...", key_triple, response_status);

‎src/providers/pkcs11_provider/asym_encryption.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright 2020 Contributors to the Parsec project.
22
// SPDX-License-Identifier: Apache-2.0
33
use super::Pkcs11Provider;
4-
use super::{key_management::get_key_info, utils, KeyPairType, ReadWriteSession, Session};
4+
use super::{utils, KeyPairType, ReadWriteSession, Session};
55
use crate::authenticators::ApplicationName;
66
use crate::key_info_managers::KeyTriple;
77
use log::{info, trace};
@@ -18,8 +18,7 @@ impl Pkcs11Provider {
1818
op: psa_asymmetric_encrypt::Operation,
1919
) -> Result<psa_asymmetric_encrypt::Result> {
2020
let key_triple = KeyTriple::new(app_name, ProviderID::Pkcs11, op.key_name.clone());
21-
let store_handle = self.key_info_store.read().expect("Key store lock poisoned");
22-
let (key_id, key_attributes) = get_key_info(&key_triple, &*store_handle)?;
21+
let (key_id, key_attributes) = self.get_key_info(&key_triple)?;
2322

2423
op.validate(key_attributes)?;
2524

@@ -69,8 +68,7 @@ impl Pkcs11Provider {
6968
op: psa_asymmetric_decrypt::Operation,
7069
) -> Result<psa_asymmetric_decrypt::Result> {
7170
let key_triple = KeyTriple::new(app_name, ProviderID::Pkcs11, op.key_name.clone());
72-
let store_handle = self.key_info_store.read().expect("Key store lock poisoned");
73-
let (key_id, key_attributes) = get_key_info(&key_triple, &*store_handle)?;
71+
let (key_id, key_attributes) = self.get_key_info(&key_triple)?;
7472

7573
op.validate(key_attributes)?;
7674

‎src/providers/pkcs11_provider/asym_sign.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright 2020 Contributors to the Parsec project.
22
// SPDX-License-Identifier: Apache-2.0
33
use super::Pkcs11Provider;
4-
use super::{key_management::get_key_info, utils, KeyPairType, ReadWriteSession, Session};
4+
use super::{utils, KeyPairType, ReadWriteSession, Session};
55
use crate::authenticators::ApplicationName;
66
use crate::key_info_managers::KeyTriple;
77
use log::{info, trace};
@@ -18,8 +18,7 @@ impl Pkcs11Provider {
1818
op: psa_sign_hash::Operation,
1919
) -> Result<psa_sign_hash::Result> {
2020
let key_triple = KeyTriple::new(app_name, ProviderID::Pkcs11, op.key_name.clone());
21-
let store_handle = self.key_info_store.read().expect("Key store lock poisoned");
22-
let (key_id, key_attributes) = get_key_info(&key_triple, &*store_handle)?;
21+
let (key_id, key_attributes) = self.get_key_info(&key_triple)?;
2322

2423
op.validate(key_attributes)?;
2524

@@ -66,8 +65,7 @@ impl Pkcs11Provider {
6665
op: psa_verify_hash::Operation,
6766
) -> Result<psa_verify_hash::Result> {
6867
let key_triple = KeyTriple::new(app_name, ProviderID::Pkcs11, op.key_name.clone());
69-
let store_handle = self.key_info_store.read().expect("Key store lock poisoned");
70-
let (key_id, key_attributes) = get_key_info(&key_triple, &*store_handle)?;
68+
let (key_id, key_attributes) = self.get_key_info(&key_triple)?;
7169

7270
op.validate(key_attributes)?;
7371

‎src/providers/pkcs11_provider/key_management.rs

Lines changed: 18 additions & 152 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
// Copyright 2020 Contributors to the Parsec project.
22
// SPDX-License-Identifier: Apache-2.0
3-
use super::{utils, KeyInfo, KeyPairType, LocalIdStore, Pkcs11Provider, ReadWriteSession, Session};
3+
use super::{utils, KeyPairType, Pkcs11Provider, ReadWriteSession, Session};
44
use crate::authenticators::ApplicationName;
55
use crate::key_info_managers::KeyTriple;
6-
use crate::key_info_managers::{self, ManageKeyInfo};
7-
use log::{error, info, trace, warn};
8-
use parsec_interface::operations::psa_key_attributes::*;
6+
use log::{error, info, trace};
7+
use parsec_interface::operations::psa_key_attributes::Type;
98
use parsec_interface::operations::{
109
psa_destroy_key, psa_export_public_key, psa_generate_key, psa_import_key,
1110
};
@@ -16,80 +15,6 @@ use picky_asn1_x509::RSAPublicKey;
1615
use pkcs11::types::{CKR_OK, CK_ATTRIBUTE, CK_OBJECT_HANDLE, CK_SESSION_HANDLE};
1716
use std::mem;
1817

19-
/// Gets a key identifier and key attributes from the Key Info Manager.
20-
pub fn get_key_info(
21-
key_triple: &KeyTriple,
22-
store_handle: &dyn ManageKeyInfo,
23-
) -> Result<([u8; 4], Attributes)> {
24-
match store_handle.get(key_triple) {
25-
Ok(Some(key_info)) => {
26-
if key_info.id.len() == 4 {
27-
let mut dst = [0; 4];
28-
dst.copy_from_slice(&key_info.id);
29-
Ok((dst, key_info.attributes))
30-
} else {
31-
error!("Stored Key ID is not valid.");
32-
Err(ResponseStatus::KeyInfoManagerError)
33-
}
34-
}
35-
Ok(None) => Err(ResponseStatus::PsaErrorDoesNotExist),
36-
Err(string) => Err(key_info_managers::to_response_status(string)),
37-
}
38-
}
39-
40-
pub fn create_key_id(
41-
key_triple: KeyTriple,
42-
key_attributes: Attributes,
43-
store_handle: &mut dyn ManageKeyInfo,
44-
local_ids_handle: &mut LocalIdStore,
45-
) -> Result<[u8; 4]> {
46-
let mut key_id = rand::random::<[u8; 4]>();
47-
while local_ids_handle.contains(&key_id) {
48-
key_id = rand::random::<[u8; 4]>();
49-
}
50-
let key_info = KeyInfo {
51-
id: key_id.to_vec(),
52-
attributes: key_attributes,
53-
};
54-
match store_handle.insert(key_triple.clone(), key_info) {
55-
Ok(insert_option) => {
56-
if insert_option.is_some() {
57-
if crate::utils::GlobalConfig::log_error_details() {
58-
warn!("Overwriting Key triple mapping ({})", key_triple);
59-
} else {
60-
warn!("Overwriting Key triple mapping");
61-
}
62-
}
63-
let _ = local_ids_handle.insert(key_id);
64-
65-
Ok(key_id)
66-
}
67-
Err(string) => Err(key_info_managers::to_response_status(string)),
68-
}
69-
}
70-
71-
pub fn remove_key_id(
72-
key_triple: &KeyTriple,
73-
key_id: [u8; 4],
74-
store_handle: &mut dyn ManageKeyInfo,
75-
local_ids_handle: &mut LocalIdStore,
76-
) -> Result<()> {
77-
match store_handle.remove(key_triple) {
78-
Ok(_) => {
79-
let _ = local_ids_handle.remove(&key_id);
80-
Ok(())
81-
}
82-
Err(string) => Err(key_info_managers::to_response_status(string)),
83-
}
84-
}
85-
86-
pub fn key_info_exists(key_triple: &KeyTriple, store_handle: &dyn ManageKeyInfo) -> Result<bool> {
87-
match store_handle.exists(key_triple) {
88-
Ok(val) => Ok(val),
89-
Err(string) => Err(key_info_managers::to_response_status(string)),
90-
}
91-
}
92-
9318
impl Pkcs11Provider {
9419
/// Find the PKCS 11 object handle corresponding to the key ID and the key type (public,
9520
/// private or any key type) given as parameters for the current session.
@@ -151,20 +76,10 @@ impl Pkcs11Provider {
15176
let key_attributes = op.attributes;
15277

15378
let key_triple = KeyTriple::new(app_name, ProviderID::Pkcs11, key_name);
154-
let mut store_handle = self
155-
.key_info_store
156-
.write()
157-
.expect("Key store lock poisoned");
158-
let mut local_ids_handle = self.local_ids.write().expect("Local ID lock poisoned");
159-
if key_info_exists(&key_triple, &*store_handle)? {
79+
if self.key_info_exists(&key_triple)? {
16080
return Err(ResponseStatus::PsaErrorAlreadyExists);
16181
}
162-
let key_id = create_key_id(
163-
key_triple.clone(),
164-
key_attributes,
165-
&mut *store_handle,
166-
&mut local_ids_handle,
167-
)?;
82+
let key_id = self.create_key_id(key_triple.clone(), key_attributes)?;
16883

16984
let (mech, mut pub_template, mut priv_template, mut allowed_mechanism) =
17085
utils::parsec_to_pkcs11_params(key_attributes, &key_id)?;
@@ -178,12 +93,7 @@ impl Pkcs11Provider {
17893

17994
let session = Session::new(self, ReadWriteSession::ReadWrite).or_else(|err| {
18095
format_error!("Error creating a new session", err);
181-
remove_key_id(
182-
&key_triple,
183-
key_id,
184-
&mut *store_handle,
185-
&mut local_ids_handle,
186-
)?;
96+
let _ = self.remove_key_id(&key_triple)?;
18797
Err(err)
18898
})?;
18999

@@ -204,12 +114,7 @@ impl Pkcs11Provider {
204114
Ok(_key) => Ok(psa_generate_key::Result {}),
205115
Err(e) => {
206116
format_error!("Generate Key Pair operation failed", e);
207-
remove_key_id(
208-
&key_triple,
209-
key_id,
210-
&mut *store_handle,
211-
&mut local_ids_handle,
212-
)?;
117+
let _ = self.remove_key_id(&key_triple)?;
213118
Err(utils::to_response_status(e))
214119
}
215120
}
@@ -228,43 +133,23 @@ impl Pkcs11Provider {
228133
let key_name = op.key_name;
229134
let key_attributes = op.attributes;
230135
let key_triple = KeyTriple::new(app_name, ProviderID::Pkcs11, key_name);
231-
let mut store_handle = self
232-
.key_info_store
233-
.write()
234-
.expect("Key store lock poisoned");
235-
let mut local_ids_handle = self.local_ids.write().expect("Local ID lock poisoned");
236-
if key_info_exists(&key_triple, &*store_handle)? {
136+
if self.key_info_exists(&key_triple)? {
237137
return Err(ResponseStatus::PsaErrorAlreadyExists);
238138
}
239-
let key_id = create_key_id(
240-
key_triple.clone(),
241-
key_attributes,
242-
&mut *store_handle,
243-
&mut local_ids_handle,
244-
)?;
139+
let key_id = self.create_key_id(key_triple.clone(), key_attributes)?;
245140

246141
let mut template: Vec<CK_ATTRIBUTE> = Vec::new();
247142

248143
let public_key: RSAPublicKey = picky_asn1_der::from_bytes(op.data.expose_secret())
249144
.or_else(|e| {
250145
format_error!("Failed to parse RsaPublicKey data", e);
251-
remove_key_id(
252-
&key_triple,
253-
key_id,
254-
&mut *store_handle,
255-
&mut local_ids_handle,
256-
)?;
146+
let _ = self.remove_key_id(&key_triple)?;
257147
Err(ResponseStatus::PsaErrorInvalidArgument)
258148
})?;
259149

260150
if public_key.modulus.is_negative() || public_key.public_exponent.is_negative() {
261151
error!("Only positive modulus and public exponent are supported.");
262-
remove_key_id(
263-
&key_triple,
264-
key_id,
265-
&mut *store_handle,
266-
&mut local_ids_handle,
267-
)?;
152+
let _ = self.remove_key_id(&key_triple)?;
268153
return Err(ResponseStatus::PsaErrorInvalidArgument);
269154
}
270155

@@ -281,6 +166,8 @@ impl Pkcs11Provider {
281166
} else {
282167
error!("`bits` field of key attributes must be either 0 or equal to the size of the key in `data`.");
283168
}
169+
170+
let _ = self.remove_key_id(&key_triple)?;
284171
return Err(ResponseStatus::PsaErrorInvalidArgument);
285172
}
286173

@@ -320,12 +207,7 @@ impl Pkcs11Provider {
320207

321208
let session = Session::new(self, ReadWriteSession::ReadWrite).or_else(|err| {
322209
format_error!("Error creating a new session", err);
323-
remove_key_id(
324-
&key_triple,
325-
key_id,
326-
&mut *store_handle,
327-
&mut local_ids_handle,
328-
)?;
210+
let _ = self.remove_key_id(&key_triple)?;
329211
Err(err)
330212
})?;
331213

@@ -344,12 +226,7 @@ impl Pkcs11Provider {
344226
Ok(_key) => Ok(psa_import_key::Result {}),
345227
Err(e) => {
346228
format_error!("Import operation failed", e);
347-
remove_key_id(
348-
&key_triple,
349-
key_id,
350-
&mut *store_handle,
351-
&mut local_ids_handle,
352-
)?;
229+
let _ = self.remove_key_id(&key_triple)?;
353230
Err(utils::to_response_status(e))
354231
}
355232
}
@@ -362,8 +239,7 @@ impl Pkcs11Provider {
362239
) -> Result<psa_export_public_key::Result> {
363240
let key_name = op.key_name;
364241
let key_triple = KeyTriple::new(app_name, ProviderID::Pkcs11, key_name);
365-
let store_handle = self.key_info_store.read().expect("Key store lock poisoned");
366-
let (key_id, _key_attributes) = get_key_info(&key_triple, &*store_handle)?;
242+
let (key_id, _key_attributes) = self.get_key_info(&key_triple)?;
367243

368244
let session = Session::new(self, ReadWriteSession::ReadOnly)?;
369245
if crate::utils::GlobalConfig::log_error_details() {
@@ -458,12 +334,7 @@ impl Pkcs11Provider {
458334
) -> Result<psa_destroy_key::Result> {
459335
let key_name = op.key_name;
460336
let key_triple = KeyTriple::new(app_name, ProviderID::Pkcs11, key_name);
461-
let mut store_handle = self
462-
.key_info_store
463-
.write()
464-
.expect("Key store lock poisoned");
465-
let mut local_ids_handle = self.local_ids.write().expect("Local ID lock poisoned");
466-
let (key_id, _) = get_key_info(&key_triple, &*store_handle)?;
337+
let (key_id, _) = self.get_key_info(&key_triple)?;
467338

468339
let session = Session::new(self, ReadWriteSession::ReadWrite)?;
469340
if crate::utils::GlobalConfig::log_error_details() {
@@ -510,12 +381,7 @@ impl Pkcs11Provider {
510381
}
511382
};
512383

513-
remove_key_id(
514-
&key_triple,
515-
key_id,
516-
&mut *store_handle,
517-
&mut local_ids_handle,
518-
)?;
384+
let _ = self.remove_key_id(&key_triple)?;
519385

520386
Ok(psa_destroy_key::Result {})
521387
}

0 commit comments

Comments
 (0)