rust/pv: Remove internal BinGuestSecret struct

The BinGuestSecret type provides no benefits. The public GuestSecret
struct can handle everything. Therefore, move the two functions from bin
to the non-bin variant. While at it, use a struct to define the binary
structure instead of copy numbers to some positions in a Vec. This
simplifies the addition of further secret types.

Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
Reviewed-by: Julian Ruess <julianr@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>

rust/pv/src/uvsecret/asrcb.rs

@@ -22,37 +22,6 @@ use crate::{
 use pv_core::request::RequestVersion;
 use zerocopy::AsBytes;
 
-/// Internal wrapper for Guest Secret, so that we can dump it in the form the UV wants it to be
-#[derive(Debug)]
-struct BinGuestSecret(GuestSecret);
-impl BinGuestSecret {
-    /// Reference to the confidential data
-    fn confidential(&self) -> &[u8] {
-        match &self.0 {
-            GuestSecret::Null => &[],
-            GuestSecret::Association { secret, .. } => secret.value().as_slice(),
-        }
-    }
-    fn dump_auth(&self) -> Vec<u8> {
-        match &self.0 {
-            GuestSecret::Null => vec![0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
-            GuestSecret::Association { id, .. } => {
-                let mut buf = vec![0; 48];
-                buf[3] = 2;
-                buf[7] = 0x20;
-                buf[16..48].copy_from_slice(id.as_ref());
-                buf
-            }
-        }
-    }
-}
-
-impl From<GuestSecret> for BinGuestSecret {
-    fn from(secret: GuestSecret) -> Self {
-        BinGuestSecret(secret)
-    }
-}
-
 /// Authenticated data w/o user data
 #[repr(C)]
 #[derive(Debug, Clone, Copy, AsBytes)]
@@ -77,7 +46,7 @@ impl ReqAuthData {
 
 #[derive(Debug)]
 struct ReqConfData {
-    secret: BinGuestSecret,
+    secret: GuestSecret,
     extension_secret: Secret<[u8; 32]>,
 }
 
@@ -191,7 +160,7 @@ impl AddSecretRequest {
         AddSecretRequest {
             conf: ReqConfData {
                 extension_secret: Secret::new([0; 32]),
-                secret: secret.into(),
+                secret,
             },
             aad: ReqAuthData::new(boot_tags, flags),
             keyslots: vec![],
@@ -227,7 +196,7 @@ impl AddSecretRequest {
 
     /// Returns a reference to the guest secret of this [`AddSecretRequest`].
     pub fn guest_secret(&self) -> &GuestSecret {
-        &self.conf.secret.0
+        &self.conf.secret
     }
 
     /// Add user-data to the Add-Secret request
@@ -249,7 +218,7 @@ impl AddSecretRequest {
     /// compiles the authenticated area of this request
     fn aad(&self, ctx: &ReqEncrCtx, conf_len: usize) -> Result<Vec<u8>> {
         let cust_pub_key = ctx.key_coords()?;
-        let secr_auth = self.conf.secret.dump_auth();
+        let secr_auth = self.conf.secret.auth();
         let user_data = self.user_data.data();
 
         let mut aad: Vec<Aad> = Vec::with_capacity(5 + self.keyslots.len());
@@ -262,7 +231,7 @@ impl AddSecretRequest {
         }
         aad.push(Aad::Plain(cust_pub_key.as_ref()));
         self.keyslots.iter().for_each(|k| aad.push(Aad::Ks(k)));
-        aad.push(Aad::Plain(&secr_auth));
+        aad.push(Aad::Plain(secr_auth.get()));
 
         ctx.build_aad(self.version.into(), &aad, conf_len, self.user_data.magic())
     }
@@ -329,34 +298,3 @@ impl Request for AddSecretRequest {
         self.keyslots.push(Keyslot::new(hostkey))
     }
 }
-
-#[cfg(test)]
-mod test {
-    use super::*;
-
-    #[test]
-    fn guest_secret_bin_null() {
-        let gs: BinGuestSecret = GuestSecret::Null.into();
-        let gs_bytes = gs.dump_auth();
-        let exp = vec![0u8, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
-
-        assert_eq!(exp, gs_bytes);
-        assert_eq!(&Vec::<u8>::new(), gs.confidential())
-    }
-
-    #[test]
-    fn guest_secret_bin_ap() {
-        let gs: BinGuestSecret = GuestSecret::Association {
-            name: "test".to_string(),
-            id: [1; 32].into(),
-            secret: [2; 32].into(),
-        }
-        .into();
-        let gs_bytes_auth = gs.dump_auth();
-        let mut exp = vec![0u8, 0, 0, 2, 0, 0, 0, 0x20, 0, 0, 0, 0, 0, 0, 0, 0];
-        exp.extend([1; 32]);
-
-        assert_eq!(exp, gs_bytes_auth);
-        assert_eq!(&[2; 32], gs.confidential());
-    }
-}

rust/pv/src/uvsecret/guest_secret.rs

@@ -8,11 +8,14 @@ use crate::{
     request::{hash, openssl::MessageDigest, random_array, Secret},
     Result,
 };
-use pv_core::uv::SecretId;
+use byteorder::BigEndian;
+use pv_core::uv::{ListableSecretType, SecretId};
 use serde::{Deserialize, Serialize};
-use std::convert::TryInto;
+use std::{convert::TryInto, fmt::Display};
+use utils::assert_size;
+use zerocopy::{AsBytes, U16, U32};
 
-const SECRET_SIZE: usize = 32;
+const ASSOC_SECRET_SIZE: usize = 32;
 
 /// A Secret to be added in [`AddSecretRequest`]
 #[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
@@ -29,7 +32,7 @@ pub enum GuestSecret {
         id: SecretId,
         /// Confidential actual association secret (32 bytes)
         #[serde(skip)]
-        secret: Secret<[u8; SECRET_SIZE]>,
+        secret: Secret<[u8; ASSOC_SECRET_SIZE]>,
     },
 }
 
@@ -44,7 +47,7 @@ impl GuestSecret {
     /// This function will return an error if OpenSSL cannot create a hash.
     pub fn association<O>(name: &str, secret: O) -> Result<GuestSecret>
     where
-        O: Into<Option<[u8; SECRET_SIZE]>>,
+        O: Into<Option<[u8; ASSOC_SECRET_SIZE]>>,
     {
         let id: [u8; SecretId::ID_SIZE] = hash(MessageDigest::sha256(), name.as_bytes())?
             .to_vec()
@@ -61,6 +64,101 @@ impl GuestSecret {
             secret: secret.into(),
         })
     }
+
+    /// Reference to the confidential data
+    pub(crate) fn confidential(&self) -> &[u8] {
+        match &self {
+            GuestSecret::Null => &[],
+            GuestSecret::Association { secret, .. } => secret.value().as_slice(),
+        }
+    }
+    /// Creates the non-confidential part of the secret ad-hoc
+    pub(crate) fn auth(&self) -> SecretAuth {
+        match &self {
+            GuestSecret::Null => SecretAuth::Null,
+            //Panic:  every non null secret type is listable -> no panic
+            listable => {
+                SecretAuth::Listable(ListableSecretHdr::from_guest_secret(listable).unwrap())
+            }
+        }
+    }
+
+    /// Returns the UV type ID
+    fn kind(&self) -> u16 {
+        match self {
+            // Null is not listable, but the ListableSecretType provides the type constant (1)
+            GuestSecret::Null => ListableSecretType::NULL,
+            GuestSecret::Association { .. } => ListableSecretType::ASSOCIATION,
+        }
+    }
+
+    /// Size of the secret value
+    fn secret_len(&self) -> u32 {
+        match self {
+            GuestSecret::Null => 0,
+            GuestSecret::Association { secret, .. } => secret.value().len() as u32,
+        }
+    }
+
+    /// Returns the ID of the secret type (if any)
+    fn id(&self) -> Option<SecretId> {
+        match self {
+            GuestSecret::Null => None,
+            GuestSecret::Association { id, .. } => Some(id.to_owned()),
+        }
+    }
+}
+
+impl Display for GuestSecret {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            GuestSecret::Null => write!(f, "Meta"),
+            gs => {
+                let kind: U16<BigEndian> = gs.kind().into();
+                let st: ListableSecretType = kind.into();
+                write!(f, "{st}")
+            }
+        }
+    }
+}
+
+#[derive(Debug)]
+pub(crate) enum SecretAuth {
+    Null,
+    Listable(ListableSecretHdr),
+}
+
+impl SecretAuth {
+    pub fn get(&self) -> &[u8] {
+        match self {
+            SecretAuth::Null => &[0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
+            SecretAuth::Listable(h) => h.as_bytes(),
+        }
+    }
+}
+
+#[repr(C)]
+#[derive(Debug, AsBytes)]
+pub(crate) struct ListableSecretHdr {
+    res0: u16,
+    kind: U16<BigEndian>,
+    secret_len: U32<BigEndian>,
+    res8: u64,
+    id: SecretId,
+}
+assert_size!(ListableSecretHdr, 0x30);
+
+impl ListableSecretHdr {
+    fn from_guest_secret(gs: &GuestSecret) -> Option<Self> {
+        let id = gs.id()?;
+        Some(Self {
+            res0: 0,
+            kind: gs.kind().into(),
+            secret_len: gs.secret_len().into(),
+            res8: 0,
+            id,
+        })
+    }
 }
 
 #[cfg(test)]
@@ -116,4 +214,29 @@ mod test {
             ],
         );
     }
+
+    #[test]
+    fn guest_secret_bin_null() {
+        let gs = GuestSecret::Null;
+        let gs_bytes = gs.auth();
+        let exp = vec![0u8, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
+
+        assert_eq!(exp, gs_bytes.get());
+        assert_eq!(&Vec::<u8>::new(), gs.confidential())
+    }
+
+    #[test]
+    fn guest_secret_bin_ap() {
+        let gs = GuestSecret::Association {
+            name: "test".to_string(),
+            id: [1; 32].into(),
+            secret: [2; 32].into(),
+        };
+        let gs_bytes_auth = gs.auth();
+        let mut exp = vec![0u8, 0, 0, 2, 0, 0, 0, 0x20, 0, 0, 0, 0, 0, 0, 0, 0];
+        exp.extend([1; 32]);
+
+        assert_eq!(exp, gs_bytes_auth.get());
+        assert_eq!(&[2; 32], gs.confidential());
+    }
 }

rust/pv_core/src/uvdevice/secret_list.rs

@@ -291,10 +291,13 @@ pub enum ListableSecretType {
     /// Unknown secret type
     Unknown(u16),
 }
+
 impl ListableSecretType {
     const RESERVED_0: u16 = 0x0000;
-    const NULL: u16 = 0x0001;
-    const ASSOCIATION: u16 = 0x0002;
+    /// UV type id for a null secret
+    pub const NULL: u16 = 0x0001;
+    /// UV type id for an association secret
+    pub const ASSOCIATION: u16 = 0x0002;
 }
 
 impl Display for ListableSecretType {