Skip to content

Commit a90fc7e

Browse files
hug-devhug-dev
authored
Merge pull request #93 from hug-dev/check-unsafe
Ensure the safety of unsafe blocks
2 parents 6a1cd93 + 2c01747 commit a90fc7e

File tree

7 files changed

+308
-140
lines changed

7 files changed

+308
-140
lines changed

‎src/providers/mbed_provider/mod.rs

Lines changed: 150 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,13 @@ use parsec_interface::operations::{OpExportPublicKey, ResultExportPublicKey};
2727
use parsec_interface::operations::{OpImportKey, ResultImportKey};
2828
use parsec_interface::operations::{OpListOpcodes, ResultListOpcodes};
2929
use parsec_interface::requests::{Opcode, ProviderID, ResponseStatus, Result};
30-
use psa_crypto_binding::psa_key_handle_t as KeyHandle;
31-
use psa_crypto_binding::psa_key_id_t as KeyId;
30+
use psa_crypto_binding::psa_key_id_t;
3231
use std::collections::HashSet;
3332
use std::convert::TryInto;
3433
use std::io::{Error, ErrorKind};
3534
use std::sync::{Arc, Mutex, RwLock};
3635
use std_semaphore::Semaphore;
37-
use utils::Key;
36+
use utils::KeyHandle;
3837
use uuid::Uuid;
3938

4039
#[allow(
@@ -53,7 +52,7 @@ mod psa_crypto_binding {
5352
mod constants;
5453
mod utils;
5554

56-
type LocalIdStore = HashSet<KeyId>;
55+
type LocalIdStore = HashSet<psa_key_id_t>;
5756

5857
const SUPPORTED_OPCODES: [Opcode; 7] = [
5958
Opcode::CreateKey,
@@ -91,7 +90,7 @@ pub struct MbedProvider {
9190
/// Gets a PSA Key ID from the Key ID Manager.
9291
/// Wrapper around the get method of the Key ID Manager to convert the key ID to the psa_key_id_t
9392
/// type.
94-
fn get_key_id(key_triple: &KeyTriple, store_handle: &dyn ManageKeyIDs) -> Result<KeyId> {
93+
fn get_key_id(key_triple: &KeyTriple, store_handle: &dyn ManageKeyIDs) -> Result<psa_key_id_t> {
9594
match store_handle.get(key_triple) {
9695
Ok(Some(key_id)) => {
9796
if let Ok(key_id_bytes) = key_id.try_into() {
@@ -114,13 +113,13 @@ fn create_key_id(
114113
key_triple: KeyTriple,
115114
store_handle: &mut dyn ManageKeyIDs,
116115
local_ids_handle: &mut LocalIdStore,
117-
) -> Result<KeyId> {
118-
let mut key_id = rand::random::<KeyId>();
116+
) -> Result<psa_key_id_t> {
117+
let mut key_id = rand::random::<psa_key_id_t>();
119118
while local_ids_handle.contains(&key_id)
120119
|| key_id == 0
121120
|| key_id > constants::PSA_MAX_PERSISTENT_KEY_IDENTIFIER
122121
{
123-
key_id = rand::random::<KeyId>();
122+
key_id = rand::random::<psa_key_id_t>();
124123
}
125124
match store_handle.insert(key_triple.clone(), key_id.to_ne_bytes().to_vec()) {
126125
Ok(insert_option) => {
@@ -140,7 +139,7 @@ fn create_key_id(
140139

141140
fn remove_key_id(
142141
key_triple: &KeyTriple,
143-
key_id: KeyId,
142+
key_id: psa_key_id_t,
144143
store_handle: &mut dyn ManageKeyIDs,
145144
local_ids_handle: &mut LocalIdStore,
146145
) -> Result<()> {
@@ -172,6 +171,8 @@ impl MbedProvider {
172171
/// if there, delete them. Adds Key IDs currently in use in the local IDs store.
173172
/// Returns `None` if the initialisation failed.
174173
fn new(key_id_store: Arc<RwLock<dyn ManageKeyIDs + Send + Sync>>) -> Option<MbedProvider> {
174+
// Safety: this function should be called before any of the other Mbed Crypto functions
175+
// are.
175176
if unsafe { psa_crypto_binding::psa_crypto_init() } != PSA_SUCCESS {
176177
error!("Error when initialising Mbed Crypto");
177178
return None;
@@ -208,23 +209,22 @@ impl MbedProvider {
208209
continue;
209210
}
210211
};
211-
let mut key_handle: KeyHandle = Default::default();
212-
let open_key_status =
213-
unsafe { psa_crypto_binding::psa_open_key(key_id, &mut key_handle) };
214-
if open_key_status == constants::PSA_ERROR_DOES_NOT_EXIST {
215-
to_remove.push(key_triple.clone());
216-
} else if open_key_status != PSA_SUCCESS {
217-
error!(
218-
"Error {} when opening a persistent Mbed Crypto key.",
219-
open_key_status
220-
);
221-
return None;
222-
} else {
223-
let _ = local_ids_handle.insert(key_id);
224-
unsafe {
225-
let _ = psa_crypto_binding::psa_close_key(key_handle);
212+
213+
// Safety: safe because:
214+
// * the Mbed Crypto library has been initialized
215+
// * this code is executed only by the main thread
216+
match unsafe { KeyHandle::open(key_id) } {
217+
Ok(_) => {
218+
let _ = local_ids_handle.insert(key_id);
219+
}
220+
Err(ResponseStatus::PsaErrorDoesNotExist) => {
221+
to_remove.push(key_triple.clone())
222+
}
223+
Err(e) => {
224+
error!("Error {} when opening a persistent Mbed Crypto key.", e);
225+
return None;
226226
}
227-
}
227+
};
228228
}
229229
}
230230
Err(string) => {
@@ -282,28 +282,30 @@ impl Provide for MbedProvider {
282282
)?;
283283

284284
let key_attrs = utils::convert_key_attributes(&key_attributes, key_id)?;
285-
let mut key = Key::new(&self.key_handle_mutex);
286-
287-
let generate_key_status = unsafe {
288-
let _guard = self
289-
.key_handle_mutex
290-
.lock()
291-
.expect("Grabbing key handle mutex failed");
292-
psa_crypto_binding::psa_generate_key(&key_attrs, key.as_mut())
293-
};
294285

295-
if generate_key_status != PSA_SUCCESS {
286+
let _guard = self
287+
.key_handle_mutex
288+
.lock()
289+
.expect("Grabbing key handle mutex failed");
290+
291+
// Safety:
292+
// * at this point the provider has been instantiated so Mbed Crypto has been initialized
293+
// * self.key_handle_mutex prevents concurrent accesses
294+
// * self.key_slot_semaphore prevents overflowing key slots
295+
let mut key_handle = unsafe { KeyHandle::generate(&key_attrs) }.or_else(|e| {
296296
remove_key_id(
297297
&key_triple,
298298
key_id,
299299
&mut *store_handle,
300300
&mut local_ids_handle,
301301
)?;
302-
error!("Generate key status: {}", generate_key_status);
303-
return Err(utils::convert_status(generate_key_status).ok_or_else(|| {
304-
error!("Failed to convert error status.");
305-
ResponseStatus::PsaErrorGenericError
306-
})?);
302+
error!("Generate key status: {}", e);
303+
Err(e)
304+
})?;
305+
306+
// Safety: same conditions than above.
307+
unsafe {
308+
key_handle.close()?;
307309
}
308310

309311
Ok(ResultCreateKey {})
@@ -328,33 +330,30 @@ impl Provide for MbedProvider {
328330
)?;
329331

330332
let key_attrs = utils::convert_key_attributes(&key_attributes, key_id)?;
331-
let mut key = Key::new(&self.key_handle_mutex);
332-
333-
let import_key_status = unsafe {
334-
let _guard = self
335-
.key_handle_mutex
336-
.lock()
337-
.expect("Grabbing key handle mutex failed");
338-
psa_crypto_binding::psa_import_key(
339-
&key_attrs,
340-
key_data.as_ptr(),
341-
key_data.len(),
342-
key.as_mut(),
343-
)
344-
};
345333

346-
if import_key_status != PSA_SUCCESS {
334+
let _guard = self
335+
.key_handle_mutex
336+
.lock()
337+
.expect("Grabbing key handle mutex failed");
338+
339+
// Safety:
340+
// * at this point the provider has been instantiated so Mbed Crypto has been initialized
341+
// * self.key_handle_mutex prevents concurrent accesses
342+
// * self.key_slot_semaphore prevents overflowing key slots
343+
let mut key_handle = unsafe { KeyHandle::import(&key_attrs, key_data) }.or_else(|e| {
347344
remove_key_id(
348345
&key_triple,
349346
key_id,
350347
&mut *store_handle,
351348
&mut local_ids_handle,
352349
)?;
353-
error!("Import key status: {}", import_key_status);
354-
return Err(utils::convert_status(import_key_status).ok_or_else(|| {
355-
error!("Failed to convert error status.");
356-
ResponseStatus::PsaErrorGenericError
357-
})?);
350+
error!("Import key status: {}", e);
351+
Err(e)
352+
})?;
353+
354+
// Safety: same conditions than above.
355+
unsafe {
356+
key_handle.close()?;
358357
}
359358

360359
Ok(ResultImportKey {})
@@ -371,24 +370,43 @@ impl Provide for MbedProvider {
371370
let key_triple = KeyTriple::new(app_name, ProviderID::MbedProvider, key_name);
372371
let store_handle = self.key_id_store.read().expect("Key store lock poisoned");
373372
let key_id = get_key_id(&key_triple, &*store_handle)?;
374-
let key = Key::open_key(key_id, &self.key_handle_mutex)?;
375373

376-
let key_attrs = key.get_attributes()?;
377-
let buffer_size = utils::psa_export_public_key_size(&key_attrs)?;
374+
let _guard = self
375+
.key_handle_mutex
376+
.lock()
377+
.expect("Grabbing key handle mutex failed");
378+
379+
let mut key_handle;
380+
let mut key_attrs;
381+
// Safety:
382+
// * at this point the provider has been instantiated so Mbed Crypto has been initialized
383+
// * self.key_handle_mutex prevents concurrent accesses
384+
// * self.key_slot_semaphore prevents overflowing key slots
385+
unsafe {
386+
key_handle = KeyHandle::open(key_id)?;
387+
key_attrs = key_handle.attributes()?;
388+
}
389+
390+
let buffer_size = utils::psa_export_public_key_size(key_attrs.as_ref())?;
378391
let mut buffer = vec![0u8; buffer_size];
379392
let mut actual_size = 0;
380393

381-
let export_status = unsafe {
382-
psa_crypto_binding::psa_export_public_key(
383-
key.raw_handle(),
394+
let export_status;
395+
// Safety: same conditions than above.
396+
unsafe {
397+
export_status = psa_crypto_binding::psa_export_public_key(
398+
key_handle.raw(),
384399
buffer.as_mut_ptr(),
385400
buffer_size,
386401
&mut actual_size,
387-
)
402+
);
403+
key_attrs.reset();
404+
key_handle.close()?;
388405
};
389406

390407
if export_status != PSA_SUCCESS {
391408
error!("Export status: {}", export_status);
409+
// Safety: same conditions than above.
392410
return Err(utils::convert_status(export_status).ok_or_else(|| {
393411
error!("Failed to convert error status.");
394412
ResponseStatus::PsaErrorGenericError
@@ -407,9 +425,23 @@ impl Provide for MbedProvider {
407425
let mut store_handle = self.key_id_store.write().expect("Key store lock poisoned");
408426
let mut local_ids_handle = self.local_ids.write().expect("Local ID lock poisoned");
409427
let key_id = get_key_id(&key_triple, &*store_handle)?;
410-
let key = Key::open_key(key_id, &self.key_handle_mutex)?;
411428

412-
let destroy_key_status = unsafe { psa_crypto_binding::psa_destroy_key(key.raw_handle()) };
429+
let _guard = self
430+
.key_handle_mutex
431+
.lock()
432+
.expect("Grabbing key handle mutex failed");
433+
434+
let key_handle;
435+
let destroy_key_status;
436+
437+
// Safety:
438+
// * at this point the provider has been instantiated so Mbed Crypto has been initialized
439+
// * self.key_handle_mutex prevents concurrent accesses
440+
// * self.key_slot_semaphore prevents overflowing key slots
441+
unsafe {
442+
key_handle = KeyHandle::open(key_id)?;
443+
destroy_key_status = psa_crypto_binding::psa_destroy_key(key_handle.raw());
444+
}
413445

414446
if destroy_key_status == PSA_SUCCESS {
415447
remove_key_id(
@@ -436,23 +468,41 @@ impl Provide for MbedProvider {
436468
let key_triple = KeyTriple::new(app_name, ProviderID::MbedProvider, key_name);
437469
let store_handle = self.key_id_store.read().expect("Key store lock poisoned");
438470
let key_id = get_key_id(&key_triple, &*store_handle)?;
439-
let key = Key::open_key(key_id, &self.key_handle_mutex)?;
440-
let key_attrs = key.get_attributes()?;
441471

442-
let buffer_size = utils::psa_asymmetric_sign_output_size(&key_attrs)?;
472+
let _guard = self
473+
.key_handle_mutex
474+
.lock()
475+
.expect("Grabbing key handle mutex failed");
476+
477+
let mut key_handle;
478+
let mut key_attrs;
479+
// Safety:
480+
// * at this point the provider has been instantiated so Mbed Crypto has been initialized
481+
// * self.key_handle_mutex prevents concurrent accesses
482+
// * self.key_slot_semaphore prevents overflowing key slots
483+
unsafe {
484+
key_handle = KeyHandle::open(key_id)?;
485+
key_attrs = key_handle.attributes()?;
486+
}
487+
488+
let buffer_size = utils::psa_asymmetric_sign_output_size(key_attrs.as_ref())?;
443489
let mut signature = vec![0u8; buffer_size];
444490
let mut signature_size: usize = 0;
445491

446-
let sign_status = unsafe {
447-
psa_crypto_binding::psa_asymmetric_sign(
448-
key.raw_handle(),
449-
key_attrs.core.policy.alg,
492+
let sign_status;
493+
// Safety: same conditions than above.
494+
unsafe {
495+
sign_status = psa_crypto_binding::psa_asymmetric_sign(
496+
key_handle.raw(),
497+
key_attrs.raw().core.policy.alg,
450498
hash.as_ptr(),
451499
hash.len(),
452500
signature.as_mut_ptr(),
453501
buffer_size,
454502
&mut signature_size,
455-
)
503+
);
504+
key_attrs.reset();
505+
key_handle.close()?;
456506
};
457507

458508
if sign_status == PSA_SUCCESS {
@@ -481,19 +531,33 @@ impl Provide for MbedProvider {
481531
let key_triple = KeyTriple::new(app_name, ProviderID::MbedProvider, key_name);
482532
let store_handle = self.key_id_store.read().expect("Key store lock poisoned");
483533
let key_id = get_key_id(&key_triple, &*store_handle)?;
484-
let key = Key::open_key(key_id, &self.key_handle_mutex)?;
485-
let key_attrs = key.get_attributes()?;
486534

487-
let verify_status = unsafe {
488-
psa_crypto_binding::psa_asymmetric_verify(
489-
key.raw_handle(),
490-
key_attrs.core.policy.alg,
535+
let _guard = self
536+
.key_handle_mutex
537+
.lock()
538+
.expect("Grabbing key handle mutex failed");
539+
540+
let mut key_handle;
541+
let mut key_attrs;
542+
let verify_status;
543+
// Safety:
544+
// * at this point the provider has been instantiated so Mbed Crypto has been initialized
545+
// * self.key_handle_mutex prevents concurrent accesses
546+
// * self.key_slot_semaphore prevents overflowing key slots
547+
unsafe {
548+
key_handle = KeyHandle::open(key_id)?;
549+
key_attrs = key_handle.attributes()?;
550+
verify_status = psa_crypto_binding::psa_asymmetric_verify(
551+
key_handle.raw(),
552+
key_attrs.raw().core.policy.alg,
491553
hash.as_ptr(),
492554
hash.len(),
493555
signature.as_ptr(),
494556
signature.len(),
495-
)
496-
};
557+
);
558+
key_attrs.reset();
559+
key_handle.close()?;
560+
}
497561

498562
if verify_status == PSA_SUCCESS {
499563
Ok(ResultAsymVerify {})
@@ -508,6 +572,7 @@ impl Provide for MbedProvider {
508572

509573
impl Drop for MbedProvider {
510574
fn drop(&mut self) {
575+
// Safety: the Provider was initialized with psa_crypto_init
511576
unsafe {
512577
psa_crypto_binding::mbedtls_psa_crypto_free();
513578
}

0 commit comments

Comments
 (0)