Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pwhash_scrypt test fails (escrypt_kdf_nosse) in mmap with invalid mem size #890

Closed
ciprian-barbu opened this issue Oct 29, 2019 · 5 comments

Comments

@ciprian-barbu
Copy link

This issue is related to two other closed issues:
#721
#749

I encountered this problem on an aarch64 server, running Ubuntu 16.04, both in a Docker container and baremetal. First I fixed the ulimit max_memlock problem, increasing from 64k to 16384k. This made the test pass on one server, but when I switched to a different server I started getting memory depletion, as described on the second bug report mentioned above.

I started debugging the actual test case and found the following:

  • the failing test case is always the same, tv3, test number 30 (starting from 0): "$7$8zzzzz/.....lgPchkGHqbeONR
  • on these server there is no SSE support (at least not as libsodium can use), which causes escrypt_r to call escrypt_kdf_nosse
  • escrypt_kdf_nosse receives some very nasty parameters, r=1073741823, p=1, N_log2=10
  • the total amount of memory computed as needed is 141149805084352 (about 128 TB!!!!)
  • the alloc_region function calls mmap, passing this huge param as size, but also specifying MAP_POPULATE flag
  • under "normal" circumstances, this mmap would fail, but in some special conditions this causes mmap to try and not only create memory pages, but also to reserve them into memory. There is no sane configuration that can ofer 128 TB of memory

So here is the tricky part. On my second aarch64 server we used to run some virtualized work (Fuel/Openstack) which for some reason made necessary to specify a special vm.memory_overcommit value of 1. This means that the kernel will no longer perform sanity checks for mmap like syscalls. This is why in this case mmap will try to allocate and map the pages into memory.

I'm not sure whether the purpose of the test case is to verify situations like this, where an invalid encrypted text would require allocating an insane and incorrect amount of memory, but when it's also coupled with this kind of sysctl config for memory_overcommit, I consider it to be a vulnerability.

I'm also attaching a file with my debug session.
libsodium_pwhash_scrypt_dbg.txt

Thanks and regards,
/Ciprian

@ciprian-barbu
Copy link
Author

Forgot to mention I have the exact same information in this related JIRA card:
https://jira.akraino.org/browse/IEC-26

@jedisct1
Copy link
Owner

Is it with libsodium 1.0.18-stable?

@ciprian-barbu
Copy link
Author

No, actually the problem has been identified in PyNaCl 1.3.0, which at this moment is the latest version available. I'm trying to setup a virtualenv with the cord-tester framework, which requires pynacl.
The version of libsodium in this case is 1.0.16, it's visible in the JIRA card I posted before, but forgot to mention it here.

@jedisct1
Copy link
Owner

The string from the test seen in your txt file doesn't exist any longer.

PyNaCl can probably use a system installation
But if it installs its own version when one cannot be found, it would be a good idea to keep it up to date.

@ciprian-barbu
Copy link
Author

Sure, that makes sense.

TBH I wasn't aware that portion of code disappeared, but l wanted to dump this info here anyway, because I noticed the other threads didn't really reach a conclusion to how it reproduces and what was going wrong.

I will try to take this to the maintainers of PyNaCl, but as a side note, I think it makes sense for you to join forces with them and maybe even incorporate the python binding support in your repo.

Thanks and regards,
/Ciprian

Repository owner locked and limited conversation to collaborators Jan 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants