Opened 5 years ago

Closed 5 years ago

#12103 closed defect (fixed)

Fully hardening firefox binary is broken since 3.5.3 on Linux

Reported by: gk Owned by: erinn
Priority: Medium Milestone:
Component: Applications/Tor bundles/installation Version:
Severity: Keywords: tbb-security, tbb-testcase
Cc: mikeperry, intrigeri Actual Points:
Parent ID: #10065 Points:
Reviewer: Sponsor:

Description

Beginning with TBB 3.5.3 we have neither for 32 nor for 64 bit firefox binaries RELRO anymore on Linux according to the checksec script.

Child Tickets

Change History (17)

comment:1 Changed 5 years ago by gk

Cc: mikeperry added

comment:2 Changed 5 years ago by gk

Bleh, nothing obvious in the diff between 3.5.2 and 3.5.3.

comment:3 Changed 5 years ago by cypherpunks

It's about 9c37c9fcc639c02e9dd7b81741bbf95a24f47a31 commit.
No GNU_RELRO for headers reported by readelf after objcopy --remove-section=.note.gnu.build-id applied.

comment:4 Changed 5 years ago by mikeperry

Keywords: tbb-testcase added

In January, there was this fix to binutils: "Update bfd to properly generate PT_GNU_RELRO segment for ld and objcopy. PRs 14207/16322/16323."
http://gcc.gnu.org/ml/gcc/2014-01/msg00286.html

It seems like RedHat may have independently patched this or a related issue in 2012: "Fix the creation of GNU_RELRO segments (#825736)"
http://pkgs.org/centos-6/centos-x86_64/binutils-2.20.51.0.2-5.36.el6.x86_64.rpm.html

It seems like running checksec regularly should be part of our test suite, to ensure against regressions like this when either the toolchain or how we use it changes.

comment:5 in reply to:  4 Changed 5 years ago by gk

Replying to mikeperry:

In January, there was this fix to binutils: "Update bfd to properly generate PT_GNU_RELRO segment for ld and objcopy. PRs 14207/16322/16323."
http://gcc.gnu.org/ml/gcc/2014-01/msg00286.html

I tried 2.24.51.0.3 but still, objcopy is removing our RELRO. :(

It seems like RedHat may have independently patched this or a related issue in 2012: "Fix the creation of GNU_RELRO segments (#825736)"
http://pkgs.org/centos-6/centos-x86_64/binutils-2.20.51.0.2-5.36.el6.x86_64.rpm.html

Hrm... that bug is not visible for me, so not sure yet what they actually fixed there.

comment:6 Changed 5 years ago by intrigeri

Cc: intrigeri added

comment:7 Changed 5 years ago by erinn

Keywords: needs-triage added

comment:8 Changed 5 years ago by gk

Keywords: needs-triage removed

comment:9 Changed 5 years ago by mikeperry

Parent ID: #10065

comment:10 Changed 5 years ago by cypherpunks

Look at rewrite_elf_program_header() from bfd/elf.c:

      if (segment->p_type != PT_LOAD)
	{
	  /* Remove PT_GNU_RELRO segment.  */
	  if (segment->p_type == PT_GNU_RELRO)
	    segment->p_type = PT_NULL;
	  continue;
	}

It used to call by setup_bfd_headers (ibfd, obfd) from copy_object() of binutils/objcopy.c

comment:11 Changed 5 years ago by cypherpunks

Remove PT_GNU_RELRO segment.

Original source of changes

+	{
+	  /* If a section is added or mofied, remove PT_GNU_RELRO
+	     segment.  */
+	  if (status != removed &&
+	      segment->p_type == PT_GNU_RELRO)
+	    segment->p_type = PT_NULL;
+	  continue;
+	}

It keeps .data.rel.ro section.

comment:12 Changed 5 years ago by cypherpunks

Binutils bugs used to remove GNU_RELRO segment: 5037, 3281

comment:13 Changed 5 years ago by cypherpunks

Lets investigate alternatives for binutils in that case. Elfutils includes eu-strip which supports remove-section command.

comment:14 Changed 5 years ago by cypherpunks

Elfutils

Non working alternative

-R option supports only .comment section
Last edited 5 years ago by cypherpunks (previous) (diff)

comment:15 Changed 5 years ago by cypherpunks

Another alternative elftoolchain. Not in debian repository, 473189

comment:16 Changed 5 years ago by cypherpunks

Instead of removing .note.gnu.build-id we could to skip it while linking stuff. If to pass option for compiler:

-Wl,--build-id=none

No Build ID then.

comment:17 in reply to:  16 Changed 5 years ago by gk

Resolution: fixed
Status: newclosed

Replying to cypherpunks:

Instead of removing .note.gnu.build-id we could to skip it while linking stuff. If to pass option for compiler:

-Wl,--build-id=none

No Build ID then.

Turns out that this idea needs some Firefox patching we can avoid if we switch to gold as our linker which has a number of other advantages like waaay faster linking and being a fix for #12743. This is fixed with commit d8e92e2f4d362216dfff1790026309e6c0a51b58 on master and commit 7df10ce04da9ed36a55e91c193fca29e88ac7a5f on maint-3.6.

Note: See TracTickets for help on using tickets.