refactor unsafe code

pkcs11/Cargo.toml

@@ -12,7 +12,6 @@ env_logger = { default-features = false, version = "0.10.0", features = [
   "auto-color",
   "humantime",
 ] }
-libc = { default-features = false, version = "0.2.80" }
 cryptoki-sys = "0.1.6"
 log = "0.4.19"
 merge = { features = [

pkcs11/src/api/decrypt.rs

@@ -1,4 +1,4 @@
-use cryptoki_sys::CK_ULONG;
+use cryptoki_sys::{CKR_ARGUMENTS_BAD, CK_ULONG};
 use log::{error, trace};
 
 use crate::{
@@ -13,13 +13,12 @@ pub extern "C" fn C_DecryptInit(
 ) -> cryptoki_sys::CK_RV {
     trace!("C_DecryptInit() called");
 
-    if pMechanism.is_null() {
-        return cryptoki_sys::CKR_ARGUMENTS_BAD;
-    }
-
-    trace!("C_DecryptInit() mech: {:?}", unsafe { *pMechanism });
-
-    let raw_mech = unsafe { CkRawMechanism::from_raw_ptr_unchecked(pMechanism) };
+    let raw_mech = match unsafe { CkRawMechanism::from_raw_ptr(pMechanism) } {
+        Some(mech) => mech,
+        None => {
+            return CKR_ARGUMENTS_BAD;
+        }
+    };
 
     let mech = match Mechanism::from_ckraw_mech(&raw_mech) {
         Ok(mech) => mech,

pkcs11/src/api/encrypt.rs

@@ -16,13 +16,12 @@ pub extern "C" fn C_EncryptInit(
 ) -> cryptoki_sys::CK_RV {
     trace!("C_EncryptInit() called");
 
-    if pMechanism.is_null() {
-        return cryptoki_sys::CKR_ARGUMENTS_BAD;
-    }
-
-    trace!("C_EncryptInit() mech: {:?}", unsafe { *pMechanism });
-
-    let raw_mech = unsafe { CkRawMechanism::from_raw_ptr_unchecked(pMechanism) };
+    let raw_mech = match unsafe { CkRawMechanism::from_raw_ptr(pMechanism) } {
+        Some(mech) => mech,
+        None => {
+            return cryptoki_sys::CKR_ARGUMENTS_BAD;
+        }
+    };
 
     let mech = match Mechanism::from_ckraw_mech(&raw_mech) {
         Ok(mech) => mech,

pkcs11/src/api/generation.rs

@@ -20,11 +20,18 @@ pub extern "C" fn C_GenerateKey(
 ) -> cryptoki_sys::CK_RV {
     trace!("C_GenerateKey() called");
 
-    if pMechanism.is_null() || phKey.is_null() || pTemplate.is_null() {
+    // pTemplate and pMechanism are checked for null with `from_raw_ptr`
+
+    if phKey.is_null() {
         return cryptoki_sys::CKR_ARGUMENTS_BAD;
     }
 
-    let mech = unsafe { CkRawMechanism::from_raw_ptr_unchecked(pMechanism) };
+    let mech = match unsafe { CkRawMechanism::from_raw_ptr(pMechanism) } {
+        Some(mech) => mech,
+        None => {
+            return cryptoki_sys::CKR_ARGUMENTS_BAD;
+        }
+    };
 
     trace!("C_GenerateKey() mech: {:?}", mech.type_());
     trace!("C_GenerateKey() mech param len: {:?}", mech.len());
@@ -36,10 +43,14 @@ pub extern "C" fn C_GenerateKey(
         }
     };
 
-    read_session!(hSession, session);
+    let template = match unsafe { CkRawAttrTemplate::from_raw_ptr(pTemplate, ulCount as usize) } {
+        Some(template) => template,
+        None => {
+            return cryptoki_sys::CKR_ARGUMENTS_BAD;
+        }
+    };
 
-    let template =
-        unsafe { CkRawAttrTemplate::from_raw_ptr_unchecked(pTemplate, ulCount as usize) };
+    read_session!(hSession, session);
 
     let key = match session.generate_key(&template, None, &mech) {
         Ok(key) => key,
@@ -76,16 +87,18 @@ pub extern "C" fn C_GenerateKeyPair(
 ) -> cryptoki_sys::CK_RV {
     trace!("C_GenerateKeyPair() called");
 
-    if pMechanism.is_null()
-        || phPublicKey.is_null()
-        || phPrivateKey.is_null()
-        || pPublicKeyTemplate.is_null()
-        || pPrivateKeyTemplate.is_null()
-    {
+    // pMechanism, pPrivateKeyTemplate, pPublicKeyTemplate  checked for null with `from_raw_ptr`
+
+    if phPublicKey.is_null() || phPrivateKey.is_null() {
         return cryptoki_sys::CKR_ARGUMENTS_BAD;
     }
 
-    let mech = unsafe { CkRawMechanism::from_raw_ptr_unchecked(pMechanism) };
+    let mech = match unsafe { CkRawMechanism::from_raw_ptr(pMechanism) } {
+        Some(mech) => mech,
+        None => {
+            return cryptoki_sys::CKR_ARGUMENTS_BAD;
+        }
+    };
 
     trace!("C_GenerateKeyPair() mech: {:?}", mech.type_());
     trace!("C_GenerateKeyPair() mech param len: {:?}", mech.len());
@@ -100,30 +113,25 @@ pub extern "C" fn C_GenerateKeyPair(
             return cryptoki_sys::CKR_MECHANISM_INVALID;
         }
     };
-
-    read_session!(hSession, session);
-
-    let public_template = unsafe {
-        CkRawAttrTemplate::from_raw_ptr_unchecked(
-            pPublicKeyTemplate,
-            ulPublicKeyAttributeCount as usize,
-        )
+    let public_template = match unsafe {
+        CkRawAttrTemplate::from_raw_ptr(pPublicKeyTemplate, ulPublicKeyAttributeCount as usize)
+    } {
+        Some(public_template) => public_template,
+        None => {
+            return cryptoki_sys::CKR_ARGUMENTS_BAD;
+        }
     };
 
-    let private_template = unsafe {
-        CkRawAttrTemplate::from_raw_ptr_unchecked(
-            pPrivateKeyTemplate,
-            ulPrivateKeyAttributeCount as usize,
-        )
+    let private_template = match unsafe {
+        CkRawAttrTemplate::from_raw_ptr(pPrivateKeyTemplate, ulPrivateKeyAttributeCount as usize)
+    } {
+        Some(private_template) => private_template,
+        None => {
+            return cryptoki_sys::CKR_ARGUMENTS_BAD;
+        }
     };
 
-    public_template.iter().for_each(|attr| {
-        trace!(
-            "Public template: {:?}, {:?}",
-            attr.type_(),
-            attr.val_bytes()
-        );
-    });
+    read_session!(hSession, session);
 
     let keys = match session.generate_key(&private_template, Some(&public_template), &mech) {
         Ok(keys) => keys,

pkcs11/src/api/object.rs

@@ -18,13 +18,9 @@ pub extern "C" fn C_FindObjectsInit(
         return cryptoki_sys::CKR_ARGUMENTS_BAD;
     }
 
-    lock_session!(hSession, session);
+    let template = unsafe { CkRawAttrTemplate::from_raw_ptr(pTemplate, ulCount as usize) };
 
-    let template = if !pTemplate.is_null() {
-        Some(unsafe { CkRawAttrTemplate::from_raw_ptr_unchecked(pTemplate, ulCount as usize) })
-    } else {
-        None
-    };
+    lock_session!(hSession, session);
     trace!("C_FindObjectsInit() template: {:?}", template);
     match session.enum_init(template) {
         Ok(_) => cryptoki_sys::CKR_OK,
@@ -55,13 +51,11 @@ pub extern "C" fn C_FindObjects(
     };
     trace!("C_FindObjects() objects: {:?}", objects);
 
+    let returned_count = objects.len();
+
     unsafe {
-        std::ptr::copy_nonoverlapping(
-            objects.as_ptr(),
-            phObject,
-            objects.len().min(ulMaxObjectCount as usize),
-        );
-        std::ptr::write(pulObjectCount, objects.len() as CK_ULONG);
+        std::ptr::copy_nonoverlapping(objects.as_ptr(), phObject, returned_count);
+        std::ptr::write(pulObjectCount, returned_count as CK_ULONG);
     }
 
     cryptoki_sys::CKR_OK
@@ -107,8 +101,13 @@ pub extern "C" fn C_GetAttributeValue(
         object.kind
     );
 
-    let mut template =
-        unsafe { CkRawAttrTemplate::from_raw_ptr_unchecked(pTemplate, ulCount as usize) };
+    let mut template = match unsafe { CkRawAttrTemplate::from_raw_ptr(pTemplate, ulCount as usize) }
+    {
+        Some(template) => template,
+        None => {
+            return cryptoki_sys::CKR_ARGUMENTS_BAD;
+        }
+    };
 
     object.fill_attr_template(&mut template)
 }
@@ -148,14 +147,20 @@ pub extern "C" fn C_CreateObject(
 ) -> cryptoki_sys::CK_RV {
     trace!("C_CreateObject() called ");
 
-    if pTemplate.is_null() || phObject.is_null() {
+    // pTemplate checked with from_raw_ptr
+
+    if phObject.is_null() {
         return cryptoki_sys::CKR_ARGUMENTS_BAD;
     }
 
-    lock_session!(hSession, session);
+    let template = match unsafe { CkRawAttrTemplate::from_raw_ptr(pTemplate, ulCount as usize) } {
+        Some(template) => template,
+        None => {
+            return cryptoki_sys::CKR_ARGUMENTS_BAD;
+        }
+    };
 
-    let template =
-        unsafe { CkRawAttrTemplate::from_raw_ptr_unchecked(pTemplate, ulCount as usize) };
+    lock_session!(hSession, session);
 
     let objects = match session.create_object(template) {
         Ok(object) => object,
@@ -209,12 +214,12 @@ pub extern "C" fn C_SetAttributeValue(
 ) -> cryptoki_sys::CK_RV {
     trace!("C_SetAttributeValue() called");
 
-    if pTemplate.is_null() {
-        return cryptoki_sys::CKR_ARGUMENTS_BAD;
-    }
-
-    let template =
-        unsafe { CkRawAttrTemplate::from_raw_ptr_unchecked(pTemplate, ulCount as usize) };
+    let template = match unsafe { CkRawAttrTemplate::from_raw_ptr(pTemplate, ulCount as usize) } {
+        Some(template) => template,
+        None => {
+            return cryptoki_sys::CKR_ARGUMENTS_BAD;
+        }
+    };
     let parsed = match key::parse_attributes(&template) {
         Ok(parsed) => parsed,
         Err(err) => {

pkcs11/src/api/session.rs

@@ -40,7 +40,7 @@ pub extern "C" fn C_OpenSession(
     trace!("C_OpenSession() created session: {:?}", session);
 
     unsafe {
-        *phSession = session;
+        std::ptr::write(phSession, session);
     }
 
     cryptoki_sys::CKR_OK

pkcs11/src/api/sign.rs

@@ -16,12 +16,13 @@ pub extern "C" fn C_SignInit(
         hKey,
         hSession
     );
-    if pMechanism.is_null() {
-        return cryptoki_sys::CKR_ARGUMENTS_BAD;
-    }
-    trace!("C_SignInit() mech: {:?}", unsafe { *pMechanism });
 
-    let raw_mech = unsafe { CkRawMechanism::from_raw_ptr_unchecked(pMechanism) };
+    let raw_mech = match unsafe { CkRawMechanism::from_raw_ptr(pMechanism) } {
+        Some(mech) => mech,
+        None => {
+            return cryptoki_sys::CKR_ARGUMENTS_BAD;
+        }
+    };
 
     let mech = match Mechanism::from_ckraw_mech(&raw_mech) {
         Ok(mech) => mech,