pvattest: Refactor firmware checking

Refactor the firmware verification client such that adding a new
request/response versions is simpler.

Reviewed-by: Marc Hartmayer <marc@linux.ibm.com>
Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>

rust/pvattest/src/cmd/check.rs

@@ -15,7 +15,7 @@ use pv::misc::{create_file, open_file, read_file};
 use serde::Serialize;
 use utils::HexSlice;
 
-use self::firmware::firmware_check;
+use self::firmware::firmware_check_v1;
 use self::host_key::{host_key_check, HostKeyCheck};
 use self::secret_store::{secret_store_check, SecretStoreCheck};
 use crate::additional::AttestationResult;
@@ -120,7 +120,7 @@ pub fn check(opt: &CheckOpt) -> Result<ExitCode> {
     let user_data = user_data_check(opt, &att_res)?.check(&mut issues);
     let secret_store = secret_store_check(opt, &att_res)?.check(&mut issues);
 
-    let firmware_check = firmware_check(opt, &att_res)?;
+    let firmware_check = firmware_check_v1(opt, &att_res)?;
     let valid_firmware = match firmware_check {
         CheckState::None => None,
         CheckState::Data(_) => Some(true),

rust/pvattest/src/cmd/check/firmware.rs

@@ -2,7 +2,7 @@
 //
 // Copyright IBM Corp. 2024
 
-use std::fmt::Display;
+use std::fmt::{Debug, Display};
 use std::time::Duration;
 
 use anyhow::{bail, Result};
@@ -16,41 +16,90 @@ use crate::additional::AttestationResult;
 use crate::cli::CheckOpt;
 
 const CHECK_DEFAULT_ENDP: &str = "https://esupport.ibm.com/eccedge/ent/z";
-const VERIFY_API: &str = "hmrs/firmware/attestation/v1/verify";
 const TIMEOUT_MAX: Duration = Duration::from_secs(3);
 const USER_AGENT: &str = "s390-tools-pvattest";
 const CONTENT_TYPE: &str = "Content-Type: application/json";
 const CLIENT_ID: &str = "x-client-id: X";
 
-#[derive(Debug, Serialize)]
-struct Request {
-    version: String,
+#[derive(Debug, Clone, Copy, Serialize, Deserialize)]
+enum Version {
+    #[serde(rename = "1.0")]
+    V1,
+}
+
+trait Request: Serialize + Debug {
+    type Response: Response;
+
+    fn new(firmware_hash: &[u8]) -> Self;
+}
+
+#[derive(Debug, Serialize, Deserialize)]
+struct RequestV1_1 {
+    version: Version,
     payload: String,
 }
 
-impl Request {
-    const VERSION_ONE: &'static str = "1.0";
+impl Request for RequestV1_1 {
+    type Response = ResponseV1;
 
-    fn new_v1(firmware_hash: &[u8]) -> Self {
+    fn new(firmware_hash: &[u8]) -> Self {
         Self {
-            version: Self::VERSION_ONE.to_string(),
+            version: Version::V1,
             payload: BASE64_STANDARD.encode(firmware_hash),
         }
     }
 }
 
+/// Trait for firmware verification response types.
+///
+/// This trait defines the interface for handling responses from the IBM firmware
+/// verification API.
+trait Response: serde::de::DeserializeOwned + Debug + Display {
+    /// The API version constant for this response type.
+    const VERSION: Version;
+
+    /// Returns whether the firmware verification was successful.
+    ///
+    /// # Returns
+    ///
+    /// `true` if the firmware is in a valid state, `false` otherwise.
+    fn valid(&self) -> bool;
+
+    /// Constructs the verification API endpoint URL for this response version.
+    ///
+    /// * `endp` - The base endpoint URL (e.g., "<https://esupport.ibm.com/eccedge/ent/z>")
+    ///
+    /// # Returns
+    ///
+    /// The complete API endpoint URL for firmware verification, including the version path.
+    fn verify_api(endp: &str) -> String {
+        let ver = match Self::VERSION {
+            Version::V1 => "v1",
+        };
+        format!("{endp}/hmrs/firmware/attestation/{ver}/verify",)
+    }
+}
+
+// allow unused because all fields are provided by the REST API but may be unused by this toolk
 #[allow(unused)]
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
-struct Response {
-    version: String,
+struct ResponseV1 {
+    version: Version,
     valid: bool,
     reference_id: String,
     #[serde(default)]
     reason: Option<String>,
 }
 
-impl Display for Response {
+impl Response for ResponseV1 {
+    const VERSION: Version = Version::V1;
+    fn valid(&self) -> bool {
+        self.valid
+    }
+}
+
+impl Display for ResponseV1 {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(
             f,
@@ -73,10 +122,14 @@ impl Handler for Buf {
     }
 }
 
-fn check<U: AsRef<[u8]>>(fw_hash: &U, endp: &str) -> Result<CheckState<()>> {
-    let req = serde_json::to_vec(&Request::new_v1(fw_hash.as_ref()))?;
+fn check<Q, U>(fw_hash: &U, endp: &str) -> Result<CheckState<()>>
+where
+    Q: Request,
+    U: AsRef<[u8]>,
+{
+    let req = serde_json::to_vec(&Q::new(fw_hash.as_ref()))?;
 
-    let url = format!("{endp}/{VERIFY_API}");
+    let url = Q::Response::verify_api(endp);
     debug!("POST {url}");
 
     let mut http_header = List::new();
@@ -105,7 +158,7 @@ fn check<U: AsRef<[u8]>>(fw_hash: &U, endp: &str) -> Result<CheckState<()>> {
         );
     }
 
-    let resp: Response = match serde_json::from_slice(&handle.get_ref().0) {
+    let resp: Q::Response = match serde_json::from_slice(&handle.get_ref().0) {
         Ok(res) => res,
         Err(e) => bail!(
             "Unexpected response from server: {} \n (\"{}\")",
@@ -117,7 +170,7 @@ fn check<U: AsRef<[u8]>>(fw_hash: &U, endp: &str) -> Result<CheckState<()>> {
 
     debug!("Firmware check {resp:?}");
 
-    match resp.valid {
+    match resp.valid() {
         true => info!("✓ {resp}"),
         false => bail_check!(&format!("{resp}")),
     }
@@ -125,7 +178,10 @@ fn check<U: AsRef<[u8]>>(fw_hash: &U, endp: &str) -> Result<CheckState<()>> {
     Ok(CheckState::Data(()))
 }
 
-pub fn firmware_check(opt: &CheckOpt, att_res: &AttestationResult) -> Result<CheckState<()>> {
+fn firmware_check<Q>(opt: &CheckOpt, att_res: &AttestationResult) -> Result<CheckState<()>>
+where
+    Q: Request,
+{
     if !opt.firmware {
         return Ok(None.into());
     }
@@ -140,7 +196,7 @@ pub fn firmware_check(opt: &CheckOpt, att_res: &AttestationResult) -> Result<Che
         .as_ref()
         .and_then(|add| add.firmware_state())
     {
-        Some(hash) => check(hash, endp),
+        Some(hash) => check::<Q, _>(hash, endp),
         None => {
             bail_check!(
                 "The Attestation response contains no firmware hash, but checking was enabled"
@@ -148,3 +204,64 @@ pub fn firmware_check(opt: &CheckOpt, att_res: &AttestationResult) -> Result<Che
         }
     }
 }
+
+pub fn firmware_check_v1(opt: &CheckOpt, att_res: &AttestationResult) -> Result<CheckState<()>> {
+    firmware_check::<RequestV1_1>(opt, att_res)
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn serialize_request_v1_1() {
+        let payload = BASE64_STANDARD.encode([42u8; 320]);
+        let req = RequestV1_1::new(&[42u8; 320]);
+
+        assert!(matches!(req.version, Version::V1));
+        assert_eq!(req.payload, payload);
+
+        let json = serde_json::to_string(&req).unwrap();
+        let expected = format!(r#"{{"version":"1.0","payload":"{payload}"}}"#);
+        assert_eq!(json, expected);
+    }
+
+    #[test]
+    fn parse_response_v1() {
+        let json = r#"{
+  "version": "1.0",
+  "valid": true,
+  "referenceId": "ref-1",
+  "reason": "string"
+}"#;
+
+        let resp: ResponseV1 = serde_json::from_str(json).unwrap();
+        assert!(resp.valid());
+        assert!(matches!(resp.version, Version::V1));
+        assert_eq!(resp.reference_id, "ref-1");
+        assert_eq!(resp.reason.as_deref(), Some("string"));
+
+        let display = resp.to_string();
+        let expected = "The firmware is in a valid state\n  Reason: string\n  ReferenceId: ref-1";
+        assert_eq!(display, expected);
+    }
+
+    #[test]
+    fn parse_response_v1_without_reason() {
+        let json = r#"{
+  "version": "1.0",
+  "valid": true,
+  "referenceId": "ref-1"
+}"#;
+
+        let resp: ResponseV1 = serde_json::from_str(json).unwrap();
+        assert!(resp.valid());
+        assert!(matches!(resp.version, Version::V1));
+        assert_eq!(resp.reference_id, "ref-1");
+        assert_eq!(resp.reason, None);
+
+        let display = resp.to_string();
+        let expected = "The firmware is in a valid state";
+        assert_eq!(display, expected);
+    }
+}