In the rush to get the Windows builds working correctly, we may have disabled hardening options that we shouldn't have. I experienced crashes on Windows 7 with a few options as well. We should get to the bottom of those issues and if they persist, we should ping the mingw-w64 people and find out what the issue is.
I played around with this a bit today and here is a tiny bit of progress and some rabid fangirling of objdump.
I enabled ASLR and DEP for tor.exe and libevent (nothing else automatically picks up LDFLAGS, so I need to look into that) and so far nothing is crashing for me on Win7.
Relatedly, I've been using objdump lately to look at the results of builds from #9444 (closed) and decided to see if it's possible to glean information about ASLR and DEP from the Windows binaries without having to check them in ProcessHacker or some other Windows app. For DLLs and EXEs (both PE), there is something called DllCharacteristics in the header which will tell you which, if any, of those are enabled. For ASLR it is 0x40 and for DEP it's 0x100, so all of our DLLs and EXEs should be showing something like: DllCharacteristics 00000140. The main reason this is cool is that you can use objdump from Linux to investigate this, you don't need to use some Microsoft tool, and it can be automated post-build as a QA measure to make sure nothing funky happens accidentally to disable these measures.
pefile is a multi-platform Python module to read and work with Portable Executable (aka PE) files. Most of the information in the PE Header is accessible, as well as all the sections, section's information and data.
I'm going to continue to tighten up the hardening options and try to reproduce the crashing. I'll also test further with the Microsoft tools mentioned above.
-pie is definitely the culprit. I built two TBBs: one with DEP/ASLR and no -pie and another with DEP/ASLR and -pie. The latter crashes and the former doesn't. I also get the same wrong AddressOfEntryPoint (00001000) in the build with -pie whereas in the non-pie build I get the correct one (000014b0).
It took a little wrangling but I eventually got all of the libs we build building with DEP/ASLR (this is including the Firefox libs). I think we should consider extending that to the libssp we build in mingw-64, but I didn't look into how to do that yet.
I will break it up further before I ask for any real merging, but I wanted to link it here because I won't be able to get to that before Monday and thought people should have a chance to look it over or at least test it if they felt like doing so. This enables DEP/ASLR and SSP on all binaries and DLLs we build. I also built a new libssp and am shipping the bundle with that instead of the MinGW one we were using before.
Issues:
None of the PT stuff is hardened at all. Is this a blocker for getting it into a beta?
skruffy's got two patches in here: one for binutils that creates a proper reloc section so we can have working ASLR, and one for gcc that prevents the use of /dev/urandom on Windows. nickm and zwol have reviewd the gcc (SSP) patch and deemed it okay, but both said someone with more binutils knowledge needs to check the ld patch. For reference they are here:
Only the libssp-0.dll is different (apart from the different tor-browser-bundle commit). I uploaded mine: https://people.torproject.org/~gk/misc/hardened_libssp-0.dll 36f7c2434e0e65ca9b6065c97a42de98992e434d80236e3619140c88ca7dfcdc. I built from commit 17425dec3a13de51b717efeb8bdde1a4460d31fa.
gk -- it turned out to be a one line patch to make libssp build reproducibly. I've updated my branch.
As for next steps, the branch needs to be rebased... I tried it today but it looks quite messy with all the meek changes that have happened in the intervening period. I either need more sleep or to sit with someone and work on fixing the conflicts, because my first glance at it made me worry I might screw it up.
Some issues remain, namely that none of the pluggable transports are hardened. The python dll we're distributing only has DEP and not ASLR. As I understand it, the default options have changed in the distributions of Python 3.x, but it seems like no small task to switch from one to the other. I looked into crosscompiling Python and while it seems possible (in the sense that there is some python-mingw port by some random person on the internet), it also might be quite a time consuming project.
As for the PTs written in Go, I refer to this thread where Russ Cox says:
Address space randomization is an OS-level workaround for a language-level problem, namely that simple C programs tend to be full of exploitable buffer overflows. Go fixes this at the language level, with bounds-checked arrays and slices and no dangling pointers, which makes the OS-level workaround much less important. In return, we receive the incredible debuggability of deterministic address space layout. I would not give that up lightly.
Anyway, feedback welcome. Putting this into needs_review. I should note that skruffy's binutils patch remains mostly unreviewed, and I still need to send it upstream, but if anyone feels like digging into it before I do, I would appreciate it (and so would he).
Edit: And Tom, since you were part of the iSEC review, can you double check that I got the hardening right? I reviewed the binaries with objdump and in procexp and Process Hacker on a Win7 VM but would love a second set of eyes.
#10077 (moved) is not done yet which means we are currently using two different compilers for the alpha series: 4.6.3 (for tor, the pluggable transports, binutils and the compiler itself) and 4.8.3 (for the other utils and Tor Browser). You don't change that but take the libs from 4.8.3 for everything which makes me a bit nervous. Could you either take the libs belonging to the respective compiler OR fix #10077 (moved) (which might be easy and needs to get done anyway)?
Why do we (still) need the wrapper scripts for the Tor Browser. We are using LDFLAGS and friends anyway. Thus, can't we just use them?
Do we really need this ld -> ld.orig and i686-w64-mingw32-ld -> /usr/bin thingy? I am wondering as you don't do that in the gitian-utils descriptor. Maybe this was a 4.6.3 issue which is no longer a problem with 4.8.3 and later (might be something to think about for 2), too)?
I've updated my branch (it was a force-update though) to address points 1) and 3).
I fixed #10077 (moved) and now everything builds with our compiler. The PTs aren't really using it, but the .pyd files are, and the way to check resulting binaries is by running strings file | grep -i gcc. gcc 4.8.3 is thoughtful enough to write its version into everything, and 4.6.3 doesn't seem to do the same. I've verified where I can.
I remember having some trouble getting Firefox to accept all of the necessary flags last time I did this and the wrappers make it very convenient. I'm not sure why the flags are not showing up in about:buildconfig -- if you inspect the binaries with PE Studio, procexp, and Process Hacker, it very clearly shows that DEP and ASLR are enabled. I'll play around with it a bit more and see if the wrappers are the cause; it might be simply a strange "cosmetic" issue.
You were right, we don't need that. It's only necessary in cases where mingw-w64 is installed and if we're building with our own, we don't need to install mingw-w64 anywhere.
As for non-reproducibility, I think I must've left something out of my branch. I've rebuilt twice with my current branch and both times the sha256sums matched. Can you tell me if you still see issues? My current sha256sum for torbrowser-install-4.0-alpha-1_en-US.exe with my changes is 2784dea6ec561d4d4225812461274730dc004a3b8ca88c6a3360e05e3fa03741.
I should have mentioned: since Yan's key isn't verifying the https-e repo right now (for me or Mike), I changed versions.alpha to have VERIFY_TAGS=0. Mike pointed out that not having this change will result in a different hash since the file is included, so I thought I should mention in case you haven't built yet.
I deleted all the hardening comments and squashed everything into the Windows hardening commit (see also #10077 (moved)). I'll submit the binutils patch upstream too.
I rebuilt with my new branch just to make sure everything's cool and it only differed in the bundle.inputs because of the commit change, otherwise everything matches. caeaad1d3b4ea982d64aaee018b58209ab569d52b4c1b50f93a8560b78530b79 is the new en-US sha256sum if you want to check my work.
My friend Graydon (founder of Rust lang) reviewed the patch and proposed some potential alternate patches to achieve the same goal. That's for a longer-term solution that we'll hopefully get into binutils, but in the short-term he said that (paraphrasing) the patch looks safe to him, so it should be fine to ship binaries built with it.
I'll begin testing his patches and seeing if we can integrate them before upstream does, but I think we should use skruffy's patch until then. I would expect to have another one ready before the next TBB release though.
I think we can call this fixed! The hardening in TBB 4.0-alpha-2 should have everything mostly enabled. There are a few stragglers (#13031 (moved) for Linux in particular), and a few extra things we can do once we have 64bit builds for Mac and Windows, but I've removed them from this ticket and put them under the tbb-security tag, as they are a bit farther out.
Trac: Resolution: N/Ato fixed Status: needs_review to closed