From 636309d3479cbbe47139cb0f66fa14a879b208f4 Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Wed, 13 Sep 2023 18:08:03 +0200 Subject: [PATCH 01/10] fix: adjust against invalid traces for message_1 --- lib/src/edhoc.rs | 165 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 116 insertions(+), 49 deletions(-) diff --git a/lib/src/edhoc.rs b/lib/src/edhoc.rs index fcb6ae5d..94d506ab 100644 --- a/lib/src/edhoc.rs +++ b/lib/src/edhoc.rs @@ -700,32 +700,37 @@ fn parse_suites_i( 0x80..=0x97 => { // the CBOR array length is encoded in the first byte, so we extract it let suites_len: usize = (rcvd_message_1.content[1] - CBOR_MAJOR_ARRAY).into(); - raw_suites_len = 1; // account for the CBOR_MAJOR_ARRAY byte - if suites_len <= EDHOC_SUITES.len() { - let mut j: usize = 0; // index for addressing cipher suites - while j < suites_len { - raw_suites_len += 1; - // match based on cipher suite identifier - match rcvd_message_1.content[raw_suites_len] { - // CBOR unsigned integer (0..23) - 0x00..=0x17 => { - suites_i[j] = rcvd_message_1.content[raw_suites_len]; - suites_i_len += 1; - } - // CBOR unsigned integer (one-byte uint8_t follows) - 0x18 => { - raw_suites_len += 1; // account for the 0x18 tag byte - suites_i[j] = rcvd_message_1.content[raw_suites_len]; - suites_i_len += 1; - } - _ => { - error = EDHOCError::ParsingError; - break; + // check surplus array encoding of ciphersuite + if suites_len > 1 { + raw_suites_len = 1; // account for the CBOR_MAJOR_ARRAY byte + if suites_len <= EDHOC_SUITES.len() { + let mut j: usize = 0; // index for addressing cipher suites + while j < suites_len { + raw_suites_len += 1; + // match based on cipher suite identifier + match rcvd_message_1.content[raw_suites_len] { + // CBOR unsigned integer (0..23) + 0x00..=0x17 => { + suites_i[j] = rcvd_message_1.content[raw_suites_len]; + suites_i_len += 1; + } + // CBOR unsigned integer (one-byte uint8_t follows) + 0x18 => { + raw_suites_len += 1; // account for the 0x18 tag byte + suites_i[j] = rcvd_message_1.content[raw_suites_len]; + suites_i_len += 1; + } + _ => { + error = EDHOCError::ParsingError; + break; + } } + j += 1; } - j += 1; + error = EDHOCError::Success; + } else { + error = EDHOCError::ParsingError; } - error = EDHOCError::Success; } else { error = EDHOCError::ParsingError; } @@ -780,6 +785,24 @@ fn parse_ead(message: &EdhocMessageBuffer, offset: usize) -> Result bool { + return (first_byte & CBOR_MAJOR_ARRAY) == CBOR_MAJOR_ARRAY; +} + +fn is_encoded_conn_id_minimal(enc_conn_id: U8) -> bool { + return (enc_conn_id >= 20 && enc_conn_id <= 37) || (enc_conn_id >= 0 && enc_conn_id <= 17); +} + +fn should_encoded_conn_id_be_minimal(conn_id_byte1: U8, conn_id_byte2: U8) -> bool { + return !is_encoded_conn_id_minimal(conn_id_byte1) + && (conn_id_byte1 == (CBOR_MAJOR_BYTE_STRING | 0x1)) // bstr with length of 1 + && is_encoded_conn_id_minimal(conn_id_byte2); +} + +fn should_encoded_ciphersuite_be_minimal(byte1: U8, byte2: U8) -> bool { + return is_cbor_array(byte1) && (byte1 - CBOR_MAJOR_ARRAY) == 1; +} + fn parse_message_1( rcvd_message_1: &BufferMessage1, ) -> Result< @@ -794,6 +817,7 @@ fn parse_message_1( EDHOCError, > { let mut error: EDHOCError = EDHOCError::UnknownError; + let mut method: U8 = 0xff; let mut g_x: BytesP256ElemLen = [0x00; P256_ELEM_LEN]; let mut suites_i: BytesSuites = [0u8; SUITES_LEN]; let mut suites_i_len: usize = 0; @@ -801,37 +825,51 @@ fn parse_message_1( let mut c_i = 0; let mut ead_1 = None::; - let method = rcvd_message_1.content[0]; - - let res_suites = parse_suites_i(rcvd_message_1); - - if res_suites.is_ok() { - (suites_i, suites_i_len, raw_suites_len) = res_suites.unwrap(); - - g_x.copy_from_slice( - &rcvd_message_1.content[3 + raw_suites_len..3 + raw_suites_len + P256_ELEM_LEN], - ); - - c_i = rcvd_message_1.content[3 + raw_suites_len + P256_ELEM_LEN]; - - // if there is still more to parse, the rest will be the EAD_1 - if rcvd_message_1.len > (4 + raw_suites_len + P256_ELEM_LEN) { - // NOTE: since the current implementation only supports one EAD handler, - // we assume only one EAD item - let ead_res = parse_ead(rcvd_message_1, 4 + raw_suites_len + P256_ELEM_LEN); - if ead_res.is_ok() { - ead_1 = ead_res.unwrap(); - error = EDHOCError::Success; + // check surplus array encoding + if !is_cbor_array(rcvd_message_1.content[0]) { + method = rcvd_message_1.content[0]; + let res_suites = parse_suites_i(rcvd_message_1); + + if res_suites.is_ok() { + (suites_i, suites_i_len, raw_suites_len) = res_suites.unwrap(); + + let g_x_type = rcvd_message_1.content[1 + raw_suites_len]; + if g_x_type == CBOR_BYTE_STRING { + g_x.copy_from_slice( + &rcvd_message_1.content[3 + raw_suites_len..3 + raw_suites_len + P256_ELEM_LEN], + ); + + // check surplus bstr encoding + c_i = rcvd_message_1.content[3 + raw_suites_len + P256_ELEM_LEN]; + let c_i_lookahead = rcvd_message_1.content[4 + raw_suites_len + P256_ELEM_LEN]; + if !should_encoded_conn_id_be_minimal(c_i, c_i_lookahead) { + // if there is still more to parse, the rest will be the EAD_1 + if rcvd_message_1.len > (4 + raw_suites_len + P256_ELEM_LEN) { + // NOTE: since the current implementation only supports one EAD handler, + // we assume only one EAD item + let ead_res = parse_ead(rcvd_message_1, 4 + raw_suites_len + P256_ELEM_LEN); + if ead_res.is_ok() { + ead_1 = ead_res.unwrap(); + error = EDHOCError::Success; + } else { + error = ead_res.unwrap_err(); + } + } else if rcvd_message_1.len == (4 + raw_suites_len + P256_ELEM_LEN) { + error = EDHOCError::Success; + } else { + error = EDHOCError::ParsingError; + } + } else { + error = EDHOCError::ParsingError; + } } else { - error = ead_res.unwrap_err(); + error = EDHOCError::ParsingError; } - } else if rcvd_message_1.len == (4 + raw_suites_len + P256_ELEM_LEN) { - error = EDHOCError::Success; } else { - error = EDHOCError::ParsingError; + error = res_suites.unwrap_err(); } } else { - error = res_suites.unwrap_err(); + error = EDHOCError::ParsingError; } match error { @@ -1599,6 +1637,35 @@ mod tests { assert!(ead_1.is_none()); } + const _MESSAGE_1_TV: &str = + "0382060258208af6f430ebe18d34184017a9a11bf511c8dff8f834730b96c1b7c8dbca2fc3b637"; + const MESSAGE_1_INVALID_ARRAY_TV: &str = + "8403025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa90e"; + const MESSAGE_1_INVALID_C_I_TV: &str = + "03025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa9410e"; + const MESSAGE_1_INVALID_CIPHERSUITE_TV: &str = + "0381025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa90e"; + const MESSAGE_1_INVALID_TEXT_EPHEMERAL_KEY_TV: &str = + "0302782020616972207370656564206F66206120756E6C6164656E207377616C6C6F77200e"; + const MESSAGE_2_INVALID_NUMBER_OF_CBOR_SEQUENCE_TV: &str = + "5820419701d7f00a26c2dc587a36dd752549f33763c893422c8ea0f955a13a4ff5d54B9862a11de42a95d785386a"; + const PLAINTEXT_2_SURPLUS_MAP_ID_CRED_TV: &str = "27a10442321048fa5efa2ebf920bf3"; + const PLAINTEXT_2_SURPLUS_BSTR_ID_CRED_TV: &str = "27413248fa5efa2ebf920bf3"; + #[test] + fn test_parse_message_1_invalid_traces() { + let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_ARRAY_TV); + assert!(parse_message_1(&message_1_tv).is_err()); + + let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_C_I_TV); + assert!(parse_message_1(&message_1_tv).is_err()); + + let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_CIPHERSUITE_TV); + assert!(parse_message_1(&message_1_tv).is_err()); + + let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_TEXT_EPHEMERAL_KEY_TV); + assert!(parse_message_1(&message_1_tv).is_err()); + } + #[test] fn test_encode_message_2() { let ciphertext_2_tv = BufferCiphertext2::from_hex(CIPHERTEXT_2_TV); From 9a60d1e3851358ef80d43e3d8abfc5370daf438e Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Thu, 14 Sep 2023 14:40:43 +0200 Subject: [PATCH 02/10] fix: adjust for invalid traces in message_2 and plaintext_2 --- lib/src/edhoc.rs | 78 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 57 insertions(+), 21 deletions(-) diff --git a/lib/src/edhoc.rs b/lib/src/edhoc.rs index 94d506ab..e0e2632f 100644 --- a/lib/src/edhoc.rs +++ b/lib/src/edhoc.rs @@ -960,17 +960,30 @@ fn encode_message_1( fn parse_message_2( rcvd_message_2: &BufferMessage2, ) -> Result<(BytesP256ElemLen, BufferCiphertext2), EDHOCError> { + let mut error: EDHOCError = EDHOCError::UnknownError; // FIXME decode negative integers as well let mut g_y: BytesP256ElemLen = [0x00; P256_ELEM_LEN]; - g_y[..].copy_from_slice(&rcvd_message_2.content[2..2 + P256_ELEM_LEN]); - let mut ciphertext_2: BufferCiphertext2 = BufferCiphertext2::new(); - ciphertext_2.len = rcvd_message_2.len - P256_ELEM_LEN - 2; // len - gy_len - 2 - ciphertext_2.content[..ciphertext_2.len].copy_from_slice( - &rcvd_message_2.content[2 + P256_ELEM_LEN..2 + P256_ELEM_LEN + ciphertext_2.len], - ); - Ok((g_y, ciphertext_2)) + // ensure the whole message is a single CBOR sequence + if rcvd_message_2.content[0] == CBOR_BYTE_STRING + && rcvd_message_2.content[1] == (rcvd_message_2.len as u8 - 2) + { + g_y[..].copy_from_slice(&rcvd_message_2.content[2..2 + P256_ELEM_LEN]); + + ciphertext_2.len = rcvd_message_2.len - P256_ELEM_LEN - 2; // len - gy_len - 2 + ciphertext_2.content[..ciphertext_2.len].copy_from_slice( + &rcvd_message_2.content[2 + P256_ELEM_LEN..2 + P256_ELEM_LEN + ciphertext_2.len], + ); + error = EDHOCError::Success; + } else { + error = EDHOCError::ParsingError; + } + + match error { + EDHOCError::Success => Ok((g_y, ciphertext_2)), + _ => Err(error), + } } fn encode_message_2(g_y: &BytesP256ElemLen, ciphertext_2: &BufferCiphertext2) -> BufferMessage2 { @@ -1554,6 +1567,20 @@ mod tests { const OSCORE_MASTER_SECRET_TV: BytesCcmKeyLen = hex!("8c409a332223ad900e44f3434d2d2ce3"); const OSCORE_MASTER_SALT_TV: Bytes8 = hex!("6163f44be862adfa"); + // invalid test vectors, should result in a parsing error + const MESSAGE_1_INVALID_ARRAY_TV: &str = + "8403025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa90e"; + const MESSAGE_1_INVALID_C_I_TV: &str = + "03025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa9410e"; + const MESSAGE_1_INVALID_CIPHERSUITE_TV: &str = + "0381025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa90e"; + const MESSAGE_1_INVALID_TEXT_EPHEMERAL_KEY_TV: &str = + "0302782020616972207370656564206F66206120756E6C6164656E207377616C6C6F77200e"; + const MESSAGE_2_INVALID_NUMBER_OF_CBOR_SEQUENCE_TV: &str = + "5820419701d7f00a26c2dc587a36dd752549f33763c893422c8ea0f955a13a4ff5d54B9862a11de42a95d785386a"; + const PLAINTEXT_2_SURPLUS_MAP_ID_CRED_TV: &str = "27a10442321048fa5efa2ebf920bf3"; + const PLAINTEXT_2_SURPLUS_BSTR_ID_CRED_TV: &str = "27413248fa5efa2ebf920bf3"; + #[test] fn test_ecdh() { let g_xy = p256_ecdh(&X_TV, &G_Y_TV); @@ -1637,20 +1664,6 @@ mod tests { assert!(ead_1.is_none()); } - const _MESSAGE_1_TV: &str = - "0382060258208af6f430ebe18d34184017a9a11bf511c8dff8f834730b96c1b7c8dbca2fc3b637"; - const MESSAGE_1_INVALID_ARRAY_TV: &str = - "8403025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa90e"; - const MESSAGE_1_INVALID_C_I_TV: &str = - "03025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa9410e"; - const MESSAGE_1_INVALID_CIPHERSUITE_TV: &str = - "0381025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa90e"; - const MESSAGE_1_INVALID_TEXT_EPHEMERAL_KEY_TV: &str = - "0302782020616972207370656564206F66206120756E6C6164656E207377616C6C6F77200e"; - const MESSAGE_2_INVALID_NUMBER_OF_CBOR_SEQUENCE_TV: &str = - "5820419701d7f00a26c2dc587a36dd752549f33763c893422c8ea0f955a13a4ff5d54B9862a11de42a95d785386a"; - const PLAINTEXT_2_SURPLUS_MAP_ID_CRED_TV: &str = "27a10442321048fa5efa2ebf920bf3"; - const PLAINTEXT_2_SURPLUS_BSTR_ID_CRED_TV: &str = "27413248fa5efa2ebf920bf3"; #[test] fn test_parse_message_1_invalid_traces() { let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_ARRAY_TV); @@ -1666,6 +1679,12 @@ mod tests { assert!(parse_message_1(&message_1_tv).is_err()); } + #[test] + fn test_parse_message_2_invalid_traces() { + let message_2_tv = BufferMessage1::from_hex(MESSAGE_2_INVALID_NUMBER_OF_CBOR_SEQUENCE_TV); + assert!(parse_message_2(&message_2_tv).is_err()); + } + #[test] fn test_encode_message_2() { let ciphertext_2_tv = BufferCiphertext2::from_hex(CIPHERTEXT_2_TV); @@ -1797,6 +1816,23 @@ mod tests { assert_eq!(plaintext_2, plaintext_2_tv); } + #[test] + fn test_parse_plaintext_2_invalid_traces() { + let plaintext_2_tv = BufferPlaintext2::from_hex(PLAINTEXT_2_SURPLUS_MAP_ID_CRED_TV); + let mut plaintext_2_tv_buffer: BytesMaxBuffer = [0x00u8; MAX_BUFFER_LEN]; + plaintext_2_tv_buffer[..plaintext_2_tv.len] + .copy_from_slice(&plaintext_2_tv.content[..plaintext_2_tv.len]); + let plaintext_2 = decode_plaintext_2(&plaintext_2_tv_buffer, plaintext_2_tv.len); + assert!(plaintext_2.is_err()); + + let plaintext_2_tv = BufferPlaintext2::from_hex(PLAINTEXT_2_SURPLUS_BSTR_ID_CRED_TV); + let mut plaintext_2_tv_buffer: BytesMaxBuffer = [0x00u8; MAX_BUFFER_LEN]; + plaintext_2_tv_buffer[..plaintext_2_tv.len] + .copy_from_slice(&plaintext_2_tv.content[..plaintext_2_tv.len]); + let plaintext_2 = decode_plaintext_2(&plaintext_2_tv_buffer, plaintext_2_tv.len); + assert!(plaintext_2.is_err()); + } + #[test] fn test_decode_plaintext_2() { let plaintext_2_tv = BufferPlaintext2::from_hex(PLAINTEXT_2_TV); From e31c578e5f2d9c628588d74d0b809b5f44bd2864 Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Thu, 14 Sep 2023 15:43:52 +0200 Subject: [PATCH 03/10] fix[hacspec]: invalid traces message_1 --- hacspec/src/lib.rs | 163 ++++++++++++++++++++++++++++++++------------- 1 file changed, 116 insertions(+), 47 deletions(-) diff --git a/hacspec/src/lib.rs b/hacspec/src/lib.rs index 51fb1ac6..e6e61266 100644 --- a/hacspec/src/lib.rs +++ b/hacspec/src/lib.rs @@ -755,31 +755,36 @@ fn parse_suites_i( // the CBOR array length is encoded in the first byte, so we extract it let suites_len: U8 = rcvd_message_1.content[1] - U8(CBOR_MAJOR_ARRAY); let suites_len: usize = suites_len.declassify().into(); - raw_suites_len = 1; // account for the CBOR_MAJOR_ARRAY byte - if suites_len <= EDHOC_SUITES.len() { - let mut error_occurred = false; - for j in 0..suites_len { - raw_suites_len += 1; - if !error_occurred { - // parse based on cipher suite identifier - let cs_id = (rcvd_message_1.content[raw_suites_len] as U8).declassify(); - if cs_id >= 0x00 && cs_id <= 0x17 { - // CBOR unsigned integer (0..23) - suites_i[j] = rcvd_message_1.content[raw_suites_len]; - suites_i_len += 1; - } else if cs_id == 0x18 { - // CBOR unsigned integer (one-byte uint8_t follows) - raw_suites_len += 1; // account for the 0x18 tag byte - suites_i[j] = rcvd_message_1.content[raw_suites_len]; - suites_i_len += 1; - } else { - error = EDHOCError::ParsingError; - error_occurred = true; + // check surplus array encoding of ciphersuite + if suites_len > 1 { + raw_suites_len = 1; // account for the CBOR_MAJOR_ARRAY byte + if suites_len <= EDHOC_SUITES.len() { + let mut error_occurred = false; + for j in 0..suites_len { + raw_suites_len += 1; + if !error_occurred { + // parse based on cipher suite identifier + let cs_id = (rcvd_message_1.content[raw_suites_len] as U8).declassify(); + if cs_id >= 0x00 && cs_id <= 0x17 { + // CBOR unsigned integer (0..23) + suites_i[j] = rcvd_message_1.content[raw_suites_len]; + suites_i_len += 1; + } else if cs_id == 0x18 { + // CBOR unsigned integer (one-byte uint8_t follows) + raw_suites_len += 1; // account for the 0x18 tag byte + suites_i[j] = rcvd_message_1.content[raw_suites_len]; + suites_i_len += 1; + } else { + error = EDHOCError::ParsingError; + error_occurred = true; + } } } - } - if !error_occurred { - error = EDHOCError::Success; + if !error_occurred { + error = EDHOCError::Success; + } + } else { + error = EDHOCError::ParsingError; } } else { error = EDHOCError::ParsingError; @@ -841,6 +846,25 @@ fn parse_ead( } } +fn is_cbor_array(first_byte: U8) -> bool { + return (first_byte.declassify() & CBOR_MAJOR_ARRAY) == CBOR_MAJOR_ARRAY; +} + +fn is_encoded_conn_id_minimal(conn_id: U8) -> bool { + let conn_id = conn_id.declassify(); + return (conn_id >= 20 && conn_id <= 37) || (conn_id >= 0 && conn_id <= 17); +} + +fn should_encoded_conn_id_be_minimal(conn_id_byte1: U8, conn_id_byte2: U8) -> bool { + return !is_encoded_conn_id_minimal(conn_id_byte1) + && (conn_id_byte1.declassify() == (CBOR_MAJOR_BYTE_STRING | 0x1)) // bstr with length of 1 + && is_encoded_conn_id_minimal(conn_id_byte2); +} + +fn should_encoded_ciphersuite_be_minimal(byte1: U8, byte2: U8) -> bool { + return is_cbor_array(byte1) && (byte1.declassify() - CBOR_MAJOR_ARRAY) == 1; +} + fn parse_message_1( rcvd_message_1: &BufferMessage1, ) -> Result< @@ -855,6 +879,7 @@ fn parse_message_1( EDHOCError, > { let mut error: EDHOCError = EDHOCError::UnknownError; + let mut method: U8 = U8(0xff); let mut g_x: BytesP256ElemLen = BytesP256ElemLen::new(); let mut suites_i = BytesSuites::new(); let mut suites_i_len: usize = 0; @@ -862,39 +887,54 @@ fn parse_message_1( let mut c_i = U8(0); let mut ead_1 = None::; - let method = rcvd_message_1.content[0]; + // check surplus array encoding + if !is_cbor_array(rcvd_message_1.content[0]) { + method = rcvd_message_1.content[0]; - let res_suites = parse_suites_i(rcvd_message_1); + let res_suites = parse_suites_i(rcvd_message_1); - if res_suites.is_ok() { - (suites_i, suites_i_len, raw_suites_len) = res_suites.unwrap(); - - g_x = BytesP256ElemLen::from_slice( - &rcvd_message_1.content, - 3 + raw_suites_len, - P256_ELEM_LEN, - ); + if res_suites.is_ok() { + (suites_i, suites_i_len, raw_suites_len) = res_suites.unwrap(); - c_i = rcvd_message_1.content[3 + raw_suites_len + P256_ELEM_LEN]; + let g_x_type = rcvd_message_1.content[1 + raw_suites_len].declassify(); + if g_x_type == CBOR_BYTE_STRING { + g_x = BytesP256ElemLen::from_slice( + &rcvd_message_1.content, + 3 + raw_suites_len, + P256_ELEM_LEN, + ); - // if there is still more to parse, the rest will be the EAD_1 - if rcvd_message_1.len > (4 + raw_suites_len + P256_ELEM_LEN) { - // NOTE: since the current implementation only supports one EAD handler, - // we assume only one EAD item - let ead_res = parse_ead(rcvd_message_1, 4 + raw_suites_len + P256_ELEM_LEN); - if ead_res.is_ok() { - ead_1 = ead_res.unwrap(); - error = EDHOCError::Success; + // check surplus bstr encoding + c_i = rcvd_message_1.content[3 + raw_suites_len + P256_ELEM_LEN]; + let c_i_lookahead = rcvd_message_1.content[4 + raw_suites_len + P256_ELEM_LEN]; + if !should_encoded_conn_id_be_minimal(c_i, c_i_lookahead) { + // if there is still more to parse, the rest will be the EAD_1 + if rcvd_message_1.len > (4 + raw_suites_len + P256_ELEM_LEN) { + // NOTE: since the current implementation only supports one EAD handler, + // we assume only one EAD item + let ead_res = parse_ead(rcvd_message_1, 4 + raw_suites_len + P256_ELEM_LEN); + if ead_res.is_ok() { + ead_1 = ead_res.unwrap(); + error = EDHOCError::Success; + } else { + error = ead_res.unwrap_err(); + } + } else if rcvd_message_1.len == (4 + raw_suites_len + P256_ELEM_LEN) { + error = EDHOCError::Success; + } else { + error = EDHOCError::ParsingError; + } + } else { + error = EDHOCError::ParsingError; + } } else { - error = ead_res.unwrap_err(); + error = EDHOCError::ParsingError; } - } else if rcvd_message_1.len == (4 + raw_suites_len + P256_ELEM_LEN) { - error = EDHOCError::Success; } else { - error = EDHOCError::ParsingError; + error = res_suites.unwrap_err(); } } else { - error = res_suites.unwrap_err(); + error = EDHOCError::ParsingError; } match error { @@ -1572,6 +1612,20 @@ mod tests { const OSCORE_MASTER_SECRET_TV: &str = "8c409a332223ad900e44f3434d2d2ce3"; const OSCORE_MASTER_SALT_TV: &str = "6163f44be862adfa"; + // invalid test vectors, should result in a parsing error + const MESSAGE_1_INVALID_ARRAY_TV: &str = + "8403025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa90e"; + const MESSAGE_1_INVALID_C_I_TV: &str = + "03025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa9410e"; + const MESSAGE_1_INVALID_CIPHERSUITE_TV: &str = + "0381025820741a13d7ba048fbb615e94386aa3b61bea5b3d8f65f32620b749bee8d278efa90e"; + const MESSAGE_1_INVALID_TEXT_EPHEMERAL_KEY_TV: &str = + "0302782020616972207370656564206F66206120756E6C6164656E207377616C6C6F77200e"; + const MESSAGE_2_INVALID_NUMBER_OF_CBOR_SEQUENCE_TV: &str = + "5820419701d7f00a26c2dc587a36dd752549f33763c893422c8ea0f955a13a4ff5d54B9862a11de42a95d785386a"; + const PLAINTEXT_2_SURPLUS_MAP_ID_CRED_TV: &str = "27a10442321048fa5efa2ebf920bf3"; + const PLAINTEXT_2_SURPLUS_BSTR_ID_CRED_TV: &str = "27413248fa5efa2ebf920bf3"; + #[test] fn test_ecdh() { let x_tv = BytesP256ElemLen::from_hex(X_TV); @@ -1668,6 +1722,21 @@ mod tests { assert_eq!(c_i.declassify(), c_i_tv.declassify()); } + #[test] + fn test_parse_message_1_invalid_traces() { + let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_ARRAY_TV); + assert!(parse_message_1(&message_1_tv).is_err()); + + let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_C_I_TV); + assert!(parse_message_1(&message_1_tv).is_err()); + + let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_CIPHERSUITE_TV); + assert!(parse_message_1(&message_1_tv).is_err()); + + let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_TEXT_EPHEMERAL_KEY_TV); + assert!(parse_message_1(&message_1_tv).is_err()); + } + #[test] fn test_encode_message_2() { let message_2_tv = BufferMessage2::from_hex(MESSAGE_2_TV); From 7ca2a4b2d4ccdfe66ef1dfe2a9c3bd0ea5d7f7cc Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Thu, 14 Sep 2023 15:48:27 +0200 Subject: [PATCH 04/10] fix[hacspec]: invalid traces plaintext_2 --- hacspec/src/lib.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/hacspec/src/lib.rs b/hacspec/src/lib.rs index e6e61266..aa4644a0 100644 --- a/hacspec/src/lib.rs +++ b/hacspec/src/lib.rs @@ -1913,6 +1913,27 @@ mod tests { assert_bytes_eq!(plaintext_2.content, plaintext_2_tv.content); } + #[test] + fn test_parse_plaintext_2_invalid_traces() { + let plaintext_2_tv_len = PLAINTEXT_2_SURPLUS_MAP_ID_CRED_TV.len() / 2; + let plaintext_2_tv = BytesMaxBuffer::from_slice( + &ByteSeq::from_hex(PLAINTEXT_2_SURPLUS_MAP_ID_CRED_TV), + 0, + plaintext_2_tv_len, + ); + let plaintext_2 = decode_plaintext_2(&plaintext_2_tv, plaintext_2_tv_len); + assert!(plaintext_2.is_err()); + + let plaintext_2_tv_len = PLAINTEXT_2_SURPLUS_BSTR_ID_CRED_TV.len() / 2; + let plaintext_2_tv = BytesMaxBuffer::from_slice( + &ByteSeq::from_hex(PLAINTEXT_2_SURPLUS_BSTR_ID_CRED_TV), + 0, + plaintext_2_tv_len, + ); + let plaintext_2 = decode_plaintext_2(&plaintext_2_tv, plaintext_2_tv_len); + assert!(plaintext_2.is_err()); + } + #[test] fn test_decode_plaintext_2() { let plaintext_2_tv = BytesMaxBuffer::from_slice( From 3c1d2bc17f3dc8a6cd06159ae7ecd1e3b9b27c33 Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Thu, 14 Sep 2023 16:08:36 +0200 Subject: [PATCH 05/10] fix[hacspec]: invalid traces message_2 --- hacspec/src/lib.rs | 177 +++++++++++++++++++++++++++------------------ 1 file changed, 105 insertions(+), 72 deletions(-) diff --git a/hacspec/src/lib.rs b/hacspec/src/lib.rs index aa4644a0..586535c2 100644 --- a/hacspec/src/lib.rs +++ b/hacspec/src/lib.rs @@ -520,86 +520,91 @@ pub fn i_process_message_2( let mut kid = U8(0xffu8); // invalidate kid if current_state == EDHOCState::WaitMessage2 { - let (g_y, ciphertext_2) = parse_message_2(message_2); - - let th_2 = compute_th_2(&g_y, &h_message_1); + let res = parse_message_2(message_2); + if res.is_ok() { + let (g_y, ciphertext_2) = res.unwrap(); - // compute prk_2e - let prk_2e = compute_prk_2e(&x, &g_y, &th_2); + let th_2 = compute_th_2(&g_y, &h_message_1); - let (plaintext_2, plaintext_2_len) = - encrypt_decrypt_ciphertext_2(&prk_2e, &th_2, &ciphertext_2); + // compute prk_2e + let prk_2e = compute_prk_2e(&x, &g_y, &th_2); - // decode plaintext_2 - let plaintext_2_decoded = decode_plaintext_2(&plaintext_2, plaintext_2_len); + let (plaintext_2, plaintext_2_len) = + encrypt_decrypt_ciphertext_2(&prk_2e, &th_2, &ciphertext_2); - if plaintext_2_decoded.is_ok() { - let (c_r_2, kid, mac_2, ead_2) = plaintext_2_decoded.unwrap(); - c_r = c_r_2; + // decode plaintext_2 + let plaintext_2_decoded = decode_plaintext_2(&plaintext_2, plaintext_2_len); - // Step 3: If EAD is present make it available to the application - let ead_success = if let Some(ead_2) = ead_2 { - i_process_ead_2(ead_2.to_public_item()).is_ok() - } else { - true - }; - if ead_success { - // verify mac_2 - let salt_3e2m = compute_salt_3e2m(&prk_2e, &th_2); - - prk_3e2m = compute_prk_3e2m(&salt_3e2m, &x, g_r); - - let expected_mac_2 = compute_mac_2( - &prk_3e2m, - id_cred_r_expected, - cred_r_expected, - cred_r_len, - &th_2, - ); + if plaintext_2_decoded.is_ok() { + let (c_r_2, kid, mac_2, ead_2) = plaintext_2_decoded.unwrap(); + c_r = c_r_2; - // Check MAC before checking KID - if mac_2.declassify_eq(&expected_mac_2) { - if kid.declassify() - == id_cred_r_expected[id_cred_r_expected.len() - 1].declassify() - { - // step is actually from processing of message_3 - // but we do it here to avoid storing plaintext_2 in State - th_3 = compute_th_3( - &th_2, - &BufferPlaintext2::from_slice(&plaintext_2, 0, plaintext_2_len), - cred_r_expected, - cred_r_len, - ); - // message 3 processing + // Step 3: If EAD is present make it available to the application + let ead_success = if let Some(ead_2) = ead_2 { + i_process_ead_2(ead_2.to_public_item()).is_ok() + } else { + true + }; + if ead_success { + // verify mac_2 + let salt_3e2m = compute_salt_3e2m(&prk_2e, &th_2); + + prk_3e2m = compute_prk_3e2m(&salt_3e2m, &x, g_r); + + let expected_mac_2 = compute_mac_2( + &prk_3e2m, + id_cred_r_expected, + cred_r_expected, + cred_r_len, + &th_2, + ); + + // Check MAC before checking KID + if mac_2.declassify_eq(&expected_mac_2) { + if kid.declassify() + == id_cred_r_expected[id_cred_r_expected.len() - 1].declassify() + { + // step is actually from processing of message_3 + // but we do it here to avoid storing plaintext_2 in State + th_3 = compute_th_3( + &th_2, + &BufferPlaintext2::from_slice(&plaintext_2, 0, plaintext_2_len), + cred_r_expected, + cred_r_len, + ); + // message 3 processing - let salt_4e3m = compute_salt_4e3m(&prk_3e2m, &th_3); + let salt_4e3m = compute_salt_4e3m(&prk_3e2m, &th_3); - prk_4e3m = compute_prk_4e3m(&salt_4e3m, i, &g_y); + prk_4e3m = compute_prk_4e3m(&salt_4e3m, i, &g_y); - error = EDHOCError::Success; - current_state = EDHOCState::ProcessedMessage2; + error = EDHOCError::Success; + current_state = EDHOCState::ProcessedMessage2; - state = construct_state( - current_state, - x, - _c_i, - g_y, - prk_3e2m, - prk_4e3m, - _prk_out, - _prk_exporter, - h_message_1, - th_3, - ); + state = construct_state( + current_state, + x, + _c_i, + g_y, + prk_3e2m, + prk_4e3m, + _prk_out, + _prk_exporter, + h_message_1, + th_3, + ); + } else { + // Unknown peer + error = EDHOCError::UnknownPeer; + } } else { - // Unknown peer - error = EDHOCError::UnknownPeer; + error = EDHOCError::MacVerificationFailed; } } else { - error = EDHOCError::MacVerificationFailed; + error = EDHOCError::EADError; } } else { - error = EDHOCError::EADError; + error = EDHOCError::ParsingError; } } else { error = EDHOCError::ParsingError; @@ -1021,14 +1026,34 @@ fn encode_message_1( output } -fn parse_message_2(rcvd_message_2: &BufferMessage2) -> (BytesP256ElemLen, BufferCiphertext2) { +fn parse_message_2( + rcvd_message_2: &BufferMessage2, +) -> Result<(BytesP256ElemLen, BufferCiphertext2), EDHOCError> { + let mut error: EDHOCError = EDHOCError::UnknownError; // FIXME decode negative integers as well - let g_y = BytesP256ElemLen::from_slice(&rcvd_message_2.content, 2, P256_ELEM_LEN); - let ciphertext_2_len = rcvd_message_2.len - P256_ELEM_LEN - 2; // len - gy_len - 2 - let ciphertext_2 = - BufferCiphertext2::from_slice(&rcvd_message_2.content, 2 + P256_ELEM_LEN, ciphertext_2_len); + let mut g_y: BytesP256ElemLen = BytesP256ElemLen::new(); + let mut ciphertext_2: BufferCiphertext2 = BufferCiphertext2::new(); + + // ensure the whole message is a single CBOR sequence + if u8::from(rcvd_message_2.content[0]) == CBOR_BYTE_STRING + && u8::from(rcvd_message_2.content[1]) == (rcvd_message_2.len as u8 - 2) + { + g_y = BytesP256ElemLen::from_slice(&rcvd_message_2.content, 2, P256_ELEM_LEN); + let ciphertext_2_len = rcvd_message_2.len - P256_ELEM_LEN - 2; // len - gy_len - 2 + ciphertext_2 = BufferCiphertext2::from_slice( + &rcvd_message_2.content, + 2 + P256_ELEM_LEN, + ciphertext_2_len, + ); + error = EDHOCError::Success; + } else { + error = EDHOCError::ParsingError; + } - (g_y, ciphertext_2) + match error { + EDHOCError::Success => Ok((g_y, ciphertext_2)), + _ => Err(error), + } } fn encode_message_2(g_y: &BytesP256ElemLen, ciphertext_2: &BufferCiphertext2) -> BufferMessage2 { @@ -1737,6 +1762,12 @@ mod tests { assert!(parse_message_1(&message_1_tv).is_err()); } + #[test] + fn test_parse_message_2_invalid_traces() { + let message_2_tv = BufferMessage1::from_hex(MESSAGE_2_INVALID_NUMBER_OF_CBOR_SEQUENCE_TV); + assert!(parse_message_2(&message_2_tv).is_err()); + } + #[test] fn test_encode_message_2() { let message_2_tv = BufferMessage2::from_hex(MESSAGE_2_TV); @@ -1754,7 +1785,9 @@ mod tests { let g_y_tv = BytesP256ElemLen::from_hex(G_Y_TV); let ciphertext_2_tv = BufferCiphertext2::from_hex(CIPHERTEXT_2_TV); - let (g_y, ciphertext_2) = parse_message_2(&message_2_tv); + let ret = parse_message_2(&message_2_tv); + assert!(ret.is_ok()); + let (g_y, ciphertext_2) = ret.unwrap(); assert_bytes_eq!(g_y, g_y_tv); assert_bytes_eq!(ciphertext_2.content, ciphertext_2_tv.content); From da754d31dfee6d760778f9e4c1e126d53f3f9a44 Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Fri, 15 Sep 2023 10:12:51 +0200 Subject: [PATCH 06/10] refactor: parsing tests --- hacspec/src/lib.rs | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/hacspec/src/lib.rs b/hacspec/src/lib.rs index 586535c2..0ffd9f9d 100644 --- a/hacspec/src/lib.rs +++ b/hacspec/src/lib.rs @@ -1035,8 +1035,8 @@ fn parse_message_2( let mut ciphertext_2: BufferCiphertext2 = BufferCiphertext2::new(); // ensure the whole message is a single CBOR sequence - if u8::from(rcvd_message_2.content[0]) == CBOR_BYTE_STRING - && u8::from(rcvd_message_2.content[1]) == (rcvd_message_2.len as u8 - 2) + if (rcvd_message_2.content[0] as U8).declassify() == CBOR_BYTE_STRING + && (rcvd_message_2.content[1] as U8).declassify() == (rcvd_message_2.len as u8 - 2) { g_y = BytesP256ElemLen::from_slice(&rcvd_message_2.content, 2, P256_ELEM_LEN); let ciphertext_2_len = rcvd_message_2.len - P256_ELEM_LEN - 2; // len - gy_len - 2 @@ -1750,22 +1750,37 @@ mod tests { #[test] fn test_parse_message_1_invalid_traces() { let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_ARRAY_TV); - assert!(parse_message_1(&message_1_tv).is_err()); + assert_eq!( + parse_message_1(&message_1_tv).unwrap_err(), + EDHOCError::ParsingError + ); let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_C_I_TV); - assert!(parse_message_1(&message_1_tv).is_err()); + assert_eq!( + parse_message_1(&message_1_tv).unwrap_err(), + EDHOCError::ParsingError + ); let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_CIPHERSUITE_TV); - assert!(parse_message_1(&message_1_tv).is_err()); + assert_eq!( + parse_message_1(&message_1_tv).unwrap_err(), + EDHOCError::ParsingError + ); let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_TEXT_EPHEMERAL_KEY_TV); - assert!(parse_message_1(&message_1_tv).is_err()); + assert_eq!( + parse_message_1(&message_1_tv).unwrap_err(), + EDHOCError::ParsingError + ); } #[test] fn test_parse_message_2_invalid_traces() { let message_2_tv = BufferMessage1::from_hex(MESSAGE_2_INVALID_NUMBER_OF_CBOR_SEQUENCE_TV); - assert!(parse_message_2(&message_2_tv).is_err()); + assert_eq!( + parse_message_2(&message_2_tv).unwrap_err(), + EDHOCError::ParsingError + ); } #[test] @@ -1955,7 +1970,7 @@ mod tests { plaintext_2_tv_len, ); let plaintext_2 = decode_plaintext_2(&plaintext_2_tv, plaintext_2_tv_len); - assert!(plaintext_2.is_err()); + assert_eq!(plaintext_2.unwrap_err(), EDHOCError::ParsingError); let plaintext_2_tv_len = PLAINTEXT_2_SURPLUS_BSTR_ID_CRED_TV.len() / 2; let plaintext_2_tv = BytesMaxBuffer::from_slice( @@ -1964,7 +1979,7 @@ mod tests { plaintext_2_tv_len, ); let plaintext_2 = decode_plaintext_2(&plaintext_2_tv, plaintext_2_tv_len); - assert!(plaintext_2.is_err()); + assert_eq!(plaintext_2.unwrap_err(), EDHOCError::ParsingError); } #[test] From 0239e090bddff1a3e523586e64b7b3306254342a Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Fri, 15 Sep 2023 11:02:23 +0200 Subject: [PATCH 07/10] refactor: c_i parsing check --- consts/src/lib.rs | 2 ++ hacspec/src/lib.rs | 19 +++++-------------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/consts/src/lib.rs b/consts/src/lib.rs index 14245d57..872f2a61 100644 --- a/consts/src/lib.rs +++ b/consts/src/lib.rs @@ -133,6 +133,8 @@ mod common { pub const CBOR_UINT_1BYTE: u8 = 0x18u8; pub const CBOR_NEG_INT_1BYTE_START: u8 = 0x20u8; pub const CBOR_NEG_INT_1BYTE_END: u8 = 0x37u8; + pub const CBOR_UINT_1BYTE_START: u8 = 0x0u8; + pub const CBOR_UINT_1BYTE_END: u8 = 0x17u8; pub const CBOR_MAJOR_TEXT_STRING: u8 = 0x60u8; pub const CBOR_MAJOR_BYTE_STRING: u8 = 0x40u8; pub const CBOR_MAJOR_ARRAY: u8 = 0x80u8; diff --git a/hacspec/src/lib.rs b/hacspec/src/lib.rs index 0ffd9f9d..feedc48e 100644 --- a/hacspec/src/lib.rs +++ b/hacspec/src/lib.rs @@ -855,17 +855,6 @@ fn is_cbor_array(first_byte: U8) -> bool { return (first_byte.declassify() & CBOR_MAJOR_ARRAY) == CBOR_MAJOR_ARRAY; } -fn is_encoded_conn_id_minimal(conn_id: U8) -> bool { - let conn_id = conn_id.declassify(); - return (conn_id >= 20 && conn_id <= 37) || (conn_id >= 0 && conn_id <= 17); -} - -fn should_encoded_conn_id_be_minimal(conn_id_byte1: U8, conn_id_byte2: U8) -> bool { - return !is_encoded_conn_id_minimal(conn_id_byte1) - && (conn_id_byte1.declassify() == (CBOR_MAJOR_BYTE_STRING | 0x1)) // bstr with length of 1 - && is_encoded_conn_id_minimal(conn_id_byte2); -} - fn should_encoded_ciphersuite_be_minimal(byte1: U8, byte2: U8) -> bool { return is_cbor_array(byte1) && (byte1.declassify() - CBOR_MAJOR_ARRAY) == 1; } @@ -909,10 +898,12 @@ fn parse_message_1( P256_ELEM_LEN, ); - // check surplus bstr encoding c_i = rcvd_message_1.content[3 + raw_suites_len + P256_ELEM_LEN]; - let c_i_lookahead = rcvd_message_1.content[4 + raw_suites_len + P256_ELEM_LEN]; - if !should_encoded_conn_id_be_minimal(c_i, c_i_lookahead) { + // check that c_i is encoded as single-byte uint (we still do not support bstr encoding) + let c_i_u8 = c_i.declassify(); + if (c_i_u8 >= CBOR_NEG_INT_1BYTE_START && c_i_u8 <= CBOR_NEG_INT_1BYTE_END) + || (c_i_u8 >= CBOR_UINT_1BYTE_START && c_i_u8 <= CBOR_UINT_1BYTE_END) + { // if there is still more to parse, the rest will be the EAD_1 if rcvd_message_1.len > (4 + raw_suites_len + P256_ELEM_LEN) { // NOTE: since the current implementation only supports one EAD handler, From e1a36d07696d3e6c90186efd14dfb6d2c8f2e65f Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Fri, 15 Sep 2023 14:08:50 +0200 Subject: [PATCH 08/10] refactor: check cbor sequence message_1 --- hacspec/src/lib.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/hacspec/src/lib.rs b/hacspec/src/lib.rs index feedc48e..6ccb8d16 100644 --- a/hacspec/src/lib.rs +++ b/hacspec/src/lib.rs @@ -851,12 +851,16 @@ fn parse_ead( } } -fn is_cbor_array(first_byte: U8) -> bool { - return (first_byte.declassify() & CBOR_MAJOR_ARRAY) == CBOR_MAJOR_ARRAY; +#[inline(always)] +fn is_cbor_uint8(byte: U8) -> bool { + let byte = byte.declassify(); + return byte >= CBOR_UINT_1BYTE_START && byte <= CBOR_UINT_1BYTE_END; } -fn should_encoded_ciphersuite_be_minimal(byte1: U8, byte2: U8) -> bool { - return is_cbor_array(byte1) && (byte1.declassify() - CBOR_MAJOR_ARRAY) == 1; +#[inline(always)] +fn is_cbor_int8(byte: U8) -> bool { + let byte = byte.declassify(); + return byte >= CBOR_NEG_INT_1BYTE_START && byte <= CBOR_NEG_INT_1BYTE_END; } fn parse_message_1( @@ -881,8 +885,8 @@ fn parse_message_1( let mut c_i = U8(0); let mut ead_1 = None::; - // check surplus array encoding - if !is_cbor_array(rcvd_message_1.content[0]) { + // first element of CBOR sequence must be an integer + if is_cbor_uint8(rcvd_message_1.content[0]) { method = rcvd_message_1.content[0]; let res_suites = parse_suites_i(rcvd_message_1); @@ -899,11 +903,8 @@ fn parse_message_1( ); c_i = rcvd_message_1.content[3 + raw_suites_len + P256_ELEM_LEN]; - // check that c_i is encoded as single-byte uint (we still do not support bstr encoding) - let c_i_u8 = c_i.declassify(); - if (c_i_u8 >= CBOR_NEG_INT_1BYTE_START && c_i_u8 <= CBOR_NEG_INT_1BYTE_END) - || (c_i_u8 >= CBOR_UINT_1BYTE_START && c_i_u8 <= CBOR_UINT_1BYTE_END) - { + // check that c_i is encoded as single-byte int (we still do not support bstr encoding) + if is_cbor_int8(c_i) || is_cbor_uint8(c_i) { // if there is still more to parse, the rest will be the EAD_1 if rcvd_message_1.len > (4 + raw_suites_len + P256_ELEM_LEN) { // NOTE: since the current implementation only supports one EAD handler, From 1413ac6b0b19a191df40c06011e36c66397d5b3f Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Fri, 15 Sep 2023 17:24:53 +0200 Subject: [PATCH 09/10] refactor: use dedicated inlined functions for cbor --- consts/src/lib.rs | 2 + hacspec/src/lib.rs | 218 +++++++++++++++++++++++++++------------------ 2 files changed, 132 insertions(+), 88 deletions(-) diff --git a/consts/src/lib.rs b/consts/src/lib.rs index 872f2a61..fb48fc74 100644 --- a/consts/src/lib.rs +++ b/consts/src/lib.rs @@ -137,7 +137,9 @@ mod common { pub const CBOR_UINT_1BYTE_END: u8 = 0x17u8; pub const CBOR_MAJOR_TEXT_STRING: u8 = 0x60u8; pub const CBOR_MAJOR_BYTE_STRING: u8 = 0x40u8; + pub const CBOR_MAJOR_BYTE_STRING_MAX: u8 = 0x57u8; pub const CBOR_MAJOR_ARRAY: u8 = 0x80u8; + pub const CBOR_MAJOR_ARRAY_MAX: u8 = 0x97u8; pub const MAX_INFO_LEN: usize = 2 + SHA256_DIGEST_LEN + // 32-byte digest as bstr 1 + MAX_KDF_LABEL_LEN + // label <24 bytes as tstr 1 + MAX_KDF_CONTEXT_LEN + // context <24 bytes as bstr diff --git a/hacspec/src/lib.rs b/hacspec/src/lib.rs index 6ccb8d16..a543d314 100644 --- a/hacspec/src/lib.rs +++ b/hacspec/src/lib.rs @@ -733,6 +733,46 @@ pub fn construct_state( ) } +/// the unsigned integer is encoded as a single byte +#[inline(always)] +fn is_cbor_uint_1byte(byte: U8) -> bool { + let byte = byte.declassify(); + return byte >= CBOR_UINT_1BYTE_START && byte <= CBOR_UINT_1BYTE_END; +} + +/// the unsigned integer is encoded as two bytes +#[inline(always)] +fn is_cbor_uint_2bytes(byte: U8) -> bool { + return byte.declassify() == CBOR_UINT_1BYTE; +} + +/// the negative integer is 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; +} + +/// a single byte signals both bstr type and content length +#[inline(always)] +fn is_cbor_bstr_1byte_prefix(byte: U8) -> bool { + let byte = byte.declassify(); + return byte >= CBOR_MAJOR_BYTE_STRING && byte <= CBOR_MAJOR_BYTE_STRING_MAX; +} + +/// two bytes are used to signal bstr type and content length +#[inline(always)] +fn is_cbor_bstr_2bytes_prefix(byte: U8) -> bool { + return byte.declassify() == CBOR_BYTE_STRING; +} + +/// a single byte signals both array type and content length +#[inline(always)] +fn is_cbor_array_1byte_prefix(byte: U8) -> bool { + let byte = byte.declassify(); + return byte >= CBOR_MAJOR_ARRAY && byte <= CBOR_MAJOR_ARRAY_MAX; +} + fn parse_suites_i( rcvd_message_1: &BufferMessage1, ) -> Result<(BytesSuites, usize, usize), EDHOCError> { @@ -742,54 +782,48 @@ fn parse_suites_i( let mut suites_i_len: usize = 0; // match based on first byte of SUITES_I, which can be either an int or an array - let suites_i_first = (rcvd_message_1.content[1] as U8).declassify(); - if suites_i_first >= 0x00 && suites_i_first <= 0x17 { + if is_cbor_uint_1byte(rcvd_message_1.content[1]) { // CBOR unsigned integer (0..=23) suites_i[0] = rcvd_message_1.content[1]; suites_i_len = 1; raw_suites_len = 1; error = EDHOCError::Success; - } else if suites_i_first == 0x18 { + } else if is_cbor_uint_2bytes(rcvd_message_1.content[1]) { // CBOR unsigned integer (one-byte uint8_t follows) suites_i[0] = rcvd_message_1.content[2]; suites_i_len = 1; raw_suites_len = 2; error = EDHOCError::Success; - } else if suites_i_first >= 0x80 && suites_i_first <= 0x97 { + } else if is_cbor_array_1byte_prefix(rcvd_message_1.content[1]) { // CBOR array (0..=23 data items follow) // the CBOR array length is encoded in the first byte, so we extract it let suites_len: U8 = rcvd_message_1.content[1] - U8(CBOR_MAJOR_ARRAY); let suites_len: usize = suites_len.declassify().into(); - // check surplus array encoding of ciphersuite - if suites_len > 1 { - raw_suites_len = 1; // account for the CBOR_MAJOR_ARRAY byte - if suites_len <= EDHOC_SUITES.len() { - let mut error_occurred = false; - for j in 0..suites_len { - raw_suites_len += 1; - if !error_occurred { - // parse based on cipher suite identifier - let cs_id = (rcvd_message_1.content[raw_suites_len] as U8).declassify(); - if cs_id >= 0x00 && cs_id <= 0x17 { - // CBOR unsigned integer (0..23) - suites_i[j] = rcvd_message_1.content[raw_suites_len]; - suites_i_len += 1; - } else if cs_id == 0x18 { - // CBOR unsigned integer (one-byte uint8_t follows) - raw_suites_len += 1; // account for the 0x18 tag byte - suites_i[j] = rcvd_message_1.content[raw_suites_len]; - suites_i_len += 1; - } else { - error = EDHOCError::ParsingError; - error_occurred = true; - } - } - } + raw_suites_len = 1; // account for the CBOR_MAJOR_ARRAY byte + if suites_len > 1 && suites_len <= EDHOC_SUITES.len() { + // cipher suite array must be at least 2 elements long, but not longer than the defined cipher suites + let mut error_occurred = false; + for j in 0..suites_len { + raw_suites_len += 1; if !error_occurred { - error = EDHOCError::Success; + // parse based on cipher suite identifier + if is_cbor_uint_1byte(rcvd_message_1.content[raw_suites_len]) { + // CBOR unsigned integer (0..23) + suites_i[j] = rcvd_message_1.content[raw_suites_len]; + suites_i_len += 1; + } else if is_cbor_uint_2bytes(rcvd_message_1.content[raw_suites_len]) { + // CBOR unsigned integer (one-byte uint8_t follows) + raw_suites_len += 1; // account for the 0x18 tag byte + suites_i[j] = rcvd_message_1.content[raw_suites_len]; + suites_i_len += 1; + } else { + error = EDHOCError::ParsingError; + error_occurred = true; + } } - } else { - error = EDHOCError::ParsingError; + } + if !error_occurred { + error = EDHOCError::Success; } } else { error = EDHOCError::ParsingError; @@ -812,14 +846,14 @@ fn parse_ead( let mut ead_item = None::; let mut ead_value = None::; - // assume label is either a single byte integer (negative or positive) - let label = message.content[offset].declassify(); - let res_label = if label >= 0x00 && label <= 0x17 { + // 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 as u8, false)) - } else if label >= 0x20 && label <= 0x37 { + Ok((label.declassify() as u8, false)) + } else if is_cbor_neg_int_1byte(label) { // CBOR negative integer (-1..=-24) - Ok((label - (CBOR_NEG_INT_1BYTE_START - 1), true)) + Ok((label.declassify() - (CBOR_NEG_INT_1BYTE_START - 1), true)) } else { Err(EDHOCError::ParsingError) }; @@ -851,18 +885,6 @@ fn parse_ead( } } -#[inline(always)] -fn is_cbor_uint8(byte: U8) -> bool { - let byte = byte.declassify(); - return byte >= CBOR_UINT_1BYTE_START && byte <= CBOR_UINT_1BYTE_END; -} - -#[inline(always)] -fn is_cbor_int8(byte: U8) -> bool { - let byte = byte.declassify(); - return byte >= CBOR_NEG_INT_1BYTE_START && byte <= CBOR_NEG_INT_1BYTE_END; -} - fn parse_message_1( rcvd_message_1: &BufferMessage1, ) -> Result< @@ -886,7 +908,7 @@ fn parse_message_1( let mut ead_1 = None::; // first element of CBOR sequence must be an integer - if is_cbor_uint8(rcvd_message_1.content[0]) { + if is_cbor_uint_1byte(rcvd_message_1.content[0]) { method = rcvd_message_1.content[0]; let res_suites = parse_suites_i(rcvd_message_1); @@ -894,8 +916,7 @@ fn parse_message_1( if res_suites.is_ok() { (suites_i, suites_i_len, raw_suites_len) = res_suites.unwrap(); - let g_x_type = rcvd_message_1.content[1 + raw_suites_len].declassify(); - if g_x_type == CBOR_BYTE_STRING { + if is_cbor_bstr_2bytes_prefix(rcvd_message_1.content[1 + raw_suites_len]) { g_x = BytesP256ElemLen::from_slice( &rcvd_message_1.content, 3 + raw_suites_len, @@ -904,7 +925,7 @@ fn parse_message_1( c_i = rcvd_message_1.content[3 + raw_suites_len + P256_ELEM_LEN]; // check that c_i is encoded as single-byte int (we still do not support bstr encoding) - if is_cbor_int8(c_i) || is_cbor_uint8(c_i) { + if is_cbor_neg_int_1byte(c_i) || is_cbor_uint_1byte(c_i) { // if there is still more to parse, the rest will be the EAD_1 if rcvd_message_1.len > (4 + raw_suites_len + P256_ELEM_LEN) { // NOTE: since the current implementation only supports one EAD handler, @@ -1027,7 +1048,7 @@ fn parse_message_2( let mut ciphertext_2: BufferCiphertext2 = BufferCiphertext2::new(); // ensure the whole message is a single CBOR sequence - if (rcvd_message_2.content[0] as U8).declassify() == CBOR_BYTE_STRING + if is_cbor_bstr_2bytes_prefix(rcvd_message_2.content[0]) && (rcvd_message_2.content[1] as U8).declassify() == (rcvd_message_2.len as u8 - 2) { g_y = BytesP256ElemLen::from_slice(&rcvd_message_2.content, 2, P256_ELEM_LEN); @@ -1160,24 +1181,34 @@ fn decode_plaintext_3( ) -> Result<(U8, BytesMac3, Option), EDHOCError> { let mut ead_3 = None::; let mut error = EDHOCError::UnknownError; - - let kid = plaintext_3.content[0usize]; + let mut kid = U8(0xff); // skip the CBOR magic byte as we know how long the MAC is - let mac_3 = BytesMac3::from_slice(&plaintext_3.content, 2, MAC_LENGTH_3); - - // if there is still more to parse, the rest will be the EAD_3 - if plaintext_3.len > (2 + MAC_LENGTH_3) { - // NOTE: since the current implementation only supports one EAD handler, - // we assume only one EAD item - let ead_res = parse_ead(plaintext_3, 2 + MAC_LENGTH_3); - if ead_res.is_ok() { - ead_3 = ead_res.unwrap(); + let mut mac_3 = BytesMac3::new(); + + // check ID_CRED_I and MAC_3 + if (is_cbor_neg_int_1byte(plaintext_3.content[0]) || is_cbor_uint_1byte(plaintext_3.content[0])) + && (is_cbor_bstr_1byte_prefix(plaintext_3.content[1])) + { + kid = plaintext_3.content[0]; + // skip the CBOR magic byte as we know how long the MAC is + mac_3 = BytesMac3::from_slice(&plaintext_3.content, 2, MAC_LENGTH_3); + + // if there is still more to parse, the rest will be the EAD_3 + if plaintext_3.len > (2 + MAC_LENGTH_3) { + // NOTE: since the current implementation only supports one EAD handler, + // we assume only one EAD item + let ead_res = parse_ead(plaintext_3, 2 + MAC_LENGTH_3); + if ead_res.is_ok() { + ead_3 = ead_res.unwrap(); + error = EDHOCError::Success; + } else { + error = ead_res.unwrap_err(); + } + } else if plaintext_3.len == (2 + MAC_LENGTH_3) { error = EDHOCError::Success; } else { - error = ead_res.unwrap_err(); + error = EDHOCError::ParsingError; } - } else if plaintext_3.len == (2 + MAC_LENGTH_3) { - error = EDHOCError::Success; } else { error = EDHOCError::ParsingError; } @@ -1397,28 +1428,39 @@ fn decode_plaintext_2( ) -> Result<(U8, U8, BytesMac2, Option), EDHOCError> { let mut error = EDHOCError::UnknownError; let mut ead_2 = None::; - - let c_r = plaintext_2[0]; - let id_cred_r = plaintext_2[1]; - // NOTE: skipping cbor byte string byte as we know how long the string is - let mac_2 = BytesMac2::from_slice(plaintext_2, 3, MAC_LENGTH_2); - - // if there is still more to parse, the rest will be the EAD_2 - if plaintext_2_len > (3 + MAC_LENGTH_2) { - // NOTE: since the current implementation only supports one EAD handler, - // we assume only one EAD item - let ead_res = parse_ead( - &EdhocMessageBufferHacspec::from_slice(plaintext_2, 0, plaintext_2_len), - 3 + MAC_LENGTH_2, - ); - if ead_res.is_ok() { - ead_2 = ead_res.unwrap(); + let mut c_r: U8 = U8(0xff); + let mut id_cred_r: U8 = U8(0xff); + let mut mac_2 = BytesMac2::new(); + + if (is_cbor_neg_int_1byte(plaintext_2[0]) || is_cbor_uint_1byte(plaintext_2[0])) + && (is_cbor_neg_int_1byte(plaintext_2[1]) || is_cbor_uint_1byte(plaintext_2[1])) + && (is_cbor_bstr_1byte_prefix(plaintext_2[2])) + // TODO: check mac length as well + { + c_r = plaintext_2[0]; + id_cred_r = plaintext_2[1]; + // NOTE: skipping cbor byte string byte as we know how long the string is + mac_2 = BytesMac2::from_slice(plaintext_2, 3, MAC_LENGTH_2); + + // if there is still more to parse, the rest will be the EAD_2 + if plaintext_2_len > (3 + MAC_LENGTH_2) { + // NOTE: since the current implementation only supports one EAD handler, + // we assume only one EAD item + let ead_res = parse_ead( + &EdhocMessageBufferHacspec::from_slice(plaintext_2, 0, plaintext_2_len), + 3 + MAC_LENGTH_2, + ); + if ead_res.is_ok() { + ead_2 = ead_res.unwrap(); + error = EDHOCError::Success; + } else { + error = ead_res.unwrap_err(); + } + } else if plaintext_2_len == (3 + MAC_LENGTH_2) { error = EDHOCError::Success; } else { - error = ead_res.unwrap_err(); + error = EDHOCError::ParsingError; } - } else if plaintext_2_len == (3 + MAC_LENGTH_2) { - error = EDHOCError::Success; } else { error = EDHOCError::ParsingError; } From 814fef6cdbf47325029d8f5d290fe4b5266beab8 Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Mon, 18 Sep 2023 10:23:34 +0200 Subject: [PATCH 10/10] refactor: bring parser improvements to rust version --- hacspec/src/lib.rs | 14 +-- lib/src/edhoc.rs | 289 ++++++++++++++++++++++++++------------------- 2 files changed, 174 insertions(+), 129 deletions(-) diff --git a/hacspec/src/lib.rs b/hacspec/src/lib.rs index a543d314..7e8f49e3 100644 --- a/hacspec/src/lib.rs +++ b/hacspec/src/lib.rs @@ -733,40 +733,40 @@ pub fn construct_state( ) } -/// the unsigned integer is encoded as a single byte +/// Check for: an unsigned integer encoded as a single byte #[inline(always)] fn is_cbor_uint_1byte(byte: U8) -> bool { let byte = byte.declassify(); return byte >= CBOR_UINT_1BYTE_START && byte <= CBOR_UINT_1BYTE_END; } -/// the unsigned integer is encoded as two bytes +/// Check for: an unsigned integer encoded as two bytes #[inline(always)] fn is_cbor_uint_2bytes(byte: U8) -> bool { return byte.declassify() == CBOR_UINT_1BYTE; } -/// the negative integer is encoded as a single byte +/// 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; } -/// a single byte signals both bstr type and content length +/// 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 { let byte = byte.declassify(); return byte >= CBOR_MAJOR_BYTE_STRING && byte <= CBOR_MAJOR_BYTE_STRING_MAX; } -/// two bytes are used to signal bstr type and content length +/// Check for: a bstr denoted by two bytes, onr for type the other for content length #[inline(always)] fn is_cbor_bstr_2bytes_prefix(byte: U8) -> bool { return byte.declassify() == CBOR_BYTE_STRING; } -/// a single byte signals both array type and content length +/// Check for: an array denoted by a single byte which encodes both type and content length #[inline(always)] fn is_cbor_array_1byte_prefix(byte: U8) -> bool { let byte = byte.declassify(); @@ -1182,7 +1182,6 @@ fn decode_plaintext_3( let mut ead_3 = None::; let mut error = EDHOCError::UnknownError; let mut kid = U8(0xff); - // skip the CBOR magic byte as we know how long the MAC is let mut mac_3 = BytesMac3::new(); // check ID_CRED_I and MAC_3 @@ -1432,6 +1431,7 @@ fn decode_plaintext_2( let mut id_cred_r: U8 = U8(0xff); let mut mac_2 = BytesMac2::new(); + // check CBOR sequence types for c_r, id_cred_r, and mac_2 if (is_cbor_neg_int_1byte(plaintext_2[0]) || is_cbor_uint_1byte(plaintext_2[0])) && (is_cbor_neg_int_1byte(plaintext_2[1]) || is_cbor_uint_1byte(plaintext_2[1])) && (is_cbor_bstr_1byte_prefix(plaintext_2[2])) diff --git a/lib/src/edhoc.rs b/lib/src/edhoc.rs index e0e2632f..e4dfb2c9 100644 --- a/lib/src/edhoc.rs +++ b/lib/src/edhoc.rs @@ -672,6 +672,42 @@ pub fn construct_state( ) } +/// Check for: an unsigned integer encoded as a single byte +#[inline(always)] +fn is_cbor_uint_1byte(byte: U8) -> bool { + return byte >= CBOR_UINT_1BYTE_START && byte <= CBOR_UINT_1BYTE_END; +} + +/// Check for: an unsigned integer encoded as two bytes +#[inline(always)] +fn is_cbor_uint_2bytes(byte: U8) -> bool { + return byte == CBOR_UINT_1BYTE; +} + +/// Check for: a negative integer encoded as a single byte +#[inline(always)] +fn is_cbor_neg_int_1byte(byte: U8) -> bool { + return byte >= CBOR_NEG_INT_1BYTE_START && byte <= CBOR_NEG_INT_1BYTE_END; +} + +/// 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 { + return byte >= CBOR_MAJOR_BYTE_STRING && byte <= CBOR_MAJOR_BYTE_STRING_MAX; +} + +/// Check for: a bstr denoted by two bytes, onr for type the other for content length +#[inline(always)] +fn is_cbor_bstr_2bytes_prefix(byte: U8) -> bool { + return byte == CBOR_BYTE_STRING; +} + +/// Check for: an array denoted by a single byte which encodes both type and content length +#[inline(always)] +fn is_cbor_array_1byte_prefix(byte: U8) -> bool { + return byte >= CBOR_MAJOR_ARRAY && byte <= CBOR_MAJOR_ARRAY_MAX; +} + fn parse_suites_i( rcvd_message_1: &BufferMessage1, ) -> Result<(BytesSuites, usize, usize), EDHOCError> { @@ -681,62 +717,55 @@ fn parse_suites_i( let mut suites_i_len: usize = 0; // match based on first byte of SUITES_I, which can be either an int or an array - match rcvd_message_1.content[1] { + if is_cbor_uint_1byte(rcvd_message_1.content[1]) { // CBOR unsigned integer (0..=23) - 0x00..=0x17 => { - suites_i[0] = rcvd_message_1.content[1]; - suites_i_len = 1; - raw_suites_len = 1; - error = EDHOCError::Success; - } + suites_i[0] = rcvd_message_1.content[1]; + suites_i_len = 1; + raw_suites_len = 1; + error = EDHOCError::Success; + } else if is_cbor_uint_2bytes(rcvd_message_1.content[1]) { // CBOR unsigned integer (one-byte uint8_t follows) - 0x18 => { - suites_i[0] = rcvd_message_1.content[2]; - suites_i_len = 1; - raw_suites_len = 2; - error = EDHOCError::Success; - } + suites_i[0] = rcvd_message_1.content[2]; + suites_i_len = 1; + raw_suites_len = 2; + error = EDHOCError::Success; + } else if is_cbor_array_1byte_prefix(rcvd_message_1.content[1]) { // CBOR array (0..=23 data items follow) - 0x80..=0x97 => { - // the CBOR array length is encoded in the first byte, so we extract it - let suites_len: usize = (rcvd_message_1.content[1] - CBOR_MAJOR_ARRAY).into(); - // check surplus array encoding of ciphersuite - if suites_len > 1 { - raw_suites_len = 1; // account for the CBOR_MAJOR_ARRAY byte - if suites_len <= EDHOC_SUITES.len() { - let mut j: usize = 0; // index for addressing cipher suites - while j < suites_len { - raw_suites_len += 1; - // match based on cipher suite identifier - match rcvd_message_1.content[raw_suites_len] { - // CBOR unsigned integer (0..23) - 0x00..=0x17 => { - suites_i[j] = rcvd_message_1.content[raw_suites_len]; - suites_i_len += 1; - } - // CBOR unsigned integer (one-byte uint8_t follows) - 0x18 => { - raw_suites_len += 1; // account for the 0x18 tag byte - suites_i[j] = rcvd_message_1.content[raw_suites_len]; - suites_i_len += 1; - } - _ => { - error = EDHOCError::ParsingError; - break; - } - } - j += 1; + // the CBOR array length is encoded in the first byte, so we extract it + let suites_len: usize = (rcvd_message_1.content[1] - CBOR_MAJOR_ARRAY).into(); + // check surplus array encoding of ciphersuite + raw_suites_len = 1; // account for the CBOR_MAJOR_ARRAY byte + if suites_len > 1 && suites_len <= EDHOC_SUITES.len() { + let mut j: usize = 0; // index for addressing cipher suites + while j < suites_len { + raw_suites_len += 1; + // match based on cipher suite identifier + match rcvd_message_1.content[raw_suites_len] { + // CBOR unsigned integer (0..23) + 0x00..=0x17 => { + suites_i[j] = rcvd_message_1.content[raw_suites_len]; + suites_i_len += 1; + } + // CBOR unsigned integer (one-byte uint8_t follows) + 0x18 => { + raw_suites_len += 1; // account for the 0x18 tag byte + suites_i[j] = rcvd_message_1.content[raw_suites_len]; + suites_i_len += 1; + } + _ => { + error = EDHOCError::ParsingError; + break; } - error = EDHOCError::Success; - } else { - error = EDHOCError::ParsingError; } - } else { - error = EDHOCError::ParsingError; + j += 1; } + error = EDHOCError::Success; + } else { + error = EDHOCError::ParsingError; } - _ => error = EDHOCError::ParsingError, - }; + } else { + error = EDHOCError::ParsingError; + } match error { EDHOCError::Success => Ok((suites_i, suites_i_len, raw_suites_len)), @@ -751,12 +780,14 @@ fn parse_ead(message: &EdhocMessageBuffer, offset: usize) -> Result Ok((label as u8, false)), + Ok((label as u8, false)) + } else if is_cbor_neg_int_1byte(label) { // CBOR negative integer (-1..=-24) - label @ 0x20..=0x37 => Ok((label - (CBOR_NEG_INT_1BYTE_START - 1), true)), - _ => Err(EDHOCError::ParsingError), + Ok((label - (CBOR_NEG_INT_1BYTE_START - 1), true)) + } else { + Err(EDHOCError::ParsingError) }; if res_label.is_ok() { @@ -785,24 +816,6 @@ fn parse_ead(message: &EdhocMessageBuffer, offset: usize) -> Result bool { - return (first_byte & CBOR_MAJOR_ARRAY) == CBOR_MAJOR_ARRAY; -} - -fn is_encoded_conn_id_minimal(enc_conn_id: U8) -> bool { - return (enc_conn_id >= 20 && enc_conn_id <= 37) || (enc_conn_id >= 0 && enc_conn_id <= 17); -} - -fn should_encoded_conn_id_be_minimal(conn_id_byte1: U8, conn_id_byte2: U8) -> bool { - return !is_encoded_conn_id_minimal(conn_id_byte1) - && (conn_id_byte1 == (CBOR_MAJOR_BYTE_STRING | 0x1)) // bstr with length of 1 - && is_encoded_conn_id_minimal(conn_id_byte2); -} - -fn should_encoded_ciphersuite_be_minimal(byte1: U8, byte2: U8) -> bool { - return is_cbor_array(byte1) && (byte1 - CBOR_MAJOR_ARRAY) == 1; -} - fn parse_message_1( rcvd_message_1: &BufferMessage1, ) -> Result< @@ -825,24 +838,22 @@ fn parse_message_1( let mut c_i = 0; let mut ead_1 = None::; - // check surplus array encoding - if !is_cbor_array(rcvd_message_1.content[0]) { + // first element of CBOR sequence must be an integer + if is_cbor_uint_1byte(rcvd_message_1.content[0]) { method = rcvd_message_1.content[0]; let res_suites = parse_suites_i(rcvd_message_1); if res_suites.is_ok() { (suites_i, suites_i_len, raw_suites_len) = res_suites.unwrap(); - let g_x_type = rcvd_message_1.content[1 + raw_suites_len]; - if g_x_type == CBOR_BYTE_STRING { + if is_cbor_bstr_2bytes_prefix(rcvd_message_1.content[1 + raw_suites_len]) { g_x.copy_from_slice( &rcvd_message_1.content[3 + raw_suites_len..3 + raw_suites_len + P256_ELEM_LEN], ); - // check surplus bstr encoding c_i = rcvd_message_1.content[3 + raw_suites_len + P256_ELEM_LEN]; - let c_i_lookahead = rcvd_message_1.content[4 + raw_suites_len + P256_ELEM_LEN]; - if !should_encoded_conn_id_be_minimal(c_i, c_i_lookahead) { + // check that c_i is encoded as single-byte int (we still do not support bstr encoding) + if is_cbor_neg_int_1byte(c_i) || is_cbor_uint_1byte(c_i) { // if there is still more to parse, the rest will be the EAD_1 if rcvd_message_1.len > (4 + raw_suites_len + P256_ELEM_LEN) { // NOTE: since the current implementation only supports one EAD handler, @@ -966,7 +977,7 @@ fn parse_message_2( let mut ciphertext_2: BufferCiphertext2 = BufferCiphertext2::new(); // ensure the whole message is a single CBOR sequence - if rcvd_message_2.content[0] == CBOR_BYTE_STRING + if is_cbor_bstr_2bytes_prefix(rcvd_message_2.content[0]) && rcvd_message_2.content[1] == (rcvd_message_2.len as u8 - 2) { g_y[..].copy_from_slice(&rcvd_message_2.content[2..2 + P256_ELEM_LEN]); @@ -1099,25 +1110,33 @@ fn decode_plaintext_3( ) -> Result<(U8, BytesMac3, Option), EDHOCError> { let mut ead_3 = None::; let mut error = EDHOCError::UnknownError; - - let kid = plaintext_3.content[0usize]; - // skip the CBOR magic byte as we know how long the MAC is + let mut kid: u8 = 0xff; let mut mac_3: BytesMac3 = [0x00; MAC_LENGTH_3]; - mac_3[..].copy_from_slice(&plaintext_3.content[2..2 + MAC_LENGTH_3]); - - // if there is still more to parse, the rest will be the EAD_3 - if plaintext_3.len > (2 + MAC_LENGTH_3) { - // NOTE: since the current implementation only supports one EAD handler, - // we assume only one EAD item - let ead_res = parse_ead(plaintext_3, 2 + MAC_LENGTH_3); - if ead_res.is_ok() { - ead_3 = ead_res.unwrap(); + + // check ID_CRED_I and MAC_3 + if (is_cbor_neg_int_1byte(plaintext_3.content[0]) || is_cbor_uint_1byte(plaintext_3.content[0])) + && (is_cbor_bstr_1byte_prefix(plaintext_3.content[1])) + { + kid = plaintext_3.content[0usize]; + // skip the CBOR magic byte as we know how long the MAC is + mac_3[..].copy_from_slice(&plaintext_3.content[2..2 + MAC_LENGTH_3]); + + // if there is still more to parse, the rest will be the EAD_3 + if plaintext_3.len > (2 + MAC_LENGTH_3) { + // NOTE: since the current implementation only supports one EAD handler, + // we assume only one EAD item + let ead_res = parse_ead(plaintext_3, 2 + MAC_LENGTH_3); + if ead_res.is_ok() { + ead_3 = ead_res.unwrap(); + error = EDHOCError::Success; + } else { + error = ead_res.unwrap_err(); + } + } else if plaintext_3.len == (2 + MAC_LENGTH_3) { error = EDHOCError::Success; } else { - error = ead_res.unwrap_err(); + error = EDHOCError::ParsingError; } - } else if plaintext_3.len == (2 + MAC_LENGTH_3) { - error = EDHOCError::Success; } else { error = EDHOCError::ParsingError; } @@ -1323,29 +1342,40 @@ fn decode_plaintext_2( ) -> Result<(U8, U8, BytesMac2, Option), EDHOCError> { let mut error = EDHOCError::UnknownError; let mut ead_2 = None::; - - let c_r = plaintext_2[0]; - let id_cred_r = plaintext_2[1]; - // skip cbor byte string byte as we know how long the string is + let mut c_r: U8 = 0xff; + let mut id_cred_r: U8 = 0xff; let mut mac_2: BytesMac2 = [0x00; MAC_LENGTH_2]; - mac_2[..].copy_from_slice(&plaintext_2[3..3 + MAC_LENGTH_2]); - - // if there is still more to parse, the rest will be the EAD_2 - if plaintext_2_len > (3 + MAC_LENGTH_2) { - // NOTE: since the current implementation only supports one EAD handler, - // we assume only one EAD item - let ead_res = parse_ead( - &plaintext_2[..plaintext_2_len].try_into().expect("too long"), - 3 + MAC_LENGTH_2, - ); - if ead_res.is_ok() { - ead_2 = ead_res.unwrap(); + + // check CBOR sequence types for c_r, id_cred_r, and mac_2 + if (is_cbor_neg_int_1byte(plaintext_2[0]) || is_cbor_uint_1byte(plaintext_2[0])) + && (is_cbor_neg_int_1byte(plaintext_2[1]) || is_cbor_uint_1byte(plaintext_2[1])) + && (is_cbor_bstr_1byte_prefix(plaintext_2[2])) + // TODO: check mac length as well + { + c_r = plaintext_2[0]; + id_cred_r = plaintext_2[1]; + // skip cbor byte string byte as we know how long the string is + mac_2[..].copy_from_slice(&plaintext_2[3..3 + MAC_LENGTH_2]); + + // if there is still more to parse, the rest will be the EAD_2 + if plaintext_2_len > (3 + MAC_LENGTH_2) { + // NOTE: since the current implementation only supports one EAD handler, + // we assume only one EAD item + let ead_res = parse_ead( + &plaintext_2[..plaintext_2_len].try_into().expect("too long"), + 3 + MAC_LENGTH_2, + ); + if ead_res.is_ok() { + ead_2 = ead_res.unwrap(); + error = EDHOCError::Success; + } else { + error = ead_res.unwrap_err(); + } + } else if plaintext_2_len == (3 + MAC_LENGTH_2) { error = EDHOCError::Success; } else { - error = ead_res.unwrap_err(); + error = EDHOCError::ParsingError; } - } else if plaintext_2_len == (3 + MAC_LENGTH_2) { - error = EDHOCError::Success; } else { error = EDHOCError::ParsingError; } @@ -1667,22 +1697,37 @@ mod tests { #[test] fn test_parse_message_1_invalid_traces() { let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_ARRAY_TV); - assert!(parse_message_1(&message_1_tv).is_err()); + assert_eq!( + parse_message_1(&message_1_tv).unwrap_err(), + EDHOCError::ParsingError + ); let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_C_I_TV); - assert!(parse_message_1(&message_1_tv).is_err()); + assert_eq!( + parse_message_1(&message_1_tv).unwrap_err(), + EDHOCError::ParsingError + ); let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_CIPHERSUITE_TV); - assert!(parse_message_1(&message_1_tv).is_err()); + assert_eq!( + parse_message_1(&message_1_tv).unwrap_err(), + EDHOCError::ParsingError + ); let message_1_tv = BufferMessage1::from_hex(MESSAGE_1_INVALID_TEXT_EPHEMERAL_KEY_TV); - assert!(parse_message_1(&message_1_tv).is_err()); + assert_eq!( + parse_message_1(&message_1_tv).unwrap_err(), + EDHOCError::ParsingError + ); } #[test] fn test_parse_message_2_invalid_traces() { let message_2_tv = BufferMessage1::from_hex(MESSAGE_2_INVALID_NUMBER_OF_CBOR_SEQUENCE_TV); - assert!(parse_message_2(&message_2_tv).is_err()); + assert_eq!( + parse_message_1(&message_2_tv).unwrap_err(), + EDHOCError::ParsingError + ); } #[test] @@ -1822,15 +1867,15 @@ mod tests { let mut plaintext_2_tv_buffer: BytesMaxBuffer = [0x00u8; MAX_BUFFER_LEN]; plaintext_2_tv_buffer[..plaintext_2_tv.len] .copy_from_slice(&plaintext_2_tv.content[..plaintext_2_tv.len]); - let plaintext_2 = decode_plaintext_2(&plaintext_2_tv_buffer, plaintext_2_tv.len); - assert!(plaintext_2.is_err()); + let ret = decode_plaintext_2(&plaintext_2_tv_buffer, plaintext_2_tv.len); + assert_eq!(ret.unwrap_err(), EDHOCError::ParsingError); let plaintext_2_tv = BufferPlaintext2::from_hex(PLAINTEXT_2_SURPLUS_BSTR_ID_CRED_TV); let mut plaintext_2_tv_buffer: BytesMaxBuffer = [0x00u8; MAX_BUFFER_LEN]; plaintext_2_tv_buffer[..plaintext_2_tv.len] .copy_from_slice(&plaintext_2_tv.content[..plaintext_2_tv.len]); - let plaintext_2 = decode_plaintext_2(&plaintext_2_tv_buffer, plaintext_2_tv.len); - assert!(plaintext_2.is_err()); + let ret = decode_plaintext_2(&plaintext_2_tv_buffer, plaintext_2_tv.len); + assert_eq!(ret.unwrap_err(), EDHOCError::ParsingError); } #[test]