From 2416870cc7ab4ab59ac86d136599fc1b0275009f Mon Sep 17 00:00:00 2001 From: Ionut Mihalcea Date: Wed, 16 Sep 2026 11:59:02 +0100 Subject: [PATCH] Fix AllowedMechanisms issue Signed-off-by: Ionut Mihalcea --- cryptoki/src/object.rs | 30 ++++----- cryptoki/tests/allowed_mechanisms.rs | 94 ++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 18 deletions(-) create mode 100644 cryptoki/tests/allowed_mechanisms.rs diff --git a/cryptoki/src/object.rs b/cryptoki/src/object.rs index 26032037..4ad07b20 100644 --- a/cryptoki/src/object.rs +++ b/cryptoki/src/object.rs @@ -1120,6 +1120,9 @@ impl TryFrom for Attribute { fn try_from(attribute: CK_ATTRIBUTE) -> Result { let attr_type = AttributeType::try_from(attribute.type_)?; + if attribute.pValue.is_null() && attribute.ulValueLen != 0 { + return Err(Error::InvalidValue); + } let val = if attribute.pValue.is_null() { // if pValue is null, return an empty slice - attribute has no value &[] @@ -1256,25 +1259,16 @@ impl TryFrom for Attribute { Ok(Attribute::ValidationVersion(Version::new(val[0], val[1]))) } AttributeType::AllowedMechanisms => { - if attribute.ulValueLen == 0 { - /* For zero-length attributes we are getting pointer to static - * buffer of length zero, which can not be used to create slices. - * Short-circuit here to avoid crash (#324) */ - Ok(Attribute::AllowedMechanisms(Vec::new())) - } else { - let val = unsafe { - std::slice::from_raw_parts( - attribute.pValue as *const CK_MECHANISM_TYPE, - attribute.ulValueLen.try_into()?, - ) - }; - let types = val - .iter() - .copied() - .map(|t| t.try_into()) - .collect::>>()?; - Ok(Attribute::AllowedMechanisms(types)) + if val.len() % size_of::() != 0 { + return Err(Error::InvalidValue); } + let types = val + .chunks_exact(size_of::()) + .map(|bytes| -> Result { + CK_MECHANISM_TYPE::from_ne_bytes(bytes.try_into()?).try_into() + }) + .collect::>>()?; + Ok(Attribute::AllowedMechanisms(types)) } AttributeType::EndDate => { if val.is_empty() { diff --git a/cryptoki/tests/allowed_mechanisms.rs b/cryptoki/tests/allowed_mechanisms.rs new file mode 100644 index 00000000..f82cde63 --- /dev/null +++ b/cryptoki/tests/allowed_mechanisms.rs @@ -0,0 +1,94 @@ +// Copyright 2026 Contributors to the Parsec project. +// SPDX-License-Identifier: Apache-2.0 + +use std::mem::size_of; + +use cryptoki::{error::Error, mechanism::MechanismType, object::Attribute}; +use cryptoki_sys::{CKA_ALLOWED_MECHANISMS, CK_ATTRIBUTE, CK_MECHANISM_TYPE}; + +#[test] +fn allowed_mechanisms_raw_length_is_measured_in_bytes() { + let mechanisms = vec![MechanismType::AES_CBC, MechanismType::AES_GCM]; + let attribute = Attribute::AllowedMechanisms(mechanisms.clone()); + + let raw = CK_ATTRIBUTE::from(&attribute); + let raw_type = raw.type_; + let raw_len = raw.ulValueLen; + + // Assert against the local, properly aligned variables + assert_eq!(raw_type, CKA_ALLOWED_MECHANISMS); + assert_eq!( + raw_len as usize, + mechanisms.len() * size_of::() + ); +} + +#[test] +fn nonempty_allowed_mechanisms_round_trip_preserves_element_count() { + let expected = vec![MechanismType::AES_CBC, MechanismType::AES_GCM]; + let attribute = Attribute::AllowedMechanisms(expected.clone()); + let raw = CK_ATTRIBUTE::from(&attribute); + + let decoded = Attribute::try_from(raw).unwrap(); + + assert_eq!(decoded, Attribute::AllowedMechanisms(expected)); +} + +#[test] +fn unaligned_allowed_mechanisms_value_is_decoded_as_bytes() { + let expected = vec![MechanismType::AES_CBC, MechanismType::AES_GCM]; + let mechanism_size = size_of::(); + let declared_byte_len = expected.len() * mechanism_size; + let mut backing = vec![0_u8; declared_byte_len + 1]; + + for (index, mechanism) in expected.iter().copied().enumerate() { + let mechanism = CK_MECHANISM_TYPE::from(mechanism).to_ne_bytes(); + let start = 1 + index * mechanism_size; + backing[start..start + mechanism_size].copy_from_slice(&mechanism); + } + + let raw = CK_ATTRIBUTE { + type_: CKA_ALLOWED_MECHANISMS, + pValue: backing[1..].as_mut_ptr().cast(), + ulValueLen: declared_byte_len.try_into().unwrap(), + }; + + let decoded = Attribute::try_from(raw).unwrap(); + + assert_eq!(decoded, Attribute::AllowedMechanisms(expected)); +} + +#[test] +fn malformed_allowed_mechanisms_byte_length_is_rejected() { + let mut backing = vec![0_u8; size_of::() + 1]; + let raw = CK_ATTRIBUTE { + type_: CKA_ALLOWED_MECHANISMS, + pValue: backing.as_mut_ptr().cast(), + ulValueLen: backing.len().try_into().unwrap(), + }; + + assert!(matches!(Attribute::try_from(raw), Err(Error::InvalidValue))); +} + +#[test] +fn null_nonempty_allowed_mechanisms_value_is_rejected() { + let raw = CK_ATTRIBUTE { + type_: CKA_ALLOWED_MECHANISMS, + pValue: std::ptr::null_mut(), + ulValueLen: size_of::().try_into().unwrap(), + }; + + assert!(matches!(Attribute::try_from(raw), Err(Error::InvalidValue))); +} + +#[cfg(miri)] +#[test] +fn direct_nonempty_allowed_mechanisms_round_trip_stays_in_bounds() { + let expected = vec![MechanismType::AES_CBC, MechanismType::AES_GCM]; + let attribute = Attribute::AllowedMechanisms(expected.clone()); + let raw = CK_ATTRIBUTE::from(&attribute); + + let decoded = Attribute::try_from(raw).unwrap(); + + assert_eq!(decoded, Attribute::AllowedMechanisms(expected)); +}