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

Add ECC functionality to PKCS11 prov #446

Merged
merged 1 commit into from
Jun 18, 2021

Conversation

ionut-arm
Copy link
Member

This commit adds support for elliptic curves in the PKCS11 provider.
Namely, elliptic curve key pairs can be created and used to sign and
verify signatures. This covers a subset of elliptic curve families and
specific curves within them.

First part of implementing #421

@ionut-arm ionut-arm added the enhancement New feature or request label Jun 3, 2021
@ionut-arm ionut-arm added this to the Parsec Release 0.8.0 milestone Jun 3, 2021
@ionut-arm ionut-arm requested a review from hug-dev June 3, 2021 19:12
@ionut-arm ionut-arm self-assigned this Jun 3, 2021
@@ -172,7 +172,7 @@ fn convert_curve_to_tpm(key_attributes: Attributes) -> Result<EccCurve> {
224 => Ok(EccCurve::NistP224),
256 => Ok(EccCurve::NistP256),
384 => Ok(EccCurve::NistP384),
512 => Ok(EccCurve::NistP521),
521 => Ok(EccCurve::NistP521),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is (was?) a bug, don't know if it's ok to fix it in this PR.

@ionut-arm ionut-arm force-pushed the pkcs11-ecc branch 4 times, most recently from d1a1ac3 to 2eae072 Compare June 4, 2021 13:18
Copy link
Member

@hug-dev hug-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Very nice to see all those tests executing on the PKCS11 provider.

Might be worth to update the operations coverage once this is merged.

src/providers/pkcs11/key_management.rs Show resolved Hide resolved
src/providers/pkcs11/key_management.rs Outdated Show resolved Hide resolved
src/providers/pkcs11/key_management.rs Outdated Show resolved Hide resolved
src/providers/pkcs11/utils.rs Outdated Show resolved Hide resolved
@ionut-arm
Copy link
Member Author

The Cross-compilation Docker image might need to be updated, as the Rust compiler version used for cross-compilation is throwing an error due to a newly stabilized feature being used in picky-asn1. We'll need to decide whether we can expect the compilation issue to vanish when distros are repackaged (because a newer Rustc will be used), or if we instead either: try to get around the problem somehow (either not using the new picky version or finding a way to not include the breaking line?), or we ask our friends at Devolutions to revert that line.

This commit adds support for elliptic curves in the PKCS11 provider.
Namely, elliptic curve key pairs can be created and used to sign and
verify signatures. This covers a subset of elliptic curve families and
specific curves within them.

Signed-off-by: Ionut Mihalcea <ionut.mihalcea@arm.com>
Comment on lines +30 to +31
picky-asn1-der = { version = "<=0.2.4", optional = true }
picky-asn1 = { version = ">=0.3.1, <=0.3.1", optional = true }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤯

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, for some reason just using <=0.3.1 didn't work....

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same but maybe =0.3.1 would work

Comment on lines -792 to -796
#[cfg(any(
feature = "mbed-crypto-provider",
feature = "tpm-provider",
feature = "cryptoauthlib-provider"
))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So nice that it works on all providers now ⭐

Ok(ECParameters::NamedCurve(match (ecc_family, bits) {
// The following "unwrap()" should be ok, as they cover constant conversions
(EccFamily::SecpR1, 192) => {
ObjectIdentifierAsn1(String::from("1.2.840.10045.3.1.1").try_into().unwrap())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there in some of the picky crates, those defined as constants or methods?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is - in the ones we can't use because of compiler limitations 😬 I added them recently, so they're in the most recent version only.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah right, that was it! No worries then.

@ionut-arm ionut-arm merged commit 25b754d into parallaxsecond:main Jun 18, 2021
@ionut-arm ionut-arm deleted the pkcs11-ecc branch June 18, 2021 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants