Merge pull request #220 from geonnave/improve-errors

Improve error handling in msg buffer and authz

Author: geonnave

ead/lakers-ead-authz/src/device.rs

@@ -1,4 +1,5 @@
 use super::shared::*;
+use crate::ZeroTouchError;
 use lakers_shared::{Crypto as CryptoTrait, *};
 
 #[derive(Debug)]
@@ -67,16 +68,19 @@ impl ZeroTouchDeviceWaitEAD2 {
         crypto: &mut Crypto,
         ead_2: EADItem,
         cred_v: &[u8],
-    ) -> Result<ZeroTouchDeviceDone, ()> {
-        if ead_2.label != EAD_ZEROCONF_LABEL || ead_2.value.is_none() {
-            return Err(());
+    ) -> Result<ZeroTouchDeviceDone, ZeroTouchError> {
+        if ead_2.label != EAD_ZEROCONF_LABEL {
+            return Err(ZeroTouchError::InvalidEADLabel);
         }
+        let Some(ead_2_value_buffer) = ead_2.value else {
+            return Err(ZeroTouchError::EmptyEADValue);
+        };
         let mut ead_2_value: BytesEncodedVoucher = Default::default();
-        ead_2_value[..].copy_from_slice(&ead_2.value.unwrap().content[..ENCODED_VOUCHER_LEN]);
+        ead_2_value[..].copy_from_slice(&ead_2_value_buffer.content[..ENCODED_VOUCHER_LEN]);
 
         match verify_voucher(crypto, &ead_2_value, &self.h_message_1, cred_v, &self.prk) {
             Ok(voucher) => Ok(ZeroTouchDeviceDone { voucher }),
-            Err(_) => Err(()),
+            Err(error) => Err(error),
         }
     }
 }
@@ -128,6 +132,23 @@ fn encode_ead_1_value(
     output
 }
 
+pub(crate) fn verify_voucher<Crypto: CryptoTrait>(
+    crypto: &mut Crypto,
+    received_voucher: &BytesEncodedVoucher,
+    h_message_1: &BytesHashLen,
+    cred_v: &[u8],
+    prk: &BytesHashLen,
+) -> Result<BytesMac, ZeroTouchError> {
+    let prepared_voucher = &prepare_voucher(crypto, h_message_1, cred_v, prk);
+    if received_voucher == prepared_voucher {
+        let mut voucher_mac: BytesMac = Default::default();
+        voucher_mac[..MAC_LENGTH].copy_from_slice(&prepared_voucher[1..1 + MAC_LENGTH]);
+        return Ok(voucher_mac);
+    } else {
+        return Err(ZeroTouchError::VoucherVerificationFailed);
+    }
+}
+
 #[cfg(test)]
 mod test_device {
     use super::*;
@@ -166,7 +187,7 @@ mod test_device {
 
     #[test]
     fn test_verify_voucher() {
-        let voucher_tv = VOUCHER_TV.try_into().unwrap();
+        let mut voucher_tv = VOUCHER_TV.try_into().unwrap();
         let h_message_1_tv = H_MESSAGE_1_TV.try_into().unwrap();
         let prk_tv = PRK_TV.try_into().unwrap();
         let voucher_mac_tv: BytesMac = VOUCHER_MAC_TV.try_into().unwrap();
@@ -180,6 +201,16 @@ mod test_device {
         );
         assert!(res.is_ok());
         assert_eq!(res.unwrap(), voucher_mac_tv);
+
+        voucher_tv[0] ^= 0x01; // change a byte to make the voucher invalid
+        let res = verify_voucher(
+            &mut default_crypto(),
+            &voucher_tv,
+            &h_message_1_tv,
+            &CRED_V_TV,
+            &prk_tv,
+        );
+        assert_eq!(res, Err(ZeroTouchError::VoucherVerificationFailed));
     }
 
     #[test]

ead/lakers-ead-authz/src/lib.rs

@@ -11,6 +11,14 @@ pub use authenticator::{ZeroTouchAuthenticator, ZeroTouchAuthenticatorWaitVouche
 pub use device::{ZeroTouchDevice, ZeroTouchDeviceDone, ZeroTouchDeviceWaitEAD2};
 pub use server::ZeroTouchServer;
 
+#[derive(PartialEq, Debug)]
+#[repr(C)]
+pub enum ZeroTouchError {
+    InvalidEADLabel,
+    EmptyEADValue,
+    VoucherVerificationFailed,
+}
+
 #[cfg(test)]
 mod test_authz {
     use crate::{

ead/lakers-ead-authz/src/shared.rs

@@ -22,23 +22,6 @@ pub(crate) fn compute_prk_from_secret<Crypto: CryptoTrait>(
     crypto.hkdf_extract(&salt, &g_ab)
 }
 
-pub(crate) fn verify_voucher<Crypto: CryptoTrait>(
-    crypto: &mut Crypto,
-    received_voucher: &BytesEncodedVoucher,
-    h_message_1: &BytesHashLen,
-    cred_v: &[u8],
-    prk: &BytesHashLen,
-) -> Result<BytesMac, ()> {
-    let prepared_voucher = &prepare_voucher(crypto, h_message_1, cred_v, prk);
-    if received_voucher == prepared_voucher {
-        let mut voucher_mac: BytesMac = Default::default();
-        voucher_mac[..MAC_LENGTH].copy_from_slice(&prepared_voucher[1..1 + MAC_LENGTH]);
-        return Ok(voucher_mac);
-    } else {
-        return Err(());
-    }
-}
-
 pub(crate) fn prepare_voucher<Crypto: CryptoTrait>(
     crypto: &mut Crypto,
     h_message_1: &BytesHashLen,

lakers-python/src/ead_authz/authenticator.rs

@@ -22,7 +22,7 @@ impl PyAuthzAutenticator {
         ead_1: EADItem,
         message_1: Vec<u8>,
     ) -> PyResult<(Vec<u8>, Vec<u8>)> {
-        let message_1 = EdhocMessageBuffer::new_from_slice(message_1.as_slice()).unwrap(); // FIXME: avoid unwrap
+        let message_1 = EdhocMessageBuffer::new_from_slice(message_1.as_slice())?;
         let (state, loc_w, voucher_request) =
             self.authenticator.process_ead_1(&ead_1, &message_1)?;
         self.authenticator_wait = state;
@@ -33,8 +33,7 @@ impl PyAuthzAutenticator {
     }
 
     pub fn prepare_ead_2(&self, voucher_response: Vec<u8>) -> PyResult<EADItem> {
-        let voucher_response =
-            EdhocMessageBuffer::new_from_slice(voucher_response.as_slice()).unwrap(); // FIXME: avoid unwrap
+        let voucher_response = EdhocMessageBuffer::new_from_slice(voucher_response.as_slice())?;
         Ok(self.authenticator_wait.prepare_ead_2(&voucher_response)?)
     }
 }

lakers-python/src/initiator.rs

@@ -61,7 +61,7 @@ impl PyEdhocInitiator {
         &mut self,
         message_2: Vec<u8>,
     ) -> PyResult<(u8, Vec<u8>, Option<EADItem>)> {
-        let message_2 = EdhocMessageBuffer::new_from_slice(message_2.as_slice()).unwrap(); // FIXME: avoid unwrap
+        let message_2 = EdhocMessageBuffer::new_from_slice(message_2.as_slice())?;
 
         match i_parse_message_2(&self.wait_m2, &mut default_crypto(), &message_2) {
             Ok((state, c_r, id_cred_r, ead_2)) => {

lakers-python/src/responder.rs

@@ -34,7 +34,7 @@ impl PyEdhocResponder {
     }
 
     fn process_message_1(&mut self, message_1: Vec<u8>) -> PyResult<Option<EADItem>> {
-        let message_1 = EdhocMessageBuffer::new_from_slice(message_1.as_slice()).unwrap(); // FIXME: avoid unwrap call
+        let message_1 = EdhocMessageBuffer::new_from_slice(message_1.as_slice())?;
         let (state, ead_1) = r_process_message_1(&self.start, &mut default_crypto(), &message_1)?;
         self.processing_m1 = state;
 
@@ -73,7 +73,7 @@ impl PyEdhocResponder {
     }
 
     pub fn parse_message_3(&mut self, message_3: Vec<u8>) -> PyResult<(Vec<u8>, Option<EADItem>)> {
-        let message_3 = EdhocMessageBuffer::new_from_slice(message_3.as_slice()).unwrap(); // FIXME: avoid unwrap call
+        let message_3 = EdhocMessageBuffer::new_from_slice(message_3.as_slice())?;
         match r_parse_message_3(&mut self.wait_m3, &mut default_crypto(), &message_3) {
             Ok((state, id_cred_i, ead_3)) => {
                 self.processing_m3 = state;

lakers-python/test/test_lakers.py

@@ -62,8 +62,14 @@ def test_handshake():
     r_prk_out_new = responder.edhoc_key_update(CONTEXT)
     assert i_prk_out_new == r_prk_out_new
 
-def test_error():
+def test_edhoc_error():
     responder = lakers.EdhocResponder(R, CRED_R)
     with pytest.raises(ValueError) as err:
-        _ead_1 = responder.process_message_1([1, 2, 3])
+        _ = responder.process_message_1([1, 2, 3])
     assert str(err.value) == "EDHOCError::ParsingError"
+
+def test_buffer_error():
+    initiator = lakers.EdhocInitiator()
+    with pytest.raises(ValueError) as err:
+        _ = initiator.parse_message_2([1] * 1000)
+    assert str(err.value) == "MessageBufferError::SliceTooLong"

shared/src/lib.rs

@@ -210,6 +210,13 @@ pub enum CredentialTransfer {
     ByValue,
 }
 
+#[derive(PartialEq, Debug)]
+#[repr(C)]
+pub enum MessageBufferError {
+    BufferAlreadyFull,
+    SliceTooLong,
+}
+
 /// An owned u8 vector of a limited length
 ///
 /// It is used to represent the various messages in encrypted and in decrypted form, as well as
@@ -238,26 +245,26 @@ impl EdhocMessageBuffer {
         }
     }
 
-    pub fn new_from_slice(slice: &[u8]) -> Result<Self, ()> {
+    pub fn new_from_slice(slice: &[u8]) -> Result<Self, MessageBufferError> {
         let mut buffer = Self::new();
         if buffer.fill_with_slice(slice).is_ok() {
             Ok(buffer)
         } else {
-            Err(())
+            Err(MessageBufferError::SliceTooLong)
         }
     }
 
     pub fn get(self, index: usize) -> Option<u8> {
         self.content.get(index).copied()
     }
 
-    pub fn push(&mut self, item: u8) -> Result<(), ()> {
+    pub fn push(&mut self, item: u8) -> Result<(), MessageBufferError> {
         if self.len < self.content.len() {
             self.content[self.len] = item;
             self.len += 1;
             Ok(())
         } else {
-            Err(())
+            Err(MessageBufferError::BufferAlreadyFull)
         }
     }
 
@@ -269,23 +276,23 @@ impl EdhocMessageBuffer {
         &self.content[0..self.len]
     }
 
-    pub fn fill_with_slice(&mut self, slice: &[u8]) -> Result<(), ()> {
+    pub fn fill_with_slice(&mut self, slice: &[u8]) -> Result<(), MessageBufferError> {
         if slice.len() <= self.content.len() {
             self.len = slice.len();
             self.content[..self.len].copy_from_slice(slice);
             Ok(())
         } else {
-            Err(())
+            Err(MessageBufferError::SliceTooLong)
         }
     }
 
-    pub fn extend_from_slice(&mut self, slice: &[u8]) -> Result<(), ()> {
+    pub fn extend_from_slice(&mut self, slice: &[u8]) -> Result<(), MessageBufferError> {
         if self.len + slice.len() <= self.content.len() {
             self.content[self.len..self.len + slice.len()].copy_from_slice(slice);
             self.len += slice.len();
             Ok(())
         } else {
-            Err(())
+            Err(MessageBufferError::SliceTooLong)
         }
     }
 

shared/src/python_bindings.rs

@@ -18,6 +18,18 @@ impl From<EDHOCError> for PyErr {
     }
 }
 
+impl fmt::Display for MessageBufferError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "MessageBufferError::{:?}", self)
+    }
+}
+
+impl From<MessageBufferError> for PyErr {
+    fn from(error: MessageBufferError) -> Self {
+        PyValueError::new_err(error.to_string())
+    }
+}
+
 #[pymethods]
 impl EADItem {
     #[new]