Verifier: IBM SE to fix review comments

Signed-off-by: Qi Feng Huo <[email protected]>

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"
+tonic-build = "0.8.0"

attestation-service/attestation-service/src/bin/grpc/mod.rs

@@ -205,14 +205,18 @@ impl AttestationService for Arc<RwLock<AttestationServer>> {
         request: Request<ChallengeRequest>,
     ) -> Result<Response<ChallengeResponse>, 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:?}")))?;
 

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<String, String>,
 }
 
 #[derive(Debug, Serialize, Deserialize)]
@@ -191,11 +193,11 @@ pub async fn get_challenge(
     request: web::Json<ChallengeRequest>,
     cocoas: web::Data<Arc<RwLock<AttestationService>>>,
 ) -> Result<HttpResponse> {
-    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))

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

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<string, string> inner = 1;
 }
 message ChallengeResponse {
     string attestation_challenge = 1;

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,

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<Public>) -> Vec<u8> {
-    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<Public>) -> Result<Vec<u8>> {
+    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<Private>) -> Vec<u8> {
-    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<Private>) -> Result<Vec<u8>> {
+    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<Self> {
-        Ok(serde_json::from_slice(request).unwrap())
-    }
-
-    pub fn from_string(request: &str) -> Result<Self> {
-        Ok(serde_json::from_str(request).unwrap())
-    }
-
     pub fn create(
         hkds: &Vec<String>,
         certs: &Vec<String>,
@@ -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<Self> {
-        Ok(serde_json::from_slice(response).unwrap())
-    }
-
-    pub fn from_string(request: &str) -> Result<Self> {
-        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<Self> {
-        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<SeAttestationClaims> {
         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<u8> 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<u8> 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<SeAttestationClaim
     // response is serialized SeAttestationResponse String bytes
     let response_str = std::str::from_utf8(response)?;
     debug!("SeAttestationResponse json: {:#?}", response_str);
-    let se_response = SeAttestationResponse::from_string(response_str)?;
+    let se_response: SeAttestationResponse = serde_json::from_str(response_str)?;
 
     let claims = se_response.verify(priv_key_file)?;
     debug!("claims json: {:#?}", claims);