From 42200813ae9d831b9f215421adac31e0b6f4f6bf Mon Sep 17 00:00:00 2001 From: Justin W Smith <103147162+justsmth@users.noreply.github.com> Date: Tue, 4 Feb 2025 16:37:59 -0500 Subject: [PATCH] Fix regression in EcdsaKeyPair::from_private_key_der (#686) * Fix regression in EcdsaKeyPair::from_private_key_der * Satisfy clippy --- aws-lc-fips-sys/builder/cmake_builder.rs | 4 +-- aws-lc-rs/src/ec/key_pair.rs | 39 +++++++++++++++++++++--- aws-lc-rs/src/evp_pkey.rs | 12 ++++++-- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/aws-lc-fips-sys/builder/cmake_builder.rs b/aws-lc-fips-sys/builder/cmake_builder.rs index 76541acd3d0..b258e753177 100644 --- a/aws-lc-fips-sys/builder/cmake_builder.rs +++ b/aws-lc-fips-sys/builder/cmake_builder.rs @@ -113,11 +113,11 @@ impl CmakeBuilder { if let Some(cc) = option_env!("AWS_LC_FIPS_SYS_CC") { env::set_var("CC", cc); - emit_warning(&format!("Setting CC: {}", cc)); + emit_warning(&format!("Setting CC: {cc}")); } if let Some(cxx) = option_env!("AWS_LC_FIPS_SYS_CXX") { env::set_var("CXX", cxx); - emit_warning(&format!("Setting CXX: {}", cxx)); + emit_warning(&format!("Setting CXX: {cxx}")); } let cc_build = cc::Build::new(); diff --git a/aws-lc-rs/src/ec/key_pair.rs b/aws-lc-rs/src/ec/key_pair.rs index 04f9b93a042..073b8895904 100644 --- a/aws-lc-rs/src/ec/key_pair.rs +++ b/aws-lc-rs/src/ec/key_pair.rs @@ -3,13 +3,12 @@ // Modifications copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 OR ISC +use crate::aws_lc::{EVP_DigestSign, EVP_DigestSignInit, EVP_PKEY, EVP_PKEY_EC}; use core::fmt; use core::fmt::{Debug, Formatter}; use core::mem::MaybeUninit; use core::ptr::{null, null_mut}; -use crate::aws_lc::{EVP_DigestSign, EVP_DigestSignInit, EVP_PKEY_cmp, EVP_PKEY, EVP_PKEY_EC}; - use crate::digest::digest_ctx::DigestContext; use crate::ec::evp_key_generate; use crate::ec::signature::{EcdsaSignatureFormat, EcdsaSigningAlgorithm, PublicKey}; @@ -162,8 +161,8 @@ impl EcdsaKeyPair { ) -> Result { let priv_evp_pkey = parse_sec1_private_bn(private_key, alg.id.nid())?; let pub_evp_pkey = parse_sec1_public_point(public_key, alg.id.nid())?; - // EVP_PKEY_cmp only compare params and public key - if 1 != unsafe { EVP_PKEY_cmp(*priv_evp_pkey.as_const(), *pub_evp_pkey.as_const()) } { + // EVP_PKEY_cmp only compares params and public key + if !priv_evp_pkey.eq(&pub_evp_pkey) { return Err(KeyRejected::inconsistent_components()); } @@ -187,7 +186,8 @@ impl EcdsaKeyPair { alg: &'static EcdsaSigningAlgorithm, private_key: &[u8], ) -> Result { - let evp_pkey = parse_rfc5915_private_key(private_key, alg.id.nid())?; + let evp_pkey = LcPtr::::parse_rfc5208_private_key(private_key, EVP_PKEY_EC) + .or(parse_rfc5915_private_key(private_key, alg.id.nid()))?; Ok(Self::new(alg, evp_pkey)?) } @@ -320,3 +320,32 @@ impl AsDer> for PrivateKey<'_> { Ok(EcPrivateKeyRfc5915Der::new(bytes)) } } + +#[cfg(test)] +mod tests { + use crate::encoding::AsDer; + use crate::signature::{EcdsaKeyPair, ECDSA_P256_SHA256_FIXED_SIGNING}; + + #[test] + fn test_from_private_key_der() { + let key_pair = EcdsaKeyPair::generate(&ECDSA_P256_SHA256_FIXED_SIGNING).unwrap(); + + let bytes_5208 = key_pair.to_pkcs8v1().unwrap(); + let bytes_5915 = key_pair.private_key().as_der().unwrap(); + + let key_pair_5208 = EcdsaKeyPair::from_private_key_der( + &ECDSA_P256_SHA256_FIXED_SIGNING, + bytes_5208.as_ref(), + ) + .unwrap(); + let key_pair_5915 = EcdsaKeyPair::from_private_key_der( + &ECDSA_P256_SHA256_FIXED_SIGNING, + bytes_5915.as_ref(), + ) + .unwrap(); + + assert_eq!(key_pair.evp_pkey, key_pair_5208.evp_pkey); + assert_eq!(key_pair.evp_pkey, key_pair_5915.evp_pkey); + assert_eq!(key_pair_5208.evp_pkey, key_pair_5915.evp_pkey); + } +} diff --git a/aws-lc-rs/src/evp_pkey.rs b/aws-lc-rs/src/evp_pkey.rs index 872e2561ecf..99e584aa5b1 100644 --- a/aws-lc-rs/src/evp_pkey.rs +++ b/aws-lc-rs/src/evp_pkey.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 OR ISC use crate::aws_lc::{ - EVP_PKEY_CTX_new, EVP_PKEY_bits, EVP_PKEY_get0_EC_KEY, EVP_PKEY_get0_RSA, + EVP_PKEY_CTX_new, EVP_PKEY_bits, EVP_PKEY_cmp, EVP_PKEY_get0_EC_KEY, EVP_PKEY_get0_RSA, EVP_PKEY_get_raw_private_key, EVP_PKEY_get_raw_public_key, EVP_PKEY_id, EVP_PKEY_new_raw_private_key, EVP_PKEY_new_raw_public_key, EVP_PKEY_size, EVP_PKEY_up_ref, EVP_marshal_private_key, EVP_marshal_private_key_v2, EVP_marshal_public_key, @@ -18,6 +18,14 @@ use crate::ptr::{ConstPointer, LcPtr}; use std::os::raw::c_int; use std::ptr::null_mut; +impl PartialEq for LcPtr { + /// Only compares params and public key + fn eq(&self, other: &Self) -> bool { + // EVP_PKEY_cmp only compares params and public key + 1 == unsafe { EVP_PKEY_cmp(*self.as_const(), *other.as_const()) } + } +} + impl LcPtr { pub(crate) fn validate_as_ed25519(&self) -> Result<(), KeyRejected> { const ED25519_KEY_TYPE: c_int = aws_lc::EVP_PKEY_ED25519; @@ -95,7 +103,7 @@ impl LcPtr { let mut cbb = LcCBB::new(self.key_size_bytes() * 5); if 1 != unsafe { EVP_marshal_public_key(cbb.as_mut_ptr(), *self.as_const()) } { return Err(Unspecified); - }; + } cbb.into_vec() }