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

Zerofill #477

Closed
wants to merge 6 commits into from

Conversation

Projects
None yet
4 participants
@garrettr
Copy link
Contributor

commented Jul 12, 2014

Use a custom version of BytesIO to handle streams of sensitive data. This custom stream type zerofills all buffers before free'ing them.

@jacksingleton

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2014

@garrettr Nice!

Maybe add a link to the original bytesio.c? Would make it obvious to anyone looking at this file that they can diff with bytesio.c when reviewing/auditing

@diracdeltas

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2014

I agree with @jacksingleton. How about including the original bytesio.c file and your modifications as a git-patch, so the safestream module is generated on-the-fly by applying the patch to the original file by the SecureDrop setup scripts? It would make the changes easier to read and maintain in the future.

@garrettr

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2014

We tested this code and found that, while it reduced the number of copies of plaintext we found in memory, it did not eliminate them entirely. To really track it down, we would have to monkeypatch Flask and potentially other libraries and that's starting to get really out of hand.

We have a more elegant solution for this problem that will be landing shortly.

@garrettr garrettr closed this Sep 17, 2014

@fpietrosanti

This comment has been minimized.

Copy link

commented Jan 1, 2016

@garrettr i were reviewing globaleaks tickets and found that this topic has not yet been addressed. Which has become the more elegant solution? Do you think there could be a general solution to this problem for python software, to be committed upstream to python?

@garrettr

This comment has been minimized.

Copy link
Contributor Author

commented Jan 3, 2016

@fpietrosanti The more elegant solution for us is to use PAX_MEMORY_SANITIZE, provided by grsecurity/PaX. This ensures that all freed buffers are zero-filled for all applications, which is important for SecureDrop because it uses Apache.

Now, there are always two questions you want to answer when you are trying to sanitize memory:

  1. Can I sanitize it? You probably already know that this is not as easy as it appears. Compilers often skip zerofill instructions as an optimization, and it can be difficult to consistently trick them into not doing so.
  2. Can I free it? Obviously you don't want to sanitize memory until you're done using it. However, in the case of garbage-collected languages like Python, it can be difficult or impossible to ensure that memory is freed/sanitized when you want it to be. Ideally you would sanitize sensitive material in memory as soon as you are done using it.

So here's how we currently solve these problems in SecureDrop:

For problem 1, we use PAX_MEMORY_SANITIZE. This zero-fills all buffers that are freed by every process (including the kernel), and is implemented in the kernel, so it's about as comprehensive a solution as you can find. The downside is that your application now requires a grsecurity kernel (not easy to build/deploy/keep up to date). It also incurs a performance penalty (about 4% according to the grsec docs).

For problem 2, we currently use a kludge (#805). Originally, my goal was to force Apache to only use a single Python process for each request. Once the request is handled, the process is terminated - which causes all of the memory it has allocated to be freed, and thus sanitized by PAX_MEMORY_SANITIZE.

You can do this in Apache by using mpm_prefork and setting MaxRequestsPerChild to 1. I experimented with this last year, using the Volatility forensic framework to analyze memory dumps to confirm that the technique was working. Unfortunately, I was unable to fully confirm my results with PAX_MEMORY_SANITIZE because I couldn't get Volatility to analyze a grsecurity kernel (grsecurity specifically takes measures to confuse forensic tools like Volatility, and although I tried to disable as many of them as I could find, I still couldn't get it working).

In the end, I decided to go with the kludge from #805. It would be worth revisiting this issue again, as I believe the mpm_prefork technique should work - I just want to confirm that before making the change, to avoid having a false sense of security.

I'm not sure if any of this is relevant to Globaleaks because, IIRC, you use a single persistent Python process with a Twisted event loop to process requests.

@fpietrosanti

This comment has been minimized.

Copy link

commented Jan 9, 2016

@garrettr super detailed, explanation, thanks!

I where looking that on 23 December 2015 the PAX_MEMORY_SANITIZE is starting being integrated in the mainline linux kernel as a part of the kernel-hardening project, that's a kind of interesting development of Linux kernel to follow! http://lwn.net/Articles/668876/

@nvesely nvesely deleted the zerofill branch Apr 22, 2017

@garrettr garrettr referenced this pull request Jun 15, 2017

Closed

Minor refactor + test coverage for secure_tempfile.py #1810

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.