Skip to content

Commit 3cad90c

Browse files
ionut-armionut-arm
authored
Merge pull request #41 from ionut-arm/locks
Add locking around key handle operations in mbed provider
2 parents 1274137 + 3bb76f3 commit 3cad90c

File tree

5 files changed

+86
-9
lines changed

5 files changed

+86
-9
lines changed

‎Cargo.lock

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ rand = "0.7.2"
1111
base64 = "0.10.1"
1212
uuid = "0.7.4"
1313
threadpool = "1.7.1"
14+
std-semaphore = "0.1.0"
1415

1516
[dev-dependencies]
1617
parsec-client-test = { git = "https://github.com/parallaxsecond/parsec-client-test", tag = "0.1.2" }

‎README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ This project uses the following third party crates:
136136
* base64 (MIT and Apache-2.0)
137137
* uuid (Apache-2.0)
138138
* threadpool (Apache-2.0)
139+
* std-semaphore (MIT and Apache-2.0)
139140
* num_cpus (MIT and Apache-2.0)
140141

141142
This project uses the following third party libraries:

‎src/providers/mbed_provider/constants.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ pub const PSA_ERROR_INVALID_PADDING: psa_status_t = -150;
3838
pub const PSA_ERROR_INSUFFICIENT_DATA: psa_status_t = -143;
3939
pub const PSA_ERROR_INVALID_HANDLE: psa_status_t = -136;
4040

41+
pub const PSA_KEY_SLOT_COUNT: isize = 32;
4142
pub const EMPTY_KEY_HANDLE: psa_key_handle_t = 0;
4243
pub const PSA_KEY_TYPE_NONE: psa_key_type_t = 0x0000_0000;
4344
pub const PSA_KEY_TYPE_VENDOR_FLAG: psa_key_type_t = 0x8000_0000;

‎src/providers/mbed_provider/mod.rs

Lines changed: 76 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::authenticators::ApplicationName;
1717
use crate::key_id_managers::{KeyTriple, ManageKeyIDs};
1818
use std::collections::HashSet;
1919
use std::convert::TryInto;
20-
use std::sync::{Arc, RwLock};
20+
use std::sync::{Arc, Mutex, RwLock};
2121

2222
use parsec_interface::operations::key_attributes::KeyLifetime;
2323
use parsec_interface::operations::ProviderInfo;
@@ -29,6 +29,7 @@ use parsec_interface::operations::{OpExportPublicKey, ResultExportPublicKey};
2929
use parsec_interface::operations::{OpImportKey, ResultImportKey};
3030
use parsec_interface::operations::{OpListOpcodes, ResultListOpcodes};
3131
use parsec_interface::requests::{Opcode, ProviderID, ResponseStatus, Result};
32+
use std_semaphore::Semaphore;
3233
use uuid::Uuid;
3334

3435
#[allow(
@@ -63,6 +64,16 @@ pub struct MbedProvider {
6364
// reference it to match with the method prototypes.
6465
key_id_store: Arc<RwLock<dyn ManageKeyIDs + Send + Sync>>,
6566
local_ids: RwLock<LocalIdStore>,
67+
// Calls to `psa_open_key`, `psa_create_key` and `psa_close_key` are not thread safe - the slot
68+
// allocation mechanism in Mbed Crypto can return the same key slot for overlapping calls.
69+
// `key_handle_mutex` is use as a way of securing access to said operations among the threads.
70+
// This issue tracks progress on fixing the original problem in Mbed Crypto:
71+
// https://github.com/ARMmbed/mbed-crypto/issues/266
72+
key_handle_mutex: Mutex<()>,
73+
// As mentioned above, calls dealing with key slot allocation are not secured for concurrency.
74+
// `key_slot_semaphore` is used to ensure that only `PSA_KEY_SLOT_COUNT` threads can have slots
75+
// assigned at any time.
76+
key_slot_semaphore: Semaphore,
6677
}
6778

6879
/// Gets a PSA Key ID from the Key ID Manager.
@@ -156,6 +167,8 @@ impl MbedProvider {
156167
let mbed_provider = MbedProvider {
157168
key_id_store,
158169
local_ids: RwLock::new(HashSet::new()),
170+
key_handle_mutex: Mutex::new(()),
171+
key_slot_semaphore: Semaphore::new(constants::PSA_KEY_SLOT_COUNT),
159172
};
160173
{
161174
// The local scope allows to drop store_handle and local_ids_handle in order to return
@@ -245,6 +258,7 @@ impl Provide for MbedProvider {
245258

246259
fn create_key(&self, app_name: ApplicationName, op: OpCreateKey) -> Result<ResultCreateKey> {
247260
println!("Mbed Provider - Create Key");
261+
let _semaphore_guard = self.key_slot_semaphore.access();
248262
let key_name = op.key_name;
249263
let key_attributes = op.key_attributes;
250264
let key_triple = KeyTriple::new(app_name, ProviderID::MbedProvider, key_name);
@@ -263,6 +277,10 @@ impl Provide for MbedProvider {
263277
let mut key_handle: psa_crypto_binding::psa_key_handle_t = 0;
264278

265279
let create_key_status = unsafe {
280+
let _guard = self
281+
.key_handle_mutex
282+
.lock()
283+
.expect("Grabbing key handle mutex failed");
266284
psa_crypto_binding::psa_create_key(key_attrs.key_lifetime, key_id, &mut key_handle)
267285
};
268286

@@ -307,6 +325,10 @@ impl Provide for MbedProvider {
307325
}
308326

309327
unsafe {
328+
let _guard = self
329+
.key_handle_mutex
330+
.lock()
331+
.expect("Grabbing key handle mutex failed");
310332
psa_crypto_binding::psa_close_key(key_handle);
311333
}
312334
} else {
@@ -328,6 +350,7 @@ impl Provide for MbedProvider {
328350

329351
fn import_key(&self, app_name: ApplicationName, op: OpImportKey) -> Result<ResultImportKey> {
330352
println!("Mbed Provider - Import Key");
353+
let _semaphore_guard = self.key_slot_semaphore.access();
331354
let key_name = op.key_name;
332355
let key_attributes = op.key_attributes;
333356
let key_data = op.key_data;
@@ -347,6 +370,10 @@ impl Provide for MbedProvider {
347370
let mut key_handle: psa_crypto_binding::psa_key_handle_t = 0;
348371

349372
let create_key_status = unsafe {
373+
let _guard = self
374+
.key_handle_mutex
375+
.lock()
376+
.expect("Grabbing key handle mutex failed");
350377
psa_crypto_binding::psa_create_key(key_attrs.key_lifetime, key_id, &mut key_handle)
351378
};
352379

@@ -390,6 +417,10 @@ impl Provide for MbedProvider {
390417
}
391418

392419
unsafe {
420+
let _guard = self
421+
.key_handle_mutex
422+
.lock()
423+
.expect("Grabbing key handle mutex failed");
393424
psa_crypto_binding::psa_close_key(key_handle);
394425
}
395426
} else {
@@ -415,6 +446,7 @@ impl Provide for MbedProvider {
415446
op: OpExportPublicKey,
416447
) -> Result<ResultExportPublicKey> {
417448
println!("Mbed Provider - Export Public Key");
449+
let _semaphore_guard = self.key_slot_semaphore.access();
418450
let key_name = op.key_name;
419451
let key_lifetime = op.key_lifetime;
420452
let key_triple = KeyTriple::new(app_name, ProviderID::MbedProvider, key_name);
@@ -426,8 +458,13 @@ impl Provide for MbedProvider {
426458

427459
let ret_val: Result<ResultExportPublicKey>;
428460

429-
let open_key_status =
430-
unsafe { psa_crypto_binding::psa_open_key(lifetime, key_id, &mut key_handle) };
461+
let open_key_status = unsafe {
462+
let _guard = self
463+
.key_handle_mutex
464+
.lock()
465+
.expect("Grabbing key handle mutex failed");
466+
psa_crypto_binding::psa_open_key(lifetime, key_id, &mut key_handle)
467+
};
431468

432469
if open_key_status == constants::PSA_SUCCESS {
433470
// TODO: There's no calculation of the buffer size here, nor is there any handling of the
@@ -455,6 +492,10 @@ impl Provide for MbedProvider {
455492
}
456493

457494
unsafe {
495+
let _guard = self
496+
.key_handle_mutex
497+
.lock()
498+
.expect("Grabbing key handle mutex failed");
458499
psa_crypto_binding::psa_close_key(key_handle);
459500
}
460501
} else {
@@ -467,6 +508,7 @@ impl Provide for MbedProvider {
467508

468509
fn destroy_key(&self, app_name: ApplicationName, op: OpDestroyKey) -> Result<ResultDestroyKey> {
469510
println!("Mbed Provider - Destroy Key");
511+
let _semaphore_guard = self.key_slot_semaphore.access();
470512
let key_name = op.key_name;
471513
let key_lifetime = op.key_lifetime;
472514
let key_triple = KeyTriple::new(app_name, ProviderID::MbedProvider, key_name);
@@ -477,8 +519,13 @@ impl Provide for MbedProvider {
477519
let lifetime = conversion_utils::convert_key_lifetime(key_lifetime);
478520
let mut key_handle: psa_crypto_binding::psa_key_handle_t = 0;
479521

480-
let open_key_status =
481-
unsafe { psa_crypto_binding::psa_open_key(lifetime, key_id, &mut key_handle) };
522+
let open_key_status = unsafe {
523+
let _guard = self
524+
.key_handle_mutex
525+
.lock()
526+
.expect("Grabbing key handle mutex failed");
527+
psa_crypto_binding::psa_open_key(lifetime, key_id, &mut key_handle)
528+
};
482529

483530
if open_key_status == constants::PSA_SUCCESS {
484531
let destroy_key_status = unsafe { psa_crypto_binding::psa_destroy_key(key_handle) };
@@ -501,6 +548,7 @@ impl Provide for MbedProvider {
501548

502549
fn asym_sign(&self, app_name: ApplicationName, op: OpAsymSign) -> Result<ResultAsymSign> {
503550
println!("Mbed Provider - Asym Sign");
551+
let _semaphore_guard = self.key_slot_semaphore.access();
504552
let key_name = op.key_name;
505553
let key_lifetime = op.key_lifetime;
506554
let hash = op.hash;
@@ -511,8 +559,13 @@ impl Provide for MbedProvider {
511559
let lifetime = conversion_utils::convert_key_lifetime(key_lifetime);
512560
let mut key_handle: psa_crypto_binding::psa_key_handle_t = 0;
513561

514-
let open_key_status =
515-
unsafe { psa_crypto_binding::psa_open_key(lifetime, key_id, &mut key_handle) };
562+
let open_key_status = unsafe {
563+
let _guard = self
564+
.key_handle_mutex
565+
.lock()
566+
.expect("Grabbing key handle mutex failed");
567+
psa_crypto_binding::psa_open_key(lifetime, key_id, &mut key_handle)
568+
};
516569

517570
if open_key_status == constants::PSA_SUCCESS {
518571
let mut policy = psa_crypto_binding::psa_key_policy_t {
@@ -543,6 +596,10 @@ impl Provide for MbedProvider {
543596
};
544597

545598
unsafe {
599+
let _guard = self
600+
.key_handle_mutex
601+
.lock()
602+
.expect("Grabbing key handle mutex failed");
546603
psa_crypto_binding::psa_close_key(key_handle);
547604
}
548605

@@ -564,6 +621,7 @@ impl Provide for MbedProvider {
564621

565622
fn asym_verify(&self, app_name: ApplicationName, op: OpAsymVerify) -> Result<ResultAsymVerify> {
566623
println!("Mbed Provider - Asym Verify");
624+
let _semaphore_guard = self.key_slot_semaphore.access();
567625
let key_name = op.key_name;
568626
let key_lifetime = op.key_lifetime;
569627
let hash = op.hash;
@@ -575,8 +633,13 @@ impl Provide for MbedProvider {
575633
let lifetime = conversion_utils::convert_key_lifetime(key_lifetime);
576634
let mut key_handle: psa_crypto_binding::psa_key_handle_t = 0;
577635

578-
let open_key_status =
579-
unsafe { psa_crypto_binding::psa_open_key(lifetime, key_id, &mut key_handle) };
636+
let open_key_status = unsafe {
637+
let _guard = self
638+
.key_handle_mutex
639+
.lock()
640+
.expect("Grabbing key handle mutex failed");
641+
psa_crypto_binding::psa_open_key(lifetime, key_id, &mut key_handle)
642+
};
580643

581644
if open_key_status == constants::PSA_SUCCESS {
582645
let mut policy = psa_crypto_binding::psa_key_policy_t {
@@ -605,6 +668,10 @@ impl Provide for MbedProvider {
605668
};
606669

607670
unsafe {
671+
let _guard = self
672+
.key_handle_mutex
673+
.lock()
674+
.expect("Grabbing key handle mutex failed");
608675
psa_crypto_binding::psa_close_key(key_handle);
609676
}
610677

0 commit comments

Comments
 (0)