Opened 5 years ago

Closed 3 years ago

#10065 closed defect (fixed)

Improve Hardening for TBB3.0

Reported by: mikeperry Owned by: erinn
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: MikePerry201408R, tbb-3.0, gitian, tbb-security, tbb-gitian, tbb-isec-report
Cc: gk, mikeperry, tom@…, intrigeri Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

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.

Here's a set of tools for Windows to validate hardening:

http://www.microsoft.com/en-us/download/details.aspx?id=11910
https://www.microsoft.com/en-us/download/details.aspx?id=29851

Here's one for Linux:

http://www.trapkit.de/tools/checksec.html

Here's some random documentation:

http://wiki.debian.org/Hardening
http://stackoverflow.com/questions/13276692/safeseh-gs-on-g
https://developer.pidgin.im/ticket/15290


Child Tickets

TicketStatusOwnerSummaryComponent
#5024closederinncompile time hardening of TBB (RELRO, canary, PIE)Applications/Tor Browser
#10505closederinnBroken ASLR in windows executableCore Tor/Tor
#10512closederinnFirefox.exe doesn't have DEP enabledApplications/Tor bundles/installation
#10515closedtbb-teamCompile Firefox with buffer overflow protectionApplications/Tor Browser
#12103closederinnFully hardening firefox binary is broken since 3.5.3 on LinuxApplications/Tor bundles/installation

Change History (33)

comment:1 Changed 5 years ago by erinn

Status: newaccepted

comment:2 Changed 5 years ago by cypherpunks

Relatedly, is there any way to implement some of the functionality of a project like IronFox?

comment:3 in reply to:  2 Changed 5 years ago by cypherpunks

Replying to cypherpunks:

Relatedly, is there any way to implement some of the functionality of a project like IronFox?

Just noting the link to https://lists.torproject.org/pipermail/tor-talk/2013-November/031111.html from the blog post at https://blog.torproject.org/blog/tor-weekly-news-%E2%80%94-november-20th-2013

comment:4 Changed 5 years ago by erinn

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 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.

When I talked about this on IRC, Yawning also mentioned this ruby script which I haven't tried yet:
https://github.com/Myne-us/dllcharacteristics

scan system for characteristics of PE files. This will enable you to find PEs with ASLR disabled, DEP disabled, and more

There is also pefile: http://code.google.com/p/pefile/

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.

As for the crashing: when was it happening? Did anything trigger it? So far I have been unable to reproduce with my test bundle. If anyone wants to test it, it's here: https://people.torproject.org/~erinn/qa/torbrowser-install-3.0-rc-1_en-US-hardened.exe e5dac7a49095a1422d82df05f67476119642c7488c8c02a7c452757fcdd769ba

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.

comment:5 Changed 5 years ago by gk

#10505 is related. See http://sourceforge.net/mailarchive/message.php?msg_id=31034877 (which is posted there) for a possible crash scenario.

comment:6 Changed 5 years ago by mikeperry

Keywords: tbb-security added

comment:7 Changed 5 years ago by mikeperry

The crash happened for me with way more than just ASLR enabled. I did:

export CFLAGS="-mwindows -fstack-protector-all -fPIE -Wstack-protector --param ssp-buffer-size=4 -fno-strict-overflow -Wno-missing-field-initializers -Wformat-security"
export LDFLAGS="-mwindows -Wl,--dynamicbase -Wl,--nxcompat -lssp -L/usr/lib/gcc/i686-w64-mingw32/4.6/"

I also wrapped g++, gcc, and ld:
https://gitweb.torproject.org/builders/tor-browser-bundle.git/blob/HEAD:/gitian/build-helpers/i686-w64-mingw32-g++
https://gitweb.torproject.org/builders/tor-browser-bundle.git/blob/HEAD:/gitian/build-helpers/i686-w64-mingw32-gcc
https://gitweb.torproject.org/builders/tor-browser-bundle.git/blob/HEAD:/gitian/build-helpers/i686-w64-mingw32-ld

I'm guessing one of those many options is the culprit. Ideally we'd find out what it is, report it, and use the rest.

But in the short term, if just ASLR and DEP can be enabled without issue, we should start building with those two at least.

comment:8 Changed 5 years ago by erinn

-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.

comment:9 Changed 5 years ago by erinn

I have a gross monolithic branch in user/erinn/tor-browser-bundle.git called windows-hardening-monolithic: https://gitweb.torproject.org/user/erinn/tor-browser-bundle.git/shortlog/refs/heads/windows-hardening-monolithic

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:

  1. None of the PT stuff is hardened at all. Is this a blocker for getting it into a beta?
  1. 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:

gcc/ssp: https://gitweb.torproject.org/user/erinn/tor-browser-bundle.git/blob/17425dec3a13de51b717efeb8bdde1a4460d31fa:/gitian/patches/windows-crypto.patch

https://gitweb.torproject.org/user/erinn/tor-browser-bundle.git/blob/17425dec3a13de51b717efeb8bdde1a4460d31fa:/gitian/patches/enable-reloc-section-ld.patch

(And yes, I am aware they don't have descriptions in the patches. I'm going to fix that when I figure out good descriptions. :))

A test bundle is here: http://lucio.erinn.org/~helix/torbrowser-install-3.6-beta-1_en-US.exe a198037a157b1f29e80d3920ad964b590e4f57b44f93fcd3a1aba98fa2915c60

comment:10 Changed 5 years ago by gk

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.

comment:11 Changed 5 years ago by cypherpunks

Only the libssp-0.dll is different

Difference about timestamp, it's actual date instead of 01/01/2000 as should be.

comment:12 Changed 5 years ago by tom

Cc: tom@… added

comment:13 Changed 5 years ago by gk

Erinn: What remains to be done here? Anything I can help with?

comment:14 Changed 5 years ago by erinn

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.

comment:15 Changed 5 years ago by erinn

Component: Tor bundles/installationTor Browser
Keywords: tbb-gitian added

comment:16 Changed 5 years ago by mikeperry

Keywords: isec-audit added

comment:17 Changed 5 years ago by intrigeri

Cc: intrigeri added

comment:18 Changed 5 years ago by mikeperry

Keywords: tbb-isec-report added; isec-audit removed

comment:19 Changed 5 years ago by erinn

Status: acceptedneeds_review

I've updated all of my patches for the 4.x series and the branch is here:

https://gitweb.torproject.org/user/erinn/tor-browser-bundle.git/shortlog/refs/heads/tbb-4.x-hardening

I have a test bundle here as well, if anyone would like to build & check for reproducibility:
http://paganini.erinn.org/~erinn/torbrowser-install-4.0-alpha-1_en-US.exe
9328f4887406667d5d578d256fe9650e7b685f02e8f9a9248b1b1c7ef81987a1

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.

Last edited 5 years ago by erinn (previous) (diff)

comment:20 Changed 5 years ago by mikeperry

Keywords: MikePerry201408R added

comment:21 Changed 5 years ago by gk

Okay, here are some points:

1) #10077 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 (which might be easy and needs to get done anyway)?

2) 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?

3) 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)?

Last edited 5 years ago by gk (previous) (diff)

comment:22 Changed 5 years ago by gk

Hmm... why are these hardening flags not showing up in about:buildconfig?

comment:23 Changed 5 years ago by gk

Another issue: Seems that at least the tor.exe and the OpenSSL stuff is not matching. I've uploaded them:
https://people.torproject.org/~gk/misc/tor-win32-hardened-gbuilt.zip
https://people.torproject.org/~gk/misc/tor-win32-hardened-gbuilt.zip.asc

libssp and the runtime lib are the same on my test machine.

comment:24 Changed 5 years ago by erinn

Hi gk,

Thanks for your great review.

I've updated my branch (it was a force-update though) to address points 1) and 3).

1) I fixed #10077 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.

2) 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.

3) 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.

comment:25 Changed 5 years ago by erinn

gk --

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.

comment:26 Changed 5 years ago by gk

I match your builds now, too :) So, looks good to me. One minor thing would be fine, deleting all the hardening comments: https://gitweb.torproject.org/user/erinn/tor-browser-bundle.git/blob/d5e72383b108714b80c617f5eb67fe244a097be1:/gitian/descriptors/windows/gitian-firefox.yml#l79

comment:27 Changed 5 years ago by gk

One thing I forgot: I'd like to have somebody reviewing skruffy's binutils patch. Erinn: Could someone from the binutils people do that?

comment:28 Changed 5 years ago by erinn

I deleted all the hardening comments and squashed everything into the Windows hardening commit (see also #10077). 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.

comment:29 Changed 5 years ago by erinn

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.

comment:30 Changed 5 years ago by mikeperry

Resolution: fixed
Status: needs_reviewclosed

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 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.

comment:31 Changed 5 years ago by arma

skruffy points out that no ssp hardening happened here.

(i'm just the messenger.)

comment:32 in reply to:  31 Changed 3 years ago by cypherpunks

Resolution: fixed
Severity: Normal
Status: closedreopened

Replying to arma:

skruffy points out that no ssp hardening happened here.

(i'm just the messenger.)

Replying to this as developers may have forgotten this comment as this ticket is closed.

comment:33 Changed 3 years ago by gk

Resolution: fixed
Status: reopenedclosed

This is no issue anymore.

Note: See TracTickets for help on using tickets.