Rust-themis: Allocate with try_reserve #1014
Conversation
It will allow us using ? with the try_* methods, like try_reserve.
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!
Because we don't have try_with_capacity or something similar.
|
Ah yeah, we should probably merge the #1013 first. It will resolve some rust issues and make the changelog clear to update. |
|
@G1gg1L3s take a look? |
|
Sorry, I somehow missed this comment:
Hmm, we have tests that accidantally corrupt the cells in a way that triggers a big allocation, for example this: themis/tests/rust/secure_cell.rs Lines 762 to 780 in 24d4fd0 But, it doesn't check that the error is |
It was intended only for 32 bit systems, but let's extend it to x64 as well.
|
actually, it's common approach to limit resources for VM in the cloud. When one server used for huge amount of the containers and they should fit into the expected RAM usage and avoid overusage by one bad container. Does it hard to test it on the VM with 20mb RAM limit, try to allocate 30m and catch NoMemory error? |
I can write a test that will catch precisely that. However, that test would fail on regular machines. In other words, when users pull themis and run Also, I don't know, maybe it would require adding additional CI step for this particular test or writing a separate Dockerfile, so it should be taken into account as well. |
|
maybe let's hide this test under the feature flag turned off by default and pass it in ci env? Found example - https://stackoverflow.com/questions/48583049/run-additional-tests-by-using-a-feature-flag-to-cargo-test Probably we will need separate workflow for github actions where container will be limited with memory and we will run this only one test for now. To make it more generic, we can pass the ENV variable with the amount of memory of the container and use it in test like |
|
Oh, it's harder than I thought 😅 Our tests are run using But even with detecting CI environment, adding a new step to CI would require building themis again and maybe setting up caching. Also, we can use the So, I think that's not worth it 🙂 |
While looking at our backlog, I found T1649. In a nutshell, some tests were failing on 32-bit machines, because they corrupt the secure cell tag. This was done in a way that triggers allocation of 2GB from the rust code, which failed. But at that time, it wasn't possible to handle this failure gracefully, so rust wrapper just panicked. It means that on 32-bit platforms it was possible to execute DoS attack with maliciously crafted input.
Since we traversed to the rust 1.58, we now have the
try_*methods, including the.try_reservewhich allows us to handle failures gracefully.So, let's use it! I've tested it manually on Pi 4 and the tests pass without issues.
Checklist