Skip to content

Commit

Permalink
Safer pointer dereferences (#505)
Browse files Browse the repository at this point in the history
  • Loading branch information
justsmth authored Aug 27, 2024
1 parent f4e09c2 commit 8fb6869
Show file tree
Hide file tree
Showing 17 changed files with 271 additions and 216 deletions.
8 changes: 4 additions & 4 deletions aws-lc-rs/src/aead/aead_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,15 +220,15 @@ impl AeadCtx {

// We are performing the allocation ourselves as EVP_AEAD_CTX_new will call EVP_AEAD_CTX_init by default
// and this avoid having to zero and reinitalize again if we need to set an explicit direction.
let aead_ctx: LcPtr<EVP_AEAD_CTX> =
let mut aead_ctx: LcPtr<EVP_AEAD_CTX> =
LcPtr::new(unsafe { OPENSSL_malloc(size_of::<EVP_AEAD_CTX>()) }.cast())?;

unsafe { EVP_AEAD_CTX_zero(*aead_ctx) };
unsafe { EVP_AEAD_CTX_zero(*aead_ctx.as_mut()) };

if 1 != match direction {
Some(direction) => unsafe {
EVP_AEAD_CTX_init_with_direction(
*aead_ctx,
*aead_ctx.as_mut(),
aead,
key_bytes.as_ptr(),
key_bytes.len(),
Expand All @@ -238,7 +238,7 @@ impl AeadCtx {
},
None => unsafe {
EVP_AEAD_CTX_init(
*aead_ctx,
*aead_ctx.as_mut(),
aead,
key_bytes.as_ptr(),
key_bytes.len(),
Expand Down
61 changes: 32 additions & 29 deletions aws-lc-rs/src/agreement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ use crate::cbb::LcCBB;
use crate::ec::{ec_group_from_nid, evp_key_generate};
use crate::error::{KeyRejected, Unspecified};
use crate::fips::indicator_check;
use crate::ptr::{ConstPointer, LcPtr, Pointer};
use crate::ptr::{ConstPointer, LcPtr};
use crate::{ec, hex};
use aws_lc::{
CBS_init, EVP_PKEY_CTX_new, EVP_PKEY_CTX_new_id, EVP_PKEY_bits, EVP_PKEY_derive,
EVP_PKEY_derive_init, EVP_PKEY_derive_set_peer, EVP_PKEY_get0_EC_KEY,
EVP_PKEY_get_raw_private_key, EVP_PKEY_get_raw_public_key, EVP_PKEY_id, EVP_PKEY_keygen,
EVP_PKEY_keygen_init, EVP_PKEY_new_raw_private_key, EVP_PKEY_new_raw_public_key,
EVP_marshal_public_key, EVP_parse_public_key, NID_X9_62_prime256v1, NID_secp384r1,
NID_secp521r1, BIGNUM, CBS, EVP_PKEY, EVP_PKEY_X25519, NID_X25519,
CBS_init, EVP_PKEY_CTX_new_id, EVP_PKEY_bits, EVP_PKEY_derive, EVP_PKEY_derive_init,
EVP_PKEY_derive_set_peer, EVP_PKEY_get0_EC_KEY, EVP_PKEY_get_raw_private_key,
EVP_PKEY_get_raw_public_key, EVP_PKEY_id, EVP_PKEY_keygen, EVP_PKEY_keygen_init,
EVP_PKEY_new_raw_private_key, EVP_PKEY_new_raw_public_key, EVP_marshal_public_key,
EVP_parse_public_key, NID_X9_62_prime256v1, NID_secp384r1, NID_secp521r1, BIGNUM, CBS,
EVP_PKEY, EVP_PKEY_X25519, NID_X25519,
};

use crate::encoding::{
Expand Down Expand Up @@ -433,7 +433,11 @@ impl PrivateKey {
let mut out_len = buffer.len();

if 1 != unsafe {
EVP_PKEY_get_raw_public_key(**priv_key, buffer.as_mut_ptr(), &mut out_len)
EVP_PKEY_get_raw_public_key(
*priv_key.as_const(),
buffer.as_mut_ptr(),
&mut out_len,
)
} {
return Err(Unspecified);
}
Expand Down Expand Up @@ -470,14 +474,14 @@ impl AsDer<EcPrivateKeyRfc5915Der<'static>> for PrivateKey {
let mut outp = null_mut::<u8>();
let ec_key = {
ConstPointer::new(unsafe {
EVP_PKEY_get0_EC_KEY(self.inner_key.get_evp_pkey().as_const_ptr())
EVP_PKEY_get0_EC_KEY(*self.inner_key.get_evp_pkey().as_const())
})?
};
let length = usize::try_from(unsafe { aws_lc::i2d_ECPrivateKey(*ec_key, &mut outp) })
.map_err(|_| Unspecified)?;
let outp = LcPtr::new(outp)?;
let mut outp = LcPtr::new(outp)?;
Ok(EcPrivateKeyRfc5915Der::take_from_slice(unsafe {
core::slice::from_raw_parts_mut(*outp, length)
core::slice::from_raw_parts_mut(*outp.as_mut(), length)
}))
}
}
Expand Down Expand Up @@ -536,15 +540,15 @@ fn from_ec_private_key(priv_key: &[u8], nid: i32) -> Result<LcPtr<EVP_PKEY>, Uns
}

pub(crate) fn generate_x25519() -> Result<LcPtr<EVP_PKEY>, Unspecified> {
let pkey_ctx = LcPtr::new(unsafe { EVP_PKEY_CTX_new_id(EVP_PKEY_X25519, null_mut()) })?;
let mut pkey_ctx = LcPtr::new(unsafe { EVP_PKEY_CTX_new_id(EVP_PKEY_X25519, null_mut()) })?;

if 1 != unsafe { EVP_PKEY_keygen_init(*pkey_ctx) } {
if 1 != unsafe { EVP_PKEY_keygen_init(*pkey_ctx.as_mut()) } {
return Err(Unspecified);
}

let mut pkey: *mut EVP_PKEY = null_mut();

if 1 != indicator_check!(unsafe { EVP_PKEY_keygen(*pkey_ctx, &mut pkey) }) {
if 1 != indicator_check!(unsafe { EVP_PKEY_keygen(*pkey_ctx.as_mut(), &mut pkey) }) {
return Err(Unspecified);
}

Expand Down Expand Up @@ -613,12 +617,11 @@ impl AsDer<PublicKeyX509Der<'static>> for PublicKey {
| KeyInner::ECDH_P521(evp_pkey)
| KeyInner::X25519(evp_pkey) => {
let key_size_bytes =
TryInto::<usize>::try_into(unsafe { EVP_PKEY_bits(evp_pkey.as_const_ptr()) })
TryInto::<usize>::try_into(unsafe { EVP_PKEY_bits(*evp_pkey.as_const()) })
.expect("fit in usize")
* 8;
let mut der = LcCBB::new(key_size_bytes * 5);
if 1 != unsafe { EVP_marshal_public_key(der.as_mut_ptr(), evp_pkey.as_const_ptr()) }
{
if 1 != unsafe { EVP_marshal_public_key(der.as_mut_ptr(), *evp_pkey.as_const()) } {
return Err(Unspecified);
};
Ok(PublicKeyX509Der::from(der.into_buffer()?))
Expand All @@ -638,7 +641,7 @@ impl AsBigEndian<EcPublicKeyCompressedBin<'static>> for PublicKey {
| KeyInner::ECDH_P521(evp_pkey) => evp_pkey,
KeyInner::X25519(_) => return Err(Unspecified),
};
let ec_key = ConstPointer::new(unsafe { EVP_PKEY_get0_EC_KEY(evp_pkey.as_const_ptr()) })?;
let ec_key = ConstPointer::new(unsafe { EVP_PKEY_get0_EC_KEY(*evp_pkey.as_const()) })?;

let mut buffer = vec![0u8; self.algorithm().id.compressed_pub_key_len()];

Expand Down Expand Up @@ -785,22 +788,22 @@ fn ec_key_ecdh<'a>(
peer_pub_key_bytes: &[u8],
nid: i32,
) -> Result<&'a [u8], ()> {
let pub_key = ec::try_parse_public_key_bytes(peer_pub_key_bytes, nid)?;
let mut pub_key = ec::try_parse_public_key_bytes(peer_pub_key_bytes, nid)?;

let pkey_ctx = LcPtr::new(unsafe { EVP_PKEY_CTX_new(**priv_key, null_mut()) })?;
let mut pkey_ctx = priv_key.create_EVP_PKEY_CTX()?;

if 1 != unsafe { EVP_PKEY_derive_init(*pkey_ctx) } {
if 1 != unsafe { EVP_PKEY_derive_init(*pkey_ctx.as_mut()) } {
return Err(());
};

if 1 != unsafe { EVP_PKEY_derive_set_peer(*pkey_ctx, *pub_key) } {
if 1 != unsafe { EVP_PKEY_derive_set_peer(*pkey_ctx.as_mut(), *pub_key.as_mut()) } {
return Err(());
}

let mut out_key_len = buffer.len();

if 1 != indicator_check!(unsafe {
EVP_PKEY_derive(*pkey_ctx, buffer.as_mut_ptr(), &mut out_key_len)
EVP_PKEY_derive(*pkey_ctx.as_mut(), buffer.as_mut_ptr(), &mut out_key_len)
}) {
return Err(());
}
Expand All @@ -818,22 +821,22 @@ fn x25519_diffie_hellman<'a>(
priv_key: &LcPtr<EVP_PKEY>,
peer_pub_key: &[u8],
) -> Result<&'a [u8], ()> {
let pkey_ctx = LcPtr::new(unsafe { EVP_PKEY_CTX_new(**priv_key, null_mut()) })?;
let mut pkey_ctx = priv_key.create_EVP_PKEY_CTX()?;

if 1 != unsafe { EVP_PKEY_derive_init(*pkey_ctx) } {
if 1 != unsafe { EVP_PKEY_derive_init(*pkey_ctx.as_mut()) } {
return Err(());
};

let pub_key = try_parse_x25519_public_key_bytes(peer_pub_key)?;
let mut pub_key = try_parse_x25519_public_key_bytes(peer_pub_key)?;

if 1 != unsafe { EVP_PKEY_derive_set_peer(*pkey_ctx, *pub_key) } {
if 1 != unsafe { EVP_PKEY_derive_set_peer(*pkey_ctx.as_mut(), *pub_key.as_mut()) } {
return Err(());
}

let mut out_key_len = buffer.len();

if 1 != indicator_check!(unsafe {
EVP_PKEY_derive(*pkey_ctx, buffer.as_mut_ptr(), &mut out_key_len)
EVP_PKEY_derive(*pkey_ctx.as_mut(), buffer.as_mut_ptr(), &mut out_key_len)
}) {
return Err(());
}
Expand Down Expand Up @@ -878,7 +881,7 @@ fn try_parse_x25519_subject_public_key_info_bytes(
}
};
let evp_pkey = LcPtr::new(unsafe { EVP_parse_public_key(&mut cbs) })?;
if EVP_PKEY_X25519 != unsafe { EVP_PKEY_id(*evp_pkey) } {
if EVP_PKEY_X25519 != unsafe { EVP_PKEY_id(*evp_pkey.as_const()) } {
return Err(Unspecified);
}
Ok(evp_pkey)
Expand Down
4 changes: 2 additions & 2 deletions aws-lc-rs/src/bn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ impl TryFrom<u64> for LcPtr<BIGNUM> {

fn try_from(value: u64) -> Result<Self, Self::Error> {
unsafe {
let bn = LcPtr::new(BN_new())?;
if 1 != BN_set_u64(*bn, value) {
let mut bn = LcPtr::new(BN_new())?;
if 1 != BN_set_u64(*bn.as_mut(), value) {
return Err(());
}
Ok(bn)
Expand Down
20 changes: 8 additions & 12 deletions aws-lc-rs/src/cipher/streaming.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::cipher::{
};
use crate::error::Unspecified;
use crate::fips::indicator_check;
use crate::ptr::{LcPtr, Pointer};
use crate::ptr::LcPtr;
use aws_lc::{
EVP_CIPHER_CTX_new, EVP_CIPHER_iv_length, EVP_CIPHER_key_length, EVP_DecryptFinal_ex,
EVP_DecryptInit_ex, EVP_DecryptUpdate, EVP_EncryptFinal_ex, EVP_EncryptInit_ex,
Expand Down Expand Up @@ -80,7 +80,7 @@ impl StreamingEncryptingKey {
// AWS-LC copies the key and iv values into the EVP_CIPHER_CTX, and thus can be dropped after this.
if 1 != unsafe {
EVP_EncryptInit_ex(
cipher_ctx.as_mut_ptr(),
*cipher_ctx.as_mut(),
*cipher,
null_mut(),
key_bytes.as_ptr(),
Expand Down Expand Up @@ -126,7 +126,7 @@ impl StreamingEncryptingKey {

if 1 != unsafe {
EVP_EncryptUpdate(
self.cipher_ctx.as_mut_ptr(),
*self.cipher_ctx.as_mut(),
output.as_mut_ptr(),
&mut outlen,
input.as_ptr(),
Expand Down Expand Up @@ -159,11 +159,7 @@ impl StreamingEncryptingKey {
let mut outlen: i32 = 0;

if 1 != indicator_check!(unsafe {
EVP_EncryptFinal_ex(
self.cipher_ctx.as_mut_ptr(),
output.as_mut_ptr(),
&mut outlen,
)
EVP_EncryptFinal_ex(*self.cipher_ctx.as_mut(), output.as_mut_ptr(), &mut outlen)
}) {
return Err(Unspecified);
}
Expand Down Expand Up @@ -269,7 +265,7 @@ impl StreamingDecryptingKey {
// AWS-LC copies the key and iv values into the EVP_CIPHER_CTX, and thus can be dropped after this.
if 1 != unsafe {
EVP_DecryptInit_ex(
cipher_ctx.as_mut_ptr(),
*cipher_ctx.as_mut(),
*cipher,
null_mut(),
key_bytes.as_ptr(),
Expand Down Expand Up @@ -314,7 +310,7 @@ impl StreamingDecryptingKey {

if 1 != unsafe {
EVP_DecryptUpdate(
self.cipher_ctx.as_mut_ptr(),
*self.cipher_ctx.as_mut(),
output.as_mut_ptr(),
&mut outlen,
input.as_ptr(),
Expand All @@ -336,14 +332,14 @@ impl StreamingDecryptingKey {
/// # Errors
/// * Returns an error if the `output` buffer is smaller than the algorithm's
/// block length.
pub fn finish(self, output: &mut [u8]) -> Result<BufferUpdate, Unspecified> {
pub fn finish(mut self, output: &mut [u8]) -> Result<BufferUpdate, Unspecified> {
if output.len() < self.algorithm().block_len() {
return Err(Unspecified);
}
let mut outlen: i32 = 0;

if 1 != indicator_check!(unsafe {
EVP_DecryptFinal_ex(*self.cipher_ctx, output.as_mut_ptr(), &mut outlen)
EVP_DecryptFinal_ex(*self.cipher_ctx.as_mut(), output.as_mut_ptr(), &mut outlen)
}) {
return Err(Unspecified);
}
Expand Down
Loading

0 comments on commit 8fb6869

Please sign in to comment.