Privatize EADItem details, increase label range to i16

consts/src/lib.rs

@@ -88,30 +88,63 @@ mod common {
 
     #[derive(Debug)]
     pub struct EADItem {
-        pub label: u8,
-        pub is_critical: bool,
+        pub(crate) label: u16,
+        pub(crate) is_critical: bool,
         // TODO[ead]: have adjustable (smaller) length for this buffer
-        pub value: Option<EdhocMessageBuffer>,
+        pub(crate) value: Option<EdhocMessageBuffer>,
     }
 
-    pub trait EADTrait {
-        fn new() -> Self;
+    #[derive(Debug)]
+    #[non_exhaustive]
+    /// Error created from [EADTrait::new]
+    pub enum EADNewError {
+        /// The value given exceeds the size that is statically allocated per EAD item.
+        SizeExceeded,
+        /// The label indicates use as padding, but non-zero values were encountered.
+        InvalidPadding,
+    }
+
+    pub trait EADTrait: Sized {
+        /// Create a new EAD option
+        fn new(label: u16, is_critical: bool, value: Option<&[u8]>) -> Result<Self, EADNewError>;
+
+        // It may make sense to add a `new_padding()` function that allows adding padding with
+        // neither the null check nor the need for the caller to allocate a slice of zeros.
+
+        fn label(&self) -> u16;
+        fn is_critical(&self) -> bool;
+        fn value(&self) -> Option<&[u8]>;
     }
 
     impl EADTrait for EADItem {
-        fn new() -> Self {
-            EADItem {
-                label: 0,
-                is_critical: false,
-                value: None,
-            }
+        #[inline(always)] // Assist const propagation that removes error states
+        fn new(label: u16, is_critical: bool, value: Option<&[u8]>) -> Result<Self, EADNewError> {
+            Ok(EADItem {
+                label,
+                is_critical,
+                value: value
+                    .map(|v| v.try_into().map_err(|_| EADNewError::SizeExceeded))
+                    .transpose()?,
+            })
+        }
+
+        fn label(&self) -> u16 {
+            self.label
+        }
+
+        fn is_critical(&self) -> bool {
+            self.is_critical
+        }
+
+        fn value(&self) -> Option<&[u8]> {
+            self.value.as_ref().map(|v| &v.content[..v.len])
         }
     }
 
     pub const MAX_MESSAGE_SIZE_LEN: usize = 64;
     pub const MAX_EAD_SIZE_LEN: usize = 64;
     pub type EADMessageBuffer = EdhocMessageBuffer; // TODO: make it of size MAX_EAD_SIZE_LEN
-    pub const EAD_ZEROCONF_LABEL: u8 = 0x1; // NOTE: in lake-authz-draft-02 it is still TBD1
+    pub const EAD_ZEROCONF_LABEL: u16 = 0x1; // NOTE: in lake-authz-draft-02 it is still TBD1
 
     pub const ID_CRED_LEN: usize = 4;
     pub const SUITES_LEN: usize = 9;
@@ -131,8 +164,11 @@ mod common {
     pub const MAX_BUFFER_LEN: usize = 220;
     pub const CBOR_BYTE_STRING: u8 = 0x58u8;
     pub const CBOR_UINT_1BYTE: u8 = 0x18u8;
+    pub const CBOR_UINT_2BYTE: u8 = 0x19u8;
     pub const CBOR_NEG_INT_1BYTE_START: u8 = 0x20u8;
     pub const CBOR_NEG_INT_1BYTE_END: u8 = 0x37u8;
+    pub const CBOR_NEG_INT_1BYTE: u8 = 0x38u8;
+    pub const CBOR_NEG_INT_2BYTE: u8 = 0x39u8;
     pub const CBOR_UINT_1BYTE_START: u8 = 0x0u8;
     pub const CBOR_UINT_1BYTE_END: u8 = 0x17u8;
     pub const CBOR_MAJOR_TEXT_STRING: u8 = 0x60u8;
@@ -267,7 +303,7 @@ mod hacspec {
 
     #[derive(Debug)]
     pub struct EADItemHacspec {
-        pub label: U8,
+        pub label: U16,
         pub is_critical: bool,
         // TODO[ead]: have adjustable (smaller) length for this buffer
         pub value: Option<EdhocMessageBufferHacspec>,
@@ -282,30 +318,31 @@ mod hacspec {
     impl EADItemHacspecTrait for EADItemHacspec {
         fn new() -> Self {
             EADItemHacspec {
-                label: U8(0),
+                label: U16(0),
                 is_critical: false,
                 value: None,
             }
         }
         fn from_public_item(item: &EADItem) -> Self {
             EADItemHacspec {
-                label: U8(item.label),
-                is_critical: item.is_critical,
+                label: U16(item.label()),
+                is_critical: item.is_critical(),
                 value: match &item.value {
                     Some(value) => Some(EdhocMessageBufferHacspec::from_public_buffer(value)),
                     None => None,
                 },
             }
         }
         fn to_public_item(&self) -> EADItem {
-            EADItem {
-                label: self.label.declassify(),
-                is_critical: self.is_critical,
-                value: match &self.value {
-                    Some(value) => Some(value.to_public_buffer()),
-                    None => None,
-                },
-            }
+            let value_full = self
+                .value
+                .as_ref()
+                .map(|v| (v.content.to_public_array(), v.len));
+            let value = value_full
+                .as_ref()
+                .map(|(value, len)| &value[..(*len as usize)]);
+
+            EADItem::new(self.label.declassify(), self.is_critical, value).unwrap()
         }
     }
 

ead/edhoc-ead-zeroconf/src/lib.rs

@@ -38,13 +38,10 @@ pub fn ead_initiator_set_global_state(new_state: EADInitiatorState) {
 }
 
 pub fn i_prepare_ead_1() -> Option<EADItem> {
-    let mut ead_1 = EADItem::new();
-
-    // this ead item is critical
-    ead_1.label = EAD_ZEROCONF_LABEL;
-    ead_1.is_critical = true;
-
     // TODO: build Voucher_Info (LOC_W, ENC_ID), and append it to the buffer
+    let mut ead_1 = EADItem::new(EAD_ZEROCONF_LABEL, true, None)
+        // Const propagation will remove this.
+        .unwrap();
 
     ead_initiator_set_global_state(EADInitiatorState {
         protocol_state: EADInitiatorProtocolState::WaitEAD2,
@@ -65,7 +62,7 @@ pub fn i_process_ead_2(ead_2: EADItem) -> Result<(), ()> {
 }
 
 pub fn i_prepare_ead_3() -> Option<EADItem> {
-    Some(EADItem::new())
+    Some(EADItem::new(0, false, None).unwrap())
 }
 
 // responder side
@@ -116,13 +113,8 @@ pub fn r_process_ead_1(ead_1: EADItem) -> Result<(), ()> {
 }
 
 pub fn r_prepare_ead_2() -> Option<EADItem> {
-    let mut ead_2 = EADItem::new();
-
-    // add the label to the buffer (non-critical)
-    ead_2.label = EAD_ZEROCONF_LABEL;
-    ead_2.is_critical = true;
-
     // TODO: append Voucher (H(message_1), CRED_V) to the buffer
+    let ead_2 = EADItem::new(EAD_ZEROCONF_LABEL, true, None).unwrap();
 
     // NOTE: see the note in lib.rs::test_ead
     // state.protocol_state = EADResponderProtocolState::WaitMessage3;

hacspec/src/lib.rs

@@ -746,13 +746,31 @@ fn is_cbor_uint_2bytes(byte: U8) -> bool {
     return byte.declassify() == CBOR_UINT_1BYTE;
 }
 
+/// Check for: an unsigned integer encoded as three bytes
+#[inline(always)]
+fn is_cbor_uint_3bytes(byte: U8) -> bool {
+    return byte.declassify() == CBOR_UINT_2BYTE;
+}
+
 /// Check for: a negative integer encoded as a single byte
 #[inline(always)]
 fn is_cbor_neg_int_1byte(byte: U8) -> bool {
     let byte = byte.declassify();
     return byte >= CBOR_NEG_INT_1BYTE_START && byte <= CBOR_NEG_INT_1BYTE_END;
 }
 
+/// Check for: an unsigned integer encoded as two bytes
+#[inline(always)]
+fn is_cbor_neg_int_2bytes(byte: U8) -> bool {
+    return byte.declassify() == CBOR_NEG_INT_1BYTE;
+}
+
+/// Check for: an unsigned integer encoded as three bytes
+#[inline(always)]
+fn is_cbor_neg_int_3bytes(byte: U8) -> bool {
+    return byte.declassify() == CBOR_NEG_INT_2BYTE;
+}
+
 /// Check for: a bstr denoted by a single byte which encodes both type and content length
 #[inline(always)]
 fn is_cbor_bstr_1byte_prefix(byte: U8) -> bool {
@@ -773,6 +791,81 @@ fn is_cbor_array_1byte_prefix(byte: U8) -> bool {
     return byte >= CBOR_MAJOR_ARRAY && byte <= CBOR_MAJOR_ARRAY_MAX;
 }
 
+/// If the first CBOR item in `message` is a CBOR (unsigned or negative) integer, return its
+/// argument value, whether the number is negative, and how many bytes were consumed. Note that the
+/// argument of a negative number is offset by 1: `(0, true, _)` means -1, `(65535, true, _)` means
+/// -65536
+// This would really profit from using a regular CBOR library, or at very least looking like one;
+// that step is left for later refactoring.
+pub fn parse_cbor16(message: &BytesMessageBuffer, offset: usize) -> Option<(u16, bool, usize)> {
+    let label = message[offset];
+    if is_cbor_uint_1byte(label) {
+        // CBOR unsigned integer (0..=23)
+        Some((label.declassify().into(), false, 1))
+    } else if is_cbor_uint_2bytes(label) {
+        Some((message[offset + 1].declassify().into(), false, 2))
+    } else if is_cbor_uint_3bytes(label) {
+        Some((
+            (u16::from(message[offset + 1].declassify()) << 8)
+                + u16::from(message[offset + 2].declassify()),
+            false,
+            3,
+        ))
+    } else if is_cbor_neg_int_1byte(label) {
+        // CBOR negative integer (-1..=-24)
+        Some((
+            (label.declassify() - CBOR_NEG_INT_1BYTE_START).into(),
+            true,
+            1,
+        ))
+    } else if is_cbor_neg_int_2bytes(label) {
+        Some((message[offset + 1].declassify().into(), true, 2))
+    } else if is_cbor_neg_int_3bytes(label) {
+        Some((
+            (u16::from(message[offset + 1].declassify()) << 8)
+                + u16::from(message[offset + 2].declassify()),
+            true,
+            3,
+        ))
+    } else {
+        None
+    }
+}
+
+/// Encode a positive or negative integer into the message at the given offset, and return how many
+/// bytes were written. Note that as with parse_cbor16, the argument is offset by 1 from the
+/// absolute numeric value.
+pub fn encode_cbor16(
+    message: &mut BytesMessageBuffer,
+    offset: usize,
+    argument: u16,
+    is_negative: bool,
+) -> usize {
+    let major_bits = if is_negative {
+        CBOR_NEG_INT_1BYTE_START
+    } else {
+        CBOR_UINT_1BYTE_START
+    };
+
+    match argument {
+        0..=0x17 => {
+            message[offset] = U8(major_bits | (u8::try_from(argument).unwrap()));
+            1
+        }
+        ..=0xff => {
+            message[offset] = U8(major_bits | CBOR_UINT_1BYTE);
+            message[offset + 1] = U8(argument.try_into().unwrap());
+            2
+        }
+        _ => {
+            message[offset] = U8(major_bits | CBOR_UINT_2BYTE);
+            message[offset + 1] = U8((argument >> 8).try_into().unwrap());
+            message[offset + 2] = U8(argument as u8);
+            3
+        }
+    }
+}
+
 fn parse_suites_i(
     rcvd_message_1: &BufferMessage1,
 ) -> Result<(BytesSuites, usize, usize), EDHOCError> {
@@ -847,30 +940,24 @@ fn parse_ead(
     let mut ead_value = None::<EdhocMessageBufferHacspec>;
 
     // assume label is a single byte integer (negative or positive)
-    let label = message.content[offset];
-    let res_label = if is_cbor_uint_1byte(label) {
-        // CBOR unsigned integer (0..=23)
-        Ok((label.declassify() as u8, false))
-    } else if is_cbor_neg_int_1byte(label) {
-        // CBOR negative integer (-1..=-24)
-        Ok((label.declassify() - (CBOR_NEG_INT_1BYTE_START - 1), true))
-    } else {
-        Err(EDHOCError::ParsingError)
-    };
+    let res_label = parse_cbor16(&message.content, offset).ok_or(EDHOCError::ParsingError);
 
     if res_label.is_ok() {
-        let (label, is_critical) = res_label.unwrap();
-        if message.len > (offset + 1) {
+        let (mut label, is_critical, consumed) = res_label.unwrap();
+        if message.len > (offset + consumed) {
             // EAD value is present
             let buffer = EdhocMessageBufferHacspec::from_slice(
                 &message.content,
-                offset + 1,
-                message.len - (offset + 1),
+                offset + consumed,
+                message.len - (offset + consumed),
             );
             ead_value = Some(buffer);
         }
+        if is_critical {
+            label = label.checked_add(1).ok_or(EDHOCError::EADError)?;
+        }
         ead_item = Some(EADItemHacspec {
-            label: U8(label),
+            label: U16(label),
             is_critical,
             value: ead_value,
         });
@@ -964,19 +1051,23 @@ fn parse_message_1(
 fn encode_ead_item(ead_1: &EADItemHacspec) -> EdhocMessageBufferHacspec {
     let mut output = EdhocMessageBufferHacspec::new();
 
-    // encode label
-    if ead_1.is_critical {
-        output.content[0] = ead_1.label + U8(CBOR_NEG_INT_1BYTE_START - 1);
+    let label = if ead_1.is_critical {
+        // A critical (padding) 0 might be constructed but makes no sense and will be encoded as
+        // regular padding.
+        ead_1.label.declassify().saturating_sub(1)
     } else {
-        output.content[0] = ead_1.label;
-    }
-    output.len = 1;
+        ead_1.label.declassify()
+    };
+    // encode label
+    let written = encode_cbor16(&mut output.content, 0, label, ead_1.is_critical);
+    output.len = written;
 
     // encode value
     if let Some(ead_1_value) = &ead_1.value {
-        output.content = output
-            .content
-            .update_slice(1, &ead_1_value.content, 0, ead_1_value.len);
+        output.content =
+            output
+                .content
+                .update_slice(written, &ead_1_value.content, 0, ead_1_value.len);
         output.len += ead_1_value.len;
     }
 
@@ -1621,7 +1712,7 @@ mod tests {
     const MESSAGE_1_TV_SUITE_ONLY_C: &str = "0382021819";
     // message with an array having too many cipher suites (more than 9)
     const MESSAGE_1_TV_SUITE_ONLY_ERR: &str = "038A02020202020202020202";
-    const EAD_DUMMY_LABEL_TV: u8 = 0x01;
+    const EAD_DUMMY_LABEL_TV: u16 = 0x01;
     const EAD_DUMMY_VALUE_TV: &str = "cccccc";
     const EAD_DUMMY_CRITICAL_TV: &str = "20cccccc";
     const MESSAGE_1_WITH_DUMMY_EAD_NO_VALUE_TV: &str =
@@ -2131,7 +2222,7 @@ mod tests {
         let ead_tv = EdhocMessageBufferHacspec::from_hex(EAD_DUMMY_CRITICAL_TV);
 
         let ead_item = EADItemHacspec {
-            label: U8(EAD_DUMMY_LABEL_TV),
+            label: U16(EAD_DUMMY_LABEL_TV),
             is_critical: true,
             value: Some(EdhocMessageBufferHacspec::from_hex(EAD_DUMMY_VALUE_TV)),
         };
@@ -2149,7 +2240,7 @@ mod tests {
         let c_i_tv = U8(C_I_TV);
         let message_1_ead_tv = BufferMessage1::from_hex(MESSAGE_1_WITH_DUMMY_CRITICAL_EAD_TV);
         let ead_item = EADItemHacspec {
-            label: U8(EAD_DUMMY_LABEL_TV),
+            label: U16(EAD_DUMMY_LABEL_TV),
             is_critical: true,
             value: Some(EdhocMessageBufferHacspec::from_hex(EAD_DUMMY_VALUE_TV)),
         };