Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup usage of unsafe blocks #499

Merged
merged 1 commit into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 11 additions & 16 deletions aws-lc-rs/src/agreement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -472,12 +469,10 @@ impl AsBigEndian<EcPrivateKeyBin<'static>> 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))
}
}
Expand Down
6 changes: 3 additions & 3 deletions aws-lc-rs/src/cbs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<CBS>::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() }
}
76 changes: 39 additions & 37 deletions aws-lc-rs/src/ec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<EVP_PKEY>,
) -> Result<Vec<u8>, 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<LcPtr<EVP_PKEY>, 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")]
Expand All @@ -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<EVP_PKEY>,
) -> Result<usize, Unspecified> {
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<EC_KEY>,
) -> Result<usize, Unspecified> {
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)
Expand All @@ -386,14 +388,12 @@ pub(crate) fn marshal_public_key(
algorithm: &'static EcdsaSigningAlgorithm,
) -> Result<PublicKey, Unspecified> {
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]
Expand Down Expand Up @@ -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_GROUP>,
ec_point: &ConstPointer<EC_POINT>,
buf: &mut [u8; PUBLIC_KEY_MAX_LEN],
) -> Result<usize, Unspecified> {
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);
}
Expand Down
14 changes: 6 additions & 8 deletions aws-lc-rs/src/ec/key_pair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ impl EcdsaKeyPair {
alg: &'static EcdsaSigningAlgorithm,
private_key: &[u8],
) -> Result<Self, KeyRejected> {
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)?)
}
Expand Down Expand Up @@ -297,13 +297,11 @@ impl AsBigEndian<EcPrivateKeyBin<'static>> for PrivateKey<'_> {
/// # Errors
/// `error::Unspecified` if serialization failed.
fn as_be_bytes(&self) -> Result<EcPrivateKeyBin<'static>, 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))
}
}

Expand Down
22 changes: 12 additions & 10 deletions aws-lc-rs/src/rsa/encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<LcPtr<EVP_PKEY>, 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) {
Expand All @@ -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<EVP_PKEY>,
) -> Result<Box<[u8]>, Unspecified> {
let mut pubkey_bytes = null_mut::<u8>();
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())
}
Expand Down Expand Up @@ -104,7 +106,7 @@ pub(in crate::rsa) mod rfc8017 {
pub(in crate::rsa) fn decode_private_key_der(
private_key: &[u8],
) -> Result<LcPtr<EVP_PKEY>, 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) })?;

Expand Down Expand Up @@ -155,7 +157,7 @@ pub(in crate::rsa) mod rfc5280 {
pub(in crate::rsa) fn decode_public_key_der(
value: &[u8],
) -> Result<LcPtr<EVP_PKEY>, 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());
Expand Down
38 changes: 18 additions & 20 deletions aws-lc-rs/src/rsa/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ unsafe impl Sync for KeyPair {}
impl KeyPair {
fn new(evp_pkey: LcPtr<EVP_PKEY>) -> Result<Self, KeyRejected> {
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,
Expand Down Expand Up @@ -312,14 +312,14 @@ impl Drop for PublicKey {
}

impl PublicKey {
pub(super) unsafe fn new(evp_pkey: &LcPtr<EVP_PKEY>) -> Result<Self, Unspecified> {
pub(super) fn new(evp_pkey: &LcPtr<EVP_PKEY>) -> Result<Self, Unspecified> {
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,
Expand Down Expand Up @@ -400,7 +400,7 @@ where
B: AsRef<[u8]> + Debug,
{
#[inline]
unsafe fn build_rsa(&self) -> Result<LcPtr<EVP_PKEY>, ()> {
fn build_rsa(&self) -> Result<LcPtr<EVP_PKEY>, ()> {
let n_bytes = self.n.as_ref();
if n_bytes.is_empty() || n_bytes[0] == 0u8 {
return Err(());
Expand All @@ -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();
Expand All @@ -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(),
)
}
}

Expand Down
2 changes: 1 addition & 1 deletion aws-lc-rs/src/unstable/kdf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
Loading