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

Fix regression in EcdsaKeyPair::from_private_key_der #686

Merged
merged 2 commits into from
Feb 4, 2025
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
4 changes: 2 additions & 2 deletions aws-lc-fips-sys/builder/cmake_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
39 changes: 34 additions & 5 deletions aws-lc-rs/src/ec/key_pair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -162,8 +161,8 @@ impl EcdsaKeyPair {
) -> Result<Self, KeyRejected> {
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());
}

Expand All @@ -187,7 +186,8 @@ impl EcdsaKeyPair {
alg: &'static EcdsaSigningAlgorithm,
private_key: &[u8],
) -> Result<Self, KeyRejected> {
let evp_pkey = parse_rfc5915_private_key(private_key, alg.id.nid())?;
let evp_pkey = LcPtr::<EVP_PKEY>::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)?)
}
Expand Down Expand Up @@ -320,3 +320,32 @@ impl AsDer<EcPrivateKeyRfc5915Der<'static>> 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);
}
}
12 changes: 10 additions & 2 deletions aws-lc-rs/src/evp_pkey.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -18,6 +18,14 @@ use crate::ptr::{ConstPointer, LcPtr};
use std::os::raw::c_int;
use std::ptr::null_mut;

impl PartialEq<Self> for LcPtr<EVP_PKEY> {
/// 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<EVP_PKEY> {
pub(crate) fn validate_as_ed25519(&self) -> Result<(), KeyRejected> {
const ED25519_KEY_TYPE: c_int = aws_lc::EVP_PKEY_ED25519;
Expand Down Expand Up @@ -95,7 +103,7 @@ impl LcPtr<EVP_PKEY> {
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()
}

Expand Down
Loading