From 92faf7e4dd7032abfd372949c2115dc31f7ad42f Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Thu, 7 Mar 2024 11:43:54 +0100 Subject: [PATCH 1/3] refactor: use ptr:read/write in lakers-c --- examples/lakers-c-native/main.c | 4 ++-- lakers-c/src/initiator.rs | 14 ++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/examples/lakers-c-native/main.c b/examples/lakers-c-native/main.c index e5a203ef..d127b4e4 100644 --- a/examples/lakers-c-native/main.c +++ b/examples/lakers-c-native/main.c @@ -151,9 +151,9 @@ int main(void) uint8_t c_r; CredentialRPK fetched_cred_r = {0}; #ifdef LAKERS_EAD_AUTHZ - res = initiator_parse_message_2(&initiator, &message_2, cred_r, &c_r, &fetched_cred_r, &ead_2); + res = initiator_parse_message_2(&initiator, &message_2, &cred_r, &c_r, &fetched_cred_r, &ead_2); #else - res = initiator_parse_message_2(&initiator, &message_2, cred_r, &c_r, &fetched_cred_r, &ead_2); + res = initiator_parse_message_2(&initiator, &message_2, &cred_r, &c_r, &fetched_cred_r, &ead_2); #endif if (res != 0) { printf("Error parse msg2: %d\n", res); diff --git a/lakers-c/src/initiator.rs b/lakers-c/src/initiator.rs index 7bc71ab8..bbc527e4 100644 --- a/lakers-c/src/initiator.rs +++ b/lakers-c/src/initiator.rs @@ -27,13 +27,15 @@ pub unsafe extern "C" fn initiator_new(initiator: *mut EdhocInitiator) -> i8 { suites_i[0..suites_i_len].copy_from_slice(&EDHOC_SUPPORTED_SUITES[..]); let (x, g_x) = default_crypto().p256_generate_key_pair(); - (*initiator).start = InitiatorStart { + let start = InitiatorStart { x, g_x, suites_i, suites_i_len, }; + core::ptr::write(&mut (*initiator).start, start); + 0 } @@ -68,8 +70,8 @@ pub unsafe extern "C" fn initiator_prepare_message_1( let result = match i_prepare_message_1(&state, crypto, c_i, &ead_1) { Ok((state, msg_1)) => { - *message_1 = msg_1; - (*initiator_c).wait_m2 = state; + core::ptr::write(&mut *message_1, msg_1); + core::ptr::write(&mut (*initiator_c).wait_m2, state); 0 } Err(err) => err as i8, @@ -83,7 +85,7 @@ pub unsafe extern "C" fn initiator_parse_message_2( // input params initiator_c: *mut EdhocInitiator, message_2: *const EdhocMessageBuffer, - expected_cred_r: CredentialRPK, + expected_cred_r: *const CredentialRPK, // output params c_r_out: *mut u8, valid_cred_r_out: *mut CredentialRPK, @@ -101,7 +103,7 @@ pub unsafe extern "C" fn initiator_parse_message_2( let crypto = &mut default_crypto(); // manually take `state` because Rust cannot move out of a dereferenced raw pointer directly - // raw pointers do not have ownership information, requiring manual handling of the data + // (raw pointers do not have ownership information, requiring manual handling of the data) let state = core::ptr::read(&(*initiator_c).wait_m2); let result = match i_parse_message_2(&state, crypto, &(*message_2)) { @@ -110,7 +112,7 @@ pub unsafe extern "C" fn initiator_parse_message_2( *c_r_out = c_r; // NOTE: checking here to avoid having IdCredOwnedC being passed across the ffi boundary - let Ok(valid_cred_r) = credential_check_or_fetch(Some(expected_cred_r), id_cred_r) + let Ok(valid_cred_r) = credential_check_or_fetch(Some(*expected_cred_r), id_cred_r) else { return -1; }; From b49693379063ddf5802cff8deb4354e9271eca81 Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Thu, 7 Mar 2024 11:45:26 +0100 Subject: [PATCH 2/3] refactor: pass ciphertext as reference instead of mutable --- lib/src/edhoc.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/src/edhoc.rs b/lib/src/edhoc.rs index 082c676f..ef801c0d 100644 --- a/lib/src/edhoc.rs +++ b/lib/src/edhoc.rs @@ -134,7 +134,7 @@ pub fn r_prepare_message_2( let mut ct: BufferCiphertext2 = BufferCiphertext2::new(); ct.fill_with_slice(plaintext_2.as_slice()).unwrap(); // TODO(hax): can we prove with hax that this won't panic since they use the same underlying buffer length? - let ciphertext_2 = encrypt_decrypt_ciphertext_2(crypto, &prk_2e, &th_2, ct); + let ciphertext_2 = encrypt_decrypt_ciphertext_2(crypto, &prk_2e, &th_2, &ct); ct.fill_with_slice(ciphertext_2.as_slice()).unwrap(); // TODO(hax): same as just above. @@ -310,7 +310,7 @@ pub fn i_parse_message_2<'a>( // compute prk_2e let prk_2e = compute_prk_2e(crypto, &state.x, &g_y, &th_2); - let plaintext_2 = encrypt_decrypt_ciphertext_2(crypto, &prk_2e, &th_2, ciphertext_2); + let plaintext_2 = encrypt_decrypt_ciphertext_2(crypto, &prk_2e, &th_2, &ciphertext_2); // decode plaintext_2 let plaintext_2_decoded = decode_plaintext_2(&plaintext_2); @@ -888,7 +888,7 @@ fn encrypt_decrypt_ciphertext_2( crypto: &mut impl CryptoTrait, prk_2e: &BytesHashLen, th_2: &BytesHashLen, - mut ciphertext_2: BufferCiphertext2, + ciphertext_2: &BufferCiphertext2, ) -> BufferCiphertext2 { // convert the transcript hash th_2 to BytesMaxContextBuffer type let mut th_2_context: BytesMaxContextBuffer = [0x00; MAX_KDF_CONTEXT_LEN]; @@ -904,11 +904,13 @@ fn encrypt_decrypt_ciphertext_2( ciphertext_2.len, ); + let mut result = BufferCiphertext2::default(); for i in 0..ciphertext_2.len { - ciphertext_2.content[i] ^= keystream_2[i]; + result.content[i] = ciphertext_2.content[i] ^ keystream_2[i]; } + result.len = ciphertext_2.len; - ciphertext_2 + result } fn compute_salt_4e3m( @@ -1417,7 +1419,7 @@ mod tests { &mut default_crypto(), &PRK_2E_TV, &TH_2_TV, - ciphertext_2_tv, + &ciphertext_2_tv, ); assert_eq!(plaintext_2.len, PLAINTEXT_2_LEN_TV); @@ -1427,7 +1429,7 @@ mod tests { // test encryption let ciphertext_2 = - encrypt_decrypt_ciphertext_2(&mut default_crypto(), &PRK_2E_TV, &TH_2_TV, plaintext_2); + encrypt_decrypt_ciphertext_2(&mut default_crypto(), &PRK_2E_TV, &TH_2_TV, &plaintext_2); assert_eq!(ciphertext_2.len, CIPHERTEXT_2_LEN_TV); for i in 0..CIPHERTEXT_2_LEN_TV { From 86394d9fb2dc96c6c0e62c9b5e7bf58872f8824d Mon Sep 17 00:00:00 2001 From: Geovane Fedrecheski Date: Fri, 8 Mar 2024 12:26:58 +0100 Subject: [PATCH 3/3] refactor(c): enable sanitizers and pass creds as pointers --- examples/lakers-c-native/Makefile | 4 +++- examples/lakers-c-native/README.md | 2 ++ examples/lakers-c-native/main.c | 19 +++++++++++-------- lakers-c/src/initiator.rs | 12 ++++++------ lakers-c/src/lib.rs | 1 + lib/src/edhoc.rs | 2 +- 6 files changed, 24 insertions(+), 16 deletions(-) diff --git a/examples/lakers-c-native/Makefile b/examples/lakers-c-native/Makefile index 69e2c91d..ff091c21 100644 --- a/examples/lakers-c-native/Makefile +++ b/examples/lakers-c-native/Makefile @@ -1,5 +1,5 @@ CC=gcc -CFLAGS=-Wall -I../../target/include +CFLAGS=-Wall -I../../target/include -fsanitize=address,undefined,leak TARGET=lakers_c_native @@ -16,6 +16,8 @@ ifeq ($(LAKERS_EAD), authz) CFLAGS += -DLAKERS_EAD_AUTHZ endif +LDFLAGS=-fsanitize=address,undefined,leak + all: $(TARGET) # rule for building the target executable diff --git a/examples/lakers-c-native/README.md b/examples/lakers-c-native/README.md index 5f94b8c7..be51e865 100644 --- a/examples/lakers-c-native/README.md +++ b/examples/lakers-c-native/README.md @@ -17,3 +17,5 @@ make LAKERS_EAD=authz && ./lakers_c_native - See the README in the `lakers-c` crate. - Install [libcoap](https://libcoap.net/install.html): - tested with the following configuration: `./configure --disable-doxygen --disable-manpages --disable-dtls --disable-oscore` + +Note: the following sanitizers are enabled in the `Makefile`: `address,undefined,leak` (see for example the [AddressSanitizer](https://clang.llvm.org/docs/AddressSanitizer.html)). They may help catch bugs but make the executable larger and slower. diff --git a/examples/lakers-c-native/main.c b/examples/lakers-c-native/main.c index d127b4e4..7cb4593b 100644 --- a/examples/lakers-c-native/main.c +++ b/examples/lakers-c-native/main.c @@ -43,10 +43,13 @@ static coap_response_t message_handler(coap_session_t *session COAP_UNUSED, has_coap_response = 1; // coap_show_pdu(COAP_LOG_WARN, received); const uint8_t *data; - coap_get_data(received, &coap_response_payload_len, &data); - memcpy(coap_response_payload, data, coap_response_payload_len); - puts("received coap response"); - print_hex((uint8_t *)coap_response_payload, coap_response_payload_len); + if (coap_get_data(received, &coap_response_payload_len, &data)) { + memcpy(coap_response_payload, data, coap_response_payload_len); + puts("received coap response"); + print_hex((uint8_t *)coap_response_payload, coap_response_payload_len); + } else { + puts("received coap response without payload"); + } return COAP_RESPONSE_OK; } @@ -169,16 +172,16 @@ int main(void) puts("ead-authz voucher received and validated"); } #endif - res = initiator_verify_message_2(&initiator, &I, cred_i, fetched_cred_r); + res = initiator_verify_message_2(&initiator, &I, &cred_i, &fetched_cred_r); if (res != 0) { printf("Error verify msg2: %d\n", res); return 1; } puts("preparing msg3"); - EdhocMessageBuffer message_3; - uint8_t prk_out[SHA256_DIGEST_LEN]; - res = initiator_prepare_message_3(&initiator, ByReference, NULL, &message_3, prk_out); + EdhocMessageBuffer message_3 = {0}; + uint8_t prk_out[SHA256_DIGEST_LEN] = {0}; + res = initiator_prepare_message_3(&initiator, ByReference, NULL, &message_3, &prk_out); if (res != 0) { printf("Error prep msg3: %d\n", res); return 1; diff --git a/lakers-c/src/initiator.rs b/lakers-c/src/initiator.rs index bbc527e4..2dc5e6bd 100644 --- a/lakers-c/src/initiator.rs +++ b/lakers-c/src/initiator.rs @@ -137,8 +137,8 @@ pub unsafe extern "C" fn initiator_verify_message_2( initiator_c: *mut EdhocInitiator, i: *const BytesP256ElemLen, // i_len: usize, - mut cred_i: CredentialRPK, - valid_cred_r: CredentialRPK, + mut cred_i: *mut CredentialRPK, + valid_cred_r: *mut CredentialRPK, ) -> i8 { if initiator_c.is_null() || i.is_null() { return -1; @@ -147,10 +147,10 @@ pub unsafe extern "C" fn initiator_verify_message_2( let state = core::ptr::read(&(*initiator_c).processing_m2).to_rust(); - match i_verify_message_2(&state, crypto, valid_cred_r, &(*i)) { + match i_verify_message_2(&state, crypto, *valid_cred_r, &(*i)) { Ok(state) => { (*initiator_c).processed_m2 = state; - (*initiator_c).cred_i = &mut cred_i as *mut CredentialRPK; + (*initiator_c).cred_i = cred_i; 0 } Err(err) => err as i8, @@ -172,7 +172,7 @@ pub unsafe extern "C" fn initiator_prepare_message_3( } let crypto = &mut default_crypto(); - let mut state = core::ptr::read(&(*initiator_c).processed_m2); + let state = core::ptr::read(&(*initiator_c).processed_m2); let ead_3 = if ead_3_c.is_null() { None @@ -182,7 +182,7 @@ pub unsafe extern "C" fn initiator_prepare_message_3( }; match i_prepare_message_3( - &mut state, + &state, crypto, *(*initiator_c).cred_i, cred_transfer, diff --git a/lakers-c/src/lib.rs b/lakers-c/src/lib.rs index 8336086b..f3675bf3 100644 --- a/lakers-c/src/lib.rs +++ b/lakers-c/src/lib.rs @@ -9,6 +9,7 @@ use lakers::*; use lakers_crypto::{default_crypto, CryptoTrait}; +#[cfg(feature = "ead-authz")] pub mod ead_authz; pub mod initiator; diff --git a/lib/src/edhoc.rs b/lib/src/edhoc.rs index ef801c0d..d3622b67 100644 --- a/lib/src/edhoc.rs +++ b/lib/src/edhoc.rs @@ -399,7 +399,7 @@ pub fn i_verify_message_2( } pub fn i_prepare_message_3( - state: &mut ProcessedM2, + state: &ProcessedM2, crypto: &mut impl CryptoTrait, cred_i: CredentialRPK, cred_transfer: CredentialTransfer,