From b63e01cb905531cdc60db0fe65f02c1ff790a27a Mon Sep 17 00:00:00 2001 From: G1gg1L3s Date: Tue, 27 Jun 2023 22:43:24 +0300 Subject: [PATCH 1/5] rust-themis: Impl From for Error It will allow us using ? with the try_* methods, like try_reserve. --- src/wrappers/themis/rust/src/error.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/wrappers/themis/rust/src/error.rs b/src/wrappers/themis/rust/src/error.rs index b1e5f4edd..711af80f0 100644 --- a/src/wrappers/themis/rust/src/error.rs +++ b/src/wrappers/themis/rust/src/error.rs @@ -149,6 +149,14 @@ impl fmt::Display for Error { } } +impl From for Error { + fn from(_: std::collections::TryReserveError) -> Self { + Self { + kind: ErrorKind::NoMemory, + } + } +} + /// A list of Themis error categories. /// /// This enumeration is used by [`Error`] type, returned by most Themis functions. Some error kinds From 33204a9d3f3b837cb74d8a94aaca258bf02b5174 Mon Sep 17 00:00:00 2001 From: G1gg1L3s Date: Tue, 27 Jun 2023 22:45:00 +0300 Subject: [PATCH 2/5] rust-themis: Use try_reserve instead of reserve --- src/wrappers/themis/rust/src/keygen.rs | 8 ++++---- src/wrappers/themis/rust/src/keys.rs | 2 +- src/wrappers/themis/rust/src/secure_cell.rs | 18 +++++++++--------- .../themis/rust/src/secure_comparator.rs | 4 ++-- src/wrappers/themis/rust/src/secure_message.rs | 8 ++++---- src/wrappers/themis/rust/src/secure_session.rs | 10 +++++----- 6 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/wrappers/themis/rust/src/keygen.rs b/src/wrappers/themis/rust/src/keygen.rs index 4de1e323b..fe3234432 100644 --- a/src/wrappers/themis/rust/src/keygen.rs +++ b/src/wrappers/themis/rust/src/keygen.rs @@ -89,8 +89,8 @@ fn try_gen_rsa_key_pair() -> Result { } } - private_key.reserve(private_key_len); - public_key.reserve(public_key_len); + private_key.try_reserve(private_key_len)?; + public_key.try_reserve(public_key_len)?; unsafe { let status = themis_gen_rsa_key_pair( @@ -149,8 +149,8 @@ fn try_gen_ec_key_pair() -> Result { } } - private_key.reserve(private_key_len); - public_key.reserve(public_key_len); + private_key.try_reserve(private_key_len)?; + public_key.try_reserve(public_key_len)?; unsafe { let status = themis_gen_ec_key_pair( diff --git a/src/wrappers/themis/rust/src/keys.rs b/src/wrappers/themis/rust/src/keys.rs index 41f58f5f5..0137ecca1 100644 --- a/src/wrappers/themis/rust/src/keys.rs +++ b/src/wrappers/themis/rust/src/keys.rs @@ -640,7 +640,7 @@ impl SymmetricKey { } } - key.reserve(key_len); + key.try_reserve(key_len)?; unsafe { let status = themis_gen_sym_key(key.as_mut_ptr(), &mut key_len); diff --git a/src/wrappers/themis/rust/src/secure_cell.rs b/src/wrappers/themis/rust/src/secure_cell.rs index 5564e5629..8cb645b66 100644 --- a/src/wrappers/themis/rust/src/secure_cell.rs +++ b/src/wrappers/themis/rust/src/secure_cell.rs @@ -935,7 +935,7 @@ fn encrypt_seal(master_key: &[u8], user_context: &[u8], message: &[u8]) -> Resul } } - encrypted_message.reserve(encrypted_message_len); + encrypted_message.try_reserve(encrypted_message_len)?; unsafe { let status = themis_secure_cell_encrypt_seal( @@ -985,7 +985,7 @@ fn decrypt_seal(master_key: &[u8], user_context: &[u8], message: &[u8]) -> Resul } } - decrypted_message.reserve(decrypted_message_len); + decrypted_message.try_reserve(decrypted_message_len)?; unsafe { let status = themis_secure_cell_decrypt_seal( @@ -1039,7 +1039,7 @@ fn encrypt_seal_with_passphrase( } } - encrypted_message.reserve(encrypted_message_len); + encrypted_message.try_reserve(encrypted_message_len)?; unsafe { let status = themis_secure_cell_encrypt_seal_with_passphrase( @@ -1093,7 +1093,7 @@ fn decrypt_seal_with_passphrase( } } - decrypted_message.reserve(decrypted_message_len); + decrypted_message.try_reserve(decrypted_message_len)?; unsafe { let status = themis_secure_cell_decrypt_seal_with_passphrase( @@ -1549,8 +1549,8 @@ fn encrypt_token_protect( } } - token.reserve(token_len); - encrypted_message.reserve(encrypted_message_len); + token.try_reserve(token_len)?; + encrypted_message.try_reserve(encrypted_message_len)?; unsafe { let status = themis_secure_cell_encrypt_token_protect( @@ -1612,7 +1612,7 @@ fn decrypt_token_protect( } } - decrypted_message.reserve(decrypted_message_len); + decrypted_message.try_reserve(decrypted_message_len)?; unsafe { let status = themis_secure_cell_decrypt_token_protect( @@ -1866,7 +1866,7 @@ fn encrypt_context_imprint(master_key: &[u8], message: &[u8], context: &[u8]) -> } } - encrypted_message.reserve(encrypted_message_len); + encrypted_message.try_reserve(encrypted_message_len)?; unsafe { let status = themis_secure_cell_encrypt_context_imprint( @@ -1916,7 +1916,7 @@ fn decrypt_context_imprint(master_key: &[u8], message: &[u8], context: &[u8]) -> } } - decrypted_message.reserve(decrypted_message_len); + decrypted_message.try_reserve(decrypted_message_len)?; unsafe { let status = themis_secure_cell_decrypt_context_imprint( diff --git a/src/wrappers/themis/rust/src/secure_comparator.rs b/src/wrappers/themis/rust/src/secure_comparator.rs index d35760b2a..44835dbb2 100644 --- a/src/wrappers/themis/rust/src/secure_comparator.rs +++ b/src/wrappers/themis/rust/src/secure_comparator.rs @@ -269,7 +269,7 @@ impl SecureComparator { } } - compare_data.reserve(compare_data_len); + compare_data.try_reserve(compare_data_len)?; unsafe { let status = secure_comparator_begin_compare( @@ -327,7 +327,7 @@ impl SecureComparator { } } - compare_data.reserve(compare_data_len); + compare_data.try_reserve(compare_data_len)?; unsafe { let status = secure_comparator_proceed_compare( diff --git a/src/wrappers/themis/rust/src/secure_message.rs b/src/wrappers/themis/rust/src/secure_message.rs index 9fdef9575..862a5c5dd 100644 --- a/src/wrappers/themis/rust/src/secure_message.rs +++ b/src/wrappers/themis/rust/src/secure_message.rs @@ -165,7 +165,7 @@ impl SecureMessage { } } - encrypted.reserve(encrypted_len); + encrypted.try_reserve(encrypted_len)?; unsafe { let status = themis_secure_message_encrypt( @@ -215,7 +215,7 @@ impl SecureMessage { } } - decrypted.reserve(decrypted_len); + decrypted.try_reserve(decrypted_len)?; unsafe { let status = themis_secure_message_decrypt( @@ -349,7 +349,7 @@ impl SecureSign { } } - signed.reserve(signed_len); + signed.try_reserve(signed_len)?; unsafe { let status = themis_secure_message_sign( @@ -477,7 +477,7 @@ impl SecureVerify { } } - original.reserve(original_len); + original.try_reserve(original_len)?; unsafe { let status = themis_secure_message_verify( diff --git a/src/wrappers/themis/rust/src/secure_session.rs b/src/wrappers/themis/rust/src/secure_session.rs index a7fb4f172..f1ea33970 100644 --- a/src/wrappers/themis/rust/src/secure_session.rs +++ b/src/wrappers/themis/rust/src/secure_session.rs @@ -434,7 +434,7 @@ impl SecureSession { } } - id.reserve(id_len); + id.try_reserve(id_len)?; unsafe { let status = secure_session_get_remote_id(self.session, id.as_mut_ptr(), &mut id_len); @@ -628,7 +628,7 @@ impl SecureSession { } } - output.reserve(output_len); + output.try_reserve(output_len)?; unsafe { let status = secure_session_generate_connect_request( @@ -681,7 +681,7 @@ impl SecureSession { } } - message.reserve(message_len); + message.try_reserve(message_len)?; unsafe { let status = secure_session_unwrap( @@ -736,7 +736,7 @@ impl SecureSession { } } - wrapped.reserve(wrapped_len); + wrapped.try_reserve(wrapped_len)?; unsafe { let status = secure_session_wrap( @@ -787,7 +787,7 @@ impl SecureSession { } } - message.reserve(message_len); + message.try_reserve(message_len)?; unsafe { let status = secure_session_unwrap( From 1dee40444fc6ac60c96b4e801379673fff4b405c Mon Sep 17 00:00:00 2001 From: G1gg1L3s Date: Tue, 27 Jun 2023 22:47:27 +0300 Subject: [PATCH 3/5] rust-themis: Run the skipped tests on the 32bit They were skipped because allocation on 32-bit platforms failed while trying to allocate something bigger than 2GB. This is due to usage of `.reserve` which panics if it couldn't fulfill the request. Since we traversed to the rust 1.58, we now have the `try_*` methods, including the `.try_reserve` which allows us handle panics gracefully. I've tested it manually on pi4 and it works! --- tests/rust/secure_cell.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/rust/secure_cell.rs b/tests/rust/secure_cell.rs index c455f4987..efd6650da 100644 --- a/tests/rust/secure_cell.rs +++ b/tests/rust/secure_cell.rs @@ -760,11 +760,6 @@ mod token_protect { } #[test] - // FIXME(ilammy, 2020-05-25): avoid capacity allocation panics (T1649) - // This tests panics on 32-bit architectures due to size overflow. - // The implementation needs to use Vec::try_reserve instead of Vec::reserve - // when it becomes available in stable Rust. - #[cfg_attr(target_pointer_width = "32", ignore)] fn detects_corrupted_token() { let cell = SecureCell::with_key(SymmetricKey::new()) .unwrap() @@ -817,11 +812,6 @@ mod token_protect { } #[test] - // FIXME(ilammy, 2020-05-25): avoid capacity allocation panics (T1649) - // This tests panics on 32-bit architectures due to size overflow. - // The implementation needs to use Vec::try_reserve instead of Vec::reserve - // when it becomes available in stable Rust. - #[cfg_attr(target_pointer_width = "32", ignore)] fn detects_data_token_swap() { let cell = SecureCell::with_key(SymmetricKey::new()) .unwrap() From 24d4fd0c3c0c0f4b65252c7bdcc967986a763d92 Mon Sep 17 00:00:00 2001 From: G1gg1L3s Date: Tue, 27 Jun 2023 23:03:42 +0300 Subject: [PATCH 4/5] rust-themis: with_capacity -> try_reserve Because we don't have try_with_capacity or something similar. --- src/wrappers/themis/rust/src/secure_session.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wrappers/themis/rust/src/secure_session.rs b/src/wrappers/themis/rust/src/secure_session.rs index f1ea33970..f4adb5a60 100644 --- a/src/wrappers/themis/rust/src/secure_session.rs +++ b/src/wrappers/themis/rust/src/secure_session.rs @@ -570,7 +570,8 @@ impl SecureSession { /// [`negotiate`]: struct.SecureSession.html#method.negotiate /// [`receive_data`]: trait.SecureSessionTransport.html#method.receive_data pub fn receive(&mut self, max_len: usize) -> Result> { - let mut message = Vec::with_capacity(max_len); + let mut message = Vec::new(); + message.try_reserve(max_len)?; unsafe { let length = secure_session_receive( From a0b709fbed6b3db49a314eb6b7a6e0bac41ceb4b Mon Sep 17 00:00:00 2001 From: G1gg1L3s Date: Tue, 8 Aug 2023 17:28:26 +0300 Subject: [PATCH 5/5] rust: Add a test for checking 4GB allocation It was intended only for 32 bit systems, but let's extend it to x64 as well. --- tests/rust/secure_cell.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/rust/secure_cell.rs b/tests/rust/secure_cell.rs index efd6650da..18d2103be 100644 --- a/tests/rust/secure_cell.rs +++ b/tests/rust/secure_cell.rs @@ -837,4 +837,38 @@ mod token_protect { assert!(cell.decrypt(&encrypted, &[]).is_err()); assert!(cell.decrypt(&[], &token).is_err()); } + + #[test] + fn corruption_can_fail_with_no_memory() { + let cell = SecureCell::with_key(SymmetricKey::new()) + .unwrap() + .token_protect(); + let message = "Не можна зупинити ідею, час якої настав".as_bytes(); + + let (encrypted, token) = cell.encrypt(&message).unwrap(); + let mut corrupted_token = token; + + // set length to maximum possible, it will produce an allocation of + // 4GiB, which should fail on the 32 bit machines + corrupted_token[12..16].fill(0xff); + + let err = cell.decrypt(&encrypted, &corrupted_token).unwrap_err(); + + if cfg!(target_pointer_width = "32") { + // On 32 bit machines, even if OS supports RAM with > 4GB, a single + // process cannot have more than that, so it should fail with no + // memory. Unless you use some exotic system. + assert_eq!(err.kind(), themis::ErrorKind::NoMemory); + } else { + // We cannot assume any particular error here. On most 64 bit machines, + // it will probably fail with the `Fail`, because the lengths are + // inconsistent.However, on systems with limited amount of memory + // (like Raspberry PI 4 with memory < 4GB), it will fail because of + // allocation. + assert!(matches!( + err.kind(), + themis::ErrorKind::NoMemory | themis::ErrorKind::Fail + )); + } + } }