From 0dc444cb160c8e7b100d476749a425b9f2865fdf Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Fri, 16 Aug 2024 14:25:14 -0400 Subject: [PATCH] Cleanup usage of unsafe blocks --- aws-lc-rs/src/agreement.rs | 27 +++++-------- aws-lc-rs/src/cbs.rs | 6 +-- aws-lc-rs/src/ec.rs | 76 ++++++++++++++++++----------------- aws-lc-rs/src/ec/key_pair.rs | 14 +++---- aws-lc-rs/src/rsa/encoding.rs | 22 +++++----- aws-lc-rs/src/rsa/key.rs | 38 +++++++++--------- aws-lc-rs/src/unstable/kdf.rs | 2 +- 7 files changed, 90 insertions(+), 95 deletions(-) diff --git a/aws-lc-rs/src/agreement.rs b/aws-lc-rs/src/agreement.rs index 03e5bc8d7b4..32d6b5d3e98 100644 --- a/aws-lc-rs/src/agreement.rs +++ b/aws-lc-rs/src/agreement.rs @@ -277,7 +277,7 @@ impl PrivateKey { if AlgorithmID::X25519 == alg.id { return Err(KeyRejected::invalid_encoding()); } - let evp_pkey = unsafe { ec::unmarshal_der_to_private_key(key_bytes, alg.id.nid())? }; + let evp_pkey = ec::unmarshal_der_to_private_key(key_bytes, alg.id.nid())?; Ok(Self::new(alg, evp_pkey)) } @@ -397,15 +397,12 @@ impl PrivateKey { | KeyInner::ECDH_P384(evp_pkey) | KeyInner::ECDH_P521(evp_pkey) => { let mut buffer = [0u8; MAX_PUBLIC_KEY_LEN]; - unsafe { - let key_len = - ec::marshal_public_key_to_buffer(&mut buffer, &evp_pkey.as_const())?; - Ok(PublicKey { - alg: self.algorithm(), - public_key: buffer, - len: key_len, - }) - } + let key_len = ec::marshal_public_key_to_buffer(&mut buffer, &evp_pkey.as_const())?; + Ok(PublicKey { + alg: self.algorithm(), + public_key: buffer, + len: key_len, + }) } KeyInner::X25519(priv_key) => { let mut buffer = [0u8; MAX_PUBLIC_KEY_LEN]; @@ -472,12 +469,10 @@ impl AsBigEndian> for PrivateKey { if AlgorithmID::X25519 == self.inner_key.algorithm().id { return Err(Unspecified); } - let buffer = unsafe { - ec::marshal_private_key_to_buffer( - self.inner_key.algorithm().id.private_key_len(), - &self.inner_key.get_evp_pkey().as_const(), - )? - }; + let buffer = ec::marshal_private_key_to_buffer( + self.inner_key.algorithm().id.private_key_len(), + &self.inner_key.get_evp_pkey().as_const(), + )?; Ok(EcPrivateKeyBin::new(buffer)) } } diff --git a/aws-lc-rs/src/cbs.rs b/aws-lc-rs/src/cbs.rs index f464d9b7e91..67fd71855ef 100644 --- a/aws-lc-rs/src/cbs.rs +++ b/aws-lc-rs/src/cbs.rs @@ -6,8 +6,8 @@ use core::mem::MaybeUninit; #[inline] #[allow(non_snake_case)] -pub unsafe fn build_CBS(data: &[u8]) -> CBS { +pub fn build_CBS(data: &[u8]) -> CBS { let mut cbs = MaybeUninit::::uninit(); - CBS_init(cbs.as_mut_ptr(), data.as_ptr(), data.len()); - cbs.assume_init() + unsafe { CBS_init(cbs.as_mut_ptr(), data.as_ptr(), data.len()) }; + unsafe { cbs.assume_init() } } diff --git a/aws-lc-rs/src/ec.rs b/aws-lc-rs/src/ec.rs index 13dab3bf8d6..fc4e2ca8610 100644 --- a/aws-lc-rs/src/ec.rs +++ b/aws-lc-rs/src/ec.rs @@ -21,7 +21,7 @@ use aws_lc::EC_KEY_check_fips; #[cfg(not(feature = "fips"))] use aws_lc::EC_KEY_check_key; use aws_lc::{ - point_conversion_form_t, BN_bn2bin_padded, BN_num_bytes, ECDSA_SIG_from_bytes, + d2i_PrivateKey, point_conversion_form_t, BN_bn2bin_padded, BN_num_bytes, ECDSA_SIG_from_bytes, ECDSA_SIG_get0_r, ECDSA_SIG_get0_s, ECDSA_SIG_new, ECDSA_SIG_set0, ECDSA_SIG_to_bytes, EC_GROUP_get_curve_name, EC_GROUP_new_by_curve_name, EC_KEY_get0_group, EC_KEY_get0_private_key, EC_KEY_get0_public_key, EC_KEY_new, EC_KEY_set_group, @@ -319,40 +319,42 @@ fn validate_evp_key( Ok(()) } -pub(crate) unsafe fn marshal_private_key_to_buffer( +pub(crate) fn marshal_private_key_to_buffer( private_size: usize, evp_pkey: &ConstPointer, ) -> Result, Unspecified> { - let ec_key = ConstPointer::new(EVP_PKEY_get0_EC_KEY(**evp_pkey))?; - let private_bn = ConstPointer::new(EC_KEY_get0_private_key(*ec_key))?; + let ec_key = ConstPointer::new(unsafe { EVP_PKEY_get0_EC_KEY(**evp_pkey) })?; + let private_bn = ConstPointer::new(unsafe { EC_KEY_get0_private_key(*ec_key) })?; { - let size: usize = BN_num_bytes(*private_bn).try_into()?; + let size: usize = unsafe { BN_num_bytes(*private_bn).try_into()? }; debug_assert!(size <= private_size); } let mut buffer = vec![0u8; private_size]; - if 1 != BN_bn2bin_padded(buffer.as_mut_ptr(), private_size, *private_bn) { + if 1 != unsafe { BN_bn2bin_padded(buffer.as_mut_ptr(), private_size, *private_bn) } { return Err(Unspecified); } Ok(buffer) } -pub(crate) unsafe fn unmarshal_der_to_private_key( +pub(crate) fn unmarshal_der_to_private_key( key_bytes: &[u8], nid: i32, ) -> Result, KeyRejected> { let mut out = null_mut(); // `d2i_PrivateKey` -> ... -> `EC_KEY_parse_private_key` -> `EC_KEY_check_key` - let evp_pkey = LcPtr::new(aws_lc::d2i_PrivateKey( - EVP_PKEY_EC, - &mut out, - &mut key_bytes.as_ptr(), - key_bytes - .len() - .try_into() - .map_err(|_| KeyRejected::too_large())?, - ))?; + let evp_pkey = LcPtr::new(unsafe { + d2i_PrivateKey( + EVP_PKEY_EC, + &mut out, + &mut key_bytes.as_ptr(), + key_bytes + .len() + .try_into() + .map_err(|_| KeyRejected::too_large())?, + ) + })?; #[cfg(not(feature = "fips"))] verify_evp_key_nid(&evp_pkey.as_const(), nid)?; #[cfg(feature = "fips")] @@ -361,21 +363,21 @@ pub(crate) unsafe fn unmarshal_der_to_private_key( Ok(evp_pkey) } -pub(crate) unsafe fn marshal_public_key_to_buffer( +pub(crate) fn marshal_public_key_to_buffer( buffer: &mut [u8; PUBLIC_KEY_MAX_LEN], evp_pkey: &ConstPointer, ) -> Result { - let ec_key = ConstPointer::new(EVP_PKEY_get0_EC_KEY(**evp_pkey))?; + let ec_key = ConstPointer::new(unsafe { EVP_PKEY_get0_EC_KEY(**evp_pkey) })?; marshal_ec_public_key_to_buffer(buffer, &ec_key) } -pub(crate) unsafe fn marshal_ec_public_key_to_buffer( +pub(crate) fn marshal_ec_public_key_to_buffer( buffer: &mut [u8; PUBLIC_KEY_MAX_LEN], ec_key: &ConstPointer, ) -> Result { - let ec_group = ConstPointer::new(EC_KEY_get0_group(**ec_key))?; + let ec_group = ConstPointer::new(unsafe { EC_KEY_get0_group(**ec_key) })?; - let ec_point = ConstPointer::new(EC_KEY_get0_public_key(**ec_key))?; + let ec_point = ConstPointer::new(unsafe { EC_KEY_get0_public_key(**ec_key) })?; let out_len = ec_point_to_bytes(&ec_group, &ec_point, buffer)?; Ok(out_len) @@ -386,14 +388,12 @@ pub(crate) fn marshal_public_key( algorithm: &'static EcdsaSigningAlgorithm, ) -> Result { let mut pub_key_bytes = [0u8; PUBLIC_KEY_MAX_LEN]; - unsafe { - let key_len = marshal_public_key_to_buffer(&mut pub_key_bytes, evp_pkey)?; + let key_len = marshal_public_key_to_buffer(&mut pub_key_bytes, evp_pkey)?; - Ok(PublicKey { - algorithm, - octets: pub_key_bytes[0..key_len].into(), - }) - } + Ok(PublicKey { + algorithm, + octets: pub_key_bytes[0..key_len].into(), + }) } #[inline] @@ -548,21 +548,23 @@ pub(crate) fn ec_point_from_bytes( } #[inline] -unsafe fn ec_point_to_bytes( +fn ec_point_to_bytes( ec_group: &ConstPointer, ec_point: &ConstPointer, buf: &mut [u8; PUBLIC_KEY_MAX_LEN], ) -> Result { let pt_conv_form = point_conversion_form_t::POINT_CONVERSION_UNCOMPRESSED; - let out_len = EC_POINT_point2oct( - **ec_group, - **ec_point, - pt_conv_form, - buf.as_mut_ptr(), - PUBLIC_KEY_MAX_LEN, - null_mut(), - ); + let out_len = unsafe { + EC_POINT_point2oct( + **ec_group, + **ec_point, + pt_conv_form, + buf.as_mut_ptr(), + PUBLIC_KEY_MAX_LEN, + null_mut(), + ) + }; if out_len == 0 { return Err(Unspecified); } diff --git a/aws-lc-rs/src/ec/key_pair.rs b/aws-lc-rs/src/ec/key_pair.rs index 8da00d2970b..db9b0552273 100644 --- a/aws-lc-rs/src/ec/key_pair.rs +++ b/aws-lc-rs/src/ec/key_pair.rs @@ -182,7 +182,7 @@ impl EcdsaKeyPair { alg: &'static EcdsaSigningAlgorithm, private_key: &[u8], ) -> Result { - let evp_pkey = unsafe { ec::unmarshal_der_to_private_key(private_key, alg.id.nid())? }; + let evp_pkey = ec::unmarshal_der_to_private_key(private_key, alg.id.nid())?; Ok(Self::new(alg, evp_pkey)?) } @@ -297,13 +297,11 @@ impl AsBigEndian> for PrivateKey<'_> { /// # Errors /// `error::Unspecified` if serialization failed. fn as_be_bytes(&self) -> Result, Unspecified> { - unsafe { - let buffer = ec::marshal_private_key_to_buffer( - self.0.algorithm.id.private_key_size(), - &self.0.evp_pkey.as_const(), - )?; - Ok(EcPrivateKeyBin::new(buffer)) - } + let buffer = ec::marshal_private_key_to_buffer( + self.0.algorithm.id.private_key_size(), + &self.0.evp_pkey.as_const(), + )?; + Ok(EcPrivateKeyBin::new(buffer)) } } diff --git a/aws-lc-rs/src/rsa/encoding.rs b/aws-lc-rs/src/rsa/encoding.rs index 4d7f3be9905..f8d5c3ce54f 100644 --- a/aws-lc-rs/src/rsa/encoding.rs +++ b/aws-lc-rs/src/rsa/encoding.rs @@ -35,7 +35,7 @@ pub(in crate::rsa) mod pkcs8 { // Supports v1 and v2 encodings through a single API entry-point. pub(in crate::rsa) fn decode_der(pkcs8: &[u8]) -> Result, KeyRejected> { - let mut cbs = unsafe { cbs::build_CBS(pkcs8) }; + let mut cbs = cbs::build_CBS(pkcs8); let key = LcPtr::new(unsafe { EVP_parse_private_key(&mut cbs) }) .map_err(|()| KeyRejected::invalid_encoding())?; if !is_rsa_key(&key) { @@ -61,20 +61,22 @@ pub(in crate::rsa) mod rfc8017 { use std::ptr::null_mut; /// DER encode a RSA public key to `RSAPublicKey` structure. - pub(in crate::rsa) unsafe fn encode_public_key_der( + pub(in crate::rsa) fn encode_public_key_der( pubkey: &LcPtr, ) -> Result, Unspecified> { let mut pubkey_bytes = null_mut::(); let mut outlen: usize = 0; - if 1 != RSA_public_key_to_bytes( - &mut pubkey_bytes, - &mut outlen, - *pubkey.get_rsa()?.as_const(), - ) { + if 1 != unsafe { + RSA_public_key_to_bytes( + &mut pubkey_bytes, + &mut outlen, + *pubkey.get_rsa()?.as_const(), + ) + } { return Err(Unspecified); } let pubkey_bytes = LcPtr::new(pubkey_bytes)?; - let pubkey_slice = pubkey_bytes.as_slice(outlen); + let pubkey_slice = unsafe { pubkey_bytes.as_slice(outlen) }; let pubkey_vec = Vec::from(pubkey_slice); Ok(pubkey_vec.into_boxed_slice()) } @@ -104,7 +106,7 @@ pub(in crate::rsa) mod rfc8017 { pub(in crate::rsa) fn decode_private_key_der( private_key: &[u8], ) -> Result, KeyRejected> { - let mut cbs = unsafe { cbs::build_CBS(private_key) }; + let mut cbs = cbs::build_CBS(private_key); let rsa = DetachableLcPtr::new(unsafe { RSA_parse_private_key(&mut cbs) })?; @@ -155,7 +157,7 @@ pub(in crate::rsa) mod rfc5280 { pub(in crate::rsa) fn decode_public_key_der( value: &[u8], ) -> Result, KeyRejected> { - let mut der = unsafe { cbs::build_CBS(value) }; + let mut der = cbs::build_CBS(value); let key = LcPtr::new(unsafe { EVP_parse_public_key(&mut der) })?; if !is_rsa_key(&key) { return Err(KeyRejected::unspecified()); diff --git a/aws-lc-rs/src/rsa/key.rs b/aws-lc-rs/src/rsa/key.rs index aa23beccd08..77e28c60ac3 100644 --- a/aws-lc-rs/src/rsa/key.rs +++ b/aws-lc-rs/src/rsa/key.rs @@ -109,7 +109,7 @@ unsafe impl Sync for KeyPair {} impl KeyPair { fn new(evp_pkey: LcPtr) -> Result { KeyPair::validate_private_key(&evp_pkey)?; - let serialized_public_key = unsafe { PublicKey::new(&evp_pkey)? }; + let serialized_public_key = PublicKey::new(&evp_pkey)?; Ok(KeyPair { evp_pkey, serialized_public_key, @@ -312,14 +312,14 @@ impl Drop for PublicKey { } impl PublicKey { - pub(super) unsafe fn new(evp_pkey: &LcPtr) -> Result { + pub(super) fn new(evp_pkey: &LcPtr) -> Result { let key = encoding::rfc8017::encode_public_key_der(evp_pkey)?; #[cfg(feature = "ring-io")] { let pubkey = evp_pkey.get_rsa()?; - let modulus = ConstPointer::new(RSA_get0_n(*pubkey))?; + let modulus = ConstPointer::new(unsafe { RSA_get0_n(*pubkey) })?; let modulus = modulus.to_be_bytes().into_boxed_slice(); - let exponent = ConstPointer::new(RSA_get0_e(*pubkey))?; + let exponent = ConstPointer::new(unsafe { RSA_get0_e(*pubkey) })?; let exponent = exponent.to_be_bytes().into_boxed_slice(); Ok(PublicKey { key, @@ -400,7 +400,7 @@ where B: AsRef<[u8]> + Debug, { #[inline] - unsafe fn build_rsa(&self) -> Result, ()> { + fn build_rsa(&self) -> Result, ()> { let n_bytes = self.n.as_ref(); if n_bytes.is_empty() || n_bytes[0] == 0u8 { return Err(()); @@ -413,15 +413,15 @@ where } let e_bn = DetachableLcPtr::try_from(e_bytes)?; - let rsa = DetachableLcPtr::new(RSA_new())?; - if 1 != RSA_set0_key(*rsa, *n_bn, *e_bn, null_mut()) { + let rsa = DetachableLcPtr::new(unsafe { RSA_new() })?; + if 1 != unsafe { RSA_set0_key(*rsa, *n_bn, *e_bn, null_mut()) } { return Err(()); } n_bn.detach(); e_bn.detach(); - let pkey = LcPtr::new(EVP_PKEY_new())?; - if 1 != EVP_PKEY_assign_RSA(*pkey, *rsa) { + let pkey = LcPtr::new(unsafe { EVP_PKEY_new() })?; + if 1 != unsafe { EVP_PKEY_assign_RSA(*pkey, *rsa) } { return Err(()); } rsa.detach(); @@ -442,17 +442,15 @@ where message: &[u8], signature: &[u8], ) -> Result<(), Unspecified> { - unsafe { - let rsa = self.build_rsa()?; - super::signature::verify_rsa_signature( - params.digest_algorithm(), - params.padding(), - &rsa, - message, - signature, - params.bit_size_range(), - ) - } + let rsa = self.build_rsa()?; + super::signature::verify_rsa_signature( + params.digest_algorithm(), + params.padding(), + &rsa, + message, + signature, + params.bit_size_range(), + ) } } diff --git a/aws-lc-rs/src/unstable/kdf.rs b/aws-lc-rs/src/unstable/kdf.rs index b3a07d85b35..01657d9cc17 100644 --- a/aws-lc-rs/src/unstable/kdf.rs +++ b/aws-lc-rs/src/unstable/kdf.rs @@ -240,7 +240,7 @@ mod tests { mod notfips { use crate::unstable::kdf::{ get_kbkdf_ctr_hmac_algorithm, get_sskdf_digest_algorithm, get_sskdf_hmac_algorithm, - kbkdf, kbkdf_ctr_hmac, sskdf::SskdfHmacAlgorithmId, sskdf_digest, sskdf_hmac, + kbkdf_ctr_hmac, sskdf::SskdfHmacAlgorithmId, sskdf_digest, sskdf_hmac, KbkdfCtrHmacAlgorithmId, SskdfDigestAlgorithmId, };