pv: Require matching versions of request and HKD

Enforce that a v{1,2} request also has a v{1,2} hostkey.
This requires to change the signature of Request::add_hostkey to return
a Result.

Co-Developed-by: Timo Keller <tkeller@linux.ibm.com>
Signed-off-by: Timo Keller <tkeller@linux.ibm.com>
Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
Reviewed-by: Marc Hartmayer <marc@linux.ibm.com>
Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>

rust/pv/src/error.rs

@@ -144,6 +144,9 @@ pub enum Error {
 
     #[error("{}", .0)]
     InvalidHkd(String),
+
+    #[error("All host keys must use the same version (all hybrid or all non-hybrid)")]
+    MixedHostkeyVersions,
 }
 
 // used in macros

rust/pv/src/req/request.rs

@@ -51,7 +51,7 @@ pub trait Request {
     /// Add a host-key to this request
     ///
     /// Must be called at least once, otherwise {`Request::encrypt`} will fail
-    fn add_hostkey(&mut self, hostkey: HostKey);
+    fn add_hostkey(&mut self, hostkey: HostKey) -> Result<()>;
 }
 
 /// A struct to represent some parts of a binary/encrypted request.

rust/pv/src/uvattest/arcb.rs

@@ -61,9 +61,9 @@ use crate::{
 /// let hkd = s390_pv::misc::read_certs(&std::fs::read("host-key-document.crt")?)?;
 /// // IBM issued HKD certificates typically have one X509
 /// let hkd = hkd.first().unwrap().public_key()?;
-/// arcb.add_hostkey(HostKey::V1(hkd));
+/// arcb.add_hostkey(HostKey::V1(hkd))?;
 /// // you can add multiple hostkeys
-/// // arcb.add_hostkey(HostKey::V1(another_hkd));
+/// // arcb.add_hostkey(HostKey::V1(another_hkd))?;
 /// // encrypt it
 /// let ctx = ReqEncrCtx::random(SymKeyType::Aes256Gcm)?;
 /// let arcb = arcb.encrypt(&ctx)?;
@@ -243,8 +243,19 @@ impl Request for AttestationRequest {
         ctx.encrypt_aead(&aad, conf).map(|res| res.into_buf())
     }
 
-    fn add_hostkey(&mut self, hostkey: HostKey) {
-        self.keyslots.push(Keyslot::new(hostkey))
+    fn add_hostkey(&mut self, hostkey: HostKey) -> Result<()> {
+        match self.version {
+            AttestationVersion::One if !hostkey.is_hybrid() => Ok(()),
+            AttestationVersion::Two if hostkey.is_hybrid() => Ok(()),
+            AttestationVersion::One => Err(Error::InvalidHkd(
+                "Add classical hostkey to a v1 attestation request".to_string(),
+            )),
+            AttestationVersion::Two => Err(Error::InvalidHkd(
+                "Add hybrid key to a v2 attetstation request".to_string(),
+            )),
+        }?;
+        self.keyslots.push(Keyslot::new(hostkey));
+        Ok(())
     }
 }
 
@@ -457,7 +468,7 @@ mod test {
         arcb.conf.value_mut().nonce = NONCE;
         arcb.conf.value_mut().meas_key = MEAS;
 
-        arcb.add_hostkey(host_key);
+        arcb.add_hostkey(host_key).unwrap();
         arcb.encrypt(&ctx).unwrap()
     }
 
@@ -487,7 +498,7 @@ mod test {
         arcb.conf.value_mut().nonce = NONCE;
         arcb.conf.value_mut().meas_key = MEAS;
 
-        arcb.add_hostkey(host_key);
+        arcb.add_hostkey(host_key).unwrap();
         arcb.encrypt(&ctx).unwrap()
     }
 
@@ -584,7 +595,8 @@ mod test {
         )
         .unwrap();
         let (_, host_key1, host_key2) = get_test_keys_hybrid();
-        arcb.add_hostkey(HostKey::V2(HybridPKey::new(host_key1, host_key2).unwrap()));
+        arcb.add_hostkey(HostKey::V2(HybridPKey::new(host_key1, host_key2).unwrap()))
+            .unwrap();
 
         assert_eq!(arcb.version(), AttestationVersion::Two);
     }

rust/pv/src/uvsecret/asrcb.rs

@@ -511,7 +511,20 @@ impl Request for AddSecretRequest {
         }
     }
 
-    fn add_hostkey(&mut self, hostkey: HostKey) {
-        self.keyslots.push(Keyslot::new(hostkey))
+    fn add_hostkey(&mut self, hostkey: HostKey) -> Result<()> {
+        match self.version {
+            AddSecretVersion::One if !hostkey.is_hybrid() => Ok(()),
+            AddSecretVersion::Two if hostkey.is_hybrid() => Ok(()),
+            AddSecretVersion::One => Err(Error::InvalidHkd(
+                "Add classical hostkey to a v1 attestation request".to_string(),
+            )),
+            AddSecretVersion::Two => Err(Error::InvalidHkd(
+                "Add hybrid key to a v2 attetstation request".to_string(),
+            )),
+            #[cfg(any(debug_assertions, test))]
+            AddSecretVersion::Inv => panic!("Invalid version for production use"),
+        }?;
+        self.keyslots.push(Keyslot::new(hostkey));
+        Ok(())
     }
 }

rust/pv/tests/add_secret_request.rs

@@ -43,7 +43,7 @@ fn create_asrcb(
         asrcb.set_cuid(c);
     };
 
-    asrcb.add_hostkey(hkd);
+    asrcb.add_hostkey(hkd)?;
     asrcb.encrypt(ctx)
 }
 
@@ -128,7 +128,7 @@ fn create_signed_asrcb(skey: PKey<Private>, user_data: Vec<u8>) -> Vec<u8> {
         AddSecretRequest::new(AddSecretVersion::One, GuestSecret::Null, TAGS, no_flag())
             .expect("AddSecretRequest::new failed");
 
-    asrcb.add_hostkey(host_key);
+    asrcb.add_hostkey(host_key).unwrap();
     asrcb.set_user_data(user_data, Some(skey)).unwrap();
     asrcb.encrypt(&ctx).unwrap()
 }
@@ -139,7 +139,7 @@ fn create_signed_asrcb_v2(skey: PKey<Private>, user_data: Vec<u8>) -> Vec<u8> {
         AddSecretRequest::new(AddSecretVersion::Two, GuestSecret::Null, TAGS, no_flag())
             .expect("AddSecretRequest::new failed");
 
-    asrcb.add_hostkey(host_key);
+    asrcb.add_hostkey(host_key).unwrap();
     asrcb.set_user_data(user_data, Some(skey)).unwrap();
     asrcb.encrypt(&ctx).unwrap()
 }
@@ -152,7 +152,7 @@ fn null_none_default_ncuid_one_user_unsgn() {
         AddSecretRequest::new(AddSecretVersion::One, GuestSecret::Null, TAGS, no_flag())
             .expect("AddSecretRequest::new failed");
 
-    asrcb.add_hostkey(host_key);
+    asrcb.add_hostkey(host_key).unwrap();
     asrcb.set_user_data(user_data_orig.clone(), None).unwrap();
     let asrcb = asrcb.encrypt(&ctx).unwrap();
 
@@ -287,7 +287,9 @@ fn null_none_default_cuid_seven() {
     let mut asrcb =
         AddSecretRequest::new(AddSecretVersion::One, GuestSecret::Null, TAGS, no_flag())
             .expect("AddSecretRequest::new failed");
-    (0..7).for_each(|_| asrcb.add_hostkey(hkd.clone()));
+    for _ in 0..7 {
+        asrcb.add_hostkey(hkd.clone()).unwrap()
+    }
     asrcb.set_cuid(CUID);
     let asrcb = asrcb.encrypt(&ctx).unwrap();
 

rust/pvattest/src/cmd/create.rs

@@ -74,7 +74,9 @@ pub fn create(opt: &CreateAttOpt) -> Result<ExitCode> {
     debug!("Generated Attestation request");
 
     // Add host-key documents
-    hkds.into_iter().for_each(|k| arcb.add_hostkey(k));
+    for k in hkds.into_iter() {
+        arcb.add_hostkey(k)?
+    }
     debug!("Added all host-keys");
 
     let encr_ctx =

rust/pvimg/src/se_img.rs

@@ -327,6 +327,15 @@ impl<W: Write + Seek> SeImgBuilder<W> {
             None => return Err(Error::NoHostkey.into()),
         };
 
+        // Verify all hostkeys have the same version -> test if all or none are hybrid
+        let hybrid = matches!(version, SeHdrVersion::V2);
+        sehdr_args
+            .keys
+            .iter()
+            .all(|k| k.is_hybrid() == hybrid)
+            .then_some(())
+            .ok_or(pv::Error::MixedHostkeyVersions)?;
+
         let mut se_hdr_builder = SeHdrBuilder::new(
             version,
             PSW {

rust/pvsecret/src/cmd/create.rs

@@ -210,7 +210,9 @@ fn build_asrcb(opt: &CreateSecretOpt) -> Result<AddSecretRequest> {
 
     let mut asrcb = AddSecretRequest::new(secret_version, secret, boot_tags, flags)?;
 
-    hkds.into_iter().for_each(|k| asrcb.add_hostkey(k));
+    for k in hkds.into_iter() {
+        asrcb.add_hostkey(k)?
+    }
 
     debug!("Added all host-keys");