Bug 16621

I commented about this problem at length in NetBSD Problem Report 39105, link http://www.netbsd.org/cgi-bin/query-pr-single.pl?number=39105

I said that the dbus-1.2.1/configure script detects that gcc has the -fPIE flag, so it compiles with -fPIE and links with -pie. The script also detects that gcc has the -Wl,--gc-sections flag, so it compiles with -ffunction-sections and -fdata-sections and links with -Wl,--gc-sections. The gcc compiler that comes with NetBSD 4.0/powerpc has all of these flags, but they do not work correctly.

I said that PIE causes the dynamic linker /libexec/ld.elf_so to give that message "Unsupported relocation type 6 in non-PLT relocations". If I remove the PIE flags but leave the gc-sections flags, and build and run dbus-daemon, then the dynamic linker gives a "Bad pAUX_base" message. If I also remove the gc-sections flags, then dbus-daemon will run normally. (There is no problem with PIC flags for shared libraries.)

I now have dbus-1.2.4 and NetBSD 4.0.1/powerpc, but the problem is the same.

----
I propose two new configure flags. The --disable-pie flag shall tell configure to not use -fPIE and -pie. The --disable-gc-sections flag shall tell configure to not use -Wl,--gc-sections and -ffunction-sections and -fdata-sections. (I do not propose a --without-pic or --disable-pic flag.)

Users who have broken systems, like NetBSD/powerpc, must know to use the --disable-pie and --disable-gc-sections flags if necessary. (The configure script will not know which systems are broken. The broken flags might work with a simple test program but fail with dbus-daemon; I could not reproduce "Bad pAUX_base" in a simple test program.)

I will attach a patch to dbus-1.2.4/configure.in that provides the --disable-pie and --disable-gc-sections flags.

Created attachment 20515 [details] [review]
Adds --disable-pie and --disable-gc-sections flags

To apply this patch
$ cd dbus-1.2.4
$ patch < configure-in.patch

Then run autoconf to regenerate this configure.

I tested this patch by running configure a few times and then checking the presence or absence of the pie or gc-sections flags in the top-level Makefile.

Personally I think they should fix their compilers.  D-Bus should not route around bugs in a compiler.  If those flags do not work in gcc then disable them there or fix the test in D-Bus to do a more in-depth check.  What happens when the option works?  I'm guessing these two OS's will miss out on these security enhancements because no one will notice.  Also, are you sure it is a bug or do those systems require different flags?  Those avenues must be explored first before suggesting a disable flag.

*** Bug 30216 has been marked as a duplicate of this bug. ***

On the duplicate bug, I wrote:

It would also be good to make PIE optional. In Maemo it's patched out of
configure.in altogether[1], because on (some version of?) their ARM toolchain,
it produces/produced executables that usually work, but not reliably (which we
can't really check in configure).

Debian has also had problems with PIE on various less mainstream architectures
at various times (at least mips[2], hppa/m68k[3], avr32[4]). Debian's dbus is
currently compiled with PIE on those architectures, though.

If we add a --disable-pie option, distros with known-unreliable PIE support in
(some of) their toolchains can at least use it as a workaround (for particular
architectures or globally) without having to patch configure.

[1]
http://maemo.gitorious.org/maemo-gnome-essentials/dbus/blobs/maemo/debian/patches/80-remove-fpie-pie.patch
which fixes NB#171940, according to debian/changelog
[2] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=526961
[3]
http://wiki.debian.org/Hardening#DEBBUILDHARDENINGPIE.28gcc.2BAC8-g.2B-.2B--fPIE-pie.29
[5] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=574716

(In reply to comment #3)
> Personally I think they should fix their compilers.  D-Bus should not route
> around bugs in a compiler.

In principle, sure; but we're enabling PIE, which is something that fairly often breaks on non-x86 architectures or non-GNU libc. So far we've seen it go wrong on NetBSD and Solaris.

As I understand it, PIE only works if your compiler, linker (as in binutils) and runtime linker (as in ld.so) all support it?

> What happens when
> the option works?  I'm guessing these two OS's will miss out on these security
> enhancements because no one will notice.

At worst, packagers for those OSs notice that PIE is broken because they had to explicitly set the option to disable it. We could rename it to --disable-pie-because-my-platform-is-broken if you like...

Platforms where PIE doesn't work don't get to benefit from ASLR in any case; it seems better that D-Bus works sub-optimally than that it doesn't work.

Created attachment 42821 [details] [review]
Allow PIE to be disabled, to work around broken toolchains

I've redone the patch a bit, so it explicitly notes this as a workaround for broken toolchains.

Is there precedent for --disable-pie in other projects?  Or is dbus one of the only low level projects automatically adding -fPIE?

I'd actually lean towards punting this one to OS builders; so we'd then need a --enable-daemon-PIE flag.  (I explictly call it --daemon-PIE since it doesn't seem really valuable to make e.g. dbus-monitor PIE).

Maemo developer Akos Pasztory points out that -fPIE can be disadvantageous even when your toolchain works correctly:

> Linux on ARM has no ASLR features (except for stack), so that's not a point for
> dbus-daemon to be PIE.  OTOH, being non-PIE allows prelinking, so 2-0 for being
> an ordinary executable (on our platform).

so we might want to weaken the wording for -fPIE to indicate that it's a trade-off, or even make it default-disabled.

(In reply to comment #7)
> Is there precedent for --disable-pie in other projects?  Or is dbus one of the
> only low level projects automatically adding -fPIE?

Debian and Ubuntu seem to rely on encouraging shared libraries (which are PIC anyway, and have other well-known security advantages), and making security-sensitive processes PIE in a targeted way. It's usually hard to implement without patching the build system, which is a pain in distro packages (requires autoreconf etc.), so if distributions are interested in making dbus-daemon PIE, it's easier if we give them a flag they can enable.

Debian builds at least openssh and sendmail as PIE.

Ubuntu builds quite a few things PIE: https://wiki.ubuntu.com/SecurityTeam/KnowledgeBase/BuiltPIE

Gentoo has a modified gcc which builds everything PIE unless asked not to, and some "interesting" workarounds for building libraries.

> I'd actually lean towards punting this one to OS builders; so we'd then need a
> --enable-daemon-PIE flag.  (I explictly call it --daemon-PIE since it doesn't
> seem really valuable to make e.g. dbus-monitor PIE).

I'll have a look at other packages and see what they do.

(In reply to comment #7)
> Is there precedent for --disable-pie in other projects?  Or is dbus one of the
> only low level projects automatically adding -fPIE?

It seems that the packages in Debian that use -fPIE do so by overriding the CFLAGS during build. The tl;dr version of the buildd log analysis below is that libtool is actually quite clever, and we can support distros that care about -fPIE by just not doing anything special: those distros can set CFLAGS appropriately, and everything will Just Work.

I'm inclined to drop the PIE/relro stuff completely, to be honest; there's no benefit in having it unless it's on-by-default, since

    ./configure CFLAGS=-fPIE LDFLAGS="-pie -Wl,-z,relro"

is no more difficult to do than

    ./configure --enable-pie --enable-relro

and works consistently for any libtool-using package.

(Our use of relro is also incomplete; to be fully tin-foil-hat compliant, I hear we should also be using eager binding - "-Wl,-z,relro -Wl,-z,now".)

The Debian package hardening-wrapper knows which architectures currently have broken toolchains (there's always one...) and works around them by not using the more obscure options there; for instance, PIE is currently disabled on hppa, m68k, mips/mipsel and avr32.

The one benefit we might derive from the current mechanisms would be having PIE on only the security-sensitive bits (dbus-daemon and dbus-launch-helper), and not on ancillary tools like dbus-send. However, we already do this wrong: dbus-launch-helper isn't PIE right now, despite being setuid root.

-----------------------------

libtool in "not actually useless" shock
=======================================

The situation we have in libdbus is that the binaries link a libtool convenience library (libdbus-internal.la, in noinst_LTLIBRARIES). One day I'd actually like them to link libdbus dynamically so we don't compile the whole thing twice, but whatever.

From some application of grep, it turns out that man-db has both (installed) shared libraries libman and libmandb (equivalent to libdbus), and a convenience library libgnu (equivalent to libdbus-internal). So, let's use man-db as our template.

The official Debian buildd logs for man-db aren't entirely helpful, because it uses --enable-silent-rules, but I rebuilt it locally without that: <http://people.collabora.co.uk/~smcv/man-db_2.5.9-4+verbose-amd64-20110221-1520>

it becomes clear that libtool is somewhat clever about PIE. The Debian package passes these in the environment of ./configure:

CFLAGS="-O2 -g -fPIE -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security"
LDFLAGS="-fPIE -pie -Wl,-z,relro -Wl,-z,now"

When compiling for a convenience library, we get this libtool invocation (extra whitespace added for clarity - note use of -fPIE):

/bin/bash ../../libtool  --tag=CC   --mode=compile gcc -std=gnu99 \
    -DHAVE_CONFIG_H -I. -I../../../../gnulib/lib -I../.. \
    -DDEFAULT_TEXT_DOMAIN=\"man-db-gnulib\" -I../../intl \
    -O2 -g  -fPIE  -fstack-protector --param ssp-buffer-size=4 \
    -D_FORTIFY_SOURCE=2  -Wformat -Wformat-security \
    -Werror=format-security  -Wall -W -Wpointer-arith \
    -Wwrite-strings -Wstrict-prototypes -Wshadow -Wformat-security \
    -Wredundant-decls -Wno-missing-field-initializers -c \
    -o argp-ba.lo ../../../../gnulib/lib/argp-ba.c

which expands into this gcc invocation (note that libtool automatically replaced -fPIE with -fPIC -DPIC):

libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. \
    -I../../../../gnulib/lib -I../.. \
    -DDEFAULT_TEXT_DOMAIN=\"man-db-gnulib\" -I../../intl \
    -O2 -g -fstack-protector --param ssp-buffer-size=4 \
    -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security \
    -Werror=format-security -Wall -W -Wpointer-arith \
    -Wwrite-strings -Wstrict-prototypes -Wshadow \
    -Wformat-security -Wredundant-decls -Wno-missing-field-initializers \
    -c ../../../../gnulib/lib/argp-ba.c  -fPIC -DPIC -o .libs/argp-ba.o

When the convenience library is linked, we get this libtool call (note -fPIE -pie):

/bin/bash ../../libtool  --tag=CC   --mode=link gcc -std=gnu99  -O2 -g \
    -fPIE  -fstack-protector --param ssp-buffer-size=4  \
    -D_FORTIFY_SOURCE=2  -Wformat -Wformat-security \
    -Werror=format-security  -Wall -W -Wpointer-arith \
    -Wwrite-strings -Wstrict-prototypes -Wshadow -Wformat-security \
    -Wredundant-decls -Wno-missing-field-initializers \
    -fPIE -pie  -Wl,-z,relro  -Wl,-z,now  -o libgnu.la \
    areadlink-with-size.lo [...] xmalloc.lo

which expands to a call to ar, without invoking gcc at all:

libtool: link: ar cru .libs/libgnu.a \
    .libs/areadlink-with-size.o [...] .libs/xmalloc.o 

When compiling for a shared library, libmandb, libtool is again invoked with -fPIE:

/bin/bash ../libtool  --tag=CC   --mode=compile gcc -std=gnu99 \
    -DHAVE_CONFIG_H -I. -I../../../libdb -I..  -I../include \
    -I../../../gnulib/lib -I../gnulib/lib -I../../../lib  \
    -O2 -g  -fPIE  -fstack-protector --param ssp-buffer-size=4 \
    -D_FORTIFY_SOURCE=2  -Wformat -Wformat-security \
    -Werror=format-security  -Wall -W -Wpointer-arith \
    -Wwrite-strings -Wstrict-prototypes -Wshadow -Wformat-security \
    -Wredundant-decls -Wno-missing-field-initializers \
    -c -o libmandb_la-db_btree.lo \
    `test -f 'db_btree.c' || echo '../../../libdb/'`db_btree.c

which it again replaces with -fPIC -DPIC:

libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I../../../libdb 
    -I.. -I../include -I../../../gnulib/lib -I../gnulib/lib \
    -I../../../lib -O2 -g -fstack-protector --param ssp-buffer-size=4 \
    -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security \
    -Wall -W -Wpointer-arith -Wwrite-strings -Wstrict-prototypes -Wshadow \
    -Wformat-security -Wredundant-decls -Wno-missing-field-initializers \
    -c ../../../libdb/db_btree.c  -fPIC -DPIC \
    -o .libs/libmandb_la-db_btree.o

When linking that library, once again we get a libtool call with -fPIE, and this time also -pie:

/bin/bash ../libtool  --tag=CC   --mode=link gcc -std=gnu99  -O2 -g \
    -fPIE [...] -fPIE -pie [...]

but libtool knows that shared libraries are different, and omits -fPIE and -pie:

libtool: link: gcc -shared [...] \
    ../lib/.libs/libman.so /usr/lib/libgdbm.so \
    -Wl,-z -Wl,relro -Wl,-z -Wl,now   -Wl,-soname -Wl,libmandb-2.5.9.so \
    -o .libs/libmandb-2.5.9.so

Finally, linking an executable against those libraries uses a call to libtool with -fPIE -pie:

/bin/bash ../libtool  --tag=CC   --mode=link gcc -std=gnu99  -O2 -g \
    -fPIE  -fstack-protector --param ssp-buffer-size=4 \
    -D_FORTIFY_SOURCE=2  -Wformat -Wformat-security \
    -Werror=format-security  -Wall -W -Wpointer-arith -Wwrite-strings \
    -Wstrict-prototypes -Wshadow -Wformat-security -Wredundant-decls \
    -Wno-missing-field-initializers  -fPIE -pie  -Wl,-z,relro \
    -Wl,-z,now  -o accessdb accessdb.o ../libdb/libmandb.la \
    ../lib/libman.la ../gnulib/lib/libgnu.la  -lgdbm

results in -fPIE -pie correctly being passed through:

libtool: link: gcc -std=gnu99 -O2 -g -fPIE -fstack-protector \
    --param ssp-buffer-size=4 -D_FORTIFY_SOURCE=2 -Wformat \
    -Wformat-security -Werror=format-security -Wall -W -Wpointer-arith \
    -Wwrite-strings -Wstrict-prototypes -Wshadow -Wformat-security \
    -Wredundant-decls -Wno-missing-field-initializers -fPIE -pie \
    -Wl,-z -Wl,relro -Wl,-z -Wl,now -o .libs/accessdb accessdb.o \
    ../libdb/.libs/libmandb.so ../lib/.libs/libman.so \
    ../gnulib/lib/.libs/libgnu.a /usr/lib/libgdbm.so \
    -Wl,-rpath -Wl,/usr/lib/man-db

Created attachment 43611 [details] [review]
Don't force use of -fPIE for the dbus-daemon if apparently supported

It's a minor security benefit, but not automatically beneficial (it
    enables ASLR, but breaks prelinking, some buggy toolchains, and some gdb
    versions). Distributions who know their infrastructure works well can
    enable it just as easily via
    
        ./configure CFLAGS="-fPIE" LDFLAGS="-pie"
    
    without extra support from us, and that's a generic solution applicable to
    many packages.
    
    Similarly, don't force libdbus and libdbus-internal to be PIC: libtool
    knows better than we do whether that's necessary/beneficial on a
    particular platform.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=16621
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=27215
Bug-NB: NB#171940

Created attachment 43612 [details] [review]
Drop outdated list of options from README, mention configure --help instead

Created attachment 43613 [details] [review]
Release notes for the -fPIE change

I think this probably deserves relnotes, so distro maintainers who care about -fPIE can take steps to re-enable it.

*** Bug 27215 has been marked as a duplicate of this bug. ***

(In reply to comment #10)
> Created an attachment (id=43611) [details]
> Don't force use of -fPIE for the dbus-daemon if apparently supported

Looks great to me.  Just as a data point, Red Hat Enterprise Linux security team wants to change all daemons to use -fPIE by default, but that can easily be done in the build wrapper as documented.

Thanks, merged to dbus-1.4 for 1.4.10, and will soon also arrive in master for 1.5.2. I also merged the README and NEWS changes.

(In reply to comment #14)
> Red Hat Enterprise Linux security
> team wants to change all daemons to use -fPIE by default, but that can easily
> be done in the build wrapper as documented.

Yeah, I'd recommend doing that by changing the configure invocation in the spec file, which should work with most autotools packages. We'll probably be doing the dpkg equivalent of that (changing the configure invocation in debian/rules) in Debian.