Skip to content

Commit 3e278a2

Browse files
committed
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 <ionut.mihalcea@arm.com>
1 parent 4d29d74 commit 3e278a2

File tree

3 files changed

+124
-12
lines changed

3 files changed

+124
-12
lines changed

tss-esapi/src/structures/tagged/public/ecc.rs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,31 @@ impl PublicEccParametersBuilder {
194194
return Err(Error::local_error(WrapperErrorKind::InconsistentParams));
195195
}
196196

197-
if (ecc_curve == EccCurve::BnP256 || ecc_curve == EccCurve::BnP638)
198-
&& ecc_scheme.algorithm() != EccSchemeAlgorithm::EcDaa
199-
{
200-
error!("Bn curve should use only EcDaa scheme");
201-
return Err(Error::local_error(WrapperErrorKind::InconsistentParams));
197+
match (ecc_curve, ecc_scheme.algorithm()) {
198+
(EccCurve::BnP256 | EccCurve::BnP638, EccSchemeAlgorithm::EcDaa)
199+
| (EccCurve::Sm2P256, EccSchemeAlgorithm::Sm2)
200+
| (
201+
EccCurve::NistP192
202+
| EccCurve::NistP224
203+
| EccCurve::NistP256
204+
| EccCurve::NistP384
205+
| EccCurve::NistP521,
206+
EccSchemeAlgorithm::EcDh
207+
| EccSchemeAlgorithm::EcDaa
208+
| EccSchemeAlgorithm::EcDsa
209+
| EccSchemeAlgorithm::EcMqv
210+
| EccSchemeAlgorithm::EcSchnorr,
211+
)
212+
| (_, EccSchemeAlgorithm::Null) => (),
213+
214+
_ => {
215+
error!(
216+
"Mismatch between elliptic curve ({:#?}) and signing scheme ({:#?}) used",
217+
ecc_curve,
218+
ecc_scheme.algorithm()
219+
);
220+
return Err(Error::local_error(WrapperErrorKind::InconsistentParams));
221+
}
202222
}
203223

204224
Ok(PublicEccParameters {

tss-esapi/tests/integration_tests/abstraction_tests/ak_tests.rs

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use tss_esapi::{
1515
session_handles::PolicySession,
1616
},
1717
structures::{Auth, Digest, PublicBuilder, SymmetricDefinition},
18+
Error, WrapperErrorKind,
1819
};
1920

2021
use crate::common::create_ctx_without_session;
@@ -51,24 +52,24 @@ fn test_create_ak_rsa_ecc() {
5152
None,
5253
)
5354
.unwrap();
54-
if ak::create_ak(
55+
if let Err(Error::WrapperError(WrapperErrorKind::InconsistentParams)) = ak::create_ak(
5556
&mut context,
5657
ek_rsa,
5758
HashingAlgorithm::Sha256,
5859
AsymmetricAlgorithmSelection::Ecc(EccCurve::NistP256),
5960
SignatureSchemeAlgorithm::Sm2,
6061
None,
6162
None,
62-
)
63-
.is_ok()
64-
{
65-
// We can't use unwrap_err because that requires Debug on the T
66-
panic!("Should have errored");
63+
) {
64+
} else {
65+
panic!(
66+
"Should've gotten an 'InconsistentParams' error when trying to create an a P256 AK with an SM2 signing scheme."
67+
);
6768
}
6869
}
6970

7071
#[test]
71-
fn test_create_ak_ecc_ecc() {
72+
fn test_create_ak_ecc() {
7273
let mut context = create_ctx_without_session();
7374

7475
let ek_ecc = ek::create_ek_object(
@@ -89,6 +90,28 @@ fn test_create_ak_ecc_ecc() {
8990
.unwrap();
9091
}
9192

93+
#[test]
94+
fn test_create_ak_ecdaa() {
95+
let mut context = create_ctx_without_session();
96+
97+
let ek_ecc = ek::create_ek_object(
98+
&mut context,
99+
AsymmetricAlgorithmSelection::Ecc(EccCurve::NistP384),
100+
None,
101+
)
102+
.unwrap();
103+
ak::create_ak(
104+
&mut context,
105+
ek_ecc,
106+
HashingAlgorithm::Sha256,
107+
AsymmetricAlgorithmSelection::Ecc(EccCurve::BnP256),
108+
SignatureSchemeAlgorithm::EcDaa,
109+
None,
110+
None,
111+
)
112+
.unwrap();
113+
}
114+
92115
#[test]
93116
fn test_create_and_use_ak() {
94117
let mut context = create_ctx_without_session();

tss-esapi/tests/integration_tests/structures_tests/tagged_tests/public_ecc_parameters_tests.rs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,72 @@ fn test_signing_with_wrong_symmetric() {
9292
Err(Error::WrapperError(WrapperErrorKind::InconsistentParams))
9393
));
9494
}
95+
96+
#[test]
97+
fn test_signing_with_mismatched_scheme_nist() {
98+
assert_eq!(
99+
PublicEccParametersBuilder::new()
100+
.with_restricted(false)
101+
.with_is_decryption_key(false)
102+
.with_is_signing_key(true)
103+
.with_curve(EccCurve::NistP256)
104+
.with_ecc_scheme(
105+
EccScheme::create(
106+
EccSchemeAlgorithm::Sm2,
107+
Some(HashingAlgorithm::Sha256),
108+
None
109+
)
110+
.unwrap()
111+
)
112+
.with_key_derivation_function_scheme(KeyDerivationFunctionScheme::Null)
113+
.with_symmetric(SymmetricDefinitionObject::Null)
114+
.build(),
115+
Err(Error::WrapperError(WrapperErrorKind::InconsistentParams))
116+
);
117+
}
118+
119+
#[test]
120+
fn test_signing_with_mismatched_scheme_sm2() {
121+
assert_eq!(
122+
PublicEccParametersBuilder::new()
123+
.with_restricted(false)
124+
.with_is_decryption_key(false)
125+
.with_is_signing_key(true)
126+
.with_curve(EccCurve::Sm2P256)
127+
.with_ecc_scheme(
128+
EccScheme::create(
129+
EccSchemeAlgorithm::EcDsa,
130+
Some(HashingAlgorithm::Sha256),
131+
None
132+
)
133+
.unwrap()
134+
)
135+
.with_key_derivation_function_scheme(KeyDerivationFunctionScheme::Null)
136+
.with_symmetric(SymmetricDefinitionObject::Null)
137+
.build(),
138+
Err(Error::WrapperError(WrapperErrorKind::InconsistentParams))
139+
);
140+
}
141+
142+
#[test]
143+
fn test_signing_with_mismatched_scheme_bn() {
144+
assert_eq!(
145+
PublicEccParametersBuilder::new()
146+
.with_restricted(false)
147+
.with_is_decryption_key(false)
148+
.with_is_signing_key(true)
149+
.with_curve(EccCurve::BnP256)
150+
.with_ecc_scheme(
151+
EccScheme::create(
152+
EccSchemeAlgorithm::EcDsa,
153+
Some(HashingAlgorithm::Sha256),
154+
None
155+
)
156+
.unwrap()
157+
)
158+
.with_key_derivation_function_scheme(KeyDerivationFunctionScheme::Null)
159+
.with_symmetric(SymmetricDefinitionObject::Null)
160+
.build(),
161+
Err(Error::WrapperError(WrapperErrorKind::InconsistentParams))
162+
);
163+
}

0 commit comments

Comments
 (0)