From 02d6c71cdac81c084e8fa01844273af81029a1bc Mon Sep 17 00:00:00 2001 From: Jesper Brynolf Date: Thu, 18 Dec 2025 23:16:15 +0100 Subject: [PATCH] Fixes lint problems reported by Clippy in 1.92 In order to fix the lint problems it was necessary update MSRV to 1.76 and address the additional lint problems that arose when that change was introduced. Signed-off-by: Jesper Brynolf --- .clippy.toml | 2 +- .github/workflows/ci.yml | 4 +- README.md | 2 +- tss-esapi-sys/Cargo.toml | 2 +- tss-esapi/Cargo.toml | 2 +- tss-esapi/examples/duplication.rs | 9 +- tss-esapi/examples/duplication_secret.rs | 9 +- tss-esapi/src/abstraction/nv.rs | 3 +- .../tpm_commands/asymmetric_primitives.rs | 8 +- .../context/tpm_commands/object_commands.rs | 5 +- .../object_commands/create_command_output.rs | 131 +++++++++++++----- .../context/tpm_commands/session_commands.rs | 7 +- tss-esapi/src/lib.rs | 2 +- tss-esapi/src/traits.rs | 1 - tss-esapi/tests/create_frozen_cargo_lock | 2 +- 15 files changed, 124 insertions(+), 65 deletions(-) diff --git a/.clippy.toml b/.clippy.toml index 3b9db9dfe..7372c60ff 100644 --- a/.clippy.toml +++ b/.clippy.toml @@ -1 +1 @@ -msrv = "1.74.0" +msrv = "1.76.0" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9af14d4e2..e2cbc4bd8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,7 +31,7 @@ jobs: - name: Build the container run: docker build -t ubuntucontainer tss-esapi/tests/ --file tss-esapi/tests/Dockerfile-ubuntu --target tpm2-tools - name: Run the container - run: docker run -v $(pwd):/tmp/rust-tss-esapi -w /tmp/rust-tss-esapi/tss-esapi --env RUST_TOOLCHAIN_VERSION=1.74.0 ubuntucontainer /tmp/rust-tss-esapi/tss-esapi/tests/all-ubuntu.sh + run: docker run -v $(pwd):/tmp/rust-tss-esapi -w /tmp/rust-tss-esapi/tss-esapi --env RUST_TOOLCHAIN_VERSION=1.76.0 ubuntucontainer /tmp/rust-tss-esapi/tss-esapi/tests/all-ubuntu.sh # All in one job as I think it is a big overhead to build and run the Docker # container? tests-ubuntu: @@ -118,7 +118,7 @@ jobs: - name: Build the container run: docker build -t ubuntucontainer tss-esapi/tests/ --file tss-esapi/tests/Dockerfile-ubuntu --target tpm2-tss - name: Check Clippy lints MSRV - run: docker run -v $(pwd):/tmp/rust-tss-esapi -w /tmp/rust-tss-esapi/tss-esapi --env RUST_TOOLCHAIN_VERSION=1.74.0 ubuntucontainer /tmp/rust-tss-esapi/tss-esapi/tests/lint-checks.sh + run: docker run -v $(pwd):/tmp/rust-tss-esapi -w /tmp/rust-tss-esapi/tss-esapi --env RUST_TOOLCHAIN_VERSION=1.76.0 ubuntucontainer /tmp/rust-tss-esapi/tss-esapi/tests/lint-checks.sh - name: Check Clippy lints latest run: docker run -v $(pwd):/tmp/rust-tss-esapi -w /tmp/rust-tss-esapi/tss-esapi ubuntucontainer /tmp/rust-tss-esapi/tss-esapi/tests/lint-checks.sh diff --git a/README.md b/README.md index dee736ba1..1b5b94551 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ At the moment we test (via CI) and support the following Rust compiler versions: * On Ubuntu we test with: - The latest stable compiler version, as accessible through `rustup`. - - The 1.74.0 compiler version. + - The 1.76.0 compiler version. * On Fedora we test with the compiler version included with the Fedora 38 release. * On Fedora rawhide we test with the compiler version included. diff --git a/tss-esapi-sys/Cargo.toml b/tss-esapi-sys/Cargo.toml index 6439ed975..d2fbbacfa 100644 --- a/tss-esapi-sys/Cargo.toml +++ b/tss-esapi-sys/Cargo.toml @@ -11,7 +11,7 @@ license = "Apache-2.0" repository = "https://github.com/parallaxsecond/rust-tss-esapi" documentation = "https://docs.rs/crate/tss-esapi-sys" links = "tss2-esys" -rust-version = "1.74.0" +rust-version = "1.76.0" [build-dependencies] bindgen = { version = "0.72.0", optional = true } diff --git a/tss-esapi/Cargo.toml b/tss-esapi/Cargo.toml index 78f2e34b6..fa8e9fed6 100644 --- a/tss-esapi/Cargo.toml +++ b/tss-esapi/Cargo.toml @@ -10,7 +10,7 @@ categories = ["api-bindings", "external-ffi-bindings", "cryptography"] license = "Apache-2.0" repository = "https://github.com/parallaxsecond/rust-tss-esapi" documentation = "https://docs.rs/crate/tss-esapi" -rust-version = "1.74.0" +rust-version = "1.76.0" [[example]] name = "hmac" diff --git a/tss-esapi/examples/duplication.rs b/tss-esapi/examples/duplication.rs index 4df49f18d..11a3580a3 100644 --- a/tss-esapi/examples/duplication.rs +++ b/tss-esapi/examples/duplication.rs @@ -215,9 +215,8 @@ fn main() { None, ) }) - .map_err(|err| { + .inspect_err(|err| { eprintln!("⚠️ {err}"); - err }) .unwrap(); @@ -268,9 +267,8 @@ fn main() { .execute_with_nullauth_session(|ctx| { ctx.create(loaded_storage_key, hmac_public, None, None, None, None) }) - .map_err(|err| { + .inspect_err(|err| { eprintln!("⚠️ {err}"); - err }) .unwrap(); @@ -370,9 +368,8 @@ fn main() { // Return the duplicate result. result }) - .map_err(|err| { + .inspect_err(|err| { eprintln!("⚠️ {err}"); - err }) .unwrap(); diff --git a/tss-esapi/examples/duplication_secret.rs b/tss-esapi/examples/duplication_secret.rs index f1819fe92..92d15b2f6 100644 --- a/tss-esapi/examples/duplication_secret.rs +++ b/tss-esapi/examples/duplication_secret.rs @@ -216,9 +216,8 @@ fn main() { None, ) }) - .map_err(|err| { + .inspect_err(|err| { eprintln!("⚠️ {err}"); - err }) .unwrap(); @@ -267,9 +266,8 @@ fn main() { .execute_with_nullauth_session(|ctx| { ctx.create(loaded_storage_key, hmac_public, None, None, None, None) }) - .map_err(|err| { + .inspect_err(|err| { eprintln!("⚠️ {err}"); - err }) .unwrap(); @@ -363,9 +361,8 @@ fn main() { // Return the duplicate result. result }) - .map_err(|err| { + .inspect_err(|err| { eprintln!("⚠️ {err}"); - err }) .unwrap(); diff --git a/tss-esapi/src/abstraction/nv.rs b/tss-esapi/src/abstraction/nv.rs index 76646fc72..01562e688 100644 --- a/tss-esapi/src/abstraction/nv.rs +++ b/tss-esapi/src/abstraction/nv.rs @@ -53,9 +53,8 @@ fn get_nv_index_info( .and_then(|mut object_handle| { context .nv_read_public(NvIndexHandle::from(object_handle)) - .map_err(|e| { + .inspect_err(|_| { let _ = context.tr_close(&mut object_handle); - e }) .and_then(|(nv_public, name)| { context.tr_close(&mut object_handle)?; diff --git a/tss-esapi/src/context/tpm_commands/asymmetric_primitives.rs b/tss-esapi/src/context/tpm_commands/asymmetric_primitives.rs index 006d777e9..40758900c 100644 --- a/tss-esapi/src/context/tpm_commands/asymmetric_primitives.rs +++ b/tss-esapi/src/context/tpm_commands/asymmetric_primitives.rs @@ -151,7 +151,9 @@ impl Context { ) -> Result { let mut out_data_ptr = null_mut(); let potential_label = label.into().map(|v| v.into()); - let label_ptr = potential_label.as_ref().map_or_else(null, |v| v); + let label_ptr = potential_label + .as_ref() + .map_or_else(null, std::ptr::from_ref); ReturnCode::ensure_success( unsafe { Esys_RSA_Encrypt( @@ -310,7 +312,9 @@ impl Context { ) -> Result { let mut message_ptr = null_mut(); let potential_label = label.into().map(|v| v.into()); - let label_ptr = potential_label.as_ref().map_or_else(null, |v| v); + let label_ptr = potential_label + .as_ref() + .map_or_else(null, std::ptr::from_ref); ReturnCode::ensure_success( unsafe { Esys_RSA_Decrypt( diff --git a/tss-esapi/src/context/tpm_commands/object_commands.rs b/tss-esapi/src/context/tpm_commands/object_commands.rs index cf5eb5604..22f2fca83 100644 --- a/tss-esapi/src/context/tpm_commands/object_commands.rs +++ b/tss-esapi/src/context/tpm_commands/object_commands.rs @@ -234,6 +234,9 @@ impl Context { } let mut object_handle = ObjectHandle::None.into(); let potential_private_in = potential_private.map(|v| v.try_into()).transpose()?; + let private_in_ptr = potential_private_in + .as_ref() + .map_or_else(null, std::ptr::from_ref); let public_in = public.try_into()?; ReturnCode::ensure_success( unsafe { @@ -242,7 +245,7 @@ impl Context { self.optional_session_1(), self.optional_session_2(), self.optional_session_3(), - potential_private_in.as_ref().map_or_else(null, |v| v), + private_in_ptr, &public_in, if cfg!(hierarchy_is_esys_tr) { ObjectHandle::from(hierarchy).into() 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 b36b649d3..2f59c1c0b 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 @@ -7,55 +7,59 @@ use crate::{ tss2_esys::{TPM2B_CREATION_DATA, TPM2B_DIGEST, TPM2B_PRIVATE, TPM2B_PUBLIC, TPMT_TK_CREATION}, Error, Result, }; -use std::convert::TryFrom; -use std::ptr::null_mut; +use std::{convert::TryFrom, ops::Drop, ptr::null_mut}; /// Struct that handles the output of the /// Create Esys_Create command and zeroizes /// the FFI data. pub(crate) struct CreateCommandOutputHandler { - ffi_out_public_ptr: *mut TPM2B_PUBLIC, - ffi_out_private_ptr: *mut TPM2B_PRIVATE, - ffi_creation_data_ptr: *mut TPM2B_CREATION_DATA, - ffi_creation_hash_ptr: *mut TPM2B_DIGEST, - ffi_creation_ticket_ptr: *mut TPMT_TK_CREATION, + ffi_out_public_ptr: Option<*mut TPM2B_PUBLIC>, + ffi_out_private_ptr: Option<*mut TPM2B_PRIVATE>, + ffi_creation_data_ptr: Option<*mut TPM2B_CREATION_DATA>, + ffi_creation_hash_ptr: Option<*mut TPM2B_DIGEST>, + ffi_creation_ticket_ptr: Option<*mut TPMT_TK_CREATION>, } /// Creates a new CreateCommandOutputHandler impl CreateCommandOutputHandler { pub(crate) fn new() -> Self { Self { - ffi_out_private_ptr: null_mut(), - ffi_out_public_ptr: null_mut(), - ffi_creation_data_ptr: null_mut(), - ffi_creation_hash_ptr: null_mut(), - ffi_creation_ticket_ptr: null_mut(), + ffi_out_private_ptr: Some(null_mut()), + ffi_out_public_ptr: Some(null_mut()), + ffi_creation_data_ptr: Some(null_mut()), + ffi_creation_hash_ptr: Some(null_mut()), + ffi_creation_ticket_ptr: Some(null_mut()), } } /// A reference to the where 'outPrivate' output parameter pointer shall be stored. pub fn ffi_out_private_ptr(&mut self) -> &mut *mut TPM2B_PRIVATE { - &mut self.ffi_out_private_ptr + // The unwrap is will always be Some until its dropped. + self.ffi_out_private_ptr.as_mut().unwrap() } /// A reference to the where 'outPublic' output parameter pointer shall be stored. pub fn ffi_out_public_ptr(&mut self) -> &mut *mut TPM2B_PUBLIC { - &mut self.ffi_out_public_ptr + // The unwrap is will always be Some until its dropped. + self.ffi_out_public_ptr.as_mut().unwrap() } /// A reference to the where 'creationData' output parameter pointer shall be stored. pub fn ffi_creation_data_ptr(&mut self) -> &mut *mut TPM2B_CREATION_DATA { - &mut self.ffi_creation_data_ptr + // The unwrap is will always be Some until its dropped. + self.ffi_creation_data_ptr.as_mut().unwrap() } /// A reference to the where 'creationHash' output parameter pointer shall be stored. pub fn ffi_creation_hash_ptr(&mut self) -> &mut *mut TPM2B_DIGEST { - &mut self.ffi_creation_hash_ptr + // The unwrap is will always be Some until its dropped. + self.ffi_creation_hash_ptr.as_mut().unwrap() } /// A reference to the where 'creationTicket' output parameter pointer shall be stored. pub fn ffi_creation_ticket_ptr(&mut self) -> &mut *mut TPMT_TK_CREATION { - &mut self.ffi_creation_ticket_ptr + // The unwrap is will always be Some until its dropped. + self.ffi_creation_ticket_ptr.as_mut().unwrap() } } @@ -65,31 +69,84 @@ impl TryFrom for CreateKeyResult { 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. + // Be sure to take ownership of any data that might have been allocated + // so that it gets properly zeroized. + let out_private_result = ffi_data_handler + .ffi_out_private_ptr + .take() + .ok_or(Error::local_error( + crate::WrapperErrorKind::InternalError, // It cannot be None + )) + .and_then(|ptr| unsafe { take_from_esys(ptr) }) + .and_then(Private::try_from); + + let out_public_result = ffi_data_handler + .ffi_out_public_ptr + .take() + .ok_or(Error::local_error( + crate::WrapperErrorKind::InternalError, // It cannot be None + )) + .and_then(|ptr| unsafe { take_from_esys(ptr) }) + .and_then(Public::try_from); + + let creation_data_result = ffi_data_handler + .ffi_creation_data_ptr + .take() + .ok_or(Error::local_error( + crate::WrapperErrorKind::InternalError, // It cannot be None + )) + .and_then(|ptr| unsafe { take_from_esys(ptr) }) + .and_then(CreationData::try_from); + + let creation_hash_result = ffi_data_handler + .ffi_creation_hash_ptr + .take() + .ok_or(Error::local_error( + crate::WrapperErrorKind::InternalError, // It cannot be None + )) + .and_then(|ptr| unsafe { take_from_esys(ptr) }) + .and_then(Digest::try_from); + + let creation_ticket_result = ffi_data_handler + .ffi_creation_ticket_ptr + .take() + .ok_or(Error::local_error( + crate::WrapperErrorKind::InternalError, // It cannot be None + )) + .and_then(|ptr| unsafe { take_from_esys(ptr) }) + .and_then(CreationTicket::try_from); - let out_private_owned = unsafe { take_from_esys(ffi_data_handler.ffi_out_private_ptr)? }; - ffi_data_handler.ffi_out_private_ptr = null_mut(); + Ok(CreateKeyResult { + out_private: out_private_result?, + out_public: out_public_result?, + creation_data: creation_data_result?, + creation_hash: creation_hash_result?, + creation_ticket: creation_ticket_result?, + }) + } +} - let out_public_owned = unsafe { take_from_esys(ffi_data_handler.ffi_out_public_ptr)? }; - ffi_data_handler.ffi_out_public_ptr = null_mut(); +impl Drop for CreateCommandOutputHandler { + fn drop(&mut self) { + // Make sure we do not have anything allocated on the heap if the handler gets dropped. + let _ = self.ffi_out_private_ptr.take().inspect(|&ptr| { + let _ = unsafe { take_from_esys(ptr) }; + }); - let creation_data_owned = - unsafe { take_from_esys(ffi_data_handler.ffi_creation_data_ptr)? }; - ffi_data_handler.ffi_creation_data_ptr = null_mut(); + let _ = self.ffi_out_public_ptr.take().inspect(|&ptr| { + let _ = unsafe { take_from_esys(ptr) }; + }); - let creation_hash_owned = - unsafe { take_from_esys(ffi_data_handler.ffi_creation_hash_ptr)? }; - ffi_data_handler.ffi_creation_hash_ptr = null_mut(); + let _ = self.ffi_creation_data_ptr.take().inspect(|&ptr| { + let _ = unsafe { take_from_esys(ptr) }; + }); - let creation_ticket_owned = - unsafe { take_from_esys(ffi_data_handler.ffi_creation_ticket_ptr)? }; - ffi_data_handler.ffi_creation_ticket_ptr = null_mut(); + let _ = self.ffi_creation_hash_ptr.take().inspect(|&ptr| { + let _ = unsafe { take_from_esys(ptr) }; + }); - Ok(CreateKeyResult { - out_private: Private::try_from(out_private_owned)?, - out_public: Public::try_from(out_public_owned)?, - creation_data: CreationData::try_from(creation_data_owned)?, - creation_hash: Digest::try_from(creation_hash_owned)?, - creation_ticket: CreationTicket::try_from(creation_ticket_owned)?, - }) + let _ = self.ffi_creation_ticket_ptr.take().inspect(|&ptr| { + let _ = unsafe { take_from_esys(ptr) }; + }); } } diff --git a/tss-esapi/src/context/tpm_commands/session_commands.rs b/tss-esapi/src/context/tpm_commands/session_commands.rs index a225748a0..bdf0387f2 100644 --- a/tss-esapi/src/context/tpm_commands/session_commands.rs +++ b/tss-esapi/src/context/tpm_commands/session_commands.rs @@ -59,7 +59,10 @@ impl Context { auth_hash: HashingAlgorithm, ) -> Result> { let mut session_handle = ObjectHandle::None.into(); - let potential_tpm2b_nonce = nonce.map(|v| v.into()); + let potential_nonce_caller = nonce.map(|v| v.into()); + let nonce_caller_ptr = potential_nonce_caller + .as_ref() + .map_or_else(null, std::ptr::from_ref); ReturnCode::ensure_success( unsafe { Esys_StartAuthSession( @@ -72,7 +75,7 @@ impl Context { self.optional_session_1(), self.optional_session_2(), self.optional_session_3(), - potential_tpm2b_nonce.as_ref().map_or_else(null, |v| v), + nonce_caller_ptr, session_type.into(), &symmetric.try_into()?, auth_hash.into(), diff --git a/tss-esapi/src/lib.rs b/tss-esapi/src/lib.rs index 1fe9fea3c..a07b2ca1b 100644 --- a/tss-esapi/src/lib.rs +++ b/tss-esapi/src/lib.rs @@ -60,7 +60,7 @@ //! are at most one level away from root. //! //! Minimum supported Rust version (MSRV): -//! We currently check with version 1.74.0 of the Rust compiler during CI builds. +//! We currently check with version 1.76.0 of the Rust compiler during CI builds. //! //! # Notes on code safety: //! * thread safety is ensured by the required mutability of the `Context` structure within the diff --git a/tss-esapi/src/traits.rs b/tss-esapi/src/traits.rs index 0d8febae8..8cfe49b1f 100644 --- a/tss-esapi/src/traits.rs +++ b/tss-esapi/src/traits.rs @@ -179,7 +179,6 @@ macro_rules! impl_mu_complex { // Make the macros usable outside of the module. pub(crate) use impl_marshall_trait; -pub(crate) use impl_mu_aliases; pub(crate) use impl_mu_complex; pub(crate) use impl_mu_simple; pub(crate) use impl_mu_standard; diff --git a/tss-esapi/tests/create_frozen_cargo_lock b/tss-esapi/tests/create_frozen_cargo_lock index 5209f1276..0d18dfcf8 100755 --- a/tss-esapi/tests/create_frozen_cargo_lock +++ b/tss-esapi/tests/create_frozen_cargo_lock @@ -3,7 +3,7 @@ # Copyright 2024 Contributors to the Parsec project. # SPDX-License-Identifier: Apache-2.0 -export MSRV=1.74.0 +export MSRV=1.76.0 export EXEC_DIR="tss-esapi" if [[ "$(basename `pwd`)" != "$EXEC_DIR" ]]; then