rust/pv_core: Add decode_hex

It's uncommon and prone to error to silently stop decoding/parsing a
hex-string if there is an invalid character. Therefore, add a new
function `decode_hex` which fixes this behavior and use it in the code.

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

rust/pv_core/src/error.rs

@@ -68,6 +68,12 @@ pub enum Error {
     Io(#[from] std::io::Error),
     #[error(transparent)]
     ParseInt(#[from] std::num::ParseIntError),
+
+    #[error("Cannot decode hex string: Size {0} is not a multiple of two")]
+    InvHexStringSize(usize),
+
+    #[error("Cannot decode hex string")]
+    InvHexStringChar { source: std::num::ParseIntError },
 }
 
 /// Error cases for I/O operations

rust/pv_core/src/lib.rs

@@ -28,7 +28,7 @@ pub mod attest {
 pub mod misc {
     pub use crate::utils::pv_guest_bit_set;
     pub use crate::utils::{create_file, open_file, read_exact_file, read_file, write_file};
-    pub use crate::utils::{parse_hex, to_u16, to_u32, try_parse_u128, try_parse_u64};
+    pub use crate::utils::{decode_hex, parse_hex, to_u16, to_u32, try_parse_u128, try_parse_u64};
     pub use crate::utils::{read, write};
     pub use crate::utils::{Flags, Lsb0Flags64, Msb0Flags64};
 }

rust/pv_core/src/utils.rs

@@ -140,7 +140,7 @@ pub fn try_parse_u128(hex_str: &str, ctx: &str) -> Result<[u8; 16]> {
             "{ctx} hexstring must be 32chars long to cover all 16 bytes"
         ));
     }
-    parse_hex(hex_str).try_into().map_err(|_| {
+    decode_hex(hex_str)?.try_into().map_err(|_| {
         Error::Specification(format!(
             "{ctx} hexstring must be 32chars long to cover all 16 bytes"
         ))
@@ -333,6 +333,26 @@ usize_to_ui! {
 #[doc = r"u16"]
 => u16, to_u16}
 
+/// Converts the hexstring into a byte vector.
+///
+/// Raises an error if a non-hex character was found or the length was not a
+/// multiple of two.
+pub fn decode_hex<S: AsRef<str>>(s: S) -> Result<Vec<u8>> {
+    let hex = s.as_ref();
+    let hex_len = hex.len();
+    if hex_len % 2 != 0 {
+        return Err(Error::InvHexStringSize(hex_len));
+    }
+
+    (0..hex_len)
+        .step_by(2)
+        .map(|i| {
+            u8::from_str_radix(&hex[i..i + 2], 16)
+                .map_err(|err| Error::InvHexStringChar { source: err })
+        })
+        .collect()
+}
+
 /// Converts the hexstring into a byte vector.
 ///
 /// Stops if the end or until a non hex chat is found
@@ -475,6 +495,49 @@ mod tests {
         assert_eq!(super::parse_hex(s), exp);
     }
 
+    #[test]
+    fn decode_hex() {
+        let s = "123456acbef0";
+        let exp = vec![0x12, 0x34, 0x56, 0xac, 0xbe, 0xf0];
+        assert_eq!(super::decode_hex(s).expect("should not fail"), exp);
+
+        let s = "00123456acbef0";
+        let exp = vec![0, 0x12, 0x34, 0x56, 0xac, 0xbe, 0xf0];
+        assert_eq!(super::decode_hex(s).expect("should not fail"), exp);
+
+        let s = "00123456acbef0";
+        let exp = vec![0, 0x12, 0x34, 0x56, 0xac, 0xbe, 0xf0];
+        assert_eq!(super::decode_hex(s).expect("should not fail"), exp);
+
+        assert_eq!(
+            super::decode_hex("c0ffee").expect("should not fail"),
+            [0xc0, 0xff, 0xee]
+        );
+        assert_eq!(super::decode_hex("c0").expect("should not fail"), [0xc0]);
+        assert_eq!(super::decode_hex("").expect("should not fail"), []);
+
+        assert!(matches!(
+            super::decode_hex(" "),
+            Err(Error::InvHexStringSize(_))
+        ));
+        assert!(matches!(
+            super::decode_hex("coffee"),
+            Err(Error::InvHexStringChar { .. })
+        ));
+        assert!(matches!(
+            super::decode_hex(" c0a"),
+            Err(Error::InvHexStringChar { .. })
+        ));
+        assert!(matches!(
+            super::decode_hex("c0 a"),
+            Err(Error::InvHexStringChar { .. })
+        ));
+        assert!(matches!(
+            super::decode_hex("c0a"),
+            Err(Error::InvHexStringSize(_))
+        ));
+    }
+
     #[test]
     fn to_u32() {
         assert_eq!(Some(17), super::to_u32(17));
@@ -504,7 +567,7 @@ mod tests {
         ));
         assert!(matches!(
             try_parse_u128("-1223344556677889900aabbccddeeff", ""),
-            Err(Error::Specification(_))
+            Err(Error::InvHexStringChar { .. })
         ));
 
         assert!(matches!(
@@ -512,7 +575,7 @@ mod tests {
             Err(Error::Specification(_))
         ));
         assert!(matches!(
-            try_parse_u128("-0x1234", ""),
+            try_parse_u128("0x123", ""),
             Err(Error::Specification(_))
         ));
         assert!(matches!(
@@ -525,7 +588,7 @@ mod tests {
         ));
         assert!(matches!(
             try_parse_u128("0x-1223344556677889900aabbccddeeff", ""),
-            Err(Error::Specification(_))
+            Err(Error::InvHexStringChar { .. })
         ));
 
         assert_eq!(

rust/pv_core/src/uvdevice/secret_list.rs

@@ -354,7 +354,8 @@ where
             let nb = s.strip_prefix("0x").ok_or_else(|| {
                 serde::de::Error::invalid_value(serde::de::Unexpected::Str(s), &self)
             })?;
-            crate::misc::parse_hex(nb)
+            crate::misc::decode_hex(nb)
+                .map_err(|_| serde::de::Error::invalid_value(serde::de::Unexpected::Str(s), &self))?
                 .try_into()
                 .map_err(|_| serde::de::Error::invalid_value(serde::de::Unexpected::Str(s), &self))
         }

rust/pvsecret/src/cmd/create.rs

@@ -7,7 +7,7 @@ use anyhow::{anyhow, bail, Context, Error, Result};
 use log::{debug, info, trace, warn};
 use pv::{
     misc::{
-        open_file, parse_hex, pv_guest_bit_set, read_exact_file, read_file, try_parse_u128,
+        decode_hex, open_file, pv_guest_bit_set, read_exact_file, read_file, try_parse_u128,
         try_parse_u64, write,
     },
     request::{
@@ -174,7 +174,7 @@ fn try_from_val(val: Value) -> anyhow::Result<ConfigUid> {
     if cuid.len() != ::std::mem::size_of::<ConfigUid>() * 2 {
         return Err(anyhow!(format!("len invalid ({})", cuid.len())));
     }
-    let cuid: ConfigUid = parse_hex(&cuid)
+    let cuid: ConfigUid = decode_hex(&cuid)?
         .try_into()
         .map_err(|_| anyhow!("Cannot parse hex number".to_string()))?;
     Ok(cuid)