From ead689f26286314bb1bb5d71e7b487acd6f3dfe4 Mon Sep 17 00:00:00 2001 From: Robert Drazkowski Date: Tue, 20 Apr 2021 11:30:12 +0200 Subject: [PATCH 1/6] PSA_IMPORT_KEY introduction, together with accompanying unit tests and e2e-tests. Signed-off-by: Robert Drazkowski --- .../cryptoauthlib/config_608a.toml | 33 ++++++++ e2e_tests/src/lib.rs | 38 +++++++++ .../normal_tests/asym_sign_verify.rs | 25 +++--- .../tests/per_provider/normal_tests/auth.rs | 3 +- .../normal_tests/create_destroy_key.rs | 24 ++++-- .../normal_tests/export_public_key.rs | 2 +- .../per_provider/normal_tests/import_key.rs | 84 ++++++++++++++++--- src/providers/cryptoauthlib/key_management.rs | 10 ++- src/providers/cryptoauthlib/key_operations.rs | 81 ++++++++++++++++-- src/providers/cryptoauthlib/key_slot.rs | 65 +++++++++----- .../cryptoauthlib/key_slot_storage.rs | 15 ++-- src/providers/cryptoauthlib/mod.rs | 33 +++++++- 12 files changed, 343 insertions(+), 70 deletions(-) create mode 100644 e2e_tests/provider_cfg/cryptoauthlib/config_608a.toml diff --git a/e2e_tests/provider_cfg/cryptoauthlib/config_608a.toml b/e2e_tests/provider_cfg/cryptoauthlib/config_608a.toml new file mode 100644 index 00000000..bbe0ea4a --- /dev/null +++ b/e2e_tests/provider_cfg/cryptoauthlib/config_608a.toml @@ -0,0 +1,33 @@ +[core_settings] +# The CI already timestamps the logs +log_timestamp = false +log_error_details = true + +# The container runs the Parsec service as root, so make sure we disable root +# checks. +allow_root = true + +[listener] +listener_type = "DomainSocket" +timeout = 200 # in milliseconds +socket_path = "/tmp/parsec.sock" + +[authenticator] +auth_type = "Direct" + +[[key_manager]] +name = "on-disk-manager" +manager_type = "OnDisk" +store_path = "./mappings" + +[[provider]] +provider_type = "CryptoAuthLib" +key_info_manager = "on-disk-manager" +device_type = "atecc608a" +iface_type = "i2c" +wake_delay = 1600 +rx_retries = 20 +# i2c parameters for i2c-pseudo proxy +slave_address = 0xc0 +bus = 1 +baud = 400000 diff --git a/e2e_tests/src/lib.rs b/e2e_tests/src/lib.rs index 8e120c66..ae211ff1 100644 --- a/e2e_tests/src/lib.rs +++ b/e2e_tests/src/lib.rs @@ -596,6 +596,44 @@ impl TestClient { self.import_key(key_name, attributes, data) } + /// Import a 256 bit ECC public key. + /// The key can only be used for verifying with the Ecdsa signing algorithm with SHA-256. + pub fn import_ecc_public_secp_r1_ecdsa_sha256_key( + &mut self, + key_name: String, + data: Vec, + ) -> Result<()> { + self.import_key( + key_name, + Attributes { + lifetime: Lifetime::Persistent, + key_type: Type::EccPublicKey { + curve_family: EccFamily::SecpR1, + }, + bits: 512, + policy: Policy { + usage_flags: UsageFlags { + sign_hash: false, + verify_hash: true, + sign_message: false, + verify_message: true, + export: false, + encrypt: false, + decrypt: false, + cache: false, + copy: false, + derive: false, + }, + permitted_algorithms: AsymmetricSignature::Ecdsa { + hash_alg: Hash::Sha256.into(), + } + .into(), + }, + }, + data, + ) + } + /// Exports a key pub fn export_key(&mut self, key_name: String) -> Result> { self.basic_client diff --git a/e2e_tests/tests/per_provider/normal_tests/asym_sign_verify.rs b/e2e_tests/tests/per_provider/normal_tests/asym_sign_verify.rs index 7f92a462..3657a602 100644 --- a/e2e_tests/tests/per_provider/normal_tests/asym_sign_verify.rs +++ b/e2e_tests/tests/per_provider/normal_tests/asym_sign_verify.rs @@ -3,7 +3,7 @@ use e2e_tests::TestClient; use parsec_client::core::interface::operations::psa_algorithm::*; use parsec_client::core::interface::operations::psa_key_attributes::*; -use parsec_client::core::interface::requests::{ Opcode, ResponseStatus, Result}; +use parsec_client::core::interface::requests::{Opcode, ResponseStatus, Result}; #[cfg(any(feature = "mbed-crypto-provider", feature = "tpm-provider"))] use ring::signature::{self, UnparsedPublicKey}; use rsa::{PaddingScheme, PublicKey, RSAPublicKey}; @@ -95,7 +95,6 @@ fn only_verify_from_internet() -> Result<()> { if !client.is_operation_supported(Opcode::PsaVerifyHash) { return Ok(()); } - // "Les carottes sont cuites." hashed with SHA256 let digest = vec![ 0x02, 0x2b, 0x26, 0xb1, 0xc3, 0x18, 0xdb, 0x73, 0x36, 0xef, 0x6f, 0x50, 0x9c, 0x35, 0xdd, @@ -141,9 +140,6 @@ fn only_verify_from_internet() -> Result<()> { fn simple_sign_hash() -> Result<()> { let key_name = String::from("simple_sign_hash"); let mut client = TestClient::new(); - let mut hasher = Sha256::new(); - hasher.update(b"Bob wrote this message."); - let hash = hasher.finalize().to_vec(); if !client.is_operation_supported(Opcode::PsaGenerateKey) { return Ok(()); } @@ -151,6 +147,10 @@ fn simple_sign_hash() -> Result<()> { return Ok(()); } + let mut hasher = Sha256::new(); + hasher.update(b"Bob wrote this message."); + let hash = hasher.finalize().to_vec(); + client.generate_rsa_sign_key(key_name.clone())?; let _ = client.sign_with_rsa_sha256(key_name, hash)?; @@ -162,10 +162,6 @@ fn simple_sign_hash() -> Result<()> { fn sign_hash_not_permitted() -> Result<()> { let key_name = String::from("sign_hash_not_permitted"); let mut client = TestClient::new(); - let mut hasher = Sha256::new(); - hasher.update(b"Bob wrote this message."); - let hash = hasher.finalize().to_vec(); - if !client.is_operation_supported(Opcode::PsaGenerateKey) { return Ok(()); } @@ -173,6 +169,10 @@ fn sign_hash_not_permitted() -> Result<()> { return Ok(()); } + let mut hasher = Sha256::new(); + hasher.update(b"Bob wrote this message."); + let hash = hasher.finalize().to_vec(); + let attributes = Attributes { lifetime: Lifetime::Persistent, key_type: Type::RsaKeyPair, @@ -211,9 +211,6 @@ fn sign_hash_not_permitted() -> Result<()> { fn sign_hash_bad_format() -> Result<()> { let key_name = String::from("sign_hash_bad_format"); let mut client = TestClient::new(); - let hash1 = vec![0xEE; 31]; - let hash2 = vec![0xBB; 33]; - if !client.is_operation_supported(Opcode::PsaGenerateKey) { return Ok(()); } @@ -221,6 +218,9 @@ fn sign_hash_bad_format() -> Result<()> { return Ok(()); } + let hash1 = vec![0xEE; 31]; + let hash2 = vec![0xBB; 33]; + client.generate_rsa_sign_key(key_name.clone())?; let status1 = client @@ -433,6 +433,7 @@ fn asym_verify_with_rsa_crate() { fn verify_with_ring() { let key_name = String::from("verify_with_ring"); let mut client = TestClient::new(); + let message = b"Bob wrote this message."; client.generate_long_rsa_sign_key(key_name.clone()).unwrap(); diff --git a/e2e_tests/tests/per_provider/normal_tests/auth.rs b/e2e_tests/tests/per_provider/normal_tests/auth.rs index 48afe8e2..3f1c9a0b 100644 --- a/e2e_tests/tests/per_provider/normal_tests/auth.rs +++ b/e2e_tests/tests/per_provider/normal_tests/auth.rs @@ -1,8 +1,7 @@ // Copyright 2019 Contributors to the Parsec project. // SPDX-License-Identifier: Apache-2.0 use e2e_tests::TestClient; -use parsec_client::core::interface::requests::{Opcode, ResponseStatus}; -use parsec_client::core::interface::requests::Result; +use parsec_client::core::interface::requests::{Opcode, Result, ResponseStatus}; #[test] fn two_auths_same_key_name() -> Result<()> { diff --git a/e2e_tests/tests/per_provider/normal_tests/create_destroy_key.rs b/e2e_tests/tests/per_provider/normal_tests/create_destroy_key.rs index 48f6d279..0da86b1c 100644 --- a/e2e_tests/tests/per_provider/normal_tests/create_destroy_key.rs +++ b/e2e_tests/tests/per_provider/normal_tests/create_destroy_key.rs @@ -18,7 +18,9 @@ fn create_and_destroy() { #[cfg(not(feature = "cryptoauthlib-provider"))] client.generate_rsa_sign_key(key_name.clone()).unwrap(); #[cfg(feature = "cryptoauthlib-provider")] - client.generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone()).unwrap(); + client + .generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone()) + .unwrap(); client.destroy_key(key_name).unwrap(); } @@ -41,7 +43,9 @@ fn create_twice() { } #[cfg(feature = "cryptoauthlib-provider")] { - client.generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone()).unwrap(); + client + .generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone()) + .unwrap(); let status = client .generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name) .expect_err("A key with the same name can not be created twice."); @@ -75,7 +79,9 @@ fn create_destroy_and_operation() { #[cfg(not(feature = "cryptoauthlib-provider"))] client.generate_rsa_sign_key(key_name.clone()).unwrap(); #[cfg(feature = "cryptoauthlib-provider")] - client.generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone()).unwrap(); + client + .generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone()) + .unwrap(); client.destroy_key(key_name.clone()).unwrap(); @@ -102,8 +108,12 @@ fn create_destroy_twice() { } #[cfg(feature = "cryptoauthlib-provider")] { - client.generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone()).unwrap(); - client.generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name_2.clone()).unwrap(); + client + .generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone()) + .unwrap(); + client + .generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name_2.clone()) + .unwrap(); } client.destroy_key(key_name).unwrap(); @@ -125,7 +135,9 @@ fn generate_public_rsa_check_modulus() { #[cfg(not(feature = "cryptoauthlib-provider"))] client.generate_rsa_sign_key(key_name.clone()).unwrap(); #[cfg(feature = "cryptoauthlib-provider")] - client.generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone()).unwrap(); + client + .generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone()) + .unwrap(); let public_key = client.export_public_key(key_name).unwrap(); diff --git a/e2e_tests/tests/per_provider/normal_tests/export_public_key.rs b/e2e_tests/tests/per_provider/normal_tests/export_public_key.rs index b2a8a294..68b6c4cf 100644 --- a/e2e_tests/tests/per_provider/normal_tests/export_public_key.rs +++ b/e2e_tests/tests/per_provider/normal_tests/export_public_key.rs @@ -3,9 +3,9 @@ use e2e_tests::TestClient; use parsec_client::core::interface::operations::psa_algorithm::*; use parsec_client::core::interface::operations::psa_key_attributes::*; +use parsec_client::core::interface::requests::Opcode; use parsec_client::core::interface::requests::ResponseStatus; use parsec_client::core::interface::requests::Result; -use parsec_client::core::interface::requests::Opcode; use picky_asn1_x509::RSAPublicKey; #[test] diff --git a/e2e_tests/tests/per_provider/normal_tests/import_key.rs b/e2e_tests/tests/per_provider/normal_tests/import_key.rs index 53558534..2689a7ae 100644 --- a/e2e_tests/tests/per_provider/normal_tests/import_key.rs +++ b/e2e_tests/tests/per_provider/normal_tests/import_key.rs @@ -1,14 +1,19 @@ // Copyright 2019 Contributors to the Parsec project. // SPDX-License-Identifier: Apache-2.0 use e2e_tests::TestClient; +#[allow(unused_imports)] use parsec_client::core::interface::operations::psa_algorithm::*; +#[allow(unused_imports)] use parsec_client::core::interface::operations::psa_key_attributes::*; +use parsec_client::core::interface::requests::Opcode; use parsec_client::core::interface::requests::ResponseStatus; use parsec_client::core::interface::requests::Result; -use parsec_client::core::interface::requests::Opcode; +#[allow(unused_imports)] use picky_asn1::wrapper::IntegerAsn1; +#[allow(unused_imports)] use picky_asn1_x509::RSAPublicKey; +#[cfg(not(feature = "cryptoauthlib-provider"))] const KEY_DATA: [u8; 140] = [ 48, 129, 137, 2, 129, 129, 0, 153, 165, 220, 135, 89, 101, 254, 229, 28, 33, 138, 247, 20, 102, 253, 217, 247, 246, 142, 107, 51, 40, 179, 149, 45, 117, 254, 236, 161, 109, 16, 81, 135, 72, @@ -62,6 +67,21 @@ const KEY_PAIR_DATA: [u8; 609] = [ 0x27, ]; +#[cfg(feature = "cryptoauthlib-provider")] +//#[allow(dead_code)] +const PRIV_KEY_ECC: [u8; 32] = [ + 0xF5, 0xDB, 0x6B, 0xA1, 0x82, 0x22, 0xCE, 0xC1, 0x54, 0x53, 0xE5, 0x63, 0xDE, 0xC5, 0xC7, 0x94, + 0xCD, 0x48, 0x95, 0xF2, 0x8C, 0xC2, 0x7F, 0x50, 0xC2, 0x7E, 0xC3, 0x1B, 0xAF, 0x44, 0xEA, 0x54, +]; +#[cfg(feature = "cryptoauthlib-provider")] +const PUB_KEY_ECC: [u8; 64] = [ + 0xBA, 0x6A, 0xB5, 0xF1, 0x19, 0xAF, 0x21, 0x73, 0x03, 0x75, 0xD1, 0x8D, 0x6B, 0x5F, 0xF1, 0x94, + 0x33, 0xE5, 0x3A, 0xEE, 0x5F, 0x6F, 0xBA, 0x22, 0x97, 0x77, 0x13, 0xEA, 0x82, 0xD3, 0x74, 0x84, + 0x8E, 0x39, 0x78, 0x66, 0xE8, 0x36, 0xB3, 0xFE, 0xD3, 0x22, 0x87, 0x74, 0xA5, 0x00, 0xC5, 0x5C, + 0x17, 0x73, 0x5A, 0x92, 0x4B, 0xB3, 0x9F, 0xE4, 0x98, 0x52, 0x62, 0xA5, 0x36, 0xC5, 0x00, 0x9C, +]; + +#[cfg(not(feature = "cryptoauthlib-provider"))] fn example_modulus_1024() -> Vec { vec![ 153, 165, 220, 135, 89, 101, 254, 229, 28, 33, 138, 247, 20, 102, 253, 217, 247, 246, 142, @@ -81,7 +101,13 @@ fn import_key() -> Result<()> { if !client.is_operation_supported(Opcode::PsaImportKey) { return Ok(()); } - client.import_rsa_public_key(key_name, KEY_DATA.to_vec()) + + #[cfg(not(feature = "cryptoauthlib-provider"))] + let result = client.import_rsa_public_key(key_name, KEY_DATA.to_vec()); + #[cfg(feature = "cryptoauthlib-provider")] + let result = client.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, PUB_KEY_ECC.to_vec()); + + result } #[test] @@ -92,10 +118,21 @@ fn create_and_import_key() -> Result<()> { return Ok(()); } - client.generate_rsa_sign_key(key_name.clone())?; - let status = client - .import_rsa_public_key(key_name, KEY_DATA.to_vec()) - .expect_err("Key should have already existed"); + let status; + #[cfg(not(feature = "cryptoauthlib-provider"))] + { + client.generate_rsa_sign_key(key_name.clone())?; + status = client + .import_rsa_public_key(key_name, KEY_DATA.to_vec()) + .expect_err("Key should have already existed"); + } + #[cfg(feature = "cryptoauthlib-provider")] + { + client.generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone())?; + status = client + .import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, PUB_KEY_ECC.to_vec()) + .expect_err("Key should have already existed"); + } assert_eq!(status, ResponseStatus::PsaErrorAlreadyExists); Ok(()) @@ -109,15 +146,27 @@ fn import_key_twice() -> Result<()> { return Ok(()); } - client.import_rsa_public_key(key_name.clone(), KEY_DATA.to_vec())?; - let status = client - .import_rsa_public_key(key_name, KEY_DATA.to_vec()) - .expect_err("The key with the same name has already been created."); + let status; + #[cfg(not(feature = "cryptoauthlib-provider"))] + { + client.import_rsa_public_key(key_name.clone(), KEY_DATA.to_vec())?; + status = client + .import_rsa_public_key(key_name, KEY_DATA.to_vec()) + .expect_err("The key with the same name has already been created."); + } + #[cfg(feature = "cryptoauthlib-provider")] + { + client.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name.clone(), PUB_KEY_ECC.to_vec())?; + status = client + .import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, PUB_KEY_ECC.to_vec()) + .expect_err("The key with the same name has already been created."); + } assert_eq!(status, ResponseStatus::PsaErrorAlreadyExists); Ok(()) } +#[cfg(not(feature = "cryptoauthlib-provider"))] #[test] fn check_format_import1() -> Result<()> { let mut client = TestClient::new(); @@ -136,6 +185,7 @@ fn check_format_import1() -> Result<()> { Ok(()) } +#[cfg(not(feature = "cryptoauthlib-provider"))] #[test] fn check_format_import2() -> Result<()> { // If the bits field of the key attributes is zero, the operation should still work. @@ -185,6 +235,7 @@ fn check_format_import2() -> Result<()> { Ok(()) } +#[cfg(not(feature = "cryptoauthlib-provider"))] #[test] fn check_format_import3() -> Result<()> { // If the bits field of the key attributes is different that the size of the key parsed @@ -238,6 +289,7 @@ fn check_format_import3() -> Result<()> { Ok(()) } +#[cfg(not(feature = "cryptoauthlib-provider"))] #[test] fn failed_imported_key_should_be_removed() -> Result<()> { let mut client = TestClient::new(); @@ -323,3 +375,15 @@ fn import_key_pair() { ) .unwrap(); } + +#[cfg(feature = "cryptoauthlib-provider")] +#[test] +fn import_ecc_private_key() { + let mut client = TestClient::new(); + let key_name = String::from("import_ecc_private_key"); + if !client.is_operation_supported(Opcode::PsaImportKey) { + return; + } + + client.import_ecc_pair_secp_r1_key(key_name, PRIV_KEY_ECC.to_vec()).unwrap(); +} diff --git a/src/providers/cryptoauthlib/key_management.rs b/src/providers/cryptoauthlib/key_management.rs index c713c9c0..0e26cdeb 100644 --- a/src/providers/cryptoauthlib/key_management.rs +++ b/src/providers/cryptoauthlib/key_management.rs @@ -30,11 +30,17 @@ impl Provider { Type::Aes => Ok(rust_cryptoauthlib::KeyType::Aes), Type::EccKeyPair { curve_family: EccFamily::SecpR1, + } => { + if attributes.bits == 256 { + Ok(rust_cryptoauthlib::KeyType::P256EccKey) + } else { + Err(ResponseStatus::PsaErrorNotSupported) + } } - | Type::EccPublicKey { + Type::EccPublicKey { curve_family: EccFamily::SecpR1, } => { - if attributes.bits == 256 { + if attributes.bits == 512 { Ok(rust_cryptoauthlib::KeyType::P256EccKey) } else { Err(ResponseStatus::PsaErrorNotSupported) diff --git a/src/providers/cryptoauthlib/key_operations.rs b/src/providers/cryptoauthlib/key_operations.rs index f9f692ec..23a7755a 100644 --- a/src/providers/cryptoauthlib/key_operations.rs +++ b/src/providers/cryptoauthlib/key_operations.rs @@ -4,8 +4,9 @@ use super::key_slot::KeySlotStatus; use super::Provider; use crate::authenticators::ApplicationName; use log::{error, warn}; -use parsec_interface::operations::{psa_destroy_key, psa_generate_key}; +use parsec_interface::operations::{psa_destroy_key, psa_generate_key, psa_import_key}; use parsec_interface::requests::{ResponseStatus, Result}; +use parsec_interface::secrecy::ExposeSecret; impl Provider { pub(super) fn psa_generate_key_internal( @@ -64,16 +65,14 @@ impl Provider { ) -> Result { let key_name = op.key_name; let key_triple = self.key_info_store.get_key_triple(app_name, key_name); + let key_id = self.key_info_store.get_key_id::(&key_triple)?; match self.key_info_store.remove_key_info(&key_triple) { - Ok(key_info) => { - match self.set_slot_status(key_info.id[0] as usize, KeySlotStatus::Free) { + Ok(_) => { + match self.set_slot_status(key_id as usize, KeySlotStatus::Free) { Ok(()) => (), Err(error) => { - warn!( - "Could not set slot {:?} as free because {}", - key_info.id[0], error, - ); + warn!("Could not set slot {:?} as free because {}", key_id, error,); } } Ok(psa_destroy_key::Result {}) @@ -84,4 +83,72 @@ impl Provider { } } } + + pub(super) fn psa_import_key_internal( + &self, + app_name: ApplicationName, + op: psa_import_key::Operation, + ) -> Result { + let key_name = op.key_name; + let key_triple = self.key_info_store.get_key_triple(app_name, key_name); + self.key_info_store.does_not_exist(&key_triple)?; + + let key_attributes = op.attributes; + let key_type = match Provider::get_calib_key_type(&key_attributes) { + 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); + } + }; + let key_data_r = op.data.expose_secret(); + + let status = self.device.import_key(key_type, key_data_r, slot_id); + let error_status: rust_cryptoauthlib::AtcaStatus; + match status { + rust_cryptoauthlib::AtcaStatus::AtcaSuccess => { + match self + .key_info_store + .insert_key_info(key_triple, &slot_id, key_attributes) + { + Ok(()) => { + return Ok(psa_import_key::Result {}); + } + Err(error) => { + warn!("Insert key triple to KeyInfoManager failed. {}", error); + return Err(error); + } + } + } + x @ rust_cryptoauthlib::AtcaStatus::AtcaInvalidSize + | x @ rust_cryptoauthlib::AtcaStatus::AtcaInvalidId + | x @ rust_cryptoauthlib::AtcaStatus::AtcaBadParam => { + error_status = x; + } + _ => { + error_status = rust_cryptoauthlib::AtcaStatus::AtcaUnimplemented; + } + } + match self.set_slot_status(slot_id as usize, KeySlotStatus::Free) { + Ok(()) => { + let error = ResponseStatus::PsaErrorInvalidArgument; + error!( + "Key import failed. {}. Storage slot status updated.", + error_status + ); + Err(error) + } + Err(error) => { + error!( + "Key import failed. {}. Storage slot status failed to update.", + error_status + ); + Err(error) + } + } + } } diff --git a/src/providers/cryptoauthlib/key_slot.rs b/src/providers/cryptoauthlib/key_slot.rs index a34d3a5f..fad8d8cd 100644 --- a/src/providers/cryptoauthlib/key_slot.rs +++ b/src/providers/cryptoauthlib/key_slot.rs @@ -41,12 +41,16 @@ impl Default for AteccKeySlot { impl AteccKeySlot { // Check if software key attributes are compatible with hardware slot configuration - pub fn key_attr_vs_config(&self, key_attr: &Attributes) -> Result<(), ResponseStatus> { + pub fn key_attr_vs_config( + &self, + slot: u8, + key_attr: &Attributes, + ) -> Result<(), ResponseStatus> { // (1) Check attributes.key_type - if !self.is_key_type_ok(key_attr) { + if !self.is_key_type_ok(slot, key_attr) { return Err(ResponseStatus::PsaErrorNotSupported); } - // (2) Check attributes.policy.usage_flags + // (2) Check attributes.policy.usage_flags and slot number if !self.is_usage_flags_ok(key_attr) { return Err(ResponseStatus::PsaErrorNotSupported); } @@ -54,7 +58,6 @@ impl AteccKeySlot { if !self.is_permitted_algorithms_ok(key_attr) { return Err(ResponseStatus::PsaErrorNotSupported); } - Ok(()) } @@ -70,21 +73,32 @@ impl AteccKeySlot { Ok(()) } - fn is_key_type_ok(&self, key_attr: &Attributes) -> bool { + fn is_key_type_ok(&self, slot: u8, key_attr: &Attributes) -> bool { match key_attr.key_type { Type::RawData => self.config.key_type == rust_cryptoauthlib::KeyType::ShaOrText, Type::Hmac => !self.config.no_mac, Type::Aes => self.config.key_type == rust_cryptoauthlib::KeyType::Aes, Type::EccKeyPair { curve_family: EccFamily::SecpR1, + } => { + // P256 private key has 256 bits (32 bytes). + // Only private key is stored - public one can be computed when needed. + // The private key can onlly be stored encrypted and the encryption key must be set, + // see set_write_encryption_key() call in new(). + key_attr.bits == 256 + && self.config.key_type == rust_cryptoauthlib::KeyType::P256EccKey + && self.config.write_config == rust_cryptoauthlib::WriteConfig::Encrypt + && self.config.ecc_key_attr.is_private + && self.config.is_secret } - | Type::EccPublicKey { + Type::EccPublicKey { curve_family: EccFamily::SecpR1, } => { - // There may be a problem here: P256 private key has 256 bits (32 bytes), - // but the uncompressed public key is 512 bits (64 bytes) - key_attr.bits == 256 + // The uncompressed public key is 512 bits (64 bytes). + // First few (7) slots are too short for ECC public key. + key_attr.bits == 512 && self.config.key_type == rust_cryptoauthlib::KeyType::P256EccKey + && slot >= rust_cryptoauthlib::ATCA_ATECC_MIN_SLOT_IDX_FOR_PUB_KEY } Type::Derive | Type::DhKeyPair { .. } | Type::DhPublicKey { .. } => { // This may change... @@ -173,9 +187,7 @@ impl AteccKeySlot { Algorithm::AsymmetricSignature(AsymmetricSignature::Ecdsa { hash_alg: SignHash::Specific(Hash::Sha256), }) => { - self.config.is_secret - && self.config.key_type == rust_cryptoauthlib::KeyType::P256EccKey - && self.config.ecc_key_attr.is_private + self.config.key_type == rust_cryptoauthlib::KeyType::P256EccKey // TODO: what is external or internal hashing? } Algorithm::AsymmetricSignature(AsymmetricSignature::DeterministicEcdsa { @@ -186,7 +198,8 @@ impl AteccKeySlot { } // AsymmetricEncryption Algorithm::AsymmetricEncryption(..) => { - // why only RSA? it could work with ECC... + // Why only RSA? it could work with ECC... + // It could not - no suuport for ECC encryption in ATECC. false } // KeyAgreement @@ -287,14 +300,20 @@ mod tests { }, }; // KeyType::P256EccKey - // Type::EccKeyPair => OK - assert_eq!(key_slot.is_key_type_ok(&attributes), true); + // Type::EccKeyPair => NOK + assert_eq!(key_slot.is_key_type_ok(0, &attributes), false); + // private key attrs => OK + key_slot.config.key_type = rust_cryptoauthlib::KeyType::P256EccKey; + key_slot.config.write_config = rust_cryptoauthlib::WriteConfig::Encrypt; + key_slot.config.is_secret = true; + key_slot.config.ecc_key_attr.is_private = true; + assert_eq!(key_slot.is_key_type_ok(0, &attributes), true); // Type::Aes => NOK attributes.key_type = Type::Aes; - assert_eq!(key_slot.is_key_type_ok(&attributes), false); + assert_eq!(key_slot.is_key_type_ok(0, &attributes), false); // Type::RawData => NOK attributes.key_type = Type::RawData; - assert_eq!(key_slot.is_key_type_ok(&attributes), false); + assert_eq!(key_slot.is_key_type_ok(0, &attributes), false); // KeyType::Aes // Type::EccKeyPair => NOK @@ -302,13 +321,13 @@ mod tests { attributes.key_type = Type::EccKeyPair { curve_family: EccFamily::SecpR1, }; - assert_eq!(key_slot.is_key_type_ok(&attributes), false); + assert_eq!(key_slot.is_key_type_ok(0, &attributes), false); // Type::Aes => OK attributes.key_type = Type::Aes; - assert_eq!(key_slot.is_key_type_ok(&attributes), true); + assert_eq!(key_slot.is_key_type_ok(0, &attributes), true); // Type::RawData => NOK attributes.key_type = Type::RawData; - assert_eq!(key_slot.is_key_type_ok(&attributes), false); + assert_eq!(key_slot.is_key_type_ok(0, &attributes), false); // KeyType::ShaOrText // Type::EccKeyPair => NOK @@ -316,13 +335,13 @@ mod tests { attributes.key_type = Type::EccKeyPair { curve_family: EccFamily::SecpR1, }; - assert_eq!(key_slot.is_key_type_ok(&attributes), false); + assert_eq!(key_slot.is_key_type_ok(0, &attributes), false); // Type::Aes => NOK attributes.key_type = Type::Aes; - assert_eq!(key_slot.is_key_type_ok(&attributes), false); + assert_eq!(key_slot.is_key_type_ok(0, &attributes), false); // Type::RawData => OK attributes.key_type = Type::RawData; - assert_eq!(key_slot.is_key_type_ok(&attributes), true); + assert_eq!(key_slot.is_key_type_ok(0, &attributes), true); } #[test] diff --git a/src/providers/cryptoauthlib/key_slot_storage.rs b/src/providers/cryptoauthlib/key_slot_storage.rs index f3836f7e..4f462167 100644 --- a/src/providers/cryptoauthlib/key_slot_storage.rs +++ b/src/providers/cryptoauthlib/key_slot_storage.rs @@ -33,7 +33,7 @@ impl KeySlotStorage { // (2) if there are no two key triples mapping to a single ATECC slot - warning only ATM // check (1) - match key_slots[key_id as usize].key_attr_vs_config(&key_attr) { + match key_slots[key_id as usize].key_attr_vs_config(key_id, &key_attr) { Ok(_) => (), Err(err) => { let error = std::format!("ATECC slot configuration mismatch: {}", err); @@ -48,6 +48,7 @@ impl KeySlotStorage { return Ok(Some(warning)); } }; + // Slot 'key_id' validated - trying to mark it busy match key_slots[key_id as usize].set_slot_status(KeySlotStatus::Busy) { Ok(()) => Ok(None), Err(err) => { @@ -97,11 +98,13 @@ impl KeySlotStorage { if !key_slots[slot as usize].is_free() { continue; } - match key_slots[slot as usize].key_attr_vs_config(key_attr) { - Ok(_) => match key_slots[slot as usize].set_slot_status(KeySlotStatus::Busy) { - Ok(()) => return Ok(slot), - Err(err) => return Err(err), - }, + match key_slots[slot as usize].key_attr_vs_config(slot, key_attr) { + Ok(_) => { + match key_slots[slot as usize].set_slot_status(KeySlotStatus::Busy) { + Ok(()) => return Ok(slot), + Err(err) => return Err(err), + }; + } Err(_) => continue, } } diff --git a/src/providers/cryptoauthlib/mod.rs b/src/providers/cryptoauthlib/mod.rs index 4ce4fd14..8f168623 100644 --- a/src/providers/cryptoauthlib/mod.rs +++ b/src/providers/cryptoauthlib/mod.rs @@ -20,6 +20,7 @@ use uuid::Uuid; use parsec_interface::operations::{ psa_destroy_key, psa_generate_key, psa_generate_random, psa_hash_compare, psa_hash_compute, + psa_import_key, }; mod generate_random; @@ -175,6 +176,13 @@ impl Provider { warn!("Failed to setup opcodes for cryptoauthlib_provider"); } + let err = cryptoauthlib_provider + .device + .set_write_encryption_key(&cryptoauthlib_provider.get_write_encrypt_key()); + if rust_cryptoauthlib::AtcaStatus::AtcaSuccess != err { + warn!("Failed to setup write encryption key. Using ECC keys may not be possible."); + } + Some(cryptoauthlib_provider) } @@ -188,6 +196,7 @@ impl Provider { self.supported_opcodes.push(Opcode::PsaHashCompute); self.supported_opcodes.push(Opcode::PsaHashCompare); self.supported_opcodes.push(Opcode::PsaGenerateRandom); + self.supported_opcodes.push(Opcode::PsaImportKey); Some(()) } rust_cryptoauthlib::AtcaDeviceType::AtcaTestDevSuccess @@ -199,6 +208,16 @@ impl Provider { _ => None, } } + + // Get the deployment specific write key. With this the keys can be stored encrypted in their slots. + // For ECC private keys it is obligatory, for Aes it is an option. + fn get_write_encrypt_key(&self) -> [u8; rust_cryptoauthlib::ATCA_KEY_SIZE] { + [ + 0x4D, 0x50, 0x72, 0x6F, 0x20, 0x49, 0x4F, 0x20, 0x4B, 0x65, 0x79, 0x20, 0x9E, 0x31, + 0xBD, 0x05, 0x82, 0x58, 0x76, 0xCE, 0x37, 0x90, 0xEA, 0x77, 0x42, 0x32, 0xBB, 0x51, + 0x81, 0x49, 0x66, 0x45, + ] + } } impl Provide for Provider { @@ -295,6 +314,19 @@ impl Provide for Provider { self.psa_destroy_key_internal(app_name, op) } } + + fn psa_import_key( + &self, + app_name: ApplicationName, + op: psa_import_key::Operation, + ) -> Result { + trace!("psa_import_key ingress"); + if !self.supported_opcodes.contains(&Opcode::PsaImportKey) { + Err(ResponseStatus::PsaErrorNotSupported) + } else { + self.psa_import_key_internal(app_name, op) + } + } } /// CryptoAuthentication Library Provider builder @@ -430,7 +462,6 @@ impl ProviderBuilder { }, None => return Err(Error::new(ErrorKind::InvalidData, "Missing inteface type")), }; - Provider::new( self.key_info_store .ok_or_else(|| Error::new(ErrorKind::InvalidData, "missing key info store"))?, From 03e9d766d31f1049269f8e3df2ab4618265cb5d9 Mon Sep 17 00:00:00 2001 From: Robert Drazkowski Date: Mon, 26 Apr 2021 17:48:24 +0200 Subject: [PATCH 2/6] Review fixes. A new function was added to simplify importing ECC keys. Signed-off-by: Robert Drazkowski --- Cargo.lock | 65 +++++++ Cargo.toml | 1 + .../cryptoauthlib/config_508a.toml | 6 + .../cryptoauthlib/config_608a.toml | 6 + .../per_provider/normal_tests/import_key.rs | 127 +++++++------ src/providers/cryptoauthlib/key_management.rs | 4 +- src/providers/cryptoauthlib/key_operations.rs | 177 +++++++++++++++--- src/providers/cryptoauthlib/key_slot.rs | 8 +- 8 files changed, 308 insertions(+), 86 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b2502064..0be74c44 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -282,6 +282,21 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37ab347416e802de484e4d03c7316c48f1ecb56574dfd4a46a80f173ce1de04d" +[[package]] +name = "foreign-types" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6f339eb8adc052cd2ca78910fda869aefa38d22d5cb648e6485e4d3fc06f3b1" +dependencies = [ + "foreign-types-shared", +] + +[[package]] +name = "foreign-types-shared" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" + [[package]] name = "getrandom" version = "0.2.2" @@ -546,6 +561,49 @@ dependencies = [ "serde", ] +[[package]] +name = "once_cell" +version = "1.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af8b08b04175473088b46763e51ee54da5f9a164bc162f615b91bc179dbf15a3" + +[[package]] +name = "openssl" +version = "0.10.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a61075b62a23fef5a29815de7536d940aa35ce96d18ce0cc5076272db678a577" +dependencies = [ + "bitflags", + "cfg-if", + "foreign-types", + "libc", + "once_cell", + "openssl-sys", +] + +[[package]] +name = "openssl-src" +version = "111.15.0+1.1.1k" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1a5f6ae2ac04393b217ea9f700cd04fa9bf3d93fae2872069f3d15d908af70a" +dependencies = [ + "cc", +] + +[[package]] +name = "openssl-sys" +version = "0.9.61" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "313752393519e876837e09e1fa183ddef0be7735868dced3196f4472d536277f" +dependencies = [ + "autocfg", + "cc", + "libc", + "openssl-src", + "pkg-config", + "vcpkg", +] + [[package]] name = "parsec-interface" version = "0.24.0" @@ -581,6 +639,7 @@ dependencies = [ "hex", "libc", "log", + "openssl", "parsec-interface", "picky-asn1", "picky-asn1-der", @@ -1219,6 +1278,12 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bc5cf98d8186244414c848017f0e2676b3fcb46807f6668a97dfe67359a3c4b7" +[[package]] +name = "vcpkg" +version = "0.2.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cbdbff6266a24120518560b5dc983096efb98462e51d0d68169895b237be3e5d" + [[package]] name = "vec_map" version = "0.8.2" diff --git a/Cargo.toml b/Cargo.toml index 54ca375e..da7c40cf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ users = "0.11.0" libc = "0.2.86" anyhow = "1.0.38" rust-cryptoauthlib = { version = "0.3.0", optional = true } +openssl = { version = "0.10", features = ["vendored"] } prost = { version = "0.7.0", optional = true } diff --git a/e2e_tests/provider_cfg/cryptoauthlib/config_508a.toml b/e2e_tests/provider_cfg/cryptoauthlib/config_508a.toml index dd8dedf6..78d970b7 100644 --- a/e2e_tests/provider_cfg/cryptoauthlib/config_508a.toml +++ b/e2e_tests/provider_cfg/cryptoauthlib/config_508a.toml @@ -1,3 +1,9 @@ +######################################################################### +# The example config file for atecc508a cryptochip. +# There must be an I2C bus with a cryptochip soldered, otherwise provider +# instantiation fails. +# Not to be used by github CI. +######################################################################### [core_settings] # The CI already timestamps the logs log_timestamp = false diff --git a/e2e_tests/provider_cfg/cryptoauthlib/config_608a.toml b/e2e_tests/provider_cfg/cryptoauthlib/config_608a.toml index bbe0ea4a..483a1e63 100644 --- a/e2e_tests/provider_cfg/cryptoauthlib/config_608a.toml +++ b/e2e_tests/provider_cfg/cryptoauthlib/config_608a.toml @@ -1,3 +1,9 @@ +######################################################################### +# The example config file for atecc608a cryptochip. +# There must be an I2C bus with a cryptochip soldered, otherwise provider +# instantiation fails. +# Not to be used by github CI. +######################################################################### [core_settings] # The CI already timestamps the logs log_timestamp = false diff --git a/e2e_tests/tests/per_provider/normal_tests/import_key.rs b/e2e_tests/tests/per_provider/normal_tests/import_key.rs index 2689a7ae..19540e97 100644 --- a/e2e_tests/tests/per_provider/normal_tests/import_key.rs +++ b/e2e_tests/tests/per_provider/normal_tests/import_key.rs @@ -1,16 +1,13 @@ // Copyright 2019 Contributors to the Parsec project. -// SPDX-License-Identifier: Apache-2.0 +// SPDX-License-Identifier: Apache-2. +#![allow(unused_imports)] use e2e_tests::TestClient; -#[allow(unused_imports)] use parsec_client::core::interface::operations::psa_algorithm::*; -#[allow(unused_imports)] use parsec_client::core::interface::operations::psa_key_attributes::*; use parsec_client::core::interface::requests::Opcode; use parsec_client::core::interface::requests::ResponseStatus; use parsec_client::core::interface::requests::Result; -#[allow(unused_imports)] use picky_asn1::wrapper::IntegerAsn1; -#[allow(unused_imports)] use picky_asn1_x509::RSAPublicKey; #[cfg(not(feature = "cryptoauthlib-provider"))] @@ -68,17 +65,15 @@ const KEY_PAIR_DATA: [u8; 609] = [ ]; #[cfg(feature = "cryptoauthlib-provider")] -//#[allow(dead_code)] -const PRIV_KEY_ECC: [u8; 32] = [ - 0xF5, 0xDB, 0x6B, 0xA1, 0x82, 0x22, 0xCE, 0xC1, 0x54, 0x53, 0xE5, 0x63, 0xDE, 0xC5, 0xC7, 0x94, - 0xCD, 0x48, 0x95, 0xF2, 0x8C, 0xC2, 0x7F, 0x50, 0xC2, 0x7E, 0xC3, 0x1B, 0xAF, 0x44, 0xEA, 0x54, -]; -#[cfg(feature = "cryptoauthlib-provider")] -const PUB_KEY_ECC: [u8; 64] = [ - 0xBA, 0x6A, 0xB5, 0xF1, 0x19, 0xAF, 0x21, 0x73, 0x03, 0x75, 0xD1, 0x8D, 0x6B, 0x5F, 0xF1, 0x94, - 0x33, 0xE5, 0x3A, 0xEE, 0x5F, 0x6F, 0xBA, 0x22, 0x97, 0x77, 0x13, 0xEA, 0x82, 0xD3, 0x74, 0x84, - 0x8E, 0x39, 0x78, 0x66, 0xE8, 0x36, 0xB3, 0xFE, 0xD3, 0x22, 0x87, 0x74, 0xA5, 0x00, 0xC5, 0x5C, - 0x17, 0x73, 0x5A, 0x92, 0x4B, 0xB3, 0x9F, 0xE4, 0x98, 0x52, 0x62, 0xA5, 0x36, 0xC5, 0x00, 0x9C, +const KEY_PAIR_ECC: [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, + 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, 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"))] @@ -94,73 +89,99 @@ fn example_modulus_1024() -> Vec { ] } +#[cfg(not(feature = "cryptoauthlib-provider"))] #[test] -fn import_key() -> Result<()> { +fn import_rsa_key() -> Result<()> { let mut client = TestClient::new(); let key_name = String::from("import_key"); if !client.is_operation_supported(Opcode::PsaImportKey) { return Ok(()); } - #[cfg(not(feature = "cryptoauthlib-provider"))] - let result = client.import_rsa_public_key(key_name, KEY_DATA.to_vec()); - #[cfg(feature = "cryptoauthlib-provider")] - let result = client.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, PUB_KEY_ECC.to_vec()); + client.import_rsa_public_key(key_name, KEY_DATA.to_vec()) +} + +#[cfg(feature = "cryptoauthlib-provider")] +#[test] +fn import_ecc_key() -> Result<()> { + let mut client = TestClient::new(); + let key_name = String::from("import_key"); + if !client.is_operation_supported(Opcode::PsaImportKey) { + return Ok(()); + } - result + client.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, KEY_PAIR_ECC.to_vec()) } +#[cfg(not(feature = "cryptoauthlib-provider"))] #[test] -fn create_and_import_key() -> Result<()> { +fn create_and_import_rsa_key() -> Result<()> { let mut client = TestClient::new(); let key_name = String::from("create_and_import_key"); if !client.is_operation_supported(Opcode::PsaImportKey) { return Ok(()); } - let status; - #[cfg(not(feature = "cryptoauthlib-provider"))] - { - client.generate_rsa_sign_key(key_name.clone())?; - status = client - .import_rsa_public_key(key_name, KEY_DATA.to_vec()) - .expect_err("Key should have already existed"); - } - #[cfg(feature = "cryptoauthlib-provider")] - { - client.generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone())?; - status = client - .import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, PUB_KEY_ECC.to_vec()) - .expect_err("Key should have already existed"); + client.generate_rsa_sign_key(key_name.clone())?; + let status = client + .import_rsa_public_key(key_name, KEY_DATA.to_vec()) + .expect_err("Key should have already existed"); + assert_eq!(status, ResponseStatus::PsaErrorAlreadyExists); + + Ok(()) +} + +#[cfg(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"); + 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()) + .expect_err("Key should have already existed"); assert_eq!(status, ResponseStatus::PsaErrorAlreadyExists); Ok(()) } +#[cfg(not(feature = "cryptoauthlib-provider"))] #[test] -fn import_key_twice() -> Result<()> { +fn import_rsa_key_twice() -> Result<()> { let mut client = TestClient::new(); let key_name = String::from("import_key_twice"); if !client.is_operation_supported(Opcode::PsaImportKey) { return Ok(()); } - let status; - #[cfg(not(feature = "cryptoauthlib-provider"))] - { - client.import_rsa_public_key(key_name.clone(), KEY_DATA.to_vec())?; - status = client - .import_rsa_public_key(key_name, KEY_DATA.to_vec()) - .expect_err("The key with the same name has already been created."); - } - #[cfg(feature = "cryptoauthlib-provider")] - { - client.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name.clone(), PUB_KEY_ECC.to_vec())?; - status = client - .import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, PUB_KEY_ECC.to_vec()) - .expect_err("The key with the same name has already been created."); + client.import_rsa_public_key(key_name.clone(), KEY_DATA.to_vec())?; + let status = client + .import_rsa_public_key(key_name, KEY_DATA.to_vec()) + .expect_err("The key with the same name has already been created."); + + assert_eq!(status, ResponseStatus::PsaErrorAlreadyExists); + + Ok(()) +} + +#[cfg(feature = "cryptoauthlib-provider")] +#[test] +fn import_ecc_key_twice() -> Result<()> { + let mut client = TestClient::new(); + let key_name = String::from("import_key_twice"); + if !client.is_operation_supported(Opcode::PsaImportKey) { + return Ok(()); } + + client.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name.clone(), KEY_PAIR_ECC.to_vec())?; + let status = client + .import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, KEY_PAIR_ECC.to_vec()) + .expect_err("The key with the same name has already been created."); + assert_eq!(status, ResponseStatus::PsaErrorAlreadyExists); Ok(()) @@ -385,5 +406,5 @@ fn import_ecc_private_key() { return; } - client.import_ecc_pair_secp_r1_key(key_name, PRIV_KEY_ECC.to_vec()).unwrap(); + client.import_ecc_pair_secp_r1_key(key_name, KEY_PAIR_ECC.to_vec()).unwrap(); } diff --git a/src/providers/cryptoauthlib/key_management.rs b/src/providers/cryptoauthlib/key_management.rs index 0e26cdeb..c472d08e 100644 --- a/src/providers/cryptoauthlib/key_management.rs +++ b/src/providers/cryptoauthlib/key_management.rs @@ -31,7 +31,7 @@ impl Provider { Type::EccKeyPair { curve_family: EccFamily::SecpR1, } => { - if attributes.bits == 256 { + if attributes.bits == 256 || attributes.bits == 0 { Ok(rust_cryptoauthlib::KeyType::P256EccKey) } else { Err(ResponseStatus::PsaErrorNotSupported) @@ -40,7 +40,7 @@ impl Provider { Type::EccPublicKey { curve_family: EccFamily::SecpR1, } => { - if attributes.bits == 512 { + if attributes.bits == 512 || attributes.bits == 0 { Ok(rust_cryptoauthlib::KeyType::P256EccKey) } else { Err(ResponseStatus::PsaErrorNotSupported) diff --git a/src/providers/cryptoauthlib/key_operations.rs b/src/providers/cryptoauthlib/key_operations.rs index 23a7755a..5c6ee8e1 100644 --- a/src/providers/cryptoauthlib/key_operations.rs +++ b/src/providers/cryptoauthlib/key_operations.rs @@ -4,9 +4,13 @@ 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 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}; -use parsec_interface::secrecy::ExposeSecret; +use parsec_interface::secrecy::{ExposeSecret, Secret}; impl Provider { pub(super) fn psa_generate_key_internal( @@ -105,50 +109,169 @@ impl Provider { return Err(error); } }; - let key_data_r = op.data.expose_secret(); + // 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 status = self.device.import_key(key_type, key_data_r, slot_id); - let error_status: rust_cryptoauthlib::AtcaStatus; - match status { + let atca_error_status = self.device.import_key(key_type, &key_data, slot_id); + + let psa_error_status: ResponseStatus = match atca_error_status { rust_cryptoauthlib::AtcaStatus::AtcaSuccess => { match self .key_info_store .insert_key_info(key_triple, &slot_id, key_attributes) { - Ok(()) => { - return Ok(psa_import_key::Result {}); - } + Ok(()) => return Ok(psa_import_key::Result {}), Err(error) => { - warn!("Insert key triple to KeyInfoManager failed. {}", error); - return Err(error); + // This is very bad. + error!("Insert key triple to KeyInfoManager failed. {}", error); + // Let the function mark the slot as free later on, + // just in case things get better somehow. + ResponseStatus::PsaErrorStorageFailure } } } - x @ rust_cryptoauthlib::AtcaStatus::AtcaInvalidSize - | x @ rust_cryptoauthlib::AtcaStatus::AtcaInvalidId - | x @ rust_cryptoauthlib::AtcaStatus::AtcaBadParam => { - error_status = x; + rust_cryptoauthlib::AtcaStatus::AtcaInvalidSize + | rust_cryptoauthlib::AtcaStatus::AtcaInvalidId + | rust_cryptoauthlib::AtcaStatus::AtcaBadParam => { + warn!("Key import failed. AtcaStatus: {}", atca_error_status); + ResponseStatus::PsaErrorInvalidArgument } _ => { - error_status = rust_cryptoauthlib::AtcaStatus::AtcaUnimplemented; + warn!("Key import failed. AtcaStatus: {}", atca_error_status); + ResponseStatus::PsaErrorGenericError } - } + }; + + // Not Ok() match self.set_slot_status(slot_id as usize, KeySlotStatus::Free) { Ok(()) => { - let error = ResponseStatus::PsaErrorInvalidArgument; - error!( - "Key import failed. {}. Storage slot status updated.", - error_status - ); - Err(error) + // Import failed but at least slot was appropriately marked as Free } Err(error) => { - error!( - "Key import failed. {}. Storage slot status failed to update.", - error_status - ); - Err(error) + // Things do never get better... + 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>) -> Result> { + let key = secret.expose_secret(); + + match key_type { + Type::Aes => Ok(key.to_vec()), + 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()), + } + } + 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), } } + _ => Err(ResponseStatus::PsaErrorNotSupported), } } + +#[test] +fn test_extract_raw_ecc_private_key() { + let ecc_key_der_array: Secret> = 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, + 0x3c, 0xef, 0xfa, 0x08, 0xa7, 0xb4, 0xc6, 0xe0, 0xce, 0x73, 0xac, 0xd0, 0x69, 0xd4, + 0xcc, 0xa8, 0xd0, 0x55, 0xee, 0x6c, 0x65, 0xb5, 0x71, + ] + .to_vec(), + ); + let ecc_priv_key: [u8; 32] = [ + 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, + ]; + let ecc_priv_key_ext = extract_raw_key( + Type::EccKeyPair { + curve_family: EccFamily::SecpR1, + }, + &ecc_key_der_array, + ) + .unwrap(); + assert_eq!(ecc_priv_key.to_vec(), ecc_priv_key_ext); +} + +#[test] +fn test_extract_raw_ecc_public_key() { + let ecc_key_der_array: Secret> = 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, + 0x3c, 0xef, 0xfa, 0x08, 0xa7, 0xb4, 0xc6, 0xe0, 0xce, 0x73, 0xac, 0xd0, 0x69, 0xd4, + 0xcc, 0xa8, 0xd0, 0x55, 0xee, 0x6c, 0x65, 0xb5, 0x71, + ] + .to_vec(), + ); + let ecc_pub_key: [u8; 64] = [ + // 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, + ]; + let ecc_pub_key_ext = extract_raw_key( + Type::EccPublicKey { + curve_family: EccFamily::SecpR1, + }, + &ecc_key_der_array, + ) + .unwrap(); + assert_eq!(ecc_pub_key.to_vec(), ecc_pub_key_ext); +} diff --git a/src/providers/cryptoauthlib/key_slot.rs b/src/providers/cryptoauthlib/key_slot.rs index fad8d8cd..360d836b 100644 --- a/src/providers/cryptoauthlib/key_slot.rs +++ b/src/providers/cryptoauthlib/key_slot.rs @@ -81,11 +81,11 @@ impl AteccKeySlot { Type::EccKeyPair { curve_family: EccFamily::SecpR1, } => { - // P256 private key has 256 bits (32 bytes). + // P256 private key has 256 bits (32 bytes). 0 means - do not care. // Only private key is stored - public one can be computed when needed. // The private key can onlly be stored encrypted and the encryption key must be set, // see set_write_encryption_key() call in new(). - key_attr.bits == 256 + (key_attr.bits == 0 || key_attr.bits == 256) && self.config.key_type == rust_cryptoauthlib::KeyType::P256EccKey && self.config.write_config == rust_cryptoauthlib::WriteConfig::Encrypt && self.config.ecc_key_attr.is_private @@ -94,9 +94,9 @@ impl AteccKeySlot { Type::EccPublicKey { curve_family: EccFamily::SecpR1, } => { - // The uncompressed public key is 512 bits (64 bytes). + // The uncompressed public key is 512 bits (64 bytes). 0 means - do not care. // First few (7) slots are too short for ECC public key. - key_attr.bits == 512 + (key_attr.bits == 0 || key_attr.bits == 512) && self.config.key_type == rust_cryptoauthlib::KeyType::P256EccKey && slot >= rust_cryptoauthlib::ATCA_ATECC_MIN_SLOT_IDX_FOR_PUB_KEY } From a0415a7945897f58e706bc6727468644b2a8170d Mon Sep 17 00:00:00 2001 From: Robert Drazkowski Date: Wed, 28 Apr 2021 19:13:58 +0200 Subject: [PATCH 3/6] DER processing correction for private and public key extraction from DER encoded format. Few tests adaptations. Signed-off-by: Robert Drazkowski --- e2e_tests/src/lib.rs | 2 +- .../per_provider/normal_tests/import_key.rs | 34 ++++++--- src/providers/cryptoauthlib/key_management.rs | 10 +-- src/providers/cryptoauthlib/key_operations.rs | 75 +++++++------------ src/providers/cryptoauthlib/key_slot.rs | 5 +- 5 files changed, 55 insertions(+), 71 deletions(-) diff --git a/e2e_tests/src/lib.rs b/e2e_tests/src/lib.rs index ae211ff1..a003c36c 100644 --- a/e2e_tests/src/lib.rs +++ b/e2e_tests/src/lib.rs @@ -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, diff --git a/e2e_tests/tests/per_provider/normal_tests/import_key.rs b/e2e_tests/tests/per_provider/normal_tests/import_key.rs index 19540e97..933ac9a7 100644 --- a/e2e_tests/tests/per_provider/normal_tests/import_key.rs +++ b/e2e_tests/tests/per_provider/normal_tests/import_key.rs @@ -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::*; @@ -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, @@ -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 { vec![ @@ -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(); @@ -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(()); } @@ -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()) .expect_err("Key should have already existed"); assert_eq!(status, ResponseStatus::PsaErrorAlreadyExists); @@ -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); @@ -406,5 +416,5 @@ fn import_ecc_private_key() { return; } - 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(); } diff --git a/src/providers/cryptoauthlib/key_management.rs b/src/providers/cryptoauthlib/key_management.rs index c472d08e..1d9261e6 100644 --- a/src/providers/cryptoauthlib/key_management.rs +++ b/src/providers/cryptoauthlib/key_management.rs @@ -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) diff --git a/src/providers/cryptoauthlib/key_operations.rs b/src/providers/cryptoauthlib/key_operations.rs index 5c6ee8e1..acaaa693 100644 --- a/src/providers/cryptoauthlib/key_operations.rs +++ b/src/providers/cryptoauthlib/key_operations.rs @@ -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}; @@ -102,6 +100,7 @@ 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) => { @@ -109,13 +108,14 @@ impl Provider { 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); let psa_error_status: ResponseStatus = match atca_error_status { rust_cryptoauthlib::AtcaStatus::AtcaSuccess => { @@ -151,7 +151,7 @@ impl Provider { // Import failed but at least slot was appropriately marked as Free } Err(error) => { - // Things do never get better... + // Things never get better... error!("Storage slot status failed to update becuase {}", error); } }; @@ -159,12 +159,11 @@ impl Provider { } } -// TODO: use picky-asn1-x509 when support for ECC keys becomes available there -fn extract_raw_key(key_type: Type, secret: &Secret>) -> Result> { - let key = secret.expose_secret(); +fn extract_raw_key(key_type: Type, secret: &Secret>) -> Result>> { + 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, } => { @@ -172,41 +171,19 @@ fn extract_raw_key(key_type: Type, secret: &Secret>) -> Result> // 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), } } @@ -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> = Secret::new( + let public_ecc_key_array: Secret> = 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, @@ -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() + ); } diff --git a/src/providers/cryptoauthlib/key_slot.rs b/src/providers/cryptoauthlib/key_slot.rs index 360d836b..2e289b6a 100644 --- a/src/providers/cryptoauthlib/key_slot.rs +++ b/src/providers/cryptoauthlib/key_slot.rs @@ -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 } From 19b22eeb64a2f44953f483aefd8d832268b69cd9 Mon Sep 17 00:00:00 2001 From: Robert Drazkowski Date: Thu, 29 Apr 2021 15:39:00 +0200 Subject: [PATCH 4/6] Private key processing adjustment. Signed-off-by: Robert Drazkowski --- Cargo.lock | 65 ------------------- Cargo.toml | 1 - .../per_provider/normal_tests/import_key.rs | 33 ++++------ src/providers/cryptoauthlib/key_operations.rs | 49 ++------------ 4 files changed, 18 insertions(+), 130 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0be74c44..b2502064 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -282,21 +282,6 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37ab347416e802de484e4d03c7316c48f1ecb56574dfd4a46a80f173ce1de04d" -[[package]] -name = "foreign-types" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6f339eb8adc052cd2ca78910fda869aefa38d22d5cb648e6485e4d3fc06f3b1" -dependencies = [ - "foreign-types-shared", -] - -[[package]] -name = "foreign-types-shared" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" - [[package]] name = "getrandom" version = "0.2.2" @@ -561,49 +546,6 @@ dependencies = [ "serde", ] -[[package]] -name = "once_cell" -version = "1.7.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af8b08b04175473088b46763e51ee54da5f9a164bc162f615b91bc179dbf15a3" - -[[package]] -name = "openssl" -version = "0.10.33" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a61075b62a23fef5a29815de7536d940aa35ce96d18ce0cc5076272db678a577" -dependencies = [ - "bitflags", - "cfg-if", - "foreign-types", - "libc", - "once_cell", - "openssl-sys", -] - -[[package]] -name = "openssl-src" -version = "111.15.0+1.1.1k" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b1a5f6ae2ac04393b217ea9f700cd04fa9bf3d93fae2872069f3d15d908af70a" -dependencies = [ - "cc", -] - -[[package]] -name = "openssl-sys" -version = "0.9.61" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "313752393519e876837e09e1fa183ddef0be7735868dced3196f4472d536277f" -dependencies = [ - "autocfg", - "cc", - "libc", - "openssl-src", - "pkg-config", - "vcpkg", -] - [[package]] name = "parsec-interface" version = "0.24.0" @@ -639,7 +581,6 @@ dependencies = [ "hex", "libc", "log", - "openssl", "parsec-interface", "picky-asn1", "picky-asn1-der", @@ -1278,12 +1219,6 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bc5cf98d8186244414c848017f0e2676b3fcb46807f6668a97dfe67359a3c4b7" -[[package]] -name = "vcpkg" -version = "0.2.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cbdbff6266a24120518560b5dc983096efb98462e51d0d68169895b237be3e5d" - [[package]] name = "vec_map" version = "0.8.2" diff --git a/Cargo.toml b/Cargo.toml index da7c40cf..54ca375e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,7 +42,6 @@ users = "0.11.0" libc = "0.2.86" anyhow = "1.0.38" rust-cryptoauthlib = { version = "0.3.0", optional = true } -openssl = { version = "0.10", features = ["vendored"] } prost = { version = "0.7.0", optional = true } diff --git a/e2e_tests/tests/per_provider/normal_tests/import_key.rs b/e2e_tests/tests/per_provider/normal_tests/import_key.rs index 933ac9a7..af890116 100644 --- a/e2e_tests/tests/per_provider/normal_tests/import_key.rs +++ b/e2e_tests/tests/per_provider/normal_tests/import_key.rs @@ -65,26 +65,21 @@ const KEY_PAIR_DATA: [u8; 609] = [ ]; #[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, - 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, 0x3c, 0xef, 0xfa, 0x08, 0xa7, 0xb4, 0xc6, 0xe0, 0xce, 0x73, 0xac, 0xd0, 0x69, 0xd4, - 0xcc, 0xa8, 0xd0, 0x55, 0xee, 0x6c, 0x65, 0xb5, 0x71, +const ECC_PRIVATE_KEY: [u8; 32] = [ + 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, ]; #[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, - ]; + 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 { @@ -152,7 +147,7 @@ fn create_and_import_ecc_key() -> Result<()> { client.generate_ecc_key_pair_secpr1_ecdsa_sha256(key_name.clone())?; let status = client - .import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, ECC_KEY_PAIR.to_vec()) + .import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, ECC_PUBLIC_KEY.to_vec()) .expect_err("Key should have already existed"); assert_eq!(status, ResponseStatus::PsaErrorAlreadyExists); @@ -407,7 +402,7 @@ fn import_key_pair() { .unwrap(); } -#[cfg(feature = "cryptoauthlib-provider")] +#[cfg(any(feature = "mbed-crypto-provider", feature = "cryptoauthlib-provider"))] #[test] fn import_ecc_private_key() { let mut client = TestClient::new(); @@ -416,5 +411,5 @@ fn import_ecc_private_key() { return; } - client.import_ecc_pair_secp_r1_key(key_name, ECC_KEY_PAIR.to_vec()).unwrap(); + client.import_ecc_pair_secp_r1_key(key_name, ECC_PRIVATE_KEY.to_vec()).unwrap(); } diff --git a/src/providers/cryptoauthlib/key_operations.rs b/src/providers/cryptoauthlib/key_operations.rs index acaaa693..08383da7 100644 --- a/src/providers/cryptoauthlib/key_operations.rs +++ b/src/providers/cryptoauthlib/key_operations.rs @@ -4,7 +4,6 @@ use super::key_slot::KeySlotStatus; use super::Provider; use crate::authenticators::ApplicationName; use log::{error, warn}; -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}; @@ -163,17 +162,11 @@ fn extract_raw_key(key_type: Type, secret: &Secret>) -> Result Ok(Secret::new(key)), - Type::EccKeyPair { + Type::Aes + | Type::RawData + | 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(Secret::new(ec_key.private_key().to_vec())), - } - } + } => Ok(Secret::new(key)), Type::EccPublicKey { curve_family: EccFamily::SecpR1, } => match key.len() { @@ -188,40 +181,6 @@ fn extract_raw_key(key_type: Type, secret: &Secret>) -> Result> = 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, - 0x3c, 0xef, 0xfa, 0x08, 0xa7, 0xb4, 0xc6, 0xe0, 0xce, 0x73, 0xac, 0xd0, 0x69, 0xd4, - 0xcc, 0xa8, 0xd0, 0x55, 0xee, 0x6c, 0x65, 0xb5, 0x71, - ] - .to_vec(), - ); - let ecc_priv_key: [u8; 32] = [ - 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, - ]; - let ecc_priv_key_ext = extract_raw_key( - Type::EccKeyPair { - curve_family: EccFamily::SecpR1, - }, - &ecc_key_der_array, - ) - .unwrap(); - assert_eq!( - ecc_priv_key.to_vec(), - ecc_priv_key_ext.expose_secret().to_owned() - ); -} - #[test] fn test_extract_raw_ecc_public_key() { let public_ecc_key_array: Secret> = Secret::new( From b1a5adda55fb4f2fdd11844b769ad00253ccfa29 Mon Sep 17 00:00:00 2001 From: Robert Drazkowski Date: Thu, 29 Apr 2021 16:24:35 +0200 Subject: [PATCH 5/6] One more tests enabled for CALib provider. Signed-off-by: Robert Drazkowski --- e2e_tests/tests/per_provider/normal_tests/import_key.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/e2e_tests/tests/per_provider/normal_tests/import_key.rs b/e2e_tests/tests/per_provider/normal_tests/import_key.rs index af890116..68eb1d80 100644 --- a/e2e_tests/tests/per_provider/normal_tests/import_key.rs +++ b/e2e_tests/tests/per_provider/normal_tests/import_key.rs @@ -315,7 +315,6 @@ fn check_format_import3() -> Result<()> { Ok(()) } -#[cfg(not(feature = "cryptoauthlib-provider"))] #[test] fn failed_imported_key_should_be_removed() -> Result<()> { let mut client = TestClient::new(); @@ -323,7 +322,7 @@ fn failed_imported_key_should_be_removed() -> Result<()> { if !client.is_operation_supported(Opcode::PsaImportKey) { return Ok(()); } - + #[cfg(not(feature = "cryptoauthlib-provider"))] let public_key = RSAPublicKey { modulus: IntegerAsn1::from_bytes_be_unsigned(example_modulus_1024()), public_exponent: IntegerAsn1::from_bytes_be_unsigned(vec![0x01, 0x00, 0x01]), @@ -359,7 +358,10 @@ fn failed_imported_key_should_be_removed() -> Result<()> { .import_key(key_name.clone(), attributes, Vec::new()) .unwrap_err(); // Should succeed because key would have been destroyed. + #[cfg(not(feature = "cryptoauthlib-provider"))] client.import_rsa_public_key(key_name, picky_asn1_der::to_vec(&public_key).unwrap())?; + #[cfg(feature = "cryptoauthlib-provider")] + client.import_ecc_public_secp_r1_ecdsa_sha256_key(key_name, ECC_PUBLIC_KEY.to_vec())?; Ok(()) } From 937e171fdce18fa04c1e9f82e053e28acd0f39f4 Mon Sep 17 00:00:00 2001 From: Robert Drazkowski Date: Thu, 29 Apr 2021 16:41:13 +0200 Subject: [PATCH 6/6] Last minute tmp fix. Signed-off-by: Robert Drazkowski --- e2e_tests/tests/per_provider/normal_tests/import_key.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e_tests/tests/per_provider/normal_tests/import_key.rs b/e2e_tests/tests/per_provider/normal_tests/import_key.rs index 68eb1d80..2cd0636d 100644 --- a/e2e_tests/tests/per_provider/normal_tests/import_key.rs +++ b/e2e_tests/tests/per_provider/normal_tests/import_key.rs @@ -318,7 +318,7 @@ fn check_format_import3() -> Result<()> { #[test] fn failed_imported_key_should_be_removed() -> Result<()> { let mut client = TestClient::new(); - let key_name = String::from("failed_imported_key_should_be_removed"); + let key_name = String::from("failed_imported_key_should_be_removed_notpm"); if !client.is_operation_supported(Opcode::PsaImportKey) { return Ok(()); }