Skip to content

Commit

Permalink
Safer pointer dereferences
Browse files Browse the repository at this point in the history
  • Loading branch information
@justsmth
justsmth committed Aug 19, 2024
1 parent bc9f59a commit 9206b55
Show file tree
Hide file tree
Showing 17 changed files with 249 additions and 174 deletions.
8 changes: 4 additions & 4 deletions aws-lc-rs/src/aead/aead_ctx.rs
View file
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
52 changes: 32 additions & 20 deletions aws-lc-rs/src/agreement.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ use crate::ec::{
};
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::{
EVP_PKEY_CTX_new, EVP_PKEY_CTX_new_id, EVP_PKEY_derive, EVP_PKEY_derive_init,
Expand Down Expand Up @@ -412,7 +412,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 @@ -449,14 +453,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 @@ -517,15 +521,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 @@ -698,22 +702,26 @@ fn ec_key_ecdh<'a>(
) -> Result<&'a [u8], ()> {
let ec_group = ec_group_from_nid(nid)?;
let pub_key_point = ec_point_from_bytes(&ec_group, peer_pub_key_bytes)?;
let pub_key = evp_pkey_from_public_point(&ec_group, &pub_key_point)?;
let mut pub_key = evp_pkey_from_public_point(&ec_group, &pub_key_point)?;

let pkey_ctx = LcPtr::new(unsafe { EVP_PKEY_CTX_new(**priv_key, null_mut()) })?;
let mut pkey_ctx =
// The only modification made by EVP_PKEY_CTX_new to `priv_key` is to increment its
// refcount. The modification is made while holding a global lock:
// /~https://github.com/aws/aws-lc/blob/61503f7fe72457e12d3446853a5452d175560c49/crypto/refcount_lock.c#L29
LcPtr::new(unsafe { EVP_PKEY_CTX_new(*priv_key.as_mut_unsafe(), null_mut()) })?;

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 @@ -731,13 +739,17 @@ 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 =
// The only modification made by EVP_PKEY_CTX_new to `priv_key` is to increment its
// refcount. The modification is made while holding a global lock:
// /~https://github.com/aws/aws-lc/blob/61503f7fe72457e12d3446853a5452d175560c49/crypto/refcount_lock.c#L29
LcPtr::new(unsafe { EVP_PKEY_CTX_new(*priv_key.as_mut_unsafe(), null_mut()) })?;

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 = LcPtr::new(unsafe {
let mut pub_key = LcPtr::new(unsafe {
EVP_PKEY_new_raw_public_key(
EVP_PKEY_X25519,
null_mut(),
Expand All @@ -746,14 +758,14 @@ fn x25519_diffie_hellman<'a>(
)
})?;

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 @@ -844,7 +856,7 @@ mod tests {
(),
|_| Ok(())
)
.is_err());
.is_err());
}

let alg_variants: [&'static Algorithm; 4] = [&X25519, &ECDH_P256, &ECDH_P384, &ECDH_P521];
Expand Down Expand Up @@ -1102,7 +1114,7 @@ mod tests {
let pubkey_re = Regex::new(
"PublicKey \\{ algorithm: Algorithm \\{ curve: P256 \\}, bytes: \"[0-9a-f]+\" \\}",
)
.unwrap();
.unwrap();
let pubkey_debug = format!("{:?}", &public_key);

assert!(
Expand Down
4 changes: 2 additions & 2 deletions aws-lc-rs/src/bn.rs
View file
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
View file
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 9206b55

Please sign in to comment.