From 166daea57ed7938d816888f645fa8cff4fef4f4f Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 16 Feb 2024 17:44:14 +0200 Subject: [PATCH 1/3] p2p: fix data corruption on longer packets The code handling chunking of data frames longer than the configured maximum was faulty. --- p2p/src/secret_connection.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/p2p/src/secret_connection.rs b/p2p/src/secret_connection.rs index 807318087..2d77ed3d1 100644 --- a/p2p/src/secret_connection.rs +++ b/p2p/src/secret_connection.rs @@ -538,15 +538,15 @@ fn encrypt_and_write( data: &[u8], ) -> io::Result { let mut n = 0_usize; - let mut data_copy = data; - while !data_copy.is_empty() { + let mut remaining = data; + while !remaining.is_empty() { let chunk: &[u8]; - if DATA_MAX_SIZE < data.len() { - chunk = &data[..DATA_MAX_SIZE]; - data_copy = &data_copy[DATA_MAX_SIZE..]; + if DATA_MAX_SIZE < remaining.len() { + chunk = &remaining[..DATA_MAX_SIZE]; + remaining = &remaining[DATA_MAX_SIZE..]; } else { - chunk = data_copy; - data_copy = &[0_u8; 0]; + chunk = remaining; + remaining = &[]; } let sealed_frame = &mut [0_u8; TAG_SIZE + TOTAL_FRAME_SIZE]; encrypt(chunk, &send_state.cipher, &send_state.nonce, sealed_frame) From c6f96a06c1c4e8b8db957c501bbed1a78e09bd20 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 16 Feb 2024 18:00:47 +0200 Subject: [PATCH 2/3] Regression test for p2p data corruption --- test/src/test/unit/p2p/secret_connection.rs | 29 +++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/src/test/unit/p2p/secret_connection.rs b/test/src/test/unit/p2p/secret_connection.rs index 410e78ace..f7d88c992 100644 --- a/test/src/test/unit/p2p/secret_connection.rs +++ b/test/src/test/unit/p2p/secret_connection.rs @@ -62,6 +62,35 @@ fn test_read_write_single_message() { receiver.join().expect("receiver thread has panicked"); } +#[test] +fn test_read_write_long_message() { + let mut message: [u8; 1025] = [0x5a; 1025]; + message[1024] = 0xa5; + + let (pipe1, pipe2) = pipe::async_bipipe_buffered(); + + let sender = thread::spawn(move || { + let mut conn1 = new_peer_conn(pipe2).expect("handshake to succeed"); + + conn1 + .write_all(&message) + .expect("expected to write message"); + }); + + let receiver = thread::spawn(move || { + let mut conn2 = new_peer_conn(pipe1).expect("handshake to succeed"); + + let mut buf = [0; 1025]; + conn2 + .read_exact(&mut buf) + .expect("expected to read message"); + assert_eq!(&message, &buf); + }); + + sender.join().expect("sender thread has panicked"); + receiver.join().expect("receiver thread has panicked"); +} + #[test] fn test_evil_peer_shares_invalid_eph_key() { let csprng = OsRng {}; From a9486b3b98764f341fb8501cdee72d92fd87e71d Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 16 Feb 2024 18:09:23 +0200 Subject: [PATCH 3/3] Changelog entry for #1393 --- .changelog/unreleased/bug-fixes/1393-p2p-data-corruption.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/bug-fixes/1393-p2p-data-corruption.md diff --git a/.changelog/unreleased/bug-fixes/1393-p2p-data-corruption.md b/.changelog/unreleased/bug-fixes/1393-p2p-data-corruption.md new file mode 100644 index 000000000..fcea9ebc7 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/1393-p2p-data-corruption.md @@ -0,0 +1,2 @@ +- `[tendermint-p2p]` Fix data corruption on sending long messages via `SecretConnection` + ([\#1393](https://github.com/informalsystems/tendermint-rs/pull/1393))