Opened 5 years ago

Closed 5 years ago

#13031 closed defect (fixed)

Provide full RELRO protection on Linux

Reported by: mikeperry Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: tbb-security, TorBrowserTeam201409
Cc: gk Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Our fix in #12103 to provide RELRO protection causes checksec to report "partial" hardening. We need to figure out what that means and why it claims its only partial.

Child Tickets

Change History (12)

comment:1 Changed 5 years ago by cypherpunks

Full RELRO reported if both:

  if readelf -l $1 2>/dev/null | grep -q 'GNU_RELRO'; then
    if readelf -d $1 2>/dev/null | grep -q 'BIND_NOW'; then

are true.

We need to figure out what that means and why it claims its only partial.

Seems like gold linker missed BIND_NOW dynamic section.

comment:2 Changed 5 years ago by cypherpunks

BIND_NOW generated if -z now passed to linker. Hardening Wrapper does it during Tor Browser building. Gold supports -z now according to manual page, then wrapper doesn't work for gold or it's bug in linker itself.

comment:3 Changed 5 years ago by cypherpunks

It's probably about d8e92e2f4d362216dfff1790026309e6c0a51b58 commit (fix for #12103 as is). It builds independent binutils and creates new ld.gold that used instead of link to hardened-ld (real ld.gold should be renamed by wrapper to ld.gold.real). Then it using such "non hardened" ld.gold later.

With hardened wrappers enabled ld files are:
ld is link to ld.gold or ld.bfd (depends preferable linker)
ld.gold and ld.bfd are links to hardened-ld
real linkers are ld.gold.real and ld.bfd.real which used by hardened-ld perl script.

Building of compiler or linker should to count existence of hardened wrappers and allow to use them instead of unconditional replacing of stuff.

comment:4 Changed 5 years ago by cypherpunks

Status: newneeds_review
--- gitian-utils.yml.original
+++ gitian-utils.yml
@@ -63,6 +63,9 @@
   # Make sure gold is used and not ld.
   cd $INSTDIR/binutils/bin
   rm ld
+  cp /usr/bin/hardened-ld ./
+  mv ld.gold ld.gold.real
+  ln -sf hardened-ld ld.gold
   ln -sf ld.gold ld
   cd ~/build
 

Lets test.

comment:5 Changed 5 years ago by gk

Status: needs_reviewneeds_revision

This hangs in |make -f client.mk configure| now. Not sure why.

comment:6 Changed 5 years ago by cypherpunks

This hangs in |make -f client.mk configure| now. Not sure why.

Zip archive can't keep links it keeping copy of target.

comment:7 Changed 5 years ago by cypherpunks

Status: needs_revisionneeds_review
--- gitian-utils.yml.original
+++ gitian-utils.yml
@@ -61,9 +61,6 @@
   make $MAKEOPTS
   make install
   # Make sure gold is used and not ld.
-  cd $INSTDIR/binutils/bin
-  rm ld
-  ln -sf ld.gold ld
   cd ~/build
 
   # Building Libevent
--- gitian-firefox.yml.original
+++ gitian-firefox.yml
@@ -65,6 +65,12 @@
   export PATH=$INSTDIR/python/bin:$PATH
   #
   unzip -d $INSTDIR binutils-linux$GBUILD_BITS-utils.zip
+  cd $INSTDIR/binutils/bin
+  rm ld
+  cp /usr/bin/hardened-ld ./
+  mv ld.gold ld.gold.real
+  ln -sf hardened-ld ld.gold
+  ln -sf ld.gold ld
   export PATH=$INSTDIR/binutils/bin:$PATH
   mkdir -p $INSTDIR/Browser/
   mkdir -p $INSTDIR/Debug/Browser/components

Lets test ver2.0

comment:8 Changed 5 years ago by gk

Okay, this works, thanks! I need to review this more closely (mainly to understand the issue behind it) and would especially like to avoid using some random hardened-ld if we are already building our linker ourselves. We'll see how to get that. But it should be available in the next release.

comment:9 Changed 5 years ago by cypherpunks

Okay, this works

Does checksec reports full RELRO protection?

some random hardened-ld if we are already building our linker ourselves.

Binutils and hardening wrappers are different packages. Wrapper passes need arguments to linker.
Only option to change default sections generated by linker, like to generate BIND_NOW section by default too.

comment:10 Changed 5 years ago by cypherpunks

Only option to change default sections generated by linker, like to generate BIND_NOW section by default too.

Or to pass LDFLAGS for every need case. Or to recompile modified gcc to pass need flags by default, like it happens for RELRO case by gcc-default-relro.diff

comment:11 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201409 added

comment:12 in reply to:  9 Changed 5 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Replying to cypherpunks:

Okay, this works

Does checksec reports full RELRO protection?

Yes. This is fixed in commit 71f2ed613d2b2ce6fb40ef27f99fd140b1b86517. Thanks.

Note: See TracTickets for help on using tickets.