From eaac8b063728c0f6bcb33bb8eba370a5e484e98a Mon Sep 17 00:00:00 2001 From: Dana Keeler Date: Fri, 15 Apr 2022 18:35:01 +0000 Subject: [PATCH] Bug 1724747 - run cargo fmt and cargo clippy on rsclientcerts/osclientcerts r=jschanck Depends on D143778 Differential Revision: https://phabricator.services.mozilla.com/D143779 --- .../ssl/osclientcerts/src/backend_macos.rs | 6 ++-- .../ssl/osclientcerts/src/backend_windows.rs | 11 +++---- security/manager/ssl/osclientcerts/src/lib.rs | 30 +++++++++---------- .../manager/ssl/rsclientcerts/src/manager.rs | 29 +++++++----------- .../manager/ssl/rsclientcerts/src/util.rs | 11 +++---- 5 files changed, 41 insertions(+), 46 deletions(-) diff --git a/security/manager/ssl/osclientcerts/src/backend_macos.rs b/security/manager/ssl/osclientcerts/src/backend_macos.rs index 90a289d46332..d638d01c9538 100644 --- a/security/manager/ssl/osclientcerts/src/backend_macos.rs +++ b/security/manager/ssl/osclientcerts/src/backend_macos.rs @@ -496,12 +496,14 @@ impl CryptokiObject for Cert { } } +#[allow(clippy::upper_case_acronyms)] #[derive(Clone, Copy, Debug)] pub enum KeyType { EC(usize), RSA, } +#[allow(clippy::upper_case_acronyms)] enum SignParams<'a> { EC(CFString, &'a [u8]), RSA(CFString, &'a [u8]), @@ -717,7 +719,7 @@ impl Key { let signing_algorithm = sign_params.get_algorithm(); let data_to_sign = CFData::from_buffer(sign_params.get_data_to_sign()); let signature = - SECURITY_FRAMEWORK.sec_key_create_signature(&key, signing_algorithm, &data_to_sign)?; + SECURITY_FRAMEWORK.sec_key_create_signature(key, signing_algorithm, &data_to_sign)?; let signature_value = match self.key_type_enum { KeyType::EC(coordinate_width) => { // We need to convert the DER Ecdsa-Sig-Value to the @@ -829,7 +831,7 @@ impl Sign for Key { } fn get_key_attribute(key: &SecKey, attr: CFStringRef) -> Result { - let attributes: CFDictionary = SECURITY_FRAMEWORK.sec_key_copy_attributes(&key)?; + let attributes: CFDictionary = SECURITY_FRAMEWORK.sec_key_copy_attributes(key)?; match attributes.find(attr as *const _) { Some(value) => Ok((*value).clone()), None => Err(error_here!(ErrorType::ExternalError)), diff --git a/security/manager/ssl/osclientcerts/src/backend_windows.rs b/security/manager/ssl/osclientcerts/src/backend_windows.rs index 804cec433042..f88d9f9b8712 100644 --- a/security/manager/ssl/osclientcerts/src/backend_windows.rs +++ b/security/manager/ssl/osclientcerts/src/backend_windows.rs @@ -98,7 +98,7 @@ impl Cert { unsafe { slice::from_raw_parts(cert.pbCertEncoded, cert.cbCertEncoded as usize) }; let value = value.to_vec(); let id = Sha256::digest(&value).to_vec(); - let label = get_cert_subject_dn(&cert_info)?; + let label = get_cert_subject_dn(cert_info)?; let (serial_number, issuer, subject) = read_encoded_certificate_identifiers(&value)?; Ok(Cert { class: serialize_uint(CKO_CERTIFICATE)?, @@ -515,15 +515,16 @@ impl SignParams { } fn flags(&self) -> u32 { - match self { - &SignParams::EC => 0, - &SignParams::RSA_PKCS1(_) => NCRYPT_PAD_PKCS1_FLAG, - &SignParams::RSA_PSS(_) => NCRYPT_PAD_PSS_FLAG, + match *self { + SignParams::EC => 0, + SignParams::RSA_PKCS1(_) => NCRYPT_PAD_PKCS1_FLAG, + SignParams::RSA_PSS(_) => NCRYPT_PAD_PSS_FLAG, } } } /// A helper enum to identify a private key's type. We support EC and RSA. +#[allow(clippy::upper_case_acronyms)] #[derive(Clone, Copy, Debug)] pub enum KeyType { EC, diff --git a/security/manager/ssl/osclientcerts/src/lib.rs b/security/manager/ssl/osclientcerts/src/lib.rs index 62991215d6cc..8ec77c9fb115 100644 --- a/security/manager/ssl/osclientcerts/src/lib.rs +++ b/security/manager/ssl/osclientcerts/src/lib.rs @@ -276,9 +276,9 @@ extern "C" fn C_GetMechanismList( log_with_thread_id!(error, "C_GetMechanismList: CKR_ARGUMENTS_BAD"); return CKR_ARGUMENTS_BAD; } - for i in 0..mechanisms.len() { + for (i, mechanism) in mechanisms.iter().enumerate() { unsafe { - *pMechanismList.offset(i as isize) = mechanisms[i]; + *pMechanismList.add(i) = *mechanism; } } } @@ -498,8 +498,8 @@ extern "C" fn C_GetAttributeValue( return CKR_ARGUMENTS_BAD; } let mut attr_types = Vec::with_capacity(ulCount as usize); - for i in 0..ulCount { - let attr = unsafe { &*pTemplate.offset(i as isize) }; + for i in 0..ulCount as usize { + let attr = unsafe { &*pTemplate.add(i) }; attr_types.push(attr.attrType); } let mut manager_guard = try_to_get_manager_guard!(); @@ -518,10 +518,9 @@ extern "C" fn C_GetAttributeValue( ); return CKR_DEVICE_ERROR; } - for i in 0..ulCount as usize { - let mut attr = unsafe { &mut *pTemplate.offset(i as isize) }; - // NB: the safety of this array access depends on the length check above - if let Some(attr_value) = &values[i] { + for (i, value) in values.iter().enumerate().take(ulCount as usize) { + let mut attr = unsafe { &mut *pTemplate.add(i) }; + if let Some(attr_value) = value { if attr.pValue.is_null() { attr.ulValueLen = attr_value.len() as CK_ULONG; } else { @@ -594,9 +593,9 @@ extern "C" fn C_FindObjectsInit( } let mut attrs = Vec::new(); log_with_thread_id!(trace, "C_FindObjectsInit:"); - for i in 0..ulCount { - let attr = unsafe { &*pTemplate.offset(i as isize) }; - trace_attr(" ", &attr); + for i in 0..ulCount as usize { + let attr = unsafe { &*pTemplate.add(i) }; + trace_attr(" ", attr); let slice = unsafe { std::slice::from_raw_parts(attr.pValue as *const u8, attr.ulValueLen as usize) }; @@ -1201,16 +1200,17 @@ static mut FUNCTION_LIST: CK_FUNCTION_LIST = CK_FUNCTION_LIST { C_WaitForSlotEvent: Some(C_WaitForSlotEvent), }; +/// # Safety +/// /// This is the only function this module exposes. NSS calls it to obtain the list of functions /// comprising this module. +/// ppFunctionList must be a valid pointer. #[no_mangle] -pub extern "C" fn C_GetFunctionList(ppFunctionList: CK_FUNCTION_LIST_PTR_PTR) -> CK_RV { +pub unsafe extern "C" fn C_GetFunctionList(ppFunctionList: CK_FUNCTION_LIST_PTR_PTR) -> CK_RV { if ppFunctionList.is_null() { return CKR_ARGUMENTS_BAD; } - unsafe { - *ppFunctionList = &mut FUNCTION_LIST; - } + *ppFunctionList = &mut FUNCTION_LIST; CKR_OK } diff --git a/security/manager/ssl/rsclientcerts/src/manager.rs b/security/manager/ssl/rsclientcerts/src/manager.rs index b8b47dcc9b7b..98c8eee32af6 100644 --- a/security/manager/ssl/rsclientcerts/src/manager.rs +++ b/security/manager/ssl/rsclientcerts/src/manager.rs @@ -45,6 +45,7 @@ pub trait ClientCertsBackend { type Cert: CryptokiObject; type Key: CryptokiObject + Sign; + #[allow(clippy::type_complexity)] fn find_objects(&self) -> Result<(Vec, Vec), Error>; } @@ -123,13 +124,7 @@ impl ManagerProxy { .name("osclientcert".into()) .spawn(move || { let mut real_manager = Manager::new(backend); - loop { - let arguments = match manager_receiver.recv() { - Ok(arguments) => arguments, - Err(_) => { - break; - } - }; + while let Ok(arguments) = manager_receiver.recv() { let results = match arguments { ManagerArguments::OpenSession(slot_type) => { ManagerReturnValue::OpenSession(real_manager.open_session(slot_type)) @@ -175,10 +170,7 @@ impl ManagerProxy { } ManagerArguments::Stop => ManagerReturnValue::Stop(Ok(())), }; - let stop_after_send = match &results { - &ManagerReturnValue::Stop(_) => true, - _ => false, - }; + let stop_after_send = matches!(&results, &ManagerReturnValue::Stop(_)); match manager_sender.send(results) { Ok(()) => {} Err(_) => { @@ -342,7 +334,7 @@ fn search_is_for_all_certificates_or_keys( if attrs.len() != 2 { return Ok(false); } - let token_bytes = vec![1 as u8]; + let token_bytes = vec![1_u8]; let mut found_token = false; let cko_certificate_bytes = serialize_uint(CKO_CERTIFICATE)?; let cko_private_key_bytes = serialize_uint(CKO_PRIVATE_KEY)?; @@ -397,7 +389,7 @@ impl Object { fn id(&self) -> Result<&[u8], Error> { self.get_attribute(CKA_ID) - .ok_or(error_here!(ErrorType::LibraryFailure)) + .ok_or_else(|| error_here!(ErrorType::LibraryFailure)) } fn get_signature_length( @@ -514,7 +506,7 @@ impl Manager { pub fn close_session(&mut self, session: CK_SESSION_HANDLE) -> Result<(), Error> { self.sessions .remove(&session) - .ok_or(error_here!(ErrorType::InvalidInput)) + .ok_or_else(|| error_here!(ErrorType::InvalidInput)) .map(|_| ()) } @@ -622,10 +614,9 @@ impl Manager { }; let mut results = Vec::with_capacity(attr_types.len()); for attr_type in attr_types { - let result = match object.get_attribute(attr_type) { - Some(value) => Some(value.to_owned()), - None => None, - }; + let result = object + .get_attribute(attr_type) + .map(|value| value.to_owned()); results.push(result); } Ok(results) @@ -657,7 +648,7 @@ impl Manager { Some((key_handle, params)) => (key_handle, params), None => return Err(error_here!(ErrorType::InvalidArgument)), }; - let key = match self.objects.get_mut(&key_handle) { + let key = match self.objects.get_mut(key_handle) { Some(key) => key, None => return Err(error_here!(ErrorType::InvalidArgument)), }; diff --git a/security/manager/ssl/rsclientcerts/src/util.rs b/security/manager/ssl/rsclientcerts/src/util.rs index b0dd39c2ce70..d0011a0a2ed3 100644 --- a/security/manager/ssl/rsclientcerts/src/util.rs +++ b/security/manager/ssl/rsclientcerts/src/util.rs @@ -91,7 +91,7 @@ pub fn read_rsa_modulus(public_key: &[u8]) -> Result, Error> { /// parameters ANY DEFINED BY algorithm OPTIONAL } /// /// Digest ::= OCTET STRING -pub fn read_digest_info<'a>(digest_info: &'a [u8]) -> Result<(&'a [u8], &'a [u8]), Error> { +pub fn read_digest_info(digest_info: &[u8]) -> Result<(&[u8], &[u8]), Error> { let mut sequence = Sequence::new(digest_info)?; let mut algorithm = sequence.read_sequence()?; let oid = algorithm.read_oid()?; @@ -112,7 +112,7 @@ pub fn read_digest_info<'a>(digest_info: &'a [u8]) -> Result<(&'a [u8], &'a [u8] /// r INTEGER, /// s INTEGER } #[cfg(target_os = "macos")] -pub fn read_ec_sig_point<'a>(signature: &'a [u8]) -> Result<(&'a [u8], &'a [u8]), Error> { +pub fn read_ec_sig_point(signature: &[u8]) -> Result<(&[u8], &[u8]), Error> { let mut sequence = Sequence::new(signature)?; let r = sequence.read_unsigned_integer()?; let s = sequence.read_unsigned_integer()?; @@ -149,6 +149,7 @@ pub fn read_ec_sig_point<'a>(signature: &'a [u8]) -> Result<(&'a [u8], &'a [u8]) /// Validity ::= SEQUENCE { /// notBefore Time, /// notAfter Time } +#[allow(clippy::type_complexity)] pub fn read_encoded_certificate_identifiers( certificate: &[u8], ) -> Result<(Vec, Vec, Vec), Error> { @@ -240,7 +241,7 @@ impl<'a> Sequence<'a> { fn read_null(&mut self) -> Result<(), Error> { let (_, _, bytes) = self.contents.read_tlv(NULL)?; - if bytes.len() == 0 { + if bytes.is_empty() { Ok(()) } else { Err(error_here!(ErrorType::InvalidInput)) @@ -311,9 +312,9 @@ impl<'a> Der<'a> { } (length[0] as usize, rest) } else if length1[0] == 0x82 { - let (lengths, rest) = try_read_bytes!(rest, 2); + let (mut lengths, rest) = try_read_bytes!(rest, 2); accumulated_length_bytes.extend_from_slice(lengths); - let length = (&mut &lengths[..]) + let length = lengths .read_u16::() .map_err(|_| error_here!(ErrorType::LibraryFailure))?; if length < 256 {