`Argon2::hash_password_into` should use fallible memory allocations

[teythoon]

Argon2::hash_password_into crashes on 32-bit architectures when using the FIRST RECOMMENDED parameter option (see Section 4 of RFC 9106) because it tries to infallibly allocate the 2 GiB buffer.

We believe that the straight-forward, easy-to-use interfaces should be safe by default. Following that, Argon2::hash_password_into should make a fallible allocation, returning allocation errors instead of crashing.

[tarcieri]

Your claim it "crashes on 32-bit architectures" seems misleading in that we do run CI on 32-bit architectures successfully:

https://github.com/RustCrypto/password-hashes/blob/master/.github/workflows/argon2.yml#L88-L105

It seems the real issue is you're attempting to allocate 2GiB of RAM on a system which doesn't have it available.

Yes, it could potentially use fallible allocations, however there is not a stable API in Rust for allocating memory fallibly, as the Allocator trait is currently unstable.

I am not sure there is a 3rd party crate for this which is both trustworthy and portable, until such a time as the Allocator trait is stabilized. We also generally dislike using 3rd party crates unless absolutely necessary.

In the meantime you can use whatever fallible allocation solution you want in conjunction with hash_password_into_with_memory

[tarcieri]

I should also note that fallible allocations don't always work the way you expect. Linux has an OOM killer, and will terminate other processes to satisfy requests for large amounts of memory on low-memory systems. So unfortunately simply adding fallible allocations may not be "safe" in the way you expect.

It would definitely be better to use less memory on low-resource systems to begin with, by selecting parameters which use less memory.

[teythoon]

Your claim it "crashes on 32-bit architectures" seems misleading in that we do run CI on 32-bit architectures successfully:
It seems the real issue is you're attempting to allocate 2GiB of RAM on a system which doesn't have it available.

I reproduced the issue that was reported to me by running a 32-bit build of Debian using Docker, with plenty of memory available. I'm pretty sure that I'm looking at address space exhaustion here, as in, there is no continuous block of unused address space with the required size.

Original bug report "sequoia-openpgp v2.0.0-alpha.2 test failure on i686 (32-bit x86)": https://gitlab.com/sequoia-pgp/sequoia/-/issues/1157

The test tries to unlock the locked sample key from RFC 9580, using the most obvious API offered by this crate. My feedback was that obvious invocation of Argon2, on recommended parameter choices, leads to a panic, which I consider very surprising, and this kind of surprises aren't great.

In the meantime you can use whatever fallible allocation solution you want in conjunction with hash_password_into_with_memory

That is what I went with. I ended up using std::alloc::alloc: https://gitlab.com/sequoia-pgp/sequoia/-/commit/3325c74c241654bc34859d95092f0b4f9b280f2e

I should also note that fallible allocations don't always work the way you expect. Linux has an OOM killer, and will terminate other processes to satisfy requests for large amounts of memory on low-memory systems. So unfortunately simply adding fallible allocations may not be "safe" in the way you expect.

I am aware.

It would definitely be better to use less memory on low-resource systems to begin with, by selecting parameters which use less memory.

As a consumer of OpenPGP artifacts, I don't have the choice of parameters, the sender chose. Now, maybe the sender chose poorly, and I won't be able to decrypt it, that is an okay outcome. Panic'ing the application is not.

[tarcieri]

Please provide a complete reproduction of the problem

[tarcieri]

I am aware.

Well, then you should also be aware that if you request too much memory, the OOM killer can kill your process.

You need to detect how much memory is available, calculate how much memory the parameters will use, and if it's too much, then fail yourself, or you're still at risk of the process being terminated by the kernel.

[conradludgate]

Vec::try_reserve is stable fwiw: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.try_reserve

[newpavlov]

A better option could be to use the alloc API directly. It would involve a bit of unsafe code, but it should be fairly straightforward and we already have unsafe code in argon2.

[conradludgate]

For prosperity, this is the gymnastics that hashbrown goes into: https://github.com/rust-lang/hashbrown/blob/master/src/raw/alloc.rs

[tarcieri]

@newpavlov what API are you talking about that's stable? e.g. the Allocator trait is unstable

[tarcieri]

Relaying another suggestion: we could take an optional memory limit parameter, and return an error if the given parameters require more memory than the limit

[newpavlov]

Why do you need the Allocator trait here? I was talking about the alloc function.

[tarcieri]

I wasn’t aware, hence why I was asking.

What makes that better than try_reserve though?

[newpavlov]

It's a more fundamental API. We don't need anything from Vec and know the desired memory size right away.