diff --git a/src/abstraction/transient.rs b/src/abstraction/transient.rs index 584685b3..8fa722b9 100644 --- a/src/abstraction/transient.rs +++ b/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 diff --git a/src/lib.rs b/src/lib.rs index bd0f3219..1a398c74 100644 --- a/src/lib.rs +++ b/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."); } } diff --git a/src/utils.rs b/src/utils.rs index 445c1dfb..83e77744 100644 --- a/src/utils.rs +++ b/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, } -impl TryFrom for Signature { - type Error = Error; - - fn try_from(tss_signature: TPMT_SIGNATURE) -> Result { +impl Signature { + pub unsafe fn try_from(tss_signature: TPMT_SIGNATURE) -> Result { 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());