From 01439e24673fc57636c66866d912ba2f28981782 Mon Sep 17 00:00:00 2001 From: Qi Feng Huo Date: Thu, 30 May 2024 16:02:16 +0800 Subject: [PATCH] Verifier: IBM SE to fix review comments Signed-off-by: Qi Feng Huo --- Cargo.toml | 5 +- .../attestation-service/src/bin/grpc/mod.rs | 10 +- .../src/bin/restful/mod.rs | 10 +- attestation-service/docs/parsed_claims.md | 7 +- attestation-service/protos/attestation.proto | 6 +- attestation-service/verifier/src/lib.rs | 1 - attestation-service/verifier/src/se/ibmse.rs | 104 ++++++------------ attestation-service/verifier/src/se/mod.rs | 81 +++++++++++--- kbs/src/api/src/attestation/coco/grpc.rs | 28 ++--- 9 files changed, 130 insertions(+), 122 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 09e060a561..9d3fef791a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,8 +37,8 @@ prost = "0.11.0" regorus = { version = "0.1.5", default-features = false, features = ["regex", "base64", "time"] } rstest = "0.18.1" serde = { version = "1.0", features = ["derive"] } -serde_with = { version = "1.11.0", features = ["base64"] } serde_json = "1.0.89" +serde_with = { version = "1.11.0", features = ["base64"] } serial_test = "0.9.0" sha2 = "0.10" shadow-rs = "0.19.0" @@ -47,5 +47,4 @@ thiserror = "1.0" tokio = { version = "1.23.0", features = ["full"] } tempfile = "3.4.0" tonic = "0.8.1" -tonic-build = "0.8.0" -s390_pv = "0.10.0" \ No newline at end of file +tonic-build = "0.8.0" \ No newline at end of file diff --git a/attestation-service/attestation-service/src/bin/grpc/mod.rs b/attestation-service/attestation-service/src/bin/grpc/mod.rs index c13d17f04a..243463aa1a 100644 --- a/attestation-service/attestation-service/src/bin/grpc/mod.rs +++ b/attestation-service/attestation-service/src/bin/grpc/mod.rs @@ -205,14 +205,18 @@ impl AttestationService for Arc> { request: Request, ) -> Result, Status> { let request: ChallengeRequest = request.into_inner(); - let tee = to_kbs_tee(&request.tee) - .map_err(|e| Status::aborted(format!("parse TEE type: {e}")))?; + let inner_tee = request.inner.get("tee") + .map_or(Err(Status::aborted("Error parse inner_tee tee")), Ok)?; + let tee_params = request.inner.get("tee_params") + .map_or(Err(Status::aborted("Error parse inner_tee tee_params")), Ok)?; + let tee = to_kbs_tee(&inner_tee) + .map_or(Err(Status::aborted(format!("Error parse TEE type: {inner_tee}"))), Ok)?; let attestation_challenge = self .read() .await .attestation_service - .generate_supplemental_challenge(tee, request.tee_params.clone()) + .generate_supplemental_challenge(tee, tee_params.clone()) .await .map_err(|e| Status::aborted(format!("Challenge: {e:?}")))?; diff --git a/attestation-service/attestation-service/src/bin/restful/mod.rs b/attestation-service/attestation-service/src/bin/restful/mod.rs index 668dadfacc..14b5217152 100644 --- a/attestation-service/attestation-service/src/bin/restful/mod.rs +++ b/attestation-service/attestation-service/src/bin/restful/mod.rs @@ -46,8 +46,10 @@ pub struct AttestationRequest { #[derive(Debug, Serialize, Deserialize)] pub struct ChallengeRequest { - tee: String, - tee_params: String, + // tee: String, + // tee_params: String, + #[flatten] + inner: HashMap, } #[derive(Debug, Serialize, Deserialize)] @@ -191,11 +193,11 @@ pub async fn get_challenge( request: web::Json, cocoas: web::Data>>, ) -> Result { - let tee = to_tee(&request.tee)?; + let tee = to_tee(&request.inner.get("tee"))?; let challenge = cocoas .read() .await - .generate_supplemental_challenge(tee, request.tee_params.clone()) + .generate_supplemental_challenge(tee, request.inner.get("tee_params").clone()) .await .context("generate challenge")?; Ok(HttpResponse::Ok().body(challenge)) diff --git a/attestation-service/docs/parsed_claims.md b/attestation-service/docs/parsed_claims.md index e818af6736..e061004f43 100644 --- a/attestation-service/docs/parsed_claims.md +++ b/attestation-service/docs/parsed_claims.md @@ -96,10 +96,11 @@ Note: The TD Report and TD Quote are fetched during early boot in this TEE. Kern ## IBM Secure Execution (SE) - `se.version`: The version this quote structure. - `se.cuid`: The config uid. -- `se.hdr.tag`: SE Header Tag (seht) -- `se.image.phkh`: SE image Public host key hash -- `se.attestation.phkh`: SE attestation Public host key hash +- `se.hdr.tag`: SE header tag (seht) +- `se.image.phkh`: SE image public host key hash +- `se.attestation.phkh`: SE attestation public host key hash - `se.user_data`: Custom attestation key owner data. + ## AMD SEV-SNP - `snp.measurement` Launch Digest covering initial guest memory diff --git a/attestation-service/protos/attestation.proto b/attestation-service/protos/attestation.proto index c60bfa1c6f..ef2b0a346f 100644 --- a/attestation-service/protos/attestation.proto +++ b/attestation-service/protos/attestation.proto @@ -91,9 +91,9 @@ message SetPolicyRequest { message SetPolicyResponse {} message ChallengeRequest { - // TEE enum. Specify the evidence type - string tee = 1; - string tee_params = 2; + // string tee = 1; + // string tee_params = 2; + map inner = 1; } message ChallengeResponse { string attestation_challenge = 1; diff --git a/attestation-service/verifier/src/lib.rs b/attestation-service/verifier/src/lib.rs index d0ada3d832..6e57e5b739 100644 --- a/attestation-service/verifier/src/lib.rs +++ b/attestation-service/verifier/src/lib.rs @@ -174,7 +174,6 @@ pub trait Verifier { /// generate the evidence /// /// A optional `tee_parameters` comes from the attester side as the input. - async fn generate_supplemental_challenge( &self, _tee_parameters: String, diff --git a/attestation-service/verifier/src/se/ibmse.rs b/attestation-service/verifier/src/se/ibmse.rs index c294b4dcb8..bc8748a8ac 100644 --- a/attestation-service/verifier/src/se/ibmse.rs +++ b/attestation-service/verifier/src/se/ibmse.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, bail, Result}; use log::{debug, warn}; use openssl::{ encrypt::{Decrypter, Encrypter}, @@ -24,28 +24,28 @@ use serde_json; use serde_with::{base64::Base64, serde_as}; use std::{fs::File, io::Read}; -fn encrypt_measurement_key(key: &[u8], rsa_public_key: &PKey) -> Vec { - let mut encrypter = Encrypter::new(rsa_public_key).unwrap(); - encrypter.set_rsa_padding(Padding::PKCS1).unwrap(); +fn encrypt_measurement_key(key: &[u8], rsa_public_key: &PKey) -> Result> { + let mut encrypter = Encrypter::new(rsa_public_key)?; + encrypter.set_rsa_padding(Padding::PKCS1)?; - let buffer_len = encrypter.encrypt_len(key).unwrap(); + let buffer_len = encrypter.encrypt_len(key)?; let mut encrypted_hmac_key = vec![0; buffer_len]; - let len = encrypter.encrypt(key, &mut encrypted_hmac_key).unwrap(); + let len = encrypter.encrypt(key, &mut encrypted_hmac_key)?; encrypted_hmac_key.truncate(len); - encrypted_hmac_key + Ok(encrypted_hmac_key) } -fn decrypt_measurement_key(key: &[u8], rsa_private_key: &PKey) -> Vec { - let mut decrypter = Decrypter::new(rsa_private_key).unwrap(); - decrypter.set_rsa_padding(Padding::PKCS1).unwrap(); +fn decrypt_measurement_key(key: &[u8], rsa_private_key: &PKey) -> Result> { + let mut decrypter = Decrypter::new(rsa_private_key)?; + decrypter.set_rsa_padding(Padding::PKCS1)?; - let buffer_len = decrypter.decrypt_len(key).unwrap(); + let buffer_len = decrypter.decrypt_len(key)?; let mut decrypted_hmac_key = vec![0; buffer_len]; - let decrypted_len = decrypter.decrypt(key, &mut decrypted_hmac_key).unwrap(); + let decrypted_len = decrypter.decrypt(key, &mut decrypted_hmac_key)?; decrypted_hmac_key.truncate(decrypted_len); - decrypted_hmac_key + Ok(decrypted_hmac_key) } #[serde_as] @@ -87,14 +87,6 @@ pub struct SeAttestationRequest { } impl SeAttestationRequest { - pub fn from_slice(request: &[u8]) -> Result { - Ok(serde_json::from_slice(request).unwrap()) - } - - pub fn from_string(request: &str) -> Result { - Ok(serde_json::from_str(request).unwrap()) - } - pub fn create( hkds: &Vec, certs: &Vec, @@ -121,8 +113,9 @@ impl SeAttestationRequest { if certs.len() != 1 { warn!("The host key document in '{hkd}' contains more than one certificate!") } - // Panic: len is == 1 -> unwrap will succeed/not panic - let c = certs.first().unwrap(); + let c = certs + .first() + .ok_or(anyhow!("File does not contain a X509 certificate"))?; verifier.verify(c)?; arcb.add_hostkey(c.public_key()?); } @@ -137,9 +130,13 @@ impl SeAttestationRequest { let conf_data = arcb.confidential_data(); let encr_measurement_key = - encrypt_measurement_key(conf_data.measurement_key(), rsa_public_key); - let encr_request_nonce = - encrypt_measurement_key(conf_data.nonce().clone().unwrap().value(), rsa_public_key); + encrypt_measurement_key(conf_data.measurement_key(), rsa_public_key)?; + let binding = conf_data + .nonce() + .clone() + .ok_or(anyhow!("Failed to get nonce binding"))?; + let nonce = binding.value(); + let encr_request_nonce = encrypt_measurement_key(nonce, rsa_public_key)?; Ok(Self { request_blob, @@ -172,34 +169,6 @@ pub struct SeAttestationResponse { } impl SeAttestationResponse { - pub fn from_slice(response: &[u8]) -> Result { - Ok(serde_json::from_slice(response).unwrap()) - } - - pub fn from_string(request: &str) -> Result { - Ok(serde_json::from_str(request).unwrap()) - } - - pub fn create( - measurement: &[u8], - additional_data: &[u8], - user_data: &[u8], - cuid: &ConfigUid, - encr_measurement_key: &[u8], - encr_request_nonce: &[u8], - image_hdr_tags: &BootHdrTags, - ) -> Result { - Ok(Self { - measurement: measurement.to_vec(), - additional_data: additional_data.to_vec(), - user_data: user_data.to_vec(), - cuid: *cuid, - encr_measurement_key: encr_measurement_key.to_vec(), - encr_request_nonce: encr_request_nonce.to_vec(), - image_hdr_tags: *image_hdr_tags, - }) - } - pub fn verify(&self, priv_key_file: &str) -> Result { let mut file = File::open(priv_key_file)?; let mut contents = vec![]; @@ -208,19 +177,16 @@ impl SeAttestationResponse { let rsa = Rsa::private_key_from_pem(&contents)?; let rsa_private_key = &PKey::from_rsa(rsa)?; - let meas_key = decrypt_measurement_key(&self.encr_measurement_key, rsa_private_key); - let nonce = decrypt_measurement_key(&self.encr_request_nonce, rsa_private_key); + let meas_key = decrypt_measurement_key(&self.encr_measurement_key, rsa_private_key)?; + let nonce = decrypt_measurement_key(&self.encr_request_nonce, rsa_private_key)?; if nonce.len() != 16 { - return Err(anyhow!("The nonce vector must have exactly 16 elements.")); + bail!("The nonce vector must have exactly 16 elements."); } - let boxed_slice: Box<[u8]> = nonce.into_boxed_slice(); - let boxed_array: Box<[u8; 16]> = match boxed_slice.try_into() { - Ok(ba) => ba, - Err(_) => return Err(anyhow!("Failed to convert nonce from Vec to [u8; 16].")), - }; - let nonce_array: [u8; 16] = *boxed_array; + let nonce_array: [u8; 16] = nonce + .try_into() + .map_err(|_| anyhow!("Failed to convert nonce from Vec to [u8; 16]."))?; let meas_key = &PKey::hmac(&meas_key)?; let items = AttestationItems::new( &self.image_hdr_tags, @@ -231,14 +197,13 @@ impl SeAttestationResponse { ); let measurement = - AttestationMeasurement::calculate(items, AttestationMeasAlg::HmacSha512, meas_key) - .unwrap(); + AttestationMeasurement::calculate(items, AttestationMeasAlg::HmacSha512, meas_key)?; if !measurement.eq_secure(&self.measurement) { debug!("Recieved: {:?}", self.measurement); debug!("Calculated: {:?}", measurement.as_ref()); warn!("Attestation measurement verification failed. Calculated and received attestation measurement are not equal."); - return Err(anyhow!("Failed to verify the measurement!")); + bail!("Failed to verify the measurement!"); } // let userdata = serde_json::from_slice(&self.user_data)?; @@ -286,7 +251,7 @@ pub fn create( debug!("pub_key_file: {:#?}", pub_key_file); let mut hdr_file = open_file(se_img_hdr)?; - let mut image_hdr_tags = BootHdrTags::from_se_image(&mut hdr_file).unwrap(); + let mut image_hdr_tags = BootHdrTags::from_se_image(&mut hdr_file)?; let root_ca = Some(ca); let se_request = SeAttestationRequest::create( @@ -296,8 +261,7 @@ pub fn create( root_ca, &mut image_hdr_tags, pub_key_file, - ) - .unwrap(); + )?; let challenge = serde_json::to_string(&se_request)?; debug!("challenge json: {:#?}", challenge); @@ -309,7 +273,7 @@ pub fn verify(response: &[u8], priv_key_file: &str) -> Result String { + if let Ok(env_path) = env::var("SE_HOST_KEY_DOCUMENTS_ROOT") { + return env_path; + } + DEFAULT_SE_HOST_KEY_DOCUMENTS_ROOT.into() +} + +fn get_certs_root() -> String { + if let Ok(env_path) = env::var("SE_CERTIFICATES_ROOT") { + return env_path; + } + DEFAULT_SE_CERTIFICATES_ROOT.into() +} + +fn get_root_ca_file() -> String { + if let Ok(env_path) = env::var("SE_CERTIFICATE_ROOT_CA") { + return env_path; + } + DEFAULT_SE_CERTIFICATE_ROOT_CA.into() +} + +fn get_crls_root() -> String { + if let Ok(env_path) = env::var("SE_CERTIFICATE_REVOCATION_LISTS_ROOT") { + return env_path; + } + DEFAULT_SE_CERTIFICATE_REVOCATION_LISTS_ROOT.into() +} + +fn get_img_hdr_file() -> String { + if let Ok(env_path) = env::var("SE_IMAGE_HEADER_FILE") { + return env_path; + } + DEFAULT_SE_IMAGE_HEADER_FILE.into() +} + +fn get_encrypt_priv_keyfile() -> String { + if let Ok(env_path) = env::var("SE_MEASUREMENT_ENCR_KEY_PRIVATE") { + return env_path; + } + DEFAULT_SE_MEASUREMENT_ENCR_KEY_PRIVATE.into() +} + +fn get_encrypt_pub_keyfile() -> String { + if let Ok(env_path) = env::var("SE_MEASUREMENT_ENCR_KEY_PUBLIC") { + return env_path; + } + DEFAULT_SE_MEASUREMENT_ENCR_KEY_PUBLIC.into() +} + fn list_files_in_folder(dir: &str) -> Result> { let mut file_paths = Vec::new(); @@ -56,19 +106,18 @@ impl Verifier for SeVerifier { &self, _tee_parameters: String, ) -> Result { - let hkds = list_files_in_folder(SE_HOST_KEY_DOCUMENTS_ROOT)?; - let certs = list_files_in_folder(SE_CERTIFICATES_ROOT)?; - let crls = list_files_in_folder(SE_CERTIFICATE_REVOCATION_LISTS_ROOT)?; - let ca = String::from(SE_CERTIFICATE_ROOT_CA); + let hkds = list_files_in_folder(&get_hkds_root())?; + let certs = list_files_in_folder(&get_certs_root())?; + let crls = list_files_in_folder(&get_crls_root())?; // challenge is Serialized SeAttestationRequest, attester uses it to do perform action // attester then generates and return Serialized SeAttestationResponse ibmse::create( &hkds, &certs, &crls, - ca, - SE_IMAGE_HEADER_FILE, - SE_MEASUREMENT_ENCR_KEY_PUBLIC, + get_root_ca_file(), + &get_img_hdr_file(), + &get_encrypt_pub_keyfile(), ) } } @@ -79,7 +128,7 @@ async fn verify_evidence( _expected_init_data_hash: &InitDataHash<'_>, ) -> Result { // evidence is serialized SeAttestationResponse String bytes - let mut se_claims = ibmse::verify(evidence, SE_MEASUREMENT_ENCR_KEY_PRIVATE)?; + let mut se_claims = ibmse::verify(evidence, &get_encrypt_priv_keyfile())?; se_generate_parsed_claim(&mut se_claims).map_err(|e| anyhow!("error from se Verifier: {:?}", e)) } diff --git a/kbs/src/api/src/attestation/coco/grpc.rs b/kbs/src/api/src/attestation/coco/grpc.rs index ef3cdf0ef3..12835c1aac 100644 --- a/kbs/src/api/src/attestation/coco/grpc.rs +++ b/kbs/src/api/src/attestation/coco/grpc.rs @@ -28,22 +28,6 @@ pub const DEFAULT_POOL_SIZE: u64 = 100; pub const COCO_AS_HASH_ALGORITHM: &str = "sha384"; -fn to_grpc_tee(tee: Tee) -> Option { - match tee { - Tee::AzSnpVtpm => Some(String::from("azsnpvtpm")), - Tee::AzTdxVtpm => Some(String::from("aztdxvtpm")), - Tee::Cca => Some(String::from("cca")), - Tee::Csv => Some(String::from("csv")), - Tee::Sample => Some(String::from("sample")), - Tee::Sev => Some(String::from("sev")), - Tee::Sgx => Some(String::from("sgx")), - Tee::Snp => Some(String::from("snp")), - Tee::Tdx => Some(String::from("tdx")), - Tee::Se => Some(String::from("se")), - _ => None, - } -} - #[derive(Clone, Debug, Deserialize)] pub struct GrpcConfig { as_addr: Option, @@ -140,10 +124,16 @@ impl Attest for GrpcClientPool { tee: Tee, tee_parameters: String, ) -> Result { - let grpc_tee = to_grpc_tee(tee).unwrap(); + let tee = serde_json::to_string(&tee) + .context("CoCo AS client: serialize tee type failed.")? + .trim_end_matches('"') + .trim_start_matches('"') + .to_string(); + let mut inner = HashMap::new(); + inner.insert("tee", grpc_tee); + inner.insert("tee_params", tee_parameters); let req = tonic::Request::new(ChallengeRequest { - tee: grpc_tee, - tee_params: tee_parameters, + inner, }); let mut client = { self.pool.lock().await.get().await? };