From 4d29d7462bd7bda4cfd25b0a99f258eaaef3de8f Mon Sep 17 00:00:00 2001 From: Ionut Mihalcea Date: Thu, 26 Oct 2023 10:32:22 +0100 Subject: [PATCH 1/2] Fix create_ak for ECC keys Fixing the parameters for creating AKs in the Endorsement Hierarchy. The `count` value part of the `EccScheme` has been adjusted, and an empty `EccPoint` was added as unique identifier for the key. Signed-off-by: Ionut Mihalcea --- tss-esapi/src/abstraction/ak.rs | 11 +++++++--- .../abstraction_tests/ak_tests.rs | 22 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/tss-esapi/src/abstraction/ak.rs b/tss-esapi/src/abstraction/ak.rs index de5ce9ad..8feeb73f 100644 --- a/tss-esapi/src/abstraction/ak.rs +++ b/tss-esapi/src/abstraction/ak.rs @@ -16,7 +16,7 @@ use crate::{ session_handles::PolicySession, }, structures::{ - Auth, CreateKeyResult, EccScheme, KeyDerivationFunctionScheme, Private, Public, + Auth, CreateKeyResult, EccPoint, EccScheme, KeyDerivationFunctionScheme, Private, Public, PublicBuilder, PublicEccParametersBuilder, PublicKeyRsa, PublicRsaParametersBuilder, RsaExponent, RsaScheme, SymmetricDefinitionObject, }, @@ -77,12 +77,17 @@ fn create_ak_public( .with_ecc_scheme(EccScheme::create( EccSchemeAlgorithm::try_from(AlgorithmIdentifier::from(sign_alg))?, Some(hash_alg), - Some(0), + if sign_alg == SignatureSchemeAlgorithm::EcDaa { + Some(0) + } else { + None + }, )?) .with_curve(ecc_curve) .with_key_derivation_function_scheme(KeyDerivationFunctionScheme::Null) .build()?, - ), + ) + .with_ecc_unique_identifier(EccPoint::default()), }; let key_builder = if let Some(ref k) = key_customization { diff --git a/tss-esapi/tests/integration_tests/abstraction_tests/ak_tests.rs b/tss-esapi/tests/integration_tests/abstraction_tests/ak_tests.rs index baed8fd5..b011de2c 100644 --- a/tss-esapi/tests/integration_tests/abstraction_tests/ak_tests.rs +++ b/tss-esapi/tests/integration_tests/abstraction_tests/ak_tests.rs @@ -67,6 +67,28 @@ fn test_create_ak_rsa_ecc() { } } +#[test] +fn test_create_ak_ecc_ecc() { + let mut context = create_ctx_without_session(); + + let ek_ecc = ek::create_ek_object( + &mut context, + AsymmetricAlgorithmSelection::Ecc(EccCurve::NistP384), + None, + ) + .unwrap(); + ak::create_ak( + &mut context, + ek_ecc, + HashingAlgorithm::Sha256, + AsymmetricAlgorithmSelection::Ecc(EccCurve::NistP256), + SignatureSchemeAlgorithm::EcDsa, + None, + None, + ) + .unwrap(); +} + #[test] fn test_create_and_use_ak() { let mut context = create_ctx_without_session(); From 3e278a2dbeca160ac568386607bf711c3ac7b7fe Mon Sep 17 00:00:00 2001 From: Ionut Mihalcea Date: Thu, 26 Oct 2023 14:42:07 +0100 Subject: [PATCH 2/2] Improve checking of elliptic curve - scheme match A proper match of elliptic curve and asymmetric scheme is more thoroughly checked to avoid cases where the user can generate PublicEccParameters that are invalid. Signed-off-by: Ionut Mihalcea --- tss-esapi/src/structures/tagged/public/ecc.rs | 30 ++++++-- .../abstraction_tests/ak_tests.rs | 37 ++++++++-- .../public_ecc_parameters_tests.rs | 69 +++++++++++++++++++ 3 files changed, 124 insertions(+), 12 deletions(-) diff --git a/tss-esapi/src/structures/tagged/public/ecc.rs b/tss-esapi/src/structures/tagged/public/ecc.rs index 6327ac92..a49394be 100644 --- a/tss-esapi/src/structures/tagged/public/ecc.rs +++ b/tss-esapi/src/structures/tagged/public/ecc.rs @@ -194,11 +194,31 @@ impl PublicEccParametersBuilder { return Err(Error::local_error(WrapperErrorKind::InconsistentParams)); } - if (ecc_curve == EccCurve::BnP256 || ecc_curve == EccCurve::BnP638) - && ecc_scheme.algorithm() != EccSchemeAlgorithm::EcDaa - { - error!("Bn curve should use only EcDaa scheme"); - return Err(Error::local_error(WrapperErrorKind::InconsistentParams)); + match (ecc_curve, ecc_scheme.algorithm()) { + (EccCurve::BnP256 | EccCurve::BnP638, EccSchemeAlgorithm::EcDaa) + | (EccCurve::Sm2P256, EccSchemeAlgorithm::Sm2) + | ( + EccCurve::NistP192 + | EccCurve::NistP224 + | EccCurve::NistP256 + | EccCurve::NistP384 + | EccCurve::NistP521, + EccSchemeAlgorithm::EcDh + | EccSchemeAlgorithm::EcDaa + | EccSchemeAlgorithm::EcDsa + | EccSchemeAlgorithm::EcMqv + | EccSchemeAlgorithm::EcSchnorr, + ) + | (_, EccSchemeAlgorithm::Null) => (), + + _ => { + error!( + "Mismatch between elliptic curve ({:#?}) and signing scheme ({:#?}) used", + ecc_curve, + ecc_scheme.algorithm() + ); + return Err(Error::local_error(WrapperErrorKind::InconsistentParams)); + } } Ok(PublicEccParameters { diff --git a/tss-esapi/tests/integration_tests/abstraction_tests/ak_tests.rs b/tss-esapi/tests/integration_tests/abstraction_tests/ak_tests.rs index b011de2c..c57bb0a0 100644 --- a/tss-esapi/tests/integration_tests/abstraction_tests/ak_tests.rs +++ b/tss-esapi/tests/integration_tests/abstraction_tests/ak_tests.rs @@ -15,6 +15,7 @@ use tss_esapi::{ session_handles::PolicySession, }, structures::{Auth, Digest, PublicBuilder, SymmetricDefinition}, + Error, WrapperErrorKind, }; use crate::common::create_ctx_without_session; @@ -51,7 +52,7 @@ fn test_create_ak_rsa_ecc() { None, ) .unwrap(); - if ak::create_ak( + if let Err(Error::WrapperError(WrapperErrorKind::InconsistentParams)) = ak::create_ak( &mut context, ek_rsa, HashingAlgorithm::Sha256, @@ -59,16 +60,16 @@ fn test_create_ak_rsa_ecc() { SignatureSchemeAlgorithm::Sm2, None, None, - ) - .is_ok() - { - // We can't use unwrap_err because that requires Debug on the T - panic!("Should have errored"); + ) { + } else { + panic!( + "Should've gotten an 'InconsistentParams' error when trying to create an a P256 AK with an SM2 signing scheme." + ); } } #[test] -fn test_create_ak_ecc_ecc() { +fn test_create_ak_ecc() { let mut context = create_ctx_without_session(); let ek_ecc = ek::create_ek_object( @@ -89,6 +90,28 @@ fn test_create_ak_ecc_ecc() { .unwrap(); } +#[test] +fn test_create_ak_ecdaa() { + let mut context = create_ctx_without_session(); + + let ek_ecc = ek::create_ek_object( + &mut context, + AsymmetricAlgorithmSelection::Ecc(EccCurve::NistP384), + None, + ) + .unwrap(); + ak::create_ak( + &mut context, + ek_ecc, + HashingAlgorithm::Sha256, + AsymmetricAlgorithmSelection::Ecc(EccCurve::BnP256), + SignatureSchemeAlgorithm::EcDaa, + None, + None, + ) + .unwrap(); +} + #[test] fn test_create_and_use_ak() { let mut context = create_ctx_without_session(); diff --git a/tss-esapi/tests/integration_tests/structures_tests/tagged_tests/public_ecc_parameters_tests.rs b/tss-esapi/tests/integration_tests/structures_tests/tagged_tests/public_ecc_parameters_tests.rs index eb23c331..d9420e33 100644 --- a/tss-esapi/tests/integration_tests/structures_tests/tagged_tests/public_ecc_parameters_tests.rs +++ b/tss-esapi/tests/integration_tests/structures_tests/tagged_tests/public_ecc_parameters_tests.rs @@ -92,3 +92,72 @@ fn test_signing_with_wrong_symmetric() { Err(Error::WrapperError(WrapperErrorKind::InconsistentParams)) )); } + +#[test] +fn test_signing_with_mismatched_scheme_nist() { + assert_eq!( + PublicEccParametersBuilder::new() + .with_restricted(false) + .with_is_decryption_key(false) + .with_is_signing_key(true) + .with_curve(EccCurve::NistP256) + .with_ecc_scheme( + EccScheme::create( + EccSchemeAlgorithm::Sm2, + Some(HashingAlgorithm::Sha256), + None + ) + .unwrap() + ) + .with_key_derivation_function_scheme(KeyDerivationFunctionScheme::Null) + .with_symmetric(SymmetricDefinitionObject::Null) + .build(), + Err(Error::WrapperError(WrapperErrorKind::InconsistentParams)) + ); +} + +#[test] +fn test_signing_with_mismatched_scheme_sm2() { + assert_eq!( + PublicEccParametersBuilder::new() + .with_restricted(false) + .with_is_decryption_key(false) + .with_is_signing_key(true) + .with_curve(EccCurve::Sm2P256) + .with_ecc_scheme( + EccScheme::create( + EccSchemeAlgorithm::EcDsa, + Some(HashingAlgorithm::Sha256), + None + ) + .unwrap() + ) + .with_key_derivation_function_scheme(KeyDerivationFunctionScheme::Null) + .with_symmetric(SymmetricDefinitionObject::Null) + .build(), + Err(Error::WrapperError(WrapperErrorKind::InconsistentParams)) + ); +} + +#[test] +fn test_signing_with_mismatched_scheme_bn() { + assert_eq!( + PublicEccParametersBuilder::new() + .with_restricted(false) + .with_is_decryption_key(false) + .with_is_signing_key(true) + .with_curve(EccCurve::BnP256) + .with_ecc_scheme( + EccScheme::create( + EccSchemeAlgorithm::EcDsa, + Some(HashingAlgorithm::Sha256), + None + ) + .unwrap() + ) + .with_key_derivation_function_scheme(KeyDerivationFunctionScheme::Null) + .with_symmetric(SymmetricDefinitionObject::Null) + .build(), + Err(Error::WrapperError(WrapperErrorKind::InconsistentParams)) + ); +}