diff --git a/tss-esapi/src/context.rs b/tss-esapi/src/context.rs index 9d10c1dcd..adb26e999 100644 --- a/tss-esapi/src/context.rs +++ b/tss-esapi/src/context.rs @@ -15,7 +15,7 @@ use handle_manager::HandleManager; use log::{debug, error}; use malloced::Malloced; use std::collections::HashMap; -use std::ptr::null_mut; +use std::{ffi::c_void, ptr, ptr::null_mut}; /// Safe abstraction over an ESYS_CONTEXT. /// @@ -454,12 +454,16 @@ impl Context { /// Private function for handling that has been allocated with /// C memory allocation functions in TSS. - fn ffi_data_to_owned(data_ptr: *mut T) -> T { - let out = unsafe { *data_ptr }; + fn ffi_data_to_owned(data_ptr: *mut T) -> Result { + if data_ptr.is_null() { + error!("Null pointer received from TSS"); + return Err(Error::local_error(ErrorKind::WrongValueFromTpm)); + } + + let out = unsafe { ptr::read(data_ptr) }; + unsafe { Esys_Free(data_ptr.cast::()) }; - // Free the malloced data. - drop(unsafe { Malloced::from_raw(data_ptr) }); - out + Ok(out) } } diff --git a/tss-esapi/src/context/general_esys_tr.rs b/tss-esapi/src/context/general_esys_tr.rs index 49981ee2a..ab44624bb 100644 --- a/tss-esapi/src/context/general_esys_tr.rs +++ b/tss-esapi/src/context/general_esys_tr.rs @@ -139,7 +139,7 @@ impl Context { error!("Error in getting name: {:#010X}", ret); }, )?; - Name::try_from(Context::ffi_data_to_owned(name_ptr)) + Name::try_from(Context::ffi_data_to_owned(name_ptr)?) } /// Used to construct an esys object from the resources inside the TPM. diff --git a/tss-esapi/src/context/tpm_commands/asymmetric_primitives.rs b/tss-esapi/src/context/tpm_commands/asymmetric_primitives.rs index 22ff04d0f..95c5f20e9 100644 --- a/tss-esapi/src/context/tpm_commands/asymmetric_primitives.rs +++ b/tss-esapi/src/context/tpm_commands/asymmetric_primitives.rs @@ -39,7 +39,7 @@ impl Context { error!("Error when performing RSA encryption: {:#010X}", ret); }, )?; - PublicKeyRsa::try_from(Context::ffi_data_to_owned(out_data_ptr)) + PublicKeyRsa::try_from(Context::ffi_data_to_owned(out_data_ptr)?) } /// Perform an asymmetric RSA decryption. @@ -69,7 +69,7 @@ impl Context { error!("Error when performing RSA decryption: {:#010X}", ret); }, )?; - PublicKeyRsa::try_from(Context::ffi_data_to_owned(message_ptr)) + PublicKeyRsa::try_from(Context::ffi_data_to_owned(message_ptr)?) } /// Generate an ephemeral key pair. @@ -199,8 +199,8 @@ impl Context { }, )?; - let z_point = Context::ffi_data_to_owned(z_point_ptr); - let pub_point = Context::ffi_data_to_owned(pub_point_ptr); + let z_point = Context::ffi_data_to_owned(z_point_ptr)?; + let pub_point = Context::ffi_data_to_owned(pub_point_ptr)?; Ok(( EccPoint::try_from(z_point.point)?, EccPoint::try_from(pub_point.point)?, @@ -335,7 +335,7 @@ impl Context { error!("Error when performing ECDH ZGen: {:#010X}", ret); }, )?; - let out_point = Context::ffi_data_to_owned(out_point_ptr); + let out_point = Context::ffi_data_to_owned(out_point_ptr)?; EccPoint::try_from(out_point.point) } diff --git a/tss-esapi/src/context/tpm_commands/attestation_commands.rs b/tss-esapi/src/context/tpm_commands/attestation_commands.rs index 6c4c5196b..570e4bade 100644 --- a/tss-esapi/src/context/tpm_commands/attestation_commands.rs +++ b/tss-esapi/src/context/tpm_commands/attestation_commands.rs @@ -147,8 +147,8 @@ impl Context { }, )?; - let certify_info = Context::ffi_data_to_owned(certify_info_ptr); - let signature = Context::ffi_data_to_owned(signature_ptr); + let certify_info = Context::ffi_data_to_owned(certify_info_ptr)?; + let signature = Context::ffi_data_to_owned(signature_ptr)?; Ok(( Attest::try_from(AttestBuffer::try_from(certify_info)?)?, Signature::try_from(signature)?, @@ -272,8 +272,8 @@ impl Context { }, )?; - let certify_info = Context::ffi_data_to_owned(certify_info_ptr); - let signature = Context::ffi_data_to_owned(signature_ptr); + let certify_info = Context::ffi_data_to_owned(certify_info_ptr)?; + let signature = Context::ffi_data_to_owned(signature_ptr)?; Ok(( Attest::try_from(AttestBuffer::try_from(certify_info)?)?, Signature::try_from(signature)?, @@ -313,8 +313,8 @@ impl Context { }, )?; - let quoted = Context::ffi_data_to_owned(quoted_ptr); - let signature = Context::ffi_data_to_owned(signature_ptr); + let quoted = Context::ffi_data_to_owned(quoted_ptr)?; + let signature = Context::ffi_data_to_owned(signature_ptr)?; Ok(( Attest::try_from(AttestBuffer::try_from(quoted)?)?, Signature::try_from(signature)?, @@ -426,8 +426,8 @@ impl Context { }, )?; - let timeinfo = Context::ffi_data_to_owned(timeinfo_ptr); - let signature = Context::ffi_data_to_owned(signature_ptr); + let timeinfo = Context::ffi_data_to_owned(timeinfo_ptr)?; + let signature = Context::ffi_data_to_owned(signature_ptr)?; Ok(( Attest::try_from(AttestBuffer::try_from(timeinfo)?)?, Signature::try_from(signature)?, diff --git a/tss-esapi/src/context/tpm_commands/capability_commands.rs b/tss-esapi/src/context/tpm_commands/capability_commands.rs index 734ab1c13..fa1c36c91 100644 --- a/tss-esapi/src/context/tpm_commands/capability_commands.rs +++ b/tss-esapi/src/context/tpm_commands/capability_commands.rs @@ -69,7 +69,7 @@ impl Context { )?; Ok(( - CapabilityData::try_from(Context::ffi_data_to_owned(capability_data_ptr))?, + CapabilityData::try_from(Context::ffi_data_to_owned(capability_data_ptr)?)?, YesNo::try_from(more_data)?.into(), )) } diff --git a/tss-esapi/src/context/tpm_commands/context_management.rs b/tss-esapi/src/context/tpm_commands/context_management.rs index b710c8083..02b701672 100644 --- a/tss-esapi/src/context/tpm_commands/context_management.rs +++ b/tss-esapi/src/context/tpm_commands/context_management.rs @@ -26,7 +26,7 @@ impl Context { error!("Error in saving context: {:#010X}", ret); }, )?; - SavedTpmContext::try_from(Context::ffi_data_to_owned(context_ptr)) + SavedTpmContext::try_from(Context::ffi_data_to_owned(context_ptr)?) } /// Load a previously saved context into the TPM and return the object handle. diff --git a/tss-esapi/src/context/tpm_commands/duplication_commands.rs b/tss-esapi/src/context/tpm_commands/duplication_commands.rs index 512a754b2..a24772480 100644 --- a/tss-esapi/src/context/tpm_commands/duplication_commands.rs +++ b/tss-esapi/src/context/tpm_commands/duplication_commands.rs @@ -327,9 +327,9 @@ impl Context { )?; Ok(( - Data::try_from(Context::ffi_data_to_owned(encryption_key_out_ptr))?, - Private::try_from(Context::ffi_data_to_owned(duplicate_ptr))?, - EncryptedSecret::try_from(Context::ffi_data_to_owned(out_sym_seed_ptr))?, + Data::try_from(Context::ffi_data_to_owned(encryption_key_out_ptr)?)?, + Private::try_from(Context::ffi_data_to_owned(duplicate_ptr)?)?, + EncryptedSecret::try_from(Context::ffi_data_to_owned(out_sym_seed_ptr)?)?, )) } @@ -683,6 +683,6 @@ impl Context { error!("Error when performing import: {:#010X}", ret); }, )?; - Private::try_from(Context::ffi_data_to_owned(out_private_ptr)) + Private::try_from(Context::ffi_data_to_owned(out_private_ptr)?) } } diff --git a/tss-esapi/src/context/tpm_commands/enhanced_authorization_ea_commands.rs b/tss-esapi/src/context/tpm_commands/enhanced_authorization_ea_commands.rs index fc5b29671..0f36bcc48 100644 --- a/tss-esapi/src/context/tpm_commands/enhanced_authorization_ea_commands.rs +++ b/tss-esapi/src/context/tpm_commands/enhanced_authorization_ea_commands.rs @@ -64,8 +64,8 @@ impl Context { }, )?; Ok(( - Timeout::try_from(Context::ffi_data_to_owned(out_timeout_ptr))?, - AuthTicket::try_from(Context::ffi_data_to_owned(out_policy_ticket_ptr))?, + Timeout::try_from(Context::ffi_data_to_owned(out_timeout_ptr)?)?, + AuthTicket::try_from(Context::ffi_data_to_owned(out_policy_ticket_ptr)?)?, )) } @@ -106,8 +106,8 @@ impl Context { }, )?; Ok(( - Timeout::try_from(Context::ffi_data_to_owned(out_timeout_ptr))?, - AuthTicket::try_from(Context::ffi_data_to_owned(out_policy_ticket_ptr))?, + Timeout::try_from(Context::ffi_data_to_owned(out_timeout_ptr)?)?, + AuthTicket::try_from(Context::ffi_data_to_owned(out_policy_ticket_ptr)?)?, )) } @@ -533,7 +533,7 @@ impl Context { }, )?; - Digest::try_from(Context::ffi_data_to_owned(policy_digest_ptr)) + Digest::try_from(Context::ffi_data_to_owned(policy_digest_ptr)?) } /// Cause conditional gating of a policy based on NV written state. diff --git a/tss-esapi/src/context/tpm_commands/hierarchy_commands.rs b/tss-esapi/src/context/tpm_commands/hierarchy_commands.rs index 8c4dca919..9c548f6df 100644 --- a/tss-esapi/src/context/tpm_commands/hierarchy_commands.rs +++ b/tss-esapi/src/context/tpm_commands/hierarchy_commands.rs @@ -70,10 +70,10 @@ impl Context { error!("Error in creating primary key: {:#010X}", ret); }, )?; - let out_public_owned = Context::ffi_data_to_owned(out_public_ptr); - let creation_data_owned = Context::ffi_data_to_owned(creation_data_ptr); - let creation_hash_owned = Context::ffi_data_to_owned(creation_hash_ptr); - let creation_ticket_owned = Context::ffi_data_to_owned(creation_ticket_ptr); + let out_public_owned = Context::ffi_data_to_owned(out_public_ptr)?; + let creation_data_owned = Context::ffi_data_to_owned(creation_data_ptr)?; + let creation_hash_owned = Context::ffi_data_to_owned(creation_hash_ptr)?; + let creation_ticket_owned = Context::ffi_data_to_owned(creation_ticket_ptr)?; let primary_key_handle = KeyHandle::from(object_handle); self.handle_manager .add_handle(primary_key_handle.into(), HandleDropAction::Flush)?; diff --git a/tss-esapi/src/context/tpm_commands/integrity_collection_pcr.rs b/tss-esapi/src/context/tpm_commands/integrity_collection_pcr.rs index 85f5d279d..72db1d09d 100644 --- a/tss-esapi/src/context/tpm_commands/integrity_collection_pcr.rs +++ b/tss-esapi/src/context/tpm_commands/integrity_collection_pcr.rs @@ -175,8 +175,8 @@ impl Context { Ok(( pcr_update_counter, - PcrSelectionList::try_from(Context::ffi_data_to_owned(pcr_selection_out_ptr))?, - DigestList::try_from(Context::ffi_data_to_owned(pcr_values_ptr))?, + PcrSelectionList::try_from(Context::ffi_data_to_owned(pcr_selection_out_ptr)?)?, + DigestList::try_from(Context::ffi_data_to_owned(pcr_values_ptr)?)?, )) } diff --git a/tss-esapi/src/context/tpm_commands/non_volatile_storage.rs b/tss-esapi/src/context/tpm_commands/non_volatile_storage.rs index f7d385726..b1b7fb682 100644 --- a/tss-esapi/src/context/tpm_commands/non_volatile_storage.rs +++ b/tss-esapi/src/context/tpm_commands/non_volatile_storage.rs @@ -476,8 +476,8 @@ impl Context { )?; Ok(( - NvPublic::try_from(Context::ffi_data_to_owned(nv_public_ptr))?, - Name::try_from(Context::ffi_data_to_owned(nv_name_ptr))?, + NvPublic::try_from(Context::ffi_data_to_owned(nv_public_ptr)?)?, + Name::try_from(Context::ffi_data_to_owned(nv_name_ptr)?)?, )) } @@ -825,7 +825,7 @@ impl Context { error!("Error when reading NV: {:#010X}", ret); }, )?; - MaxNvBuffer::try_from(Context::ffi_data_to_owned(data_ptr)) + MaxNvBuffer::try_from(Context::ffi_data_to_owned(data_ptr)?) } // Missing function: NV_ReadLock diff --git a/tss-esapi/src/context/tpm_commands/object_commands.rs b/tss-esapi/src/context/tpm_commands/object_commands.rs index 321a73c63..094f20062 100644 --- a/tss-esapi/src/context/tpm_commands/object_commands.rs +++ b/tss-esapi/src/context/tpm_commands/object_commands.rs @@ -216,9 +216,9 @@ impl Context { }, )?; Ok(( - Public::try_from(Context::ffi_data_to_owned(out_public_ptr))?, - Name::try_from(Context::ffi_data_to_owned(name_ptr))?, - Name::try_from(Context::ffi_data_to_owned(qualified_name_ptr))?, + Public::try_from(Context::ffi_data_to_owned(out_public_ptr)?)?, + Name::try_from(Context::ffi_data_to_owned(name_ptr)?)?, + Name::try_from(Context::ffi_data_to_owned(qualified_name_ptr)?)?, )) } @@ -250,7 +250,7 @@ impl Context { }, )?; - Digest::try_from(Context::ffi_data_to_owned(cert_info_ptr)) + Digest::try_from(Context::ffi_data_to_owned(cert_info_ptr)?) } /// Perform actions to create a [IdObject] containing an activation credential. @@ -283,8 +283,8 @@ impl Context { }, )?; Ok(( - IdObject::try_from(Context::ffi_data_to_owned(credential_blob_ptr))?, - EncryptedSecret::try_from(Context::ffi_data_to_owned(secret_ptr))?, + IdObject::try_from(Context::ffi_data_to_owned(credential_blob_ptr)?)?, + EncryptedSecret::try_from(Context::ffi_data_to_owned(secret_ptr)?)?, )) } @@ -307,7 +307,7 @@ impl Context { error!("Error in unsealing: {:#010X}", ret); }, )?; - SensitiveData::try_from(Context::ffi_data_to_owned(out_data_ptr)) + SensitiveData::try_from(Context::ffi_data_to_owned(out_data_ptr)?) } /// Change authorization for a TPM-resident object. @@ -335,7 +335,7 @@ impl Context { error!("Error changing object auth: {:#010X}", ret); }, )?; - Private::try_from(Context::ffi_data_to_owned(out_private_ptr)) + Private::try_from(Context::ffi_data_to_owned(out_private_ptr)?) } // Missing function: CreateLoaded diff --git a/tss-esapi/src/context/tpm_commands/object_commands/create_command_output.rs b/tss-esapi/src/context/tpm_commands/object_commands/create_command_output.rs index aaa0dbc63..b36b649d3 100644 --- a/tss-esapi/src/context/tpm_commands/object_commands/create_command_output.rs +++ b/tss-esapi/src/context/tpm_commands/object_commands/create_command_output.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ + ffi::take_from_esys, structures::{CreateKeyResult, CreationData, CreationTicket, Digest, Private, Public}, tss2_esys::{TPM2B_CREATION_DATA, TPM2B_DIGEST, TPM2B_PRIVATE, TPM2B_PUBLIC, TPMT_TK_CREATION}, Error, Result, @@ -61,17 +62,28 @@ impl CreateCommandOutputHandler { impl TryFrom for CreateKeyResult { type Error = Error; - fn try_from(ffi_data_handler: CreateCommandOutputHandler) -> Result { - let out_private_owned = - crate::ffi::to_owned_with_zeroized_source(ffi_data_handler.ffi_out_private_ptr); - let out_public_owned = - crate::ffi::to_owned_with_zeroized_source(ffi_data_handler.ffi_out_public_ptr); + fn try_from(mut ffi_data_handler: CreateCommandOutputHandler) -> Result { + // Take and free with Esys_Free; then null out the handler's fields so Drop (if any) + // won't free them a second time. + + let out_private_owned = unsafe { take_from_esys(ffi_data_handler.ffi_out_private_ptr)? }; + ffi_data_handler.ffi_out_private_ptr = null_mut(); + + let out_public_owned = unsafe { take_from_esys(ffi_data_handler.ffi_out_public_ptr)? }; + ffi_data_handler.ffi_out_public_ptr = null_mut(); + let creation_data_owned = - crate::ffi::to_owned_with_zeroized_source(ffi_data_handler.ffi_creation_data_ptr); + unsafe { take_from_esys(ffi_data_handler.ffi_creation_data_ptr)? }; + ffi_data_handler.ffi_creation_data_ptr = null_mut(); + let creation_hash_owned = - crate::ffi::to_owned_with_zeroized_source(ffi_data_handler.ffi_creation_hash_ptr); + unsafe { take_from_esys(ffi_data_handler.ffi_creation_hash_ptr)? }; + ffi_data_handler.ffi_creation_hash_ptr = null_mut(); + let creation_ticket_owned = - crate::ffi::to_owned_with_zeroized_source(ffi_data_handler.ffi_creation_ticket_ptr); + unsafe { take_from_esys(ffi_data_handler.ffi_creation_ticket_ptr)? }; + ffi_data_handler.ffi_creation_ticket_ptr = null_mut(); + Ok(CreateKeyResult { out_private: Private::try_from(out_private_owned)?, out_public: Public::try_from(out_public_owned)?, diff --git a/tss-esapi/src/context/tpm_commands/random_number_generator.rs b/tss-esapi/src/context/tpm_commands/random_number_generator.rs index 2d2b5376f..12a5176cc 100644 --- a/tss-esapi/src/context/tpm_commands/random_number_generator.rs +++ b/tss-esapi/src/context/tpm_commands/random_number_generator.rs @@ -33,7 +33,7 @@ impl Context { error!("Error in getting random bytes: {:#010X}", ret); }, )?; - Digest::try_from(Context::ffi_data_to_owned(random_bytes_ptr)) + Digest::try_from(Context::ffi_data_to_owned(random_bytes_ptr)?) } /// Add additional information into the TPM RNG state diff --git a/tss-esapi/src/context/tpm_commands/signing_and_signature_verification.rs b/tss-esapi/src/context/tpm_commands/signing_and_signature_verification.rs index f8456d1e5..3fbc3ee1a 100644 --- a/tss-esapi/src/context/tpm_commands/signing_and_signature_verification.rs +++ b/tss-esapi/src/context/tpm_commands/signing_and_signature_verification.rs @@ -36,7 +36,7 @@ impl Context { error!("Error when verifying signature: {:#010X}", ret); }, )?; - VerifiedTicket::try_from(Context::ffi_data_to_owned(validation_ptr)) + VerifiedTicket::try_from(Context::ffi_data_to_owned(validation_ptr)?) } /// Sign a digest with a key present in the TPM and return the signature. @@ -118,6 +118,6 @@ impl Context { error!("Error when signing: {:#010X}", ret); }, )?; - Signature::try_from(Context::ffi_data_to_owned(signature_ptr)) + Signature::try_from(Context::ffi_data_to_owned(signature_ptr)?) } } diff --git a/tss-esapi/src/context/tpm_commands/symmetric_primitives.rs b/tss-esapi/src/context/tpm_commands/symmetric_primitives.rs index c77157c48..797cb256b 100644 --- a/tss-esapi/src/context/tpm_commands/symmetric_primitives.rs +++ b/tss-esapi/src/context/tpm_commands/symmetric_primitives.rs @@ -215,8 +215,8 @@ impl Context { }, )?; Ok(( - MaxBuffer::try_from(Context::ffi_data_to_owned(out_data_ptr))?, - InitialValue::try_from(Context::ffi_data_to_owned(iv_out_ptr))?, + MaxBuffer::try_from(Context::ffi_data_to_owned(out_data_ptr)?)?, + InitialValue::try_from(Context::ffi_data_to_owned(iv_out_ptr)?)?, )) } @@ -290,8 +290,8 @@ impl Context { }, )?; Ok(( - Digest::try_from(Context::ffi_data_to_owned(out_hash_ptr))?, - HashcheckTicket::try_from(Context::ffi_data_to_owned(validation_ptr))?, + Digest::try_from(Context::ffi_data_to_owned(out_hash_ptr)?)?, + HashcheckTicket::try_from(Context::ffi_data_to_owned(validation_ptr)?)?, )) } @@ -369,7 +369,7 @@ impl Context { error!("Error in hmac: {:#010X}", ret); }, )?; - Digest::try_from(Context::ffi_data_to_owned(out_hmac_ptr)) + Digest::try_from(Context::ffi_data_to_owned(out_hmac_ptr)?) } // Missing function: MAC diff --git a/tss-esapi/src/context/tpm_commands/testing.rs b/tss-esapi/src/context/tpm_commands/testing.rs index 6da8d0144..fad19855a 100644 --- a/tss-esapi/src/context/tpm_commands/testing.rs +++ b/tss-esapi/src/context/tpm_commands/testing.rs @@ -75,7 +75,7 @@ impl Context { }, )?; Ok(( - MaxBuffer::try_from(Context::ffi_data_to_owned(out_data_ptr))?, + MaxBuffer::try_from(Context::ffi_data_to_owned(out_data_ptr)?)?, ReturnCode::ensure_success(test_result, |_| {}), )) } diff --git a/tss-esapi/src/ffi.rs b/tss-esapi/src/ffi.rs index 7812336e9..0a8a380d9 100644 --- a/tss-esapi/src/ffi.rs +++ b/tss-esapi/src/ffi.rs @@ -2,29 +2,33 @@ // SPDX-License-Identifier: Apache-2.0 pub mod data_zeroize; - -use crate::{ffi::data_zeroize::FfiDataZeroize, Error, Result, WrapperErrorKind}; +use crate::{ + ffi::data_zeroize::FfiDataZeroize, tss2_esys::Esys_Free, Error, Result, WrapperErrorKind, +}; use log::error; use malloced::Malloced; -use std::{convert::TryFrom, ops::Deref}; +use std::convert::TryFrom; +use std::{ffi::c_void, ptr}; -/// Function that takes ownership of data that has been -/// allocated with C memory allocation functions in TSS while also -/// zeroizing the memory before freeing it. -/// -/// # Arguments -/// * `ffi_data_ptr` - A pointer to the FFI data. +/// Move a value `T` out of ESAPI-allocated memory and free the source with Esys_Free. +/// The memory is zeroized before being freed. /// /// # Returns -/// The owned version of the FFI data. -pub(crate) fn to_owned_with_zeroized_source(ffi_data_ptr: *mut T) -> T +/// Returns an error if the pointer is null. +pub(crate) unsafe fn take_from_esys(ptr: *mut T) -> Result where T: FfiDataZeroize + Copy, { - let mut ffi_data = unsafe { Malloced::from_raw(ffi_data_ptr) }; - let owned_ffi_data: T = *ffi_data.deref(); - ffi_data.ffi_data_zeroize(); - owned_ffi_data + if ptr.is_null() { + error!("Received null pointer from ESAPI"); + return Err(Error::local_error(WrapperErrorKind::WrongValueFromTpm)); + } + + let out = ptr::read(ptr); + (*ptr).ffi_data_zeroize(); + Esys_Free(ptr.cast::()); + + Ok(out) } /// Function that takes ownership of bytes that are stored in a