Skip to content

Commit

Permalink
feat: Allow full EAD label range
Browse files Browse the repository at this point in the history
This increases the usable size of EAD labels to +-65535.

It also adds a bit of CBOR processing, to the point where it may make
sense to use a proper CBOR library instead; that is likely most easily
achieved after completion of [105].

[105]: #105
  • Loading branch information
chrysn committed Oct 2, 2023
1 parent 0d8f058 commit 0160855
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 49 deletions.
41 changes: 20 additions & 21 deletions consts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ mod common {

#[derive(Debug)]
pub struct EADItem {
pub(crate) label: u8,
pub(crate) label: u16,
pub(crate) is_critical: bool,
// TODO[ead]: have adjustable (smaller) length for this buffer
pub(crate) value: Option<EdhocMessageBuffer>,
Expand All @@ -100,9 +100,6 @@ mod common {
pub enum EADNewError {
/// The value given exceeds the size that is statically allocated per EAD item.
SizeExceeded,
/// The label value given at construction exceeds the range of EAD labels the library can
/// handle.
InexpressibleLabel,
/// The label indicates use as padding, but non-zero values were encountered.
InvalidPadding,
}
Expand All @@ -123,9 +120,7 @@ mod common {
#[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: label
.try_into()
.map_err(|_| EADNewError::InexpressibleLabel)?,
label,
is_critical,
value: value
.map(|v| v.try_into().map_err(|_| EADNewError::SizeExceeded))
Expand All @@ -134,7 +129,7 @@ mod common {
}

fn label(&self) -> u16 {
self.label.into()
self.label
}

fn is_critical(&self) -> bool {
Expand All @@ -149,7 +144,7 @@ mod common {
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;
Expand All @@ -169,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;
Expand Down Expand Up @@ -305,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>,
Expand All @@ -320,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()
}
}

Expand Down
4 changes: 2 additions & 2 deletions ead/edhoc-ead-zeroconf/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub fn ead_initiator_set_global_state(new_state: EADInitiatorState) {

pub fn i_prepare_ead_1() -> Option<EADItem> {
// TODO: build Voucher_Info (LOC_W, ENC_ID), and append it to the buffer
let mut ead_1 = EADItem::new(EAD_ZEROCONF_LABEL.into(), true, None)
let mut ead_1 = EADItem::new(EAD_ZEROCONF_LABEL, true, None)
// Const propagation will remove this.
.unwrap();

Expand Down Expand Up @@ -114,7 +114,7 @@ pub fn r_process_ead_1(ead_1: EADItem) -> Result<(), ()> {

pub fn r_prepare_ead_2() -> Option<EADItem> {
// TODO: append Voucher (H(message_1), CRED_V) to the buffer
let ead_2 = EADItem::new(EAD_ZEROCONF_LABEL.into(), true, None).unwrap();
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;
Expand Down
123 changes: 98 additions & 25 deletions hacspec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -773,6 +791,62 @@ 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> {
Expand Down Expand Up @@ -847,30 +921,26 @@ 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,
});
Expand Down Expand Up @@ -964,19 +1034,22 @@ 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);
.update_slice(written, &ead_1_value.content, 0, ead_1_value.len);
output.len += ead_1_value.len;
}

Expand Down Expand Up @@ -1621,7 +1694,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 =
Expand Down Expand Up @@ -2131,7 +2204,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)),
};
Expand All @@ -2149,7 +2222,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)),
};
Expand Down
2 changes: 1 addition & 1 deletion lib/src/edhoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1533,7 +1533,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 =
Expand Down

0 comments on commit 0160855

Please sign in to comment.