Bug 978949 - libtool + %global _hardened_build 1 = no full hardening
libtool + %global _hardened_build 1 = no full hardening
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: redhat-rpm-config (Show other bugs)
rawhide
All Linux
unspecified Severity high
: ---
: ---
Assigned To: Panu Matilainen
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 985592 988165
  Show dependency treegraph
 
Reported: 2013-06-27 07:08 EDT by Björn 'besser82' Esser
Modified: 2014-01-13 15:43 EST (History)
11 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 985592 (view as bug list)
Environment:
Last Closed: 2014-01-13 15:38:34 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
negativo17: needinfo+


Attachments (Terms of Use)
PATCH: make libtool pass hardening flags to the linker (938 bytes, patch)
2013-06-27 07:08 EDT, Björn 'besser82' Esser
no flags Details | Diff
PATCH: make libtool pass gcc-spec with hardening ldflags to the linker (881 bytes, patch)
2013-07-10 03:33 EDT, Björn 'besser82' Esser
no flags Details | Diff
fixed patch - covers ltmain.sh-cases as well (937 bytes, patch)
2013-07-25 07:48 EDT, Björn 'besser82' Esser
no flags Details | Diff
Libtool hardening hack again. (5.28 KB, patch)
2013-08-02 03:16 EDT, Pavel Raiskup
no flags Details | Diff
Updated patch to allow opt-out this hack. (1.26 KB, patch)
2013-08-02 18:51 EDT, Pavel Raiskup
no flags Details | Diff
Just adjust the opt-out macro name. (1.28 KB, patch)
2013-12-19 07:05 EST, Pavel Raiskup
no flags Details | Diff

  None (edit)
Description Björn 'besser82' Esser 2013-06-27 07:08:34 EDT
Created attachment 766058 [details]
PATCH: make libtool pass hardening flags to the linker

Description of problem:

  When a package with hardening-flags explicitly enabled in specs-file
  builds a lib using autotools and/or libtool, the result always shows
  "partial" RELRO, only.  This is perfectly the expected result of a
  build when hardening-flags are NOT explicitly enabled in spec-file.


Version-Release number of selected component (if applicable):

  redhat-rpm-config-9.1.0-28.fc17.noarch
  redhat-rpm-config-9.1.0-37.1.fc18.noarch
  redhat-rpm-config-9.1.0-45.fc19.noarch
  redhat-rpm-config-9.1.0-45.fc20.noarch


How reproducible:

  always, when a package with hardening-flags enabled builds a lib using
  autotools and/or libtool


Steps to Reproduce:

  1. rpmbuild a random package which ships lib(s) using libtool during
     %%build and explicitly enables hardening-flags

  2. extract the build binary-rpm and run `hardening-check --color --verbose $file`
     on the libs shipped by that pkg

  3. rebuild the same pkg and disable hardening in spec-file

  4. repeat 2. on the new build pkg and compare the results - no difference

  5. rebuild the previous pkg, re-enable hardening and insert this
     scriptlet immediatly after invoking %configure-macro into it's spec-file:
     
       test -f ./libtool && \
       sed -i \
	   -e 's! \\\\\\$compiler_flags ! \\\\\\$CFLAGS \\\\\\$LDFLAGS -Wl,--as-needed&!g' \
           libtool ;

  6. repeat 2. on the new build pkg and compare the results - full hardening


Actual results:

  with hardening disabled, hardening-check reports:
     ...
     Read-only relocations: yes
     Immediate binding: no, not found!

  with hardening enabled, hardening-check reports:
     ...
     Read-only relocations: yes
     Immediate binding: no, not found!

  It's actually the same, which is not expected to be.  The one having hardening enabled
  shows "partial" RELRO, too.


Expected results:

  with hardening enabled, hardening-check reports:
     ...
     Read-only relocations: yes
     Immediate binding: yes

  The way it should be.  Hardening is applied completely.


Additional info:

  At least gdm (libgdm*.so*) and abrtd (libabrt*.so*) are directly affected
  by this bug.

  The attached patch will fix this problem by doing all needed changes without
  doing anything in a hardening-enabled spec-file.  This patch will fix the
  "unused-direct-shlib-dependency"-issue encoutered with some pakages, too.
Comment 1 Panu Matilainen 2013-06-27 07:45:13 EDT
Has libtool upstream contacted on this particular issue?

From http://lists.gnu.org/archive/html/bug-libtool/2005-10/msg00003.html:
> This change is not correct, however.
> Linker flags are supposed to be interpreted by `libtool', because it may
> have to adjust (both itself and them).  Having LDFLAGS directly in
> $archive_cmds is just wrong, as it bypasses this.

So the sed-hack does something against upstream intentions, and while it might "fix" the hardening case, who knows what it might break? Not to mention such hacks are fragile to version changes and all.

CC'ing ajax, whose work the hardening-stuff in redhat-rpm-config is.
Comment 2 Matthew Miller 2013-07-03 14:28:48 EDT
Can we do a more tailored fix which addresses just the hardening flags and not all of $compiler_flags ?
Comment 3 Stephen Gallagher 2013-07-03 15:28:44 EDT
At the FESCo meeting on 2013-07-03, we deferred judgement on this pending input from Adam Jackson. Adding needsinfo
Comment 4 Toshio Ernie Kuratomi 2013-07-09 12:30:44 EDT
ajax: ping?  FESCo meeting is tomorrow and just seeing if there's an update to this.
Comment 5 Björn 'besser82' Esser 2013-07-10 03:33:59 EDT
Created attachment 771424 [details]
PATCH: make libtool pass gcc-spec with hardening ldflags to the linker

Refactored my patch.  Now it just injects the gcc-spec with hardening ldflags.
Comment 6 Toshio Ernie Kuratomi 2013-07-11 11:21:05 EDT
FESCo deferred a decision on how to deal with hardened_build again at their 2013-07-10 meeting as we're still waiting for input from ajax to this bug.
Comment 7 Adam Jackson 2013-07-11 15:03:51 EDT
I'm a little baffled why I'm being asked.  I don't pretend to understand libtool's implementation.  Nor should I be
Comment 8 Adam Jackson 2013-07-11 15:05:41 EDT
Whoa, misfire.  "Nor should I be" on the critical path for this, in my opinion; fesco ought to be entirely competent to decide this on their own.  Pretty sure I wrote that code while I was on fesco, and only because it happened to seem trivial.

I'd rather see us fix our system libtool to be sane and relibtoolize things that need it.  But seriously, do whatever works.
Comment 9 Miloslav Trmač 2013-07-17 16:05:52 EDT
FESCO agreed on today's meeting that we'd prefer a real fix in libtool (filed bug #985592), and a short-term workaround in redhat-rpm-config in the meantime.

Note that this bug (having at least the redhat-rpm-config workaround) is one of the two items blocking the Fedora 20 mass rebuild (which would ideally start on Jul 20), so applying the redhat-rpm-config workaround soon would be very appreciated.
Comment 10 Panu Matilainen 2013-07-17 23:28:15 EDT
Somebody else better take this bug then, I'm on vacation for two more weeks mostly AFK. Its lucky enough I happened to notice this at all.
Comment 11 Kevin Fenzi 2013-07-24 16:13:46 EDT
Scratch build with this and the docs change in it. 

Please scream if you see anything wrong with it. 

I will push this out later tonight if not: 

http://koji.fedoraproject.org/koji/taskinfo?taskID=5653042
Comment 12 Kevin Fenzi 2013-07-24 20:00:39 EDT
This should be live in rawhide, however, we found a issue with ltmain.sh using packages still not getting hardened.

Björn Esser is going to work on improving the patch tomorrow.
Comment 13 Panu Matilainen 2013-07-25 05:06:50 EDT
The patch also appears to break stuff:
https://lists.fedoraproject.org/pipermail/devel/2013-July/186774.html

Just FWIW, this is why I wasn't willing to address this while on vacation: applying a patch doesn't take long, but babysitting for the almost inevitable regressions and other issues in a hack like this is an entirely different thing.
Comment 14 Björn 'besser82' Esser 2013-07-25 07:48:30 EDT
Created attachment 778235 [details]
fixed patch - covers ltmain.sh-cases as well

(In reply to Kevin Fenzi from comment #12)
> This should be live in rawhide, however, we found a issue with ltmain.sh
> using packages still not getting hardened.
> 
> Björn Esser is going to work on improving the patch tomorrow.

Here it is :)  Should fithe build using ltmain.sh, too.  Checked with abrt and works there.
Comment 15 Tomas Mraz 2013-07-25 08:23:03 EDT
Unfortunately it does not cover the breakage as in comment 13.
Comment 16 Tomas Mraz 2013-07-25 08:24:45 EDT
Could the libtool hack be moved before the ./configure invocation?
Comment 17 Björn 'besser82' Esser 2013-07-25 09:28:56 EDT
Unfortunately we cannot move this before ./configure, because files that need to have sed-hack applied are created during configure-run :(

If params for %configure are all in one line, everything should be running.  This is some problem of the specfile-parser, I suppose...

I need to think about a proper solution.  If anyone has an idea, you're welcome to suggest.
Comment 18 Tomas Mraz 2013-07-25 09:33:25 EDT
Hack ugly as hell - append the commands to the configure script.
Comment 19 Björn 'besser82' Esser 2013-07-25 09:53:33 EDT
This gives me an idea...

Something like a background-process being invoked before, waiting for ./configure to finish and do the sed-hack...

Any ideas how to implement?
Comment 20 Kevin Fenzi 2013-07-25 12:31:34 EDT
Hum. Looking at it, could we take a different approach? 

Could we do something in /usr/share/config.site ?

If we can't come up with a good solution here, I'd say perhaps we should revert for now, and look at any packages that aren't properly hardened and fix things in their specs for now?
Comment 21 Simone Caronni 2013-07-26 08:06:06 EDT
(In reply to Tomas Mraz from comment #15)
> Unfortunately it does not cover the breakage as in comment 13.

Yep... it's breaking most of the builds I see around.
Any ETA for reverting/fix? 

Thanks.
--Simone
Comment 22 Stephen Gallagher 2013-07-26 08:13:35 EDT
(In reply to Simone Caronni from comment #21)
> (In reply to Tomas Mraz from comment #15)
> > Unfortunately it does not cover the breakage as in comment 13.
> 
> Yep... it's breaking most of the builds I see around.
> Any ETA for reverting/fix? 
> 


Tomas removed the hack and rebuilt yesterday afternoon. The Koji buildroot *should* be working now. If you're still seeing build breakage, it's possibly related to the docdir changes. Can you point out some specific examples?
Comment 23 Simone Caronni 2013-07-26 08:39:42 EDT
Yes it's fixed, tried now. Sorry for the noise. I was looking here or on the mailing list for announcements and did not notice it was already fixed.

Thanks.
--Simone
Comment 24 Pavel Raiskup 2013-08-02 03:16:05 EDT
Created attachment 781859 [details]
Libtool hardening hack again.

I tried to sum up this problem, repair me or add anything you think of.

1. The hack is really needed for this issue (and will be probably needed for
   generations) because even if it was fixed upstream (libtool) immediately,
   the fix will be propagated to all packages very slowly — basically once
   all upstream maintainers hit 'autoreconf -if' in their package.

2. Regarding possible upstream fix:  the problem is that libtool does not
   handle '-specs=.*' parameter for gcc (in LDFLAGS).  This may be pretty
   easily enhanced - and I will try to propose some fix upstream.  The question
   is how it will be seen by upstream.

For this concrete ugly hack
===========================

3. this hack should be OK for libtool 1.4+ (+- year 2001) and at least it
   should not break things (if some concrete package does not have this hacked
   on its own, does not use ltmain.sh name for other purposes, etc..).  Also
   note that the ltmain.sh may be in other place than directly in %{_builddir}.

4. Problems: don't handling escaping in hardened ldflags (so be careful when
   changing its content).  Also, adjusting of this patch may be needed once
   this is fixed libtool-upstream.  Most probably also, it does not fix all
   packages requesting %_hardened_build.

5. Other workarounds:  We may pass (instead of -specs=..) the -Wl,-z,now
   parameter (this is successfully propagated to gcc when linking by libtool).
   I would also prefer to add some /usr/lib/rpm/redhat/configure-wrapper bash
   script to make these changes more obvious — and this would allow us to make
   nasty changes in post-./configure time.
   Libtool also has -XCClinker, -Xlinker and -Xcompiler flags, see output of
   $ libtool --mode=link --help.

6. The output of hardening-check is following, is that correct?

   $ hardening-check gc-7.2/.libs/libgc.so
   gc-7.2/.libs/libgc.so:
    Position Independent Executable: no, regular shared library (ignored)
    Stack protected: yes
    Fortify Source functions: yes (some protected functions found)
    Read-only relocations: yes
    Immediate binding: yes

===

Some testing is really needed and it is big question whether to use this (or
something else) or postpone it to fc21 rather.  IMO, (if the build is not
broken by %_hardened_build itself now) it should not break builds in fc20.

Pavel
Comment 25 Stephen Gallagher 2013-08-02 07:59:14 EDT
(In reply to Pavel Raiskup from comment #24)
> Created attachment 781859 [details]
> Libtool hardening hack again.
> 
> I tried to sum up this problem, repair me or add anything you think of.
> 
> 1. The hack is really needed for this issue (and will be probably needed for
>    generations) because even if it was fixed upstream (libtool) immediately,
>    the fix will be propagated to all packages very slowly — basically once
>    all upstream maintainers hit 'autoreconf -if' in their package.
> 

This doesn't need to be done upstream. I have always thought it odd that Debian/Ubuntu requires this in their packaging guidelines but Fedora does not. In any case, I've always made a point of running 'autoreconf -if' in all of my autotools packages for reasons just like this: autotools and libtool make advances much faster than the packages that bundle them do.


> 2. Regarding possible upstream fix:  the problem is that libtool does not
>    handle '-specs=.*' parameter for gcc (in LDFLAGS).  This may be pretty
>    easily enhanced - and I will try to propose some fix upstream.  The
> question
>    is how it will be seen by upstream.
> 

If it's not accepted by upstream, this seems like the sort of thing that might warrant us carrying a patch in our package.
Comment 26 Pavel Raiskup 2013-08-02 08:51:46 EDT
> >    all upstream maintainers hit 'autoreconf -if' in their package.
>
> This doesn't need to be done upstream. I have always thought it odd that
> Debian/Ubuntu requires this in their packaging guidelines but Fedora does
> not.

We may not generally suggest people to do it.  Maintainer should know what
works for his package — IMO the Fedora's way is more correct.  Running
'autoreconf -fi' without compromises replaces out your m4 macros and other
stuff packaged in tarball with newer files from system -- and it may break the
build (and in some cases it may even change runtime behaviour silently) same
way as it sometimes the only way how to enable build.

Other thing is that a lot of (outdated) packages cost a lot of work to be able
to please autoreconf..  but this is becoming OT.

> > 2. Regarding possible upstream fix:  the problem is that libtool does not
> >    handle '-specs=.*' parameter for gcc (in LDFLAGS).  This may be pretty
> >    easily enhanced - and I will try to propose some fix upstream.  The
> >    question is how it will be seen by upstream.
>
> If it's not accepted by upstream, this seems like the sort of thing that
> might warrant us carrying a patch in our package.

Agree, but this is just a hack (unrelated to possible changes in libtool) to
enable the %_hardened_build to work in redhat/macros.  Upstream would
definitely consider the %_hardened_build as hack completely..  And I agree
that the most clean way how to deal with this is in per package context.

It depends on how much maintainer-work may we save applying this hack.

Pavel
Comment 27 Kevin Fenzi 2013-08-02 12:30:48 EDT
Does the patch from comment 24 fix those cases we know where the previous one failed?

(There should be at least a small pool of packages to test with mentioned on fedora devel list when the orig patch was landed). 

I'm pretty leary of landing this now, since we are going to start mass rebuild very very soon, so there's not much time to shake it out. However, if someone could confirm all the cases mentioned on the list work ok with this we could consider it more.
Comment 28 Pavel Raiskup 2013-08-02 18:51:05 EDT
Created attachment 782186 [details]
Updated patch to allow opt-out this hack.

Based on (a) Mamoru's notes [1], (b) taking into account that this may fix
potential build fails and (c) theoretically it could be useful by some packages
(e.g. by 'libtool' itself - if the _hardened_build was enabled) — I'm attaching
the patch again having also the '%configure_libtool_hack' option.

So you may define "%global configure_libtool_hack 0" if you want to opt-out
this hack from your hardened-build-enabled package.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=951442#c13
Comment 29 Fedora End Of Life 2013-09-16 12:44:51 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 20 development cycle.
Changing version to '20'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora20
Comment 30 Kevin Fenzi 2013-12-18 13:30:44 EST
Any news here? We still have this disabled currently, it would be nice to come up with a way to enable it.
Comment 31 Tomas Mraz 2013-12-19 04:45:13 EST
I think the hack from comment 28 should be applied to Rawhide. We will see if it causes any serious build breakage. (I think it should not.)
Comment 32 Pavel Raiskup 2013-12-19 07:05:47 EST
Created attachment 838954 [details]
Just adjust the opt-out macro name.

s|configure_libtool_hack|_configure_libtool_hardening_hack|
Comment 33 Kevin Fenzi 2014-01-12 13:38:50 EST
ok, here's the scratch build I will look at pushing tomorrow and announcing to devel-announce: 

http://koji.fedoraproject.org/koji/taskinfo?taskID=6393233

Please test. ;) 

proposed announcement: 

Packages currently using the _hardened_build macro that also use libtool 
may have been built only with "partial" RELRO instead of full RELRO protections. 

A workaround has been added today to the redhat-rpm-config package in rawhide to fix this issue.
Maintainers are encouraged to check their _hardened_build packages to confirm that they 
rebuild correctly with full RELRO protections. Additionally since this change is in the %configure macro used by many packages, maintainers that find regressions due to this change should file bugs on the redhat-rpm-macros package. 

(do we have a page listing how maintainers can check packages easily for full relro?)
Comment 34 Björn 'besser82' Esser 2014-01-13 02:34:08 EST
From me POV this looks good.  :D  I did some basic test building a random package which uses libtool for building it's ELF with hardening-flags enabled and no special tweaks applied.  After build I unpacked the resulting rpms with ELF content and examinated the ELFs with "hardening-check":

for _elf in `find -type f $unpacked_rpm_dir` ;
do
  [[ "´head -c 4 ${_elf} | tail -c 3´" == "ELF" ]] && \
  hardening-check -c ${_elf};
done;

The expected output of that should be

  Position Independent Executable: yes
  Stack protected: yes
  Fortify Source functions: yes (some protected functions found)
  Read-only relocations: yes
  Immediate binding: yes

for an executable and

  Position Independent Executable: no, regular shared library (ignored)
  Stack protected: yes
  Fortify Source functions: yes (some protected functions found)
  Read-only relocations: yes
  Immediate binding: yes

for a lib.so.

If "Immediate binding:" shows "no, not found!", then something went wrong and hardening-flags aren't properly applied during linking by libtool.  If "  Read-only relocations:" shows "no, not found!", then the is a general problem in properly passing in LDFLAGS into the build-process.

btw:  We should probably add some fuctionality to "fedora-review" checking for properly applying LDFLAGS in every build && change the guidelines accordingly.  I always see review-request, which don't use any buildsystem, but makefile-only, and don't apply LDFLAGS at all.  They simply use
`make CFLAGS="%{optflags}"` instead of
`make CFLAGS="%{optflags}" LDFLAGS="%{?__global_ldflags}"`, which causes not even partial-RELRO as a result.
Comment 35 Kevin Fenzi 2014-01-13 15:38:34 EST
Landed in rawhide. 

Announced: 
https://lists.fedoraproject.org/pipermail/devel-announce/2014-January/001292.html

Closing now. Please reopen if any further issues are detected with this patch.

Note You need to log in before you can comment on or make changes to this bug.