Skip to content

Commit

Permalink
Address Mark's review comments at
Browse files Browse the repository at this point in the history 
  aws#256
  • Loading branch information
Hanson Char committed Oct 31, 2023
1 parent 9f2028a commit cd74eca
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 85 deletions.
97 changes: 21 additions & 76 deletions aws-lc-rs/src/agreement.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
//! ```
use crate::ec::{
ec_group_from_nid, ec_point_from_bytes, evp_key_generate, evp_pkey_from_public_point,
evp_pkey_from_x509_pubkey, marshal_x509_public_key_to_buffer, PUBLIC_KEY_MAX_LEN,
evp_pkey_from_x509_pubkey, marshal_x509_public_key_to_buffer,
};
use crate::error::Unspecified;
use crate::fips::indicator_check;
Expand All @@ -60,9 +60,9 @@ use crate::rand::SecureRandom;
use crate::{ec, test};
use aws_lc::{
d2i_X509_PUBKEY, EVP_PKEY_CTX_new, EVP_PKEY_CTX_new_id, EVP_PKEY_derive, EVP_PKEY_derive_init,
EVP_PKEY_derive_set_peer, EVP_PKEY_get_raw_private_key, EVP_PKEY_get_raw_public_key,
EVP_PKEY_keygen, EVP_PKEY_keygen_init, EVP_PKEY_new_raw_public_key, NID_X9_62_prime256v1,
NID_secp384r1, NID_secp521r1, EVP_PKEY, EVP_PKEY_X25519, NID_X25519, X25519_PUBLIC_VALUE_LEN,
EVP_PKEY_derive_set_peer, EVP_PKEY_get_raw_public_key, EVP_PKEY_keygen, EVP_PKEY_keygen_init,
EVP_PKEY_new_raw_public_key, NID_X9_62_prime256v1, NID_secp384r1, NID_secp521r1, EVP_PKEY,
EVP_PKEY_X25519, NID_X25519, X25519_PUBLIC_VALUE_LEN,
};

use core::fmt;
Expand Down Expand Up @@ -175,6 +175,7 @@ pub static X509: Encoding = Encoding {
pub static X25519: Algorithm = Algorithm {
id: AlgorithmID::X25519,
};
#[cfg(test)]
const X25519_PRIVATE_KEY_LEN: usize = aws_lc::X25519_PRIVATE_KEY_LEN as usize;
#[cfg(test)]
const ECDH_P256_PRIVATE_KEY_LEN: usize = 32;
Expand Down Expand Up @@ -497,15 +498,11 @@ impl<B: AsRef<[u8]>> UnparsedPublicKey<B> {
}

/// Constructs a new `UnparsedPublicKey` with the specified [Encoding].
pub fn new_with_encoding(
algorithm: &'static Algorithm,
bytes: B,
encoding: &'static Encoding,
) -> Self {
pub fn new_with_x509(algorithm: &'static Algorithm, bytes: B) -> Self {
UnparsedPublicKey {
alg: algorithm,
bytes,
encoding,
encoding: &X509,
}
}

Expand Down Expand Up @@ -568,14 +565,14 @@ where
F: FnOnce(&[u8]) -> Result<R, E>,
{
let expected_alg = my_private_key.algorithm();
let expected_pub_key_len = expected_alg.id.pub_key_len();
let expected_nid = expected_alg.id.nid();

if peer_public_key.alg != expected_alg {
return Err(error_value);
}
let peer_pub_bytes = peer_public_key.bytes.as_ref();
if peer_public_key.encoding == &OCTET_STRING {
let expected_pub_key_len = expected_alg.id.pub_key_len();
let peer_pub_bytes_len = peer_pub_bytes.len();
if peer_pub_bytes_len != expected_pub_key_len {
return Err(error_value);
Expand All @@ -587,12 +584,8 @@ where
let secret: &[u8] = match &my_private_key.inner_key {
KeyInner::X25519(priv_key, ..) => match peer_public_key.encoding.id {
EncodingID::OctetString => {
x25519_dh_with_octstr_peer_pubkey(
peer_pub_bytes,
&mut buffer,
priv_key,
error_value,
)?;
x25519_diffie_hellman(&mut buffer, priv_key, peer_pub_bytes)
.map_err(|()| error_value)?;
&buffer[0..X25519_SHARED_KEY_LEN]
}
EncodingID::X509 => {
Expand Down Expand Up @@ -620,19 +613,6 @@ where
kdf(secret)
}

#[inline]
fn x25519_dh_with_octstr_peer_pubkey<E>(
peer_pub_bytes: &[u8],
shared_secret: &mut [u8; MAX_AGREEMENT_SECRET_LEN],
priv_key: &LcPtr<EVP_PKEY>,
error_value: E,
) -> Result<(), E> {
let mut pub_key = [0u8; X25519_PUBLIC_VALUE_LEN as usize];
pub_key.copy_from_slice(peer_pub_bytes);
x25519_diffie_hellman(shared_secret, priv_key, &pub_key).map_err(|()| error_value)?;
Ok(())
}

#[inline]
fn x25519_dh_with_x509_peer_pubkey<E: Copy>(
peer_x509_pubkey: &[u8],
Expand All @@ -647,16 +627,17 @@ fn x25519_dh_with_x509_peer_pubkey<E: Copy>(
let x509_pubkey = LcPtr::new(unsafe {
d2i_X509_PUBKEY(
null_mut(),
&peer_x509_pubkey.as_ptr() as *const *const u8 as *mut *const c_uchar,
&mut peer_x509_pubkey.as_ptr() as *const *const u8 as *mut *const c_uchar,
len,
)
})
.map_err(|()| error_value)?;
let mut peer_octstr_pubkey = [0u8; PUBLIC_KEY_MAX_LEN];
let mut peer_octstr_pubkey = [0u8; X25519_PUBLIC_VALUE_LEN as usize];
marshal_x509_public_key_to_buffer(&mut peer_octstr_pubkey, &x509_pubkey)
.map_err(|_| error_value)?;
x25519_diffie_hellman_x509(shared_secret, priv_key, &peer_octstr_pubkey)
.map_err(|()| error_value)
.map_err(|()| error_value)?;
x25519_diffie_hellman(shared_secret, priv_key, &peer_octstr_pubkey)
.map_err(|()| error_value)?;
Ok(())
}

// Current max secret length is P-521's.
Expand All @@ -671,7 +652,6 @@ fn ec_key_ecdh<'a>(
peer_pub_key_bytes_encoding: &Encoding,
nid: i32,
) -> Result<&'a [u8], ()> {
// let peer_ec_key = match peer_pub_key_bytes_encoding.id {
let pub_key = match peer_pub_key_bytes_encoding.id {
EncodingID::OctetString => {
let ec_group = unsafe { ec_group_from_nid(nid)? };
Expand Down Expand Up @@ -707,11 +687,11 @@ fn ec_key_ecdh<'a>(
}

#[inline]
fn x25519_diffie_hellman<'a>(
buffer: &'a mut [u8; MAX_AGREEMENT_SECRET_LEN],
fn x25519_diffie_hellman(
buffer: &mut [u8; MAX_AGREEMENT_SECRET_LEN],
priv_key: &LcPtr<EVP_PKEY>,
peer_pub_key: &[u8],
) -> Result<&'a [u8], ()> {
) -> Result<(), ()> {
let pkey_ctx = LcPtr::new(unsafe { EVP_PKEY_CTX_new(**priv_key, null_mut()) })?;

if 1 != unsafe { EVP_PKEY_derive_init(*pkey_ctx) } {
Expand Down Expand Up @@ -740,36 +720,6 @@ fn x25519_diffie_hellman<'a>(
}

debug_assert!(out_key_len == X25519_SHARED_KEY_LEN);

Ok(&buffer[0..X25519_SHARED_KEY_LEN])
}

#[inline]
fn x25519_diffie_hellman_x509(
buffer: &mut [u8],
evp_priv_key: &LcPtr<EVP_PKEY>,
peer_pub_key: &[u8],
) -> Result<(), ()> {
let mut priv_key = [0u8; X25519_PRIVATE_KEY_LEN];
if 1 != unsafe {
EVP_PKEY_get_raw_private_key(
**evp_priv_key,
priv_key.as_mut_ptr(),
&priv_key.len() as *const _ as *mut usize,
)
} {
return Err(());
}
debug_assert!(buffer.len() >= X25519_SHARED_KEY_LEN);
if 1 != unsafe {
aws_lc::X25519(
buffer.as_mut_ptr(),
priv_key.as_ptr(),
peer_pub_key.as_ptr(),
)
} {
return Err(());
}
Ok(())
}

Expand Down Expand Up @@ -1214,13 +1164,8 @@ mod tests {
out.truncate(out_len);
let my_private_key = agreement::EphemeralPrivateKey::generate(algorithm, rng)
.expect("Failed to generate key");
let peer_public_key = {
agreement::UnparsedPublicKey::new_with_encoding(
algorithm,
out.as_slice(),
&agreement::X509,
)
};
let peer_public_key =
{ agreement::UnparsedPublicKey::new_with_x509(algorithm, out.as_slice()) };
agreement::agree_ephemeral(
my_private_key,
&peer_public_key,
Expand Down
26 changes: 17 additions & 9 deletions aws-lc-rs/src/ec.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use aws_lc::{
EVP_PKEY_CTX_set_ec_paramgen_curve_nid, EVP_PKEY_assign_EC_KEY, EVP_PKEY_get0_EC_KEY,
EVP_PKEY_keygen, EVP_PKEY_keygen_init, EVP_PKEY_new, NID_X9_62_prime256v1, NID_secp256k1,
NID_secp384r1, NID_secp521r1, X509_PUBKEY_get, BIGNUM, ECDSA_SIG, EC_GROUP, EC_POINT, EVP_PKEY,
EVP_PKEY_EC, X509_PUBKEY,
EVP_PKEY_EC, X25519_PUBLIC_VALUE_LEN, X509_PUBKEY,
};

#[cfg(test)]
Expand Down Expand Up @@ -266,21 +266,29 @@ unsafe fn validate_evp_key(
Ok(())
}

#[allow(clippy::cast_ptr_alignment)]
pub(crate) fn marshal_x509_public_key_to_buffer(
buffer: &mut [u8; PUBLIC_KEY_MAX_LEN],
buffer: &mut [u8; X25519_PUBLIC_VALUE_LEN as usize],
x509_pubkey: &LcPtr<X509_PUBKEY>,
) -> Result<usize, Unspecified> {
) -> Result<(), ()> {
let buffer_len = buffer.len();
let evp_pkey = LcPtr::new(unsafe { X509_PUBKEY_get(**x509_pubkey) })?;
let bio = LcPtr::new(unsafe { BIO_new(BIO_s_mem()) })?;

if unsafe { i2d_PUBKEY_bio(*bio, *evp_pkey) } <= 0 {
return Err(Unspecified);
return Err(());
}
let size = BIO_get_mem_data(*bio, buffer.as_mut_ptr().cast::<*mut c_char>());
if size <= 0 {
return Err(Unspecified);

let mut ptr = std::ptr::null_mut::<c_char>();
let size = BIO_get_mem_data(*bio, &mut ptr);
let size = usize::try_from(size).map_err(|_| ())?;

if size < buffer_len {
return Err(());
}
usize::try_from(size).map_err(|_| Unspecified)

let slice = unsafe { std::slice::from_raw_parts(ptr as *const u8, size) };
buffer.copy_from_slice(&slice[..buffer_len]);
Ok(())
}

pub(crate) unsafe fn marshal_public_key_to_buffer(
Expand Down

0 comments on commit cd74eca

Please sign in to comment.