From e867c21eb405fce1e36818d0981bcf3175140779 Mon Sep 17 00:00:00 2001 From: Tanveer Wahid Date: Mon, 6 Jul 2026 13:26:07 -0700 Subject: [PATCH 1/2] fix: cleanup crypto issues --- keetanetwork-account/src/account.rs | 5 +- keetanetwork-asn1/src/der.rs | 35 ++++ keetanetwork-asn1/src/rasn.rs | 37 ++++ keetanetwork-crypto/src/algorithms/aes_cbc.rs | 9 +- .../src/algorithms/aes_common.rs | 3 +- keetanetwork-crypto/src/algorithms/aes_ctr.rs | 9 +- keetanetwork-x509/src/certificates.rs | 30 ++- keetanetwork-x509/src/utils.rs | 177 +++++------------- keetanetwork-x509/tests/builders.rs | 53 +++++- 9 files changed, 220 insertions(+), 138 deletions(-) diff --git a/keetanetwork-account/src/account.rs b/keetanetwork-account/src/account.rs index 042d4e2..1183863 100644 --- a/keetanetwork-account/src/account.rs +++ b/keetanetwork-account/src/account.rs @@ -922,7 +922,10 @@ macro_rules! impl_crypto_keypair { .as_ref() .ok_or(AccountError::InvalidConstruction)?; - Ok(private_key.decrypt(ciphertext.as_ref())?) + let ciphertext = ciphertext.as_ref(); + let plaintext = private_key.decrypt(ciphertext)?; + + Ok(plaintext) } fn supports_encryption(&self) -> bool { diff --git a/keetanetwork-asn1/src/der.rs b/keetanetwork-asn1/src/der.rs index 3941954..2d0bb27 100644 --- a/keetanetwork-asn1/src/der.rs +++ b/keetanetwork-asn1/src/der.rs @@ -50,6 +50,16 @@ impl AlgorithmIdentifier { pub fn new_with_params(oid: &str, parameters: Any) -> Result { Ok(Self { algorithm: ObjectIdentifier::new(oid)?, parameters: Some(parameters) }) } + + /// Decode the parameters as an OBJECT IDENTIFIER + /// + /// Returns `None` when parameters are absent or not an OBJECT IDENTIFIER. + pub fn parameters_oid(&self) -> Option { + let parameters = self.parameters.as_ref()?; + let decoded_oid = parameters.decode_as::(); + + decoded_oid.ok() + } } impl TryFrom for AlgorithmIdentifier { @@ -229,6 +239,31 @@ mod tests { }, } + #[test] + fn test_parameters_oid_decodes_oid_parameters() -> Result<(), Asn1Error> { + let curve_oid = ObjectIdentifier::new(oids::SECP256K1)?; + let oid_param = Any::encode_from(&curve_oid)?; + + let alg_with_oid = AlgorithmIdentifier::new_with_params(oids::EC_PUBLIC_KEY, oid_param)?; + assert_eq!(alg_with_oid.parameters_oid(), Some(curve_oid)); + Ok(()) + } + + #[test] + fn test_parameters_oid_rejects_non_oid_parameters() -> Result<(), Asn1Error> { + let null_param = Any::from_der(&[0x05, 0x00])?; + let alg_with_null = AlgorithmIdentifier::new_with_params(oids::RSA_ENCRYPTION, null_param)?; + assert_eq!(alg_with_null.parameters_oid(), None); + Ok(()) + } + + #[test] + fn test_parameters_oid_absent_parameters() -> Result<(), Asn1Error> { + let alg_without_params = AlgorithmIdentifier::new(oids::ED25519)?; + assert_eq!(alg_without_params.parameters_oid(), None); + Ok(()) + } + #[test] fn test_spki_conversions() -> Result<(), Asn1Error> { // Test AlgorithmIdentifier round-trip diff --git a/keetanetwork-asn1/src/rasn.rs b/keetanetwork-asn1/src/rasn.rs index 10ac4d2..690b069 100644 --- a/keetanetwork-asn1/src/rasn.rs +++ b/keetanetwork-asn1/src/rasn.rs @@ -34,6 +34,17 @@ impl AlgorithmIdentifier { Ok(AlgorithmIdentifier { algorithm: oid, parameters: Some(parameters) }) } + /// Decode the parameters as an OBJECT IDENTIFIER + /// + /// Returns `None` when parameters are absent or not an OBJECT IDENTIFIER. + pub fn parameters_oid(&self) -> Option { + let parameters = self.parameters.as_ref()?; + let parameter_bytes = parameters.as_bytes(); + let decoded_oid = rasn::der::decode::(parameter_bytes); + + decoded_oid.ok() + } + /// Decode an AlgorithmIdentifier from DER format pub fn from_der(bytes: &[u8]) -> Result { Ok(rasn::der::decode::(bytes)?) @@ -63,6 +74,7 @@ impl TryFrom for AlgorithmIdentifier { .map(|p| DerEncode::to_der(&p)) .transpose()? .map(Any::new); + Ok(Self { algorithm: oid, parameters }) } } @@ -76,6 +88,7 @@ impl TryFrom for spki::AlgorithmIdentifierOwned { .parameters .map(|p| DerDecode::from_der(p.as_bytes())) .transpose()?; + Ok(Self { oid, parameters }) } } @@ -545,6 +558,30 @@ mod tests { Ok(()) } + #[test] + fn test_parameters_oid_decodes_oid_parameters() -> Result<(), Asn1Error> { + let curve_oid = ObjectIdentifier::from_str(oids::SECP256K1)?; + let oid_param = Any::new(rasn::der::encode(&curve_oid)?); + + let alg_with_oid = AlgorithmIdentifier::new_with_params(oids::EC_PUBLIC_KEY, oid_param)?; + assert_eq!(alg_with_oid.parameters_oid(), Some(curve_oid)); + Ok(()) + } + + #[test] + fn test_parameters_oid_rejects_non_oid_parameters() -> Result<(), Asn1Error> { + let alg_with_null = AlgorithmIdentifier::new_with_params(oids::RSA_ENCRYPTION, create_null_any())?; + assert_eq!(alg_with_null.parameters_oid(), None); + Ok(()) + } + + #[test] + fn test_parameters_oid_absent_parameters() -> Result<(), Asn1Error> { + let alg_without_params = AlgorithmIdentifier::new(oids::ED25519)?; + assert_eq!(alg_without_params.parameters_oid(), None); + Ok(()) + } + #[test] fn test_spki_conversions() -> Result<(), Asn1Error> { let alg_basic = AlgorithmIdentifier::new(oids::ED25519)?; diff --git a/keetanetwork-crypto/src/algorithms/aes_cbc.rs b/keetanetwork-crypto/src/algorithms/aes_cbc.rs index 13d9ecf..34e2de0 100644 --- a/keetanetwork-crypto/src/algorithms/aes_cbc.rs +++ b/keetanetwork-crypto/src/algorithms/aes_cbc.rs @@ -49,10 +49,12 @@ impl SymmetricEncryption for Aes256Cbc { // Create cipher let cipher = Aes256CbcEnc::new_from_slices(key, &iv_bytes)?; // Encrypt with PKCS#7 padding - let ciphertext = cipher.encrypt_padded_vec_mut::(plaintext.as_ref()); + let plaintext = plaintext.as_ref(); + let ciphertext = cipher.encrypt_padded_vec_mut::(plaintext); // Return IV + ciphertext - Ok(prepend_iv(&iv_bytes, &ciphertext)) + let result = prepend_iv(&iv_bytes, &ciphertext); + Ok(result) } /// Decrypt data using AES-256-CBC. @@ -64,7 +66,8 @@ impl SymmetricEncryption for Aes256Cbc { ensure_key_size(key, 32)?; // Extract IV and encrypted payload - let (iv, encrypted_data) = split_iv(ciphertext.as_ref())?; + let ciphertext = ciphertext.as_ref(); + let (iv, encrypted_data) = split_iv(ciphertext)?; // Create cipher let cipher = Aes256CbcDec::new_from_slices(key, iv)?; diff --git a/keetanetwork-crypto/src/algorithms/aes_common.rs b/keetanetwork-crypto/src/algorithms/aes_common.rs index ce05315..d31caec 100644 --- a/keetanetwork-crypto/src/algorithms/aes_common.rs +++ b/keetanetwork-crypto/src/algorithms/aes_common.rs @@ -52,5 +52,6 @@ pub(crate) fn split_iv(ciphertext: &[u8]) -> Result<(&[u8], &[u8]), CryptoError> return Err(CryptoError::DecryptionFailed); } - Ok(ciphertext.split_at(IV_SIZE)) + let iv_and_payload = ciphertext.split_at(IV_SIZE); + Ok(iv_and_payload) } diff --git a/keetanetwork-crypto/src/algorithms/aes_ctr.rs b/keetanetwork-crypto/src/algorithms/aes_ctr.rs index f364f08..3c18868 100644 --- a/keetanetwork-crypto/src/algorithms/aes_ctr.rs +++ b/keetanetwork-crypto/src/algorithms/aes_ctr.rs @@ -66,7 +66,8 @@ impl Aes128CtrCipher { // Create CTR cipher instance let mut cipher = Aes128Ctr::new_from_slices(key, iv)?; // CTR mode works in-place, so we need a mutable copy - let mut output = data.as_ref().to_vec(); + let data = data.as_ref(); + let mut output = data.to_vec(); cipher.apply_keystream(&mut output); @@ -130,7 +131,8 @@ impl SymmetricEncryption for Aes128CtrCipher { let ciphertext = self.encrypt_with_iv(key, iv_bytes, plaintext)?; // Prepend IV to ciphertext - Ok(prepend_iv(&iv_bytes, &ciphertext)) + let result = prepend_iv(&iv_bytes, &ciphertext); + Ok(result) } /// Decrypt data with IV extracted from the beginning @@ -141,7 +143,8 @@ impl SymmetricEncryption for Aes128CtrCipher { ensure_key_size(key, 16)?; // Extract IV from the beginning - let (iv, encrypted_data) = split_iv(ciphertext.as_ref())?; + let ciphertext = ciphertext.as_ref(); + let (iv, encrypted_data) = split_iv(ciphertext)?; // Decrypt with the extracted IV self.decrypt_with_iv(key, iv, encrypted_data) diff --git a/keetanetwork-x509/src/certificates.rs b/keetanetwork-x509/src/certificates.rs index 2f9d51a..5840208 100644 --- a/keetanetwork-x509/src/certificates.rs +++ b/keetanetwork-x509/src/certificates.rs @@ -1019,6 +1019,30 @@ fn check_duplicate_extensions(extensions: &[Extension]) -> Result<(), Certificat Ok(()) } +/// Verify an ECDSA certificate signature using the curve declared by the +/// issuer's SubjectPublicKeyInfo. +/// +/// RFC 5480 Section 2.1.1 requires EC public keys to carry their named-curve +/// OID in the algorithm parameters, so the curve is always known. +fn verify_ecdsa_declared_curve( + issuer_public_key: &SubjectPublicKeyInfo, + signature_bytes: &[u8], + tbs_der: &[u8], + hash: HashAlgorithm, +) -> Result { + let Some(curve_oid) = issuer_public_key.algorithm.parameters_oid() else { + return Ok(false); + }; + + let curve_oid = curve_oid.to_string(); + let public_key_bytes = issuer_public_key.subject_public_key.raw_bytes(); + match curve_oid.as_str() { + oids::SECP256R1 => utils::try_verify_ecdsa_secp256r1(public_key_bytes, signature_bytes, tbs_der, hash), + oids::SECP256K1 => utils::try_verify_ecdsa_secp256k1(public_key_bytes, signature_bytes, tbs_der, hash), + _ => Ok(false), + } +} + impl Certificate { /// Check if the certificate is valid at a specific time pub fn is_valid_at(&self, time: DateTime) -> Result { @@ -1209,13 +1233,11 @@ impl Certificate { oids::ED25519 => utils::verify_ed25519_signature(public_key_bytes, signature_bytes, &tbs_der), oids::ECDSA_WITH_SHA3_256 => { - // For ECDSA, try both curves since the verification function handles curve detection - utils::verify_ecdsa_signature(public_key_bytes, signature_bytes, &tbs_der, HashAlgorithm::Sha3_256) + verify_ecdsa_declared_curve(issuer_public_key, signature_bytes, &tbs_der, HashAlgorithm::Sha3_256) } oids::ECDSA_WITH_SHA256 => { - // For ECDSA, try both curves since the verification function handles curve detection - utils::verify_ecdsa_signature(public_key_bytes, signature_bytes, &tbs_der, HashAlgorithm::Sha2_256) + verify_ecdsa_declared_curve(issuer_public_key, signature_bytes, &tbs_der, HashAlgorithm::Sha2_256) } oids::SHA256_WITH_RSA => Err(CertificateError::InvalidCertificate), diff --git a/keetanetwork-x509/src/utils.rs b/keetanetwork-x509/src/utils.rs index 8d96379..fba6c2d 100644 --- a/keetanetwork-x509/src/utils.rs +++ b/keetanetwork-x509/src/utils.rs @@ -468,11 +468,11 @@ pub fn raw_to_der_signature(signature_bytes: &[u8]) -> Result, Certifica Ok(der_encoded) } -/// Generic ECDSA signature verification with multiple signature format support. +/// Generic ECDSA signature verification for DER-encoded signatures. /// /// This internal helper function implements the common ECDSA verification logic -/// for both Secp256r1 and Secp256k1 curves, handling both DER-encoded and raw -/// signature formats. +/// for both Secp256r1 and Secp256k1 curves, accepting only DER-encoded +/// signatures (RFC 5480 Section 2.2). /// /// # Type Parameters /// @@ -482,7 +482,7 @@ pub fn raw_to_der_signature(signature_bytes: &[u8]) -> Result, Certifica /// # Arguments /// /// * `public_key` - The public key for verification -/// * `signature_bytes` - Signature bytes (DER or raw format) +/// * `signature_bytes` - DER-encoded signature bytes /// * `tbs_der` - To-be-signed certificate data /// * `hash` - Hash algorithm declared by the certificate's signature /// algorithm OID, used to pre-hash the TBS data @@ -504,53 +504,37 @@ where K: CryptoVerifierWithOptions, F: Fn(&[u8; 64]) -> Result, { + // X.509 ECDSA signatures are always DER-encoded Ecdsa-Sig-Value + // structures (RFC 5480 Section 2.2); anything else is invalid let signature_bytes = signature_bytes.as_ref(); + let Ok(sig_array) = der_to_raw_signature(signature_bytes) else { + return Ok(false); + }; + + let signature = sig_from_bytes(&sig_array)?; // Pre-hash the TBS data with the OID-declared hash, then verify the // digest directly (raw mode) - let tbs_digest = hash.hash(tbs_der.as_ref()); + let tbs_der = tbs_der.as_ref(); + let tbs_digest = hash.hash(tbs_der); let options = SigningOptions::raw(); + let verified = public_key + .verify_with_options(&tbs_digest, &signature, options) + .is_ok(); - // Try DER-encoded signature first - if signature_bytes.len() >= 2 && signature_bytes[0] == 0x30 { - if let Ok(sig_array) = der_to_raw_signature(signature_bytes) { - let signature = sig_from_bytes(&sig_array)?; - - if public_key - .verify_with_options(&tbs_digest, &signature, options) - .is_ok() - { - return Ok(true); - } - } - } - // Try raw 64-byte signature - else if signature_bytes.len() == 64 { - let sig_array: [u8; 64] = signature_bytes.try_into().or_invalid_certificate()?; - let signature = sig_from_bytes(&sig_array)?; - - if public_key - .verify_with_options(&tbs_digest, &signature, options) - .is_ok() - { - return Ok(true); - } - } - - Ok(false) + Ok(verified) } -/// Verify ECDSA signature using Secp256r1 curve with multiple format support. +/// Verify ECDSA signature using Secp256r1 curve. /// /// This function attempts to verify an ECDSA signature using the Secp256r1 -/// elliptic curve. It supports both DER-encoded and raw 64-byte signature -/// formats, and tries multiple verification approaches including direct -/// verification and hash-based verification. +/// elliptic curve. The signature must be DER-encoded as required for X.509 +/// certificates (RFC 5480 Section 2.2). /// /// # Arguments /// /// * `public_key_bytes` - Raw bytes of the Secp256r1 public key -/// * `signature_bytes` - Signature bytes (DER or raw format) +/// * `signature_bytes` - DER-encoded signature bytes /// * `tbs_der` - To-be-signed certificate data in DER format /// * `hash` - Hash algorithm declared by the certificate's signature /// algorithm OID @@ -568,7 +552,7 @@ where /// use keetanetwork_x509::utils::try_verify_ecdsa_secp256r1; /// /// let public_key_bytes = &[/* 65 bytes of uncompressed public key */]; -/// let signature_bytes = &[/* DER or raw signature bytes */]; +/// let signature_bytes = &[/* DER-encoded signature bytes */]; /// let tbs_data = &[/* certificate data to verify */]; /// /// let result = try_verify_ecdsa_secp256r1( @@ -591,17 +575,16 @@ pub fn try_verify_ecdsa_secp256r1( }) } -/// Verify ECDSA signature using Secp256k1 curve with multiple format support. +/// Verify ECDSA signature using Secp256k1 curve. /// /// This function attempts to verify an ECDSA signature using the Secp256k1 -/// elliptic curve. It supports both DER-encoded and raw 64-byte signature -/// formats, and tries multiple verification approaches including direct -/// verification and hash-based verification. +/// elliptic curve. The signature must be DER-encoded as required for X.509 +/// certificates (RFC 5480 Section 2.2). /// /// # Arguments /// /// * `public_key_bytes` - Raw bytes of the Secp256k1 public key -/// * `signature_bytes` - Signature bytes (DER or raw format) +/// * `signature_bytes` - DER-encoded signature bytes /// * `tbs_der` - To-be-signed certificate data in DER format /// * `hash` - Hash algorithm declared by the certificate's signature /// algorithm OID @@ -619,7 +602,7 @@ pub fn try_verify_ecdsa_secp256r1( /// use keetanetwork_x509::utils::try_verify_ecdsa_secp256k1; /// /// let public_key_bytes = &[/* 65 bytes of uncompressed public key */]; -/// let signature_bytes = &[/* DER or raw signature bytes */]; +/// let signature_bytes = &[/* DER-encoded signature bytes */]; /// let tbs_data = &[/* certificate data to verify */]; /// /// let result = try_verify_ecdsa_secp256k1( @@ -696,66 +679,6 @@ pub fn verify_ed25519_signature( .map_err(|_| CertificateError::CertificateSignatureVerificationFailed) } -/// Verify ECDSA signature trying both Secp256r1 and Secp256k1 curves. -/// -/// This function attempts to verify an ECDSA signature by trying both supported -/// elliptic curves (Secp256r1/P-256 and Secp256k1). This is useful when the -/// specific curve is not known from the algorithm identifier, as is the case -/// with the generic "ECDSA with SHA-256" algorithm identifier. -/// -/// The function tries Secp256r1 first (as it's more common in X.509), then -/// falls back to Secp256k1 if verification fails. -/// -/// # Arguments -/// -/// * `public_key_bytes` - Raw bytes of the ECDSA public key -/// * `signature_bytes` - Signature bytes (DER or raw format) -/// * `tbs_der` - To-be-signed certificate data in DER format -/// * `hash` - Hash algorithm declared by the certificate's signature -/// algorithm OID -/// -/// # Returns -/// -/// * `Ok(true)` - Signature verification succeeded with one of the curves -/// * `Ok(false)` - Signature verification failed with both curves -/// * `Err(CertificateError)` - Error during verification process -/// -/// # Example -/// -/// ```rust,no_run -/// use keetanetwork_crypto::prelude::HashAlgorithm; -/// use keetanetwork_x509::utils::verify_ecdsa_signature; -/// -/// // This example shows the function signature but does not run -/// // because it would require valid cryptographic data -/// let public_key_bytes = &[/* ECDSA public key bytes */]; -/// let signature_bytes = &[/* signature bytes */]; -/// let tbs_data = &[/* certificate data to verify */]; -/// -/// let result = verify_ecdsa_signature( -/// public_key_bytes, -/// signature_bytes, -/// tbs_data, -/// HashAlgorithm::Sha3_256 -/// ); -/// ``` -pub fn verify_ecdsa_signature( - public_key_bytes: impl AsRef<[u8]>, - signature_bytes: impl AsRef<[u8]>, - tbs_der: impl AsRef<[u8]>, - hash: HashAlgorithm, -) -> Result { - // Try Secp256r1 first (more common in X.509) - if let Ok(result) = try_verify_ecdsa_secp256r1(&public_key_bytes, &signature_bytes, &tbs_der, hash) { - if result { - return Ok(true); - } - } - - // Try Secp256k1 if Secp256r1 failed - try_verify_ecdsa_secp256k1(public_key_bytes, signature_bytes, tbs_der, hash) -} - #[cfg(test)] mod tests { use super::*; @@ -1289,7 +1212,7 @@ mod tests { // Test cases: (pub_key_len, signature_data, description, should_error) let test_cases = [ - // Invalid signature lengths for raw format + // Non-DER signatures (65, vec![0u8; 32], true), (65, vec![0u8; 96], true), (65, vec![], true), @@ -1300,7 +1223,7 @@ mod tests { // Invalid DER signature format (65, vec![0x30, 0x02, 0x01], true), (65, vec![0x30], true), - // Valid lengths but dummy data (should error due to invalid key) + // Non-DER signature with dummy data (should error due to invalid key) (65, vec![0u8; 64], true), ]; @@ -1326,31 +1249,16 @@ mod tests { } #[test] - fn test_verify_ecdsa_signature_fallback() { - let public_key_bytes = vec![0u8; 65]; // Invalid but correct length - let signature_bytes = vec![0u8; 64]; - let tbs_der = b"test data to sign"; - - let result = verify_ecdsa_signature(&public_key_bytes, &signature_bytes, tbs_der, HashAlgorithm::Sha3_256); - assert!(result.is_err()); - - let public_key_bytes = vec![0u8; 16]; - let result = verify_ecdsa_signature(&public_key_bytes, &signature_bytes, tbs_der, HashAlgorithm::Sha3_256); - assert!(result.is_err()); - } - - #[test] - fn test_signature_format_detection() { + fn test_signature_format_rejection_with_invalid_key() { let public_key_bytes = vec![0u8; 65]; let tbs_der = b"test data"; let test_cases = [ - // DER format detection (starts with 0x30) + // Truncated DER SEQUENCE vec![0x30, 0x44, 0x02, 0x20], - // Raw format (64 bytes, not starting with 0x30) + // Non-DER 64-byte signature (raw format is not accepted) vec![0x01; 64], - // Raw format with different starting byte vec![0xFF; 64], - // Invalid length (not 64 and not DER) + // Invalid lengths vec![0x01; 48], vec![0x01; 32], // Empty signature @@ -1365,4 +1273,23 @@ mod tests { assert!(result.is_err()); } } + + #[test] + fn test_non_der_signature_rejected_with_valid_key() -> Result<(), CertificateError> { + // P-256 generator point (uncompressed): a valid on-curve public key + let public_key_bytes: [u8; 65] = [ + 0x04, // Uncompressed point indicator + 0x6B, 0x17, 0xD1, 0xF2, 0xE1, 0x2C, 0x42, 0x47, 0xF8, 0xBC, 0xE6, 0xE5, 0x63, 0xA4, 0x40, 0xF2, 0x77, + 0x03, 0x7D, 0x81, 0x2D, 0xEB, 0x33, 0xA0, 0xF4, 0xA1, 0x39, 0x45, 0xD8, 0x98, 0xC2, 0x96, 0x4F, 0xE3, + 0x42, 0xE2, 0xFE, 0x1A, 0x7F, 0x9B, 0x8E, 0xE7, 0xEB, 0x4A, 0x7C, 0x0F, 0x9E, 0x16, 0x2B, 0xCE, 0x33, + 0x57, 0x6B, 0x31, 0x5E, 0xCE, 0xCB, 0xB6, 0x40, 0x68, 0x37, 0xBF, 0x51, 0xF5, + ]; + + let tbs_der = b"test data"; + // A raw 64-byte (non-DER) signature must fail verification + let raw_signature = [0x01u8; 64]; + let result = try_verify_ecdsa_secp256r1(public_key_bytes, raw_signature, tbs_der, HashAlgorithm::Sha3_256)?; + assert!(!result); + Ok(()) + } } diff --git a/keetanetwork-x509/tests/builders.rs b/keetanetwork-x509/tests/builders.rs index 798d451..d38ed0b 100644 --- a/keetanetwork-x509/tests/builders.rs +++ b/keetanetwork-x509/tests/builders.rs @@ -3,7 +3,7 @@ mod common; use chrono::{TimeZone, Utc}; use der::{Decode, Encode}; use keetanetwork_account::{Account, KeyECDSASECP256K1, KeyECDSASECP256R1, KeyPair}; -use keetanetwork_asn1::{BitString, SubjectPublicKeyInfo}; +use keetanetwork_asn1::{AlgorithmIdentifier, BitString, SubjectPublicKeyInfo}; use keetanetwork_crypto::algorithms::secp256k1::Secp256k1Derivation; use keetanetwork_crypto::algorithms::secp256r1::Secp256r1Derivation; use keetanetwork_crypto::prelude::Algorithm; @@ -348,3 +348,54 @@ fn test_ecdsa_signature_hash_selection() -> Result<(), Box Result<(), Box> { + // Build a self-signed secp256k1 certificate + let seed = generate_random_seed()?; + let seed_bytes = (*seed.expose_secret()).into_secret(); + let private_key = Secp256k1Derivation::derive_from_seed(seed_bytes)?; + let account = Account::::from(private_key); + let public_key = account.keypair.to_public_key(); + let public_key_info = SubjectPublicKeyInfo::from(public_key.clone()); + + let subject_dn = utils::create_dn(&[(oids::CN, "Curve Declaration Test")])?; + let certificate = keetanetwork_x509::builder::CertificateBuilder::new() + .with_subject_public_key(public_key_info.clone()) + .with_subject_dn(subject_dn.clone()) + .with_issuer_dn(subject_dn) + .with_serial_number(SerialNumber::from(1u64)) + .with_validity_days(365) + .build(&account)?; + + assert!( + certificate.verify_signature(&public_key_info)?, + "certificate with correctly declared secp256k1 curve should verify" + ); + + // Same key bytes but SPKI declaring secp256r1: verification must not succeed + let r1_seed = generate_random_seed()?; + let r1_seed_bytes = (*r1_seed.expose_secret()).into_secret(); + let r1_private_key = Secp256r1Derivation::derive_from_seed(r1_seed_bytes)?; + let r1_account = Account::::from(r1_private_key); + let r1_public_key = r1_account.keypair.to_public_key(); + let r1_spki = SubjectPublicKeyInfo::from(r1_public_key); + + let key_bytes = Vec::::from(public_key); + let wrong_curve_spki = SubjectPublicKeyInfo::new(r1_spki.algorithm, &key_bytes)?; + let wrong_curve_result = certificate.verify_signature(&wrong_curve_spki); + assert!( + !matches!(wrong_curve_result, Ok(true)), + "secp256k1 key declared as secp256r1 must not verify" + ); + + // SPKI with no curve parameters at all: verification must fail + let no_params_algorithm = AlgorithmIdentifier::new(oids::EC_PUBLIC_KEY)?; + let no_params_spki = SubjectPublicKeyInfo::new(no_params_algorithm, &key_bytes)?; + assert!( + !certificate.verify_signature(&no_params_spki)?, + "EC key without a declared curve must not verify" + ); + + Ok(()) +} From 1081ff0a1aaf4b14ac912aa1cabe162f5d4cec8e Mon Sep 17 00:00:00 2001 From: Tanveer Wahid Date: Mon, 6 Jul 2026 13:39:26 -0700 Subject: [PATCH 2/2] chore: lint --- keetanetwork-x509/src/certificates.rs | 5 +++-- keetanetwork-x509/src/utils.rs | 8 ++++---- keetanetwork-x509/tests/builders.rs | 10 ++-------- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/keetanetwork-x509/src/certificates.rs b/keetanetwork-x509/src/certificates.rs index 5840208..b441e7b 100644 --- a/keetanetwork-x509/src/certificates.rs +++ b/keetanetwork-x509/src/certificates.rs @@ -1022,8 +1022,9 @@ fn check_duplicate_extensions(extensions: &[Extension]) -> Result<(), Certificat /// Verify an ECDSA certificate signature using the curve declared by the /// issuer's SubjectPublicKeyInfo. /// -/// RFC 5480 Section 2.1.1 requires EC public keys to carry their named-curve -/// OID in the algorithm parameters, so the curve is always known. +/// RFC 5480 Section 2.1.1 requires the algorithm parameters to be present and +/// restricts them to a named-curve OID. This implementation accepts the +/// secp256r1 and secp256k1 named curves. fn verify_ecdsa_declared_curve( issuer_public_key: &SubjectPublicKeyInfo, signature_bytes: &[u8], diff --git a/keetanetwork-x509/src/utils.rs b/keetanetwork-x509/src/utils.rs index fba6c2d..b527611 100644 --- a/keetanetwork-x509/src/utils.rs +++ b/keetanetwork-x509/src/utils.rs @@ -1279,10 +1279,10 @@ mod tests { // P-256 generator point (uncompressed): a valid on-curve public key let public_key_bytes: [u8; 65] = [ 0x04, // Uncompressed point indicator - 0x6B, 0x17, 0xD1, 0xF2, 0xE1, 0x2C, 0x42, 0x47, 0xF8, 0xBC, 0xE6, 0xE5, 0x63, 0xA4, 0x40, 0xF2, 0x77, - 0x03, 0x7D, 0x81, 0x2D, 0xEB, 0x33, 0xA0, 0xF4, 0xA1, 0x39, 0x45, 0xD8, 0x98, 0xC2, 0x96, 0x4F, 0xE3, - 0x42, 0xE2, 0xFE, 0x1A, 0x7F, 0x9B, 0x8E, 0xE7, 0xEB, 0x4A, 0x7C, 0x0F, 0x9E, 0x16, 0x2B, 0xCE, 0x33, - 0x57, 0x6B, 0x31, 0x5E, 0xCE, 0xCB, 0xB6, 0x40, 0x68, 0x37, 0xBF, 0x51, 0xF5, + 0x6B, 0x17, 0xD1, 0xF2, 0xE1, 0x2C, 0x42, 0x47, 0xF8, 0xBC, 0xE6, 0xE5, 0x63, 0xA4, 0x40, 0xF2, 0x77, 0x03, + 0x7D, 0x81, 0x2D, 0xEB, 0x33, 0xA0, 0xF4, 0xA1, 0x39, 0x45, 0xD8, 0x98, 0xC2, 0x96, 0x4F, 0xE3, 0x42, 0xE2, + 0xFE, 0x1A, 0x7F, 0x9B, 0x8E, 0xE7, 0xEB, 0x4A, 0x7C, 0x0F, 0x9E, 0x16, 0x2B, 0xCE, 0x33, 0x57, 0x6B, 0x31, + 0x5E, 0xCE, 0xCB, 0xB6, 0x40, 0x68, 0x37, 0xBF, 0x51, 0xF5, ]; let tbs_der = b"test data"; diff --git a/keetanetwork-x509/tests/builders.rs b/keetanetwork-x509/tests/builders.rs index d38ed0b..cc58be2 100644 --- a/keetanetwork-x509/tests/builders.rs +++ b/keetanetwork-x509/tests/builders.rs @@ -384,18 +384,12 @@ fn test_ecdsa_verification_requires_declared_curve() -> Result<(), Box::from(public_key); let wrong_curve_spki = SubjectPublicKeyInfo::new(r1_spki.algorithm, &key_bytes)?; let wrong_curve_result = certificate.verify_signature(&wrong_curve_spki); - assert!( - !matches!(wrong_curve_result, Ok(true)), - "secp256k1 key declared as secp256r1 must not verify" - ); + assert!(!matches!(wrong_curve_result, Ok(true)), "secp256k1 key declared as secp256r1 must not verify"); // SPKI with no curve parameters at all: verification must fail let no_params_algorithm = AlgorithmIdentifier::new(oids::EC_PUBLIC_KEY)?; let no_params_spki = SubjectPublicKeyInfo::new(no_params_algorithm, &key_bytes)?; - assert!( - !certificate.verify_signature(&no_params_spki)?, - "EC key without a declared curve must not verify" - ); + assert!(!certificate.verify_signature(&no_params_spki)?, "EC key without a declared curve must not verify"); Ok(()) }