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

PSA_IMPORT_KEY introduction. #399

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion e2e_tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ impl TestClient {
key_type: Type::EccPublicKey {
curve_family: EccFamily::SecpR1,
},
bits: 512,
bits: 256,
policy: Policy {
usage_flags: UsageFlags {
sign_hash: false,
Expand Down
34 changes: 22 additions & 12 deletions e2e_tests/tests/per_provider/normal_tests/import_key.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Copyright 2019 Contributors to the Parsec project.
// SPDX-License-Identifier: Apache-2.
// SPDX-License-Identifier: Apache-2.0
#![allow(unused_imports)]
use e2e_tests::TestClient;
use parsec_client::core::interface::operations::psa_algorithm::*;
Expand Down Expand Up @@ -64,8 +64,8 @@ const KEY_PAIR_DATA: [u8; 609] = [
0x27,
];

#[cfg(feature = "cryptoauthlib-provider")]
const KEY_PAIR_ECC: [u8; 121] = [
#[cfg(any(feature = "mbed-crypto-provider", feature = "cryptoauthlib-provider"))]
const ECC_KEY_PAIR: [u8; 121] = [
0x30, 0x77, 0x02, 0x01, 0x01, 0x04, 0x20, 0x26, 0xc8, 0x82, 0x9e, 0x22, 0xe3, 0x0c, 0xa6, 0x3d,
0x29, 0xf5, 0xf7, 0x27, 0x39, 0x58, 0x47, 0x41, 0x81, 0xf6, 0x57, 0x4f, 0xdb, 0xcb, 0x4d, 0xbb,
0xdd, 0x52, 0xff, 0x3a, 0xc0, 0xf6, 0x0d, 0xa0, 0x0a, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d,
Expand All @@ -76,6 +76,16 @@ const KEY_PAIR_ECC: [u8; 121] = [
0xcc, 0xa8, 0xd0, 0x55, 0xee, 0x6c, 0x65, 0xb5, 0x71,
];

#[cfg(any(feature = "mbed-crypto-provider", feature = "cryptoauthlib-provider"))]
const ECC_PUBLIC_KEY: [u8; 65] = [
0x04,
0x01, 0xf7, 0x69, 0xe2, 0x40, 0x3a, 0xeb, 0x0d, 0x64, 0x3e, 0x81, 0xb8, 0xda, 0x95, 0xb0,
0x1c, 0x25, 0x80, 0xfe, 0xa3, 0xd3, 0xd0, 0x5b, 0x2f, 0xef, 0x6a, 0x31, 0x9c, 0xa9, 0xca,
0x5d, 0xe5, 0x2b, 0x4b, 0x49, 0x2c, 0x24, 0x2c, 0xef, 0xf4, 0xf2, 0x3c, 0xef, 0xfa, 0x08,
0xa7, 0xb4, 0xc6, 0xe0, 0xce, 0x73, 0xac, 0xd0, 0x69, 0xd4, 0xcc, 0xa8, 0xd0, 0x55, 0xee,
0x6c, 0x65, 0xb5, 0x71,
];

#[cfg(not(feature = "cryptoauthlib-provider"))]
fn example_modulus_1024() -> Vec<u8> {
vec![
Expand All @@ -101,7 +111,7 @@ fn import_rsa_key() -> Result<()> {
client.import_rsa_public_key(key_name, KEY_DATA.to_vec())
}

#[cfg(feature = "cryptoauthlib-provider")]
#[cfg(any(feature = "mbed-crypto-provider", feature = "cryptoauthlib-provider"))]
#[test]
fn import_ecc_key() -> Result<()> {
let mut client = TestClient::new();
Expand All @@ -110,14 +120,14 @@ fn import_ecc_key() -> Result<()> {
return Ok(());
}

client.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, KEY_PAIR_ECC.to_vec())
client.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, ECC_PUBLIC_KEY.to_vec())
}

#[cfg(not(feature = "cryptoauthlib-provider"))]
#[test]
fn create_and_import_rsa_key() -> Result<()> {
let mut client = TestClient::new();
let key_name = String::from("create_and_import_key");
let key_name = String::from("create_and_import_rsa_key");
if !client.is_operation_supported(Opcode::PsaImportKey) {
return Ok(());
}
Expand All @@ -131,18 +141,18 @@ fn create_and_import_rsa_key() -> Result<()> {
Ok(())
}

#[cfg(feature = "cryptoauthlib-provider")]
#[cfg(any(feature = "mbed-crypto-provider", feature = "cryptoauthlib-provider"))]
#[test]
fn create_and_import_ecc_key() -> Result<()> {
let mut client = TestClient::new();
let key_name = String::from("create_and_import_key");
let key_name = String::from("create_and_import_ecc_key");
if !client.is_operation_supported(Opcode::PsaImportKey) {
return Ok(());
}

client.generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone())?;
let status = client
.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, KEY_PAIR_ECC.to_vec())
.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, ECC_KEY_PAIR.to_vec())
Copy link
Member

Choose a reason for hiding this comment

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

I guess it makes no difference here as we check if the key already exists (so that the code does not go as far as parsing it), but I think it should be ECC_PUBLIC_KEY.

.expect_err("Key should have already existed");
assert_eq!(status, ResponseStatus::PsaErrorAlreadyExists);

Expand Down Expand Up @@ -177,9 +187,9 @@ fn import_ecc_key_twice() -> Result<()> {
return Ok(());
}

client.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name.clone(), KEY_PAIR_ECC.to_vec())?;
client.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name.clone(), ECC_PUBLIC_KEY.to_vec())?;
let status = client
.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, KEY_PAIR_ECC.to_vec())
.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, ECC_PUBLIC_KEY.to_vec())
.expect_err("The key with the same name has already been created.");

assert_eq!(status, ResponseStatus::PsaErrorAlreadyExists);
Expand Down Expand Up @@ -406,5 +416,5 @@ fn import_ecc_private_key() {
return;
Copy link
Member

Choose a reason for hiding this comment

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

Can you also execute this test on the Mbed Crypto provider 😄? That way we will be sure that both the formats for the public key and key pair are correct 👌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. And it looks like Mbed Crypto uses just the raw private key octet string from the whole structure.

}

client.import_ecc_pair_secp_r1_key(key_name, KEY_PAIR_ECC.to_vec()).unwrap();
client.import_ecc_pair_secp_r1_key(key_name, ECC_KEY_PAIR.to_vec()).unwrap();
}
10 changes: 2 additions & 8 deletions src/providers/cryptoauthlib/key_management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,11 @@ impl Provider {
Type::Aes => Ok(rust_cryptoauthlib::KeyType::Aes),
Type::EccKeyPair {
curve_family: EccFamily::SecpR1,
} => {
if attributes.bits == 256 || attributes.bits == 0 {
Ok(rust_cryptoauthlib::KeyType::P256EccKey)
} else {
Err(ResponseStatus::PsaErrorNotSupported)
}
}
Type::EccPublicKey {
| Type::EccPublicKey {
curve_family: EccFamily::SecpR1,
} => {
if attributes.bits == 512 || attributes.bits == 0 {
if attributes.bits == 256 || attributes.bits == 0 {
Ok(rust_cryptoauthlib::KeyType::P256EccKey)
} else {
Err(ResponseStatus::PsaErrorNotSupported)
Expand Down
75 changes: 27 additions & 48 deletions src/providers/cryptoauthlib/key_operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ use super::key_slot::KeySlotStatus;
use super::Provider;
use crate::authenticators::ApplicationName;
use log::{error, warn};
use openssl::bn::BigNumContext;
use openssl::ec::{EcGroup, EcKey, PointConversionForm};
use openssl::nid::Nid;
use openssl::ec::EcKey;
use parsec_interface::operations::psa_key_attributes::{EccFamily, Type};
use parsec_interface::operations::{psa_destroy_key, psa_generate_key, psa_import_key};
use parsec_interface::requests::{ResponseStatus, Result};
Expand Down Expand Up @@ -102,20 +100,22 @@ impl Provider {
Ok(x) => x,
Err(error) => return Err(error),
};

let slot_id = match self.find_suitable_slot(&key_attributes) {
Ok(slot) => slot,
Err(error) => {
warn!("Failed to find suitable storage slot for key. {}", error);
return Err(error);
}
};
// There is no need check op.data.len() - ler the extraction functions do this.
let key_data = match extract_raw_key(key_attributes.key_type, &op.data) {
Ok(raw_key) => raw_key,
Err(error) => return Err(error),
};

let atca_error_status = self.device.import_key(key_type, &key_data, slot_id);
let atca_error_status =
self.device
.import_key(key_type, &key_data.expose_secret(), slot_id);

hug-dev marked this conversation as resolved.
Show resolved Hide resolved
let psa_error_status: ResponseStatus = match atca_error_status {
rust_cryptoauthlib::AtcaStatus::AtcaSuccess => {
Expand Down Expand Up @@ -151,62 +151,39 @@ impl Provider {
// Import failed but at least slot was appropriately marked as Free
}
Err(error) => {
// Things do never get better...
// Things never get better...
Copy link
Member

Choose a reason for hiding this comment

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

Some improvement there 😄

error!("Storage slot status failed to update becuase {}", error);
}
};
Err(psa_error_status)
}
}

// TODO: use picky-asn1-x509 when support for ECC keys becomes available there
fn extract_raw_key(key_type: Type, secret: &Secret<Vec<u8>>) -> Result<Vec<u8>> {
let key = secret.expose_secret();
fn extract_raw_key(key_type: Type, secret: &Secret<Vec<u8>>) -> Result<Secret<Vec<u8>>> {
let mut key = secret.expose_secret().to_vec();

match key_type {
Type::Aes => Ok(key.to_vec()),
Type::Aes | Type::RawData => Ok(Secret::new(key)),
Type::EccKeyPair {
curve_family: EccFamily::SecpR1,
} => {
match EcKey::private_key_from_der(&key) {
// Assumption - if key cannot be extracted, the argument (here: key) is invalid.
Err(_) => Err(ResponseStatus::PsaErrorInvalidArgument),
// The key was successfully extracted from DER encoded string
Ok(ec_key) => Ok(ec_key.private_key().to_vec()),
Ok(ec_key) => Ok(Secret::new(ec_key.private_key().to_vec())),
}
}
Type::EccPublicKey {
curve_family: EccFamily::SecpR1,
} => {
// Strange - why private_key_from_der()?
// As long as it works - who cares?
let ec_key_wrapped = match EcKey::private_key_from_der(&key) {
// Assumption - if key cannot be extracted, the argument (here: key) is invalid.
Err(_) => return Err(ResponseStatus::PsaErrorInvalidArgument),
// The key was successfully extracted from DER encoded string
Ok(ec_key_ref) => {
let mut ctx = match BigNumContext::new() {
Ok(bn_context) => bn_context,
Err(_) => return Err(ResponseStatus::PsaErrorInsufficientMemory),
};
ec_key_ref.public_key().to_bytes(
&EcGroup::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(),
PointConversionForm::UNCOMPRESSED,
&mut ctx,
)
}
};
match ec_key_wrapped {
Ok(mut result) => {
// There is this strange 0x04 byte extracted by OpenSSL at the beginning of
// public key. Encapsulation of octet- into bitstring?
// Remove it.
let raw_public_key: Vec<_> = result.drain(1..).collect();
Ok(raw_public_key)
}
Err(_) => Err(ResponseStatus::PsaErrorInvalidArgument),
} => match key.len() {
// 512+8 bits == 64+1 octets
65 => {
let raw_public_key: Vec<_> = key.drain(1..).collect();
Ok(Secret::new(raw_public_key))
}
}
_ => Err(ResponseStatus::PsaErrorInvalidArgument),
},
_ => Err(ResponseStatus::PsaErrorNotSupported),
}
}
Expand Down Expand Up @@ -239,17 +216,16 @@ fn test_extract_raw_ecc_private_key() {
&ecc_key_der_array,
)
.unwrap();
assert_eq!(ecc_priv_key.to_vec(), ecc_priv_key_ext);
assert_eq!(
ecc_priv_key.to_vec(),
ecc_priv_key_ext.expose_secret().to_owned()
);
}

#[test]
fn test_extract_raw_ecc_public_key() {
let ecc_key_der_array: Secret<Vec<u8>> = Secret::new(
let public_ecc_key_array: Secret<Vec<u8>> = Secret::new(
[
0x30, 0x77, 0x02, 0x01, 0x01, 0x04, 0x20, 0x26, 0xc8, 0x82, 0x9e, 0x22, 0xe3, 0x0c,
0xa6, 0x3d, 0x29, 0xf5, 0xf7, 0x27, 0x39, 0x58, 0x47, 0x41, 0x81, 0xf6, 0x57, 0x4f,
0xdb, 0xcb, 0x4d, 0xbb, 0xdd, 0x52, 0xff, 0x3a, 0xc0, 0xf6, 0x0d, 0xa0, 0x0a, 0x06,
0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07, 0xa1, 0x44, 0x03, 0x42, 0x00,
0x04, 0x01, 0xf7, 0x69, 0xe2, 0x40, 0x3a, 0xeb, 0x0d, 0x64, 0x3e, 0x81, 0xb8, 0xda,
0x95, 0xb0, 0x1c, 0x25, 0x80, 0xfe, 0xa3, 0xd3, 0xd0, 0x5b, 0x2f, 0xef, 0x6a, 0x31,
0x9c, 0xa9, 0xca, 0x5d, 0xe5, 0x2b, 0x4b, 0x49, 0x2c, 0x24, 0x2c, 0xef, 0xf4, 0xf2,
Expand All @@ -270,8 +246,11 @@ fn test_extract_raw_ecc_public_key() {
Type::EccPublicKey {
curve_family: EccFamily::SecpR1,
},
&ecc_key_der_array,
&public_ecc_key_array,
)
.unwrap();
assert_eq!(ecc_pub_key.to_vec(), ecc_pub_key_ext);
assert_eq!(
ecc_pub_key.to_vec(),
ecc_pub_key_ext.expose_secret().to_owned()
);
}
5 changes: 3 additions & 2 deletions src/providers/cryptoauthlib/key_slot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,10 @@ impl AteccKeySlot {
Type::EccPublicKey {
curve_family: EccFamily::SecpR1,
} => {
// The uncompressed public key is 512 bits (64 bytes). 0 means - do not care.
// The uncompressed public key is 512 bits (64 bytes).
// But this is a length of a private key.
// First few (7) slots are too short for ECC public key.
(key_attr.bits == 0 || key_attr.bits == 512)
(key_attr.bits == 0 || key_attr.bits == 256)
&& self.config.key_type == rust_cryptoauthlib::KeyType::P256EccKey
&& slot >= rust_cryptoauthlib::ATCA_ATECC_MIN_SLOT_IDX_FOR_PUB_KEY
}
Expand Down