Created attachment 57585 [details] [review]
Respect SUID_CFLAGS and SUID_LDFLAGS for dbus-daemon-launch-helper

Is there any reason not to apply PIE and similar "hardening" options to the entire package? That's what we do in Debian.

dbus-daemon is also a privilege boundary (it's not suid, but system services trust it to not give them misleading information), so it makes sense to harden that too. Part of the code in dbus-daemon and dbus-daemon-launch-helper is a statically compiled version of libdbus (the dbus/ directory), so if you're going to apply special compiler options to the executables, you should also apply it to libdbus... and if you're going to do that, all that'll be left non-PIE are some minor tools which aren't in a critical path for performance (dbus-send, dbus-monitor, dbus-launch).

(In reply to comment #2)
> Is there any reason not to apply PIE and similar "hardening" options to the
> entire package? That's what we do in Debian.

My understanding is that we don't want to apply PIE to the entire package because of the performance hit it implies. However...

> dbus-daemon is also a privilege boundary (it's not suid, but system services
> trust it to not give them misleading information), so it makes sense to harden
> that too.

... that's a good point.

> Part of the code in dbus-daemon and dbus-daemon-launch-helper is a
> statically compiled version of libdbus (the dbus/ directory), so if you're
> going to apply special compiler options to the executables, you should also
> apply it to libdbus... and if you're going to do that, all that'll be left
> non-PIE are some minor tools which aren't in a critical path for performance
> (dbus-send, dbus-monitor, dbus-launch).

Hrm. We build everything else with PIC; but that might not result in what we want if there's a static library involved, I'm not sure.

If your recommendation is to simply harden the whole build, then we can do that.

(In reply to comment #3)
> (In reply to comment #2)
> > Part of the code in dbus-daemon and dbus-daemon-launch-helper is a
> > statically compiled version of libdbus (the dbus/ directory)
> 
> Hrm. We build everything else with PIC; but that might not result in what we
> want if there's a static library involved, I'm not sure.

It's actually a libtool convenience library, which I believe means it's built twice (PIC and non-PIC), and libtool transparently chooses which one to use when it gets linked into something - in this case, the PIC version for executables compiled as PIE, and the non-PIC version for dbus-send or whatever. If it was linked into a shared library, that'd also use the PIC version.

dbus-send and friends are pretty insignificant compared with dbus-daemon, and the shared libdbus used by applications is going to be PIC anyway, so I think everything-PIE is the way forward for distributions that use PIE as a "hardening" feature.

dbus used to have specific support for building dbus-daemon as PIE (indeed, it wasn't optional!), but I got rid of that because dbus isn't really enough of a special unique snowflake to justify having special requirements from the platform's toolchain, and PIE support seems to be broken on enough niche platforms that "PIE must work" counts as a special requirement. We briefly had it as an optional feature, but that was more complexity than we really wanted, for very little benefit. Distributions are much better-placed to know how well PIE and similar things work than upstreams are - for instance, Debian's hardening-wrapper package disables some hardening features on some of the more obscure CPU architectures, to work around broken toolchain support for them.

> If your recommendation is to simply harden the whole build, then we can do
> that.

Yes, I think it is, unless you have good reasons not to. Resolving as NOTABUG.

My recommended way to build with PIE is mentioned in NEWS, for what it's worth.