Improve usage of unsafe blocks

src/abstraction/transient.rs

@@ -157,7 +157,8 @@ impl TransientObjectContext {
             self.context.flush_context(key_handle)?;
             Err(e)
         })?;
-        let key = match PublicIdUnion::from_public(&key_pub_id) {
+        let key = match unsafe { PublicIdUnion::from_public(&key_pub_id) } {
+            // call should be safe given our trust in the TSS library
             PublicIdUnion::Rsa(pub_key) => {
                 let mut key = pub_key.buffer.to_vec();
                 key.truncate(pub_key.size.try_into().unwrap()); // should not fail on supported targets

src/lib.rs

@@ -12,7 +12,24 @@
 // WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+
 #![allow(dead_code)]
+///! # Notes on code safety:
+///! * the `unsafe` keyword is used to denote methods that could panic, crash or cause undefined
+///! behaviour. Whenever this is the case, the properties that need to be checked against
+///! parameters before passing them in will be stated in the documentation of the method.
+///! * `unsafe` blocks within this crate need to be documented through code comments if they
+///! are not covered by the points of trust described here.
+///! * the TSS2.0 library that this crate links to is trusted to return consistent values and to
+///! not crash or lead to undefined behaviour when presented with valid arguments.
+///! * the `Mbox` crate is trusted to perform operations safely on the pointers provided to it, if
+///! the pointers are trusted to be valid.
+///! * methods not marked `unsafe` are trusted to behave safely, potentially returning appropriate
+///! erorr messages when encountering any problems.
+///! * whenever `unwrap`, `expect`, `panic` or derivatives of these are used, they need to be
+///! thoroughly documented and justified - preferably `unwrap` and `expect` should *never* fail
+///! during normal operation.
+///! * these rules can be broken in test-only code and in tests.
 
 #[allow(
     non_snake_case,
@@ -412,7 +429,7 @@ impl Context {
 
         if ret.is_success() {
             let signature = unsafe { MBox::from_raw(signature) };
-            Ok((*signature).try_into()?)
+            Ok(unsafe { Signature::try_from(*signature)? })
         } else {
             error!("Error in loading: {}.", ret);
             Err(ret)
@@ -659,14 +676,10 @@ impl Drop for Context {
         let tcti_context = self.tcti_context.take().unwrap(); // should not fail based on how the context is initialised/used
 
         // Close the TCTI context.
-        unsafe {
-            tss2_esys::Tss2_TctiLdr_Finalize(
-                &mut tcti_context.into_raw() as *mut *mut TSS2_TCTI_CONTEXT
-            )
-        };
+        unsafe { tss2_esys::Tss2_TctiLdr_Finalize(&mut tcti_context.into_raw()) };
 
         // Close the context.
-        unsafe { tss2_esys::Esys_Finalize(&mut esys_context.into_raw() as *mut *mut ESYS_CONTEXT) };
+        unsafe { tss2_esys::Esys_Finalize(&mut esys_context.into_raw()) };
         info!("Context closed.");
     }
 }

src/utils.rs

@@ -322,12 +322,9 @@ pub enum PublicIdUnion {
 }
 
 impl PublicIdUnion {
-    pub fn from_public(public: &TPM2B_PUBLIC) -> Self {
+    pub unsafe fn from_public(public: &TPM2B_PUBLIC) -> Self {
         match public.publicArea.type_ {
-            TPM2_ALG_RSA => {
-                // TODO Issue #2: Should this method be unsafe?
-                PublicIdUnion::Rsa(Box::from(unsafe { public.publicArea.unique.rsa }))
-            }
+            TPM2_ALG_RSA => PublicIdUnion::Rsa(Box::from(public.publicArea.unique.rsa)),
             TPM2_ALG_ECC => unimplemented!(),
             TPM2_ALG_SYMCIPHER => unimplemented!(),
             TPM2_ALG_KEYEDHASH => unimplemented!(),
@@ -425,15 +422,13 @@ pub struct Signature {
     pub signature: Vec<u8>,
 }
 
-impl TryFrom<TPMT_SIGNATURE> for Signature {
-    type Error = Error;
-
-    fn try_from(tss_signature: TPMT_SIGNATURE) -> Result<Self> {
+impl Signature {
+    pub unsafe fn try_from(tss_signature: TPMT_SIGNATURE) -> Result<Self> {
         match tss_signature.sigAlg {
             TPM2_ALG_RSASSA => {
-                let hash_alg = unsafe { tss_signature.signature.rsassa.hash };
+                let hash_alg = tss_signature.signature.rsassa.hash;
                 let scheme = AsymSchemeUnion::RSASSA(hash_alg);
-                let signature_buf = unsafe { tss_signature.signature.rsassa.sig };
+                let signature_buf = tss_signature.signature.rsassa.sig;
                 let mut signature = signature_buf.buffer.to_vec();
                 signature.truncate(signature_buf.size.into());