From ec911e34619f1bda12b02cfcf7c10a65088b2f0e Mon Sep 17 00:00:00 2001 From: Eugene Koira Date: Thu, 11 Apr 2024 12:40:23 +0000 Subject: [PATCH 1/4] Add support to use KMS for signing EIF files when building them We extend EIF building functionality with additional option of signing EIF files with KMS. sign_info parameter of EifBuilder now turned to a structure that contains enum for the signing key. This enum can be represented as local private key (previous functionality) or KMS signing key (implemented in COSE library). Also a couple of wrappers and helper methods are introduced to store initial information about signing keys and transform it to the keys itself. The information about KMS signing key is represented as unique KMS key id string and optional region string. In case region is missed it will be read from AWS_REGION environment variable as a standard way for SDK configuration. Signed-off-by: Eugene Koira --- Cargo.toml | 10 ++- examples/eif_build.rs | 61 ++++++++++++++---- src/utils/mod.rs | 140 +++++++++++++++++++++++++++++++----------- 3 files changed, 162 insertions(+), 49 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f1d4f1a..0825bee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,15 @@ byteorder = "1.3" clap = "3.2" hex = "0.4" crc = "1.8" -aws-nitro-enclaves-cose = "0.5" +aws-nitro-enclaves-cose = { version = "0.5", features = ["key_kms"] } openssl = "0.10" serde_cbor = "0.11" chrono = { version = "0.4", default-features = false, features = ["clock"]} +aws-sdk-kms = "<=1.20" +aws-config = "<=1.1" +aws-types = "<=1.1" +aws-smithy-runtime = { version = "<=1.2" } +tokio = { version = "1.20", features = ["rt-multi-thread"] } + +[dev-dependencies] +tempfile = "3.5" diff --git a/examples/eif_build.rs b/examples/eif_build.rs index 9ca33f2..681431b 100644 --- a/examples/eif_build.rs +++ b/examples/eif_build.rs @@ -1,4 +1,4 @@ -// Copyright 2019-2022 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// Copyright 2019-2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 #![deny(warnings)] @@ -18,9 +18,9 @@ use aws_nitro_enclaves_image_format::defs::EifIdentityInfo; use aws_nitro_enclaves_image_format::utils::identity::parse_custom_metadata; use aws_nitro_enclaves_image_format::{ generate_build_info, - utils::{get_pcrs, EifBuilder, SignEnclaveInfo}, + utils::{get_pcrs, EifBuilder, SignKeyData, SignKeyDataInfo, SignKeyInfo}, }; -use clap::{App, Arg}; +use clap::{App, Arg, ArgGroup}; use serde_json::json; use sha2::{Digest, Sha256, Sha384, Sha512}; use std::fmt::Debug; @@ -76,7 +76,8 @@ fn main() { Arg::with_name("signing-certificate") .long("signing-certificate") .help("Specify the path to the signing certificate") - .takes_value(true), + .takes_value(true) + .requires("signing-key"), ) .arg( Arg::with_name("private-key") @@ -84,6 +85,25 @@ fn main() { .help("Specify the path to the private-key") .takes_value(true), ) + .arg( + Arg::with_name("kms-key-id") + .long("kms-key-id") + .help("Specify unique id of the KMS key") + .takes_value(true), + ) + .arg( + Arg::with_name("kms-key-region") + .long("kms-key-region") + .help("Specify region in which the KMS key resides") + .takes_value(true) + .requires("kms-key-id") + ) + .group( + ArgGroup::new("signing-key") + .args(&["kms-key-id", "private-key"]) + .multiple(false) + .requires("signing-certificate") + ) .arg( Arg::with_name("sha256") .long("sha256") @@ -150,14 +170,29 @@ fn main() { let private_key = matches.value_of("private-key"); - let sign_info = match (signing_certificate, private_key) { + let kms_key_id = matches.value_of("kms-key-id"); + let kms_key_region = matches.value_of("kms-key-region"); + + let sign_key_info = match (kms_key_id, private_key) { (None, None) => None, - (Some(cert_path), Some(key_path)) => { - Some(SignEnclaveInfo::new(cert_path, key_path).expect("Could not read signing info")) - } - _ => panic!("Both signing-certificate and private-key parameters must be provided"), + (Some(kms_id), None) => Some(SignKeyInfo::KmsKeyInfo { + id: kms_id.to_string(), + region: kms_key_region.map(str::to_string), + }), + (None, Some(key_path)) => Some(SignKeyInfo::LocalPrivateKeyInfo { + path: key_path.to_string(), + }), + _ => panic!("kms-key-id and private-key parameters are mutually exclusive"), }; + let sign_key_data = sign_key_info.map(|key_info| { + SignKeyData::new(&SignKeyDataInfo { + cert_path: signing_certificate.unwrap().to_string(), + key_info: key_info, + }) + .expect("Could not read signing info") + }); + let img_name = matches.value_of("image_name").map(|val| val.to_string()); let img_version = matches.value_of("image_name").map(|val| val.to_string()); let metadata_path = matches.value_of("metadata").map(|val| val.to_string()); @@ -190,7 +225,7 @@ fn main() { cmdline, ramdisks, output_path, - sign_info, + sign_key_data, Sha512::new(), eif_info, ); @@ -200,7 +235,7 @@ fn main() { cmdline, ramdisks, output_path, - sign_info, + sign_key_data, Sha256::new(), eif_info, ); @@ -210,7 +245,7 @@ fn main() { cmdline, ramdisks, output_path, - sign_info, + sign_key_data, Sha384::new(), eif_info, ); @@ -222,7 +257,7 @@ pub fn build_eif( cmdline: &str, ramdisks: Vec<&str>, output_path: &str, - sign_info: Option, + sign_info: Option, hasher: T, eif_info: EifIdentityInfo, ) { diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 99b04d4..d966a66 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -1,4 +1,4 @@ -// Copyright 2019-2022 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// Copyright 2019-2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 #![deny(warnings)] pub mod eif_reader; @@ -9,7 +9,12 @@ use crate::defs::{ EifHeader, EifIdentityInfo, EifSectionHeader, EifSectionType, PcrInfo, PcrSignature, EIF_MAGIC, MAX_NUM_SECTIONS, }; -use aws_nitro_enclaves_cose::{crypto::Openssl, header_map::HeaderMap, CoseSign1}; +use aws_config::BehaviorVersion; +use aws_nitro_enclaves_cose::{ + crypto::kms::KmsKey, crypto::Openssl, header_map::HeaderMap, CoseSign1, +}; +use aws_sdk_kms::client::Client; +use aws_types::region::Region; use crc::{crc32, Hasher32}; use openssl::asn1::Asn1Time; use openssl::pkey::PKey; @@ -34,35 +39,96 @@ use std::fs::File; use std::io::{Read, Seek, SeekFrom, Write}; use std::mem::size_of; use std::path::Path; +use tokio::runtime::Runtime; const DEFAULT_SECTIONS_COUNT: u16 = 3; +// Signing key for eif images +pub enum SignKey { + // Local private key + LocalPrivateKey(Vec), + + // KMS signer implementation from Cose library + KmsKey(KmsKey), +} + +// Full signining key data +pub struct SignKeyData { + // x509 certificate + pub cert: Vec, + + // Signing key itself + pub key: SignKey, +} + +// Signing key details +#[derive(Clone, Debug)] +pub enum SignKeyInfo { + // Local private key file path + LocalPrivateKeyInfo { path: String }, + + // KMS key details + KmsKeyInfo { id: String, region: Option }, +} + +// Full details of signing key #[derive(Clone, Debug)] -pub struct SignEnclaveInfo { - pub signing_certificate: Vec, - pub private_key: Vec, +pub struct SignKeyDataInfo { + // Path to the certificate file + pub cert_path: String, + + // Details of signing key itself + pub key_info: SignKeyInfo, } -impl SignEnclaveInfo { - pub fn new(cert_path: &str, key_path: &str) -> Result { - let mut certificate_file = File::open(cert_path) +impl SignKeyData { + pub fn new(sign_info: &SignKeyDataInfo) -> Result { + let mut cert_file = File::open(&sign_info.cert_path) .map_err(|err| format!("Could not open the certificate file: {:?}", err))?; - let mut signing_certificate = Vec::new(); - certificate_file - .read_to_end(&mut signing_certificate) + let mut cert = Vec::new(); + cert_file + .read_to_end(&mut cert) .map_err(|err| format!("Could not read the certificate file: {:?}", err))?; - let mut key_file = File::open(key_path) - .map_err(|err| format!("Could not open the key file: {:?}", err))?; - let mut private_key = Vec::new(); - key_file - .read_to_end(&mut private_key) - .map_err(|err| format!("Could not read the key file: {:?}", err))?; + let key = match &sign_info.key_info { + SignKeyInfo::LocalPrivateKeyInfo { path } => { + let mut key_file = File::open(path) + .map_err(|err| format!("Could not open the key file: {:?}", err))?; + let mut key_data = Vec::new(); + key_file + .read_to_end(&mut key_data) + .map_err(|err| format!("Could not read the key file: {:?}", err))?; - Ok(SignEnclaveInfo { - signing_certificate, - private_key, - }) + SignKey::LocalPrivateKey(key_data) + } + SignKeyInfo::KmsKeyInfo { id, region } => { + let act = async { + let mut config_loader = aws_config::defaults(BehaviorVersion::latest()); + if let Some(region_id) = region { + config_loader = config_loader.region(Region::new(region_id.clone())); + } + + let sdk_config = config_loader.load().await; + if sdk_config.region().is_none() { + return Err("AWS region for KMS is not specified".to_string()); + } + + let id_copy = id.clone(); + tokio::task::spawn_blocking(move || { + let client = Client::new(&sdk_config); + KmsKey::new_with_public_key(client, id_copy, None) + .map_err(|e| e.to_string()) + }) + .await + .unwrap() + }; + let runtime = Runtime::new().unwrap(); + let key = runtime.block_on(act)?; + SignKey::KmsKey(key) + } + }; + + Ok(SignKeyData { cert, key }) } } @@ -118,7 +184,7 @@ pub struct EifBuilder { kernel: File, cmdline: Vec, ramdisks: Vec, - sign_info: Option, + sign_info: Option, signature: Option>, signature_size: u64, metadata: Vec, @@ -142,7 +208,7 @@ impl EifBuilder { pub fn new( kernel_path: &Path, cmdline: String, - sign_info: Option, + sign_info: Option, hasher: T, flags: u16, eif_info: EifIdentityInfo, @@ -285,23 +351,27 @@ impl EifBuilder { register_index: i32, register_value: Vec, ) -> PcrSignature { - let sign_info = self.sign_info.as_ref().unwrap(); - let signing_certificate = sign_info.signing_certificate.clone(); + let sign_info = self + .sign_info + .as_ref() + .expect("Signing key is expected to be provided"); let pcr_info = PcrInfo::new(register_index, register_value); let payload = to_vec(&pcr_info).expect("Could not serialize PCR info"); - let private_key = PKey::private_key_from_pem(&sign_info.private_key) - .expect("Could not deserialize the PEM-formatted private key"); - let signature = - CoseSign1::new::(&payload, &HeaderMap::new(), private_key.as_ref()) - .unwrap() - .as_bytes(false) - .unwrap(); + let cose_sign = match &sign_info.key { + SignKey::LocalPrivateKey(key) => { + let pkey = PKey::private_key_from_pem(key) + .expect("Could not deserialize the PEM-formatted private key"); + + CoseSign1::new::(&payload, &HeaderMap::new(), &pkey) + } + SignKey::KmsKey(key) => CoseSign1::new::(&payload, &HeaderMap::new(), key), + }; PcrSignature { - signing_certificate, - signature, + signing_certificate: sign_info.cert.clone(), + signature: cose_sign.unwrap().as_bytes(false).unwrap(), } } @@ -614,7 +684,7 @@ impl EifBuilder { } if let Some(sign_info) = self.sign_info.as_ref() { - let cert = openssl::x509::X509::from_pem(&sign_info.signing_certificate[..]).unwrap(); + let cert = openssl::x509::X509::from_pem(&sign_info.cert[..]).unwrap(); let cert_der = cert.to_der().unwrap(); // This is equivalent to extend(cert.digest(sha384)), since hasher is going to // hash the DER certificate (cert.digest()) and then tpm_extend_finalize_reset From 7a16811e50ce4deaa80312d652f4351155984d75 Mon Sep 17 00:00:00 2001 From: Eugene Koira Date: Fri, 12 Apr 2024 12:26:45 +0000 Subject: [PATCH 2/4] Unit tests for KMS signing functionality Small part of reading and initializing key and certficate data is covered with tests. Signed-off-by: Eugene Koira --- .github/workflows/ci.yml | 4 +- src/utils/mod.rs | 169 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e9880c8..cc642c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,9 @@ jobs: - uses: actions/checkout@v3 - run: rustup install ${{ matrix.rust }} - run: cargo +${{ matrix.rust }} build --verbose - - run: cargo +${{ matrix.rust }} test --verbose + - run: | + cargo +${{ matrix.rust }} test --verbose \ + -- --skip utils::tests::kms # Requires AWS creds clippy: runs-on: ubuntu-latest steps: diff --git a/src/utils/mod.rs b/src/utils/mod.rs index d966a66..008fe6b 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -807,3 +807,172 @@ impl PcrSignatureChecker { Ok(()) } } + +#[cfg(test)] +mod tests { + use crate::utils::{SignKey, SignKeyData, SignKeyDataInfo, SignKeyInfo}; + use std::{env, io::Write}; + use tempfile::{NamedTempFile, TempPath}; + + const TEST_CERT_CONTENT: &[u8] = "test cert content".as_bytes(); + const TEST_PKEY_CONTENT: &[u8] = "test key content".as_bytes(); + + fn generate_certificate_file() -> Result { + let cert_file = NamedTempFile::new()?; + cert_file.as_file().write(TEST_CERT_CONTENT)?; + Ok(cert_file.into_temp_path()) + } + + fn generate_pkey_file() -> Result { + let key_file = NamedTempFile::new()?; + key_file.as_file().write(TEST_PKEY_CONTENT)?; + Ok(key_file.into_temp_path()) + } + + #[test] + fn test_local_sign_key_data_from_invalid_local_key_info() -> Result<(), std::io::Error> { + let cert_file_path = generate_certificate_file()?; + + let key_data = SignKeyData::new(&SignKeyDataInfo { + cert_path: cert_file_path.to_str().unwrap().to_string(), + key_info: SignKeyInfo::LocalPrivateKeyInfo { + path: "/invalid/path".to_string(), + }, + }); + + assert!(key_data.is_err()); + Ok(()) + } + + #[test] + fn test_local_sign_key_data_from_invalid_cert_key_info() -> Result<(), std::io::Error> { + let key_file_path = generate_pkey_file()?; + + let key_data = SignKeyData::new(&SignKeyDataInfo { + cert_path: "/invalid/path".to_string(), + key_info: SignKeyInfo::LocalPrivateKeyInfo { + path: key_file_path.to_str().unwrap().to_string(), + }, + }); + + assert!(key_data.is_err()); + Ok(()) + } + + #[test] + fn test_local_sign_key_data_from_valid_key_info() -> Result<(), std::io::Error> { + let cert_file_path = generate_certificate_file()?; + let key_file_path = generate_pkey_file()?; + + let key_data = SignKeyData::new(&SignKeyDataInfo { + cert_path: cert_file_path.to_str().unwrap().to_string(), + key_info: SignKeyInfo::LocalPrivateKeyInfo { + path: key_file_path.to_str().unwrap().to_string(), + }, + }) + .unwrap(); + + assert_eq!(key_data.cert, TEST_CERT_CONTENT); + assert!(matches!(key_data.key, SignKey::LocalPrivateKey(key) if key == TEST_PKEY_CONTENT)); + + Ok(()) + } + + mod kms { + use std::sync::Mutex; + + use super::*; + + // Mutex to lock and prevent running tests that modify AWS_REGION env variable + // within multiple threads + static ENV_MUTEX: std::sync::Mutex = Mutex::new(0); + + #[test] + fn test_kms_sign_key_data_from_invalid_cert_key_info() -> Result<(), std::io::Error> { + let key_id = env::var("AWS_KMS_TEST_KEY_ID").expect("Please set AWS_KMS_TEST_KEY_ID"); + let key_region = env::var("AWS_KMS_TEST_KEY_REGION").ok(); + + let key_data = SignKeyData::new(&SignKeyDataInfo { + cert_path: "/invalid/path".to_string(), + key_info: SignKeyInfo::KmsKeyInfo { + id: key_id, + region: key_region, + }, + }); + + assert!(key_data.is_err()); + Ok(()) + } + + #[test] + fn test_kms_sign_key_data_from_valid_key_info_explicit_region() -> Result<(), std::io::Error> + { + let cert_file_path = generate_certificate_file()?; + let key_id = env::var("AWS_KMS_TEST_KEY_ID").expect("Please set AWS_KMS_TEST_KEY_ID"); + let key_region = + env::var("AWS_KMS_TEST_KEY_REGION").expect("Please set AWS_KMS_TEST_KEY_REGION"); + let _m = ENV_MUTEX.lock().unwrap(); + env::remove_var("AWS_REGION"); + + let key_data = SignKeyData::new(&SignKeyDataInfo { + cert_path: cert_file_path.to_str().unwrap().to_string(), + key_info: SignKeyInfo::KmsKeyInfo { + id: key_id, + region: Some(key_region), + }, + }) + .unwrap(); + + assert_eq!(key_data.cert, TEST_CERT_CONTENT); + assert!(matches!(key_data.key, SignKey::KmsKey(_))); + + Ok(()) + } + + #[test] + fn test_kms_sign_key_data_from_valid_key_info_region_from_env() -> Result<(), std::io::Error> + { + let cert_file_path = generate_certificate_file()?; + let key_id = env::var("AWS_KMS_TEST_KEY_ID").expect("Please set AWS_KMS_TEST_KEY_ID"); + let key_region = + env::var("AWS_KMS_TEST_KEY_REGION").expect("Please set AWS_KMS_TEST_KEY_REGION"); + + let _m = ENV_MUTEX.lock().unwrap(); + env::set_var("AWS_REGION", key_region); + + let key_data = SignKeyData::new(&SignKeyDataInfo { + cert_path: cert_file_path.to_str().unwrap().to_string(), + key_info: SignKeyInfo::KmsKeyInfo { + id: key_id, + region: None, + }, + }) + .unwrap(); + + assert_eq!(key_data.cert, TEST_CERT_CONTENT); + assert!(matches!(key_data.key, SignKey::KmsKey(_))); + + Ok(()) + } + + #[test] + fn test_kms_sign_key_data_from_valid_key_info_no_region() -> Result<(), std::io::Error> { + let cert_file_path = generate_certificate_file()?; + let key_id = env::var("AWS_KMS_TEST_KEY_ID").expect("Please set AWS_KMS_TEST_KEY_ID"); + + let _m = ENV_MUTEX.lock().unwrap(); + env::remove_var("AWS_REGION"); + + let key_data = SignKeyData::new(&SignKeyDataInfo { + cert_path: cert_file_path.to_str().unwrap().to_string(), + key_info: SignKeyInfo::KmsKeyInfo { + id: key_id, + region: None, + }, + }); + + assert!(key_data.is_err()); + Ok(()) + } + } +} From 1203ba97cec8bf29efd0f31842dbae938dd0dc5d Mon Sep 17 00:00:00 2001 From: Eugene Koira Date: Thu, 18 Apr 2024 15:59:28 +0000 Subject: [PATCH 3/4] Fix clippy errors Signed-off-by: Eugene Koira --- src/utils/eif_reader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/eif_reader.rs b/src/utils/eif_reader.rs index 52583a1..f2defc7 100644 --- a/src/utils/eif_reader.rs +++ b/src/utils/eif_reader.rs @@ -242,7 +242,7 @@ impl EifReader { // Parse issuer into a BTreeMap let mut issuer_name = BTreeMap::new(); - for (_, e) in cert.issuer_name().entries().enumerate() { + for e in cert.issuer_name().entries() { issuer_name.insert( e.object().to_string(), format!("{:?}", e.data()).replace(&['\"'][..], ""), From 40586a93862b4834085687d3887eec7a97e80a87 Mon Sep 17 00:00:00 2001 From: Eugene Koira Date: Thu, 18 Apr 2024 16:03:37 +0000 Subject: [PATCH 4/4] Rust version bump Update minimum supported rust version (MSRV) to 1.68.2. Signed-off-by: Eugene Koira --- .github/workflows/ci.yml | 2 +- Cargo.toml | 2 +- README.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cc642c2..d547fcb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - rust: [1.58, stable, nightly] + rust: [1.68.2, stable, nightly] steps: - uses: actions/checkout@v3 - run: rustup install ${{ matrix.rust }} diff --git a/Cargo.toml b/Cargo.toml index 0825bee..cfb746a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ description = "This library provides the definition of the enclave image format repository = "https://github.com/aws/aws-nitro-enclaves-image-format" readme = "README.md" keywords = ["Nitro", "Enclaves", "AWS"] -rust-version = "1.58" +rust-version = "1.68" [dependencies] sha2 = "0.9.5" diff --git a/README.md b/README.md index b1fc95a..72714c3 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ [crates.io]: https://crates.io/crates/aws-nitro-enclaves-image-format [docs]: https://img.shields.io/docsrs/aws-nitro-enclaves-image-format [docs.rs]: https://docs.rs/aws-nitro-enclaves-image-format -[msrv]: https://img.shields.io/badge/MSRV-1.58.1-blue +[msrv]: https://img.shields.io/badge/MSRV-1.68.2-blue This library provides the definition of the enclave image format (EIF) file.