Makefile: Apply minimum hardening to libs and applications.

[besser82]

• -Wl,-z,defs: Disallows undefined symbols at link-time.
• -Wl,-z,relro: Relocation Read-Only protects the Global Offset Table (GOT) in ELF binaries from being overwritten.
• -Wl,-z,now: Tell the dynamic linker to resolve all symbols when the program is started, or when the shared library is loaded.

[solardiz]

This kind of options are normally applied by the distro, not by flags coming from the package itself, but for a rather system-specific package like this I suppose we may.

On Owl, our patched gcc had -Wl,-z,relro -Wl,-z,now implied by default. I am not familiar with -Wl,-z,defs, where and why exactly is it recommended for hardening?

Is the name HARDENING_FLAGS used anywhere else or would it be new with this package? I guess the idea is to have those hardening flags added even if the package overrides CFLAGS, unless the package also knows to override HARDENING_FLAGS in whatever way it may know it wants?

[besser82]

I am not familiar with -Wl,-z,defs, where and why exactly is it recommended for hardening?

That flag ensures that there are no undefined symbols left during link-time, and thus should be used toghether with -Wl,z,now. If there are undefined symbols, RELRO will have no effect to those. It's not a required flag, but supportive.

Is the name HARDENING_FLAGS used anywhere else or would it be new with this package? I guess the idea is to have those hardening flags added even if the package overrides CFLAGS, unless the package also knows to override HARDENING_FLAGS in whatever way it may know it wants?

Yes, the idea is to have them applied even if CFLAGS and/or LDFLAGS are overridden. One can disable them selectively though, by passing the opposite flags from within LDFLAGS.

[ldv-alt]

Given that all of HARDENING_FLAGS are linker flags, shouldn't the variable be called HARDENING_LDFLAGS instead?

[solardiz]

Given that all of HARDENING_FLAGS are linker flags, shouldn't the variable be called HARDENING_LDFLAGS instead?

I think the intent is to use it for compiler flags as well, so no... or would we prefer two variables?

[ldv-alt]

Given that all of HARDENING_FLAGS are linker flags, shouldn't the variable be called HARDENING_LDFLAGS instead?

I think the intent is to use it for compiler flags as well, so no... or would we prefer two variables?

The compiler flags should be passed to the linker as well (because of LTO), but the linker-only flags don't have to be passed to the compiler (they would be ignored).

If the idea is to have a single variable for both, then the patch is good as it is.

[besser82]

Rebased onto main, and updated to not required an additional variable.

[solardiz]

Prepend hardening flags to either CFLAGS or LDFLAGS.

So now the idea is to have those hardening flags where appropriate in all builds, and if a build doesn't want them it has to pass the corresponding no flags in CFLAGS or/and LDFLAGS as appropriate. I'm not sure I like this in general (no way to override all hardening flags without knowing what they were), but it may be OK for this specialized package.

@ldv-alt What do you think?

[besser82]

Just to give abit more of context: Those flags are supported since gcc v4.0, and define the absolute minimum of portable hardening; the same flags are the ones applied to packages like shadow-utils and alike by default since RHEL 5 or so.

[solardiz]

Those flags are supported since gcc v4.0

That's not quite true. gcc 4.6.3 that we had in Owl (until we've essentially stopped maintaining it) recognizes -fstack-protector and -fstack-protector-all, but not -fstack-protector-strong.

Also, we're running into some issue on linking, but maybe that's a gcc bug or our gcc packaging bug?

$ CFLAGS='-O2 -fstack-protector-all -D_FORTIFY_SOURCE=2' LDFLAGS='-Wl,-z,defs -Wl,-z,relro -Wl,-z,now' make
make -C misc all
make[1]: Entering directory `/space/home/git/tcb/misc'
sed -e "s!@PREFIX@!/usr!g" -e "s!@SLIBDIR@!/lib!g" \
		-e "s!@INCLUDEDIR@!/usr/include!g" -e "s!@VERSION@!!g" \
		< tcb.pc.in > tcb.pc
sed -e "s!@LIBEXECDIR@!/usr/libexec!g" < tcb.sysusers.in > tcb.sysusers
make[1]: Leaving directory `/space/home/git/tcb/misc'
make -C libs all
make[1]: Entering directory `/space/home/git/tcb/libs'
gcc -O2 -fstack-protector-all -D_FORTIFY_SOURCE=2  -I../include -Wall -Wextra  -I../include -Wall -Wextra -fPIC -c libtcb.c -o libtcb.o
gcc -O2 -fstack-protector-all -D_FORTIFY_SOURCE=2  -I../include -Wall -Wextra  -I../include -Wall -Wextra -Wl,-z,defs -Wl,-z,relro -Wl,-z,now  -L../libs  -L../libs -shared -o libtcb.so.0.9.8 -Wl,-soname,libtcb.so.0 \
		-Wl,--version-script=libtcb.map libtcb.o
/usr/lib/gcc/i686-openwall-linux/4.6.3/../../../libssp.a(ssp.o): In function `__stack_chk_fail_local':
(.text+0x170): multiple definition of `__stack_chk_fail_local'
/usr/lib/gcc/i686-openwall-linux/4.6.3/../../../libssp_nonshared.a(libssp_nonshared_la-ssp-local.o):(.text+0x0): first defined here
collect2: ld returned 1 exit status

[solardiz]

I think I'll merge this, and if we want to use a legacy Owl system for testing of tcb after this change then we'll be able to override the flags for such build.

[ldv-alt]

Hmm, this -D_FORTIFY_SOURCE=2 might be problematic for ALT because there the default is -D_FORTIFY_SOURCE=3.

[solardiz]

I think it should be OK since per my reading of the changes here HARDENING_CFLAGS is prepended before the passed CFLAGS. Unless you have that as compiler default, not as part of default RPM optflags?

[ldv-alt]

Apparently, it's a compiler default, so now I see the following warnings:

<command-line>: warning: "_FORTIFY_SOURCE" redefined
<built-in>: note: this is the location of the previous definition

"info gcc" on that system displays the following note:

NOTE: In ALT 12.2.1-alt2 and later versions, '-D_FORTIFY_SOURCE=3' is set by default, and is activated when '-O' is set to 1 or higher.

[solardiz]

We'd probably avoid this specific issue by upgrading to =3, but overall it's not good we may override a potentially better default.