Makefile: Pass CFLAGS or RPM %optflags also to the linker

[solardiz]

Perhaps we need the equivalent of openwall/tcb#23 also here. This may be trickier here because our Makefile here is smart to recognize the different systems and pass custom linker flags.

Here's the current checksec output on files coming from the Rocky Linux 9 SIG/Security package:

Partial RELRO   Canary found      NX enabled    No PIE          No RPATH   No RUNPATH   No Symbols	N/A	0		0	/bin/pwqcheck
Partial RELRO   Canary found      NX enabled    No PIE          No RPATH   No RUNPATH   No Symbols	N/A	0		0	/bin/pwqfilter
Partial RELRO   Canary found      NX enabled    No PIE          No RPATH   No RUNPATH   No Symbols	N/A	0		0	/bin/pwqgen
Partial RELRO   Canary found      NX enabled    DSO             No RPATH   No RUNPATH   No Symbols	N/A	0		0	/lib64/libpasswdqc.so.1
Partial RELRO   Canary found      NX enabled    DSO             No RPATH   No RUNPATH   No Symbols	N/A	0		0	/lib64/security/pam_passwdqc.so

For comparison, most other binaries and libraries on this distro get Full RELRO, and most binaries PIE enabled.

Since the above reuses Fedora's packaging almost verbatim, I suspect the same issue is also present on Fedora. We could fix it in the Makefile (this issue) or in Fedora's passwdqc.spec.

[solardiz]

Here's what Fedora's passwdqc.spec does currently:

make %{?_smp_mflags} all locales \
        CPPFLAGS="-DENABLE_NLS=1 -DHAVE_LIBAUDIT=1 -DLINUX_PAM=1" \
        CFLAGS_lib="$RPM_OPT_FLAGS -W -DLINUX_PAM -fPIC" \
        CFLAGS_bin="$RPM_OPT_FLAGS -W" \

We should maybe add:

        LDFLAGS="$RPM_OPT_FLAGS"

as our Makefile appears to correctly add system-specific linker flags on top of what's in simple LDFLAGS.

We also have a passwdqc.spec right in here, coming from Owl, and it does:

%__make \
        CPPFLAGS='-DLINUX_PAM' \
        CFLAGS_bin='-Wall -W %optflags' \
        CFLAGS_lib='-Wall -W -fPIC %optflags_lib'

Should we update it similarly?

Or should we deal with this issue in Makefile, like we just did for the tcb package?

[solardiz]

@ldv-alt Do you have a preference for how we approach this? I think I may start by updating the spec file in Rocky SIG/Security.

[ldv-alt]

Indeed. I think we should rather deal with the issue in Makefile.

[solardiz]

deal with the issue in Makefile.

I don't know exactly how. If we do:

+++ b/Makefile
@@ -75,7 +75,7 @@ XGETTEXT = xgettext
 XGETTEXT_OPTS = --keyword=_ --keyword=P2_:1,1 --keyword=P3_:1,2 --language=C --add-comments
 MSGMERGE = msgmerge
 
-LDFLAGS =
+LDFLAGS = $(CFLAGS)
 LDFLAGS_shared = $(LDFLAGS) --shared
 LDFLAGS_shared_LINUX = $(LDFLAGS) --shared
 LDFLAGS_shared_SUN = $(LDFLAGS) -G

this will probably break some non-Linux builds, where we pass e.g. LD_lib=/usr/ccs/bin/ld on Solaris.

Alternatively, we could do:

+++ b/Makefile
@@ -128,8 +128,9 @@ default: all
 all locales pam utils install install_lib install_locales install_pam install_utils uninstall remove remove_lib remove_locales remove_pam remove_utils:
        case "`uname -s`" in \
        Linux)  $(MAKE) CPPFLAGS_lib="$(CPPFLAGS_lib) -DHAVE_SHADOW" \
-                       LDFLAGS_lib="$(LDFLAGS_lib_LINUX)" \
-                       LDFLAGS_pam="$(LDFLAGS_pam_LINUX)" \
+                       LDFLAGS="$(LDFLAGS) $(CFLAGS)" \
+                       LDFLAGS_lib="$(LDFLAGS_lib_LINUX) $(CFLAGS)" \
+                       LDFLAGS_pam="$(LDFLAGS_pam_LINUX) $(CFLAGS)" \
                        LDLIBS_pam="$(LDLIBS_pam_LINUX)" \
                        $@_wrapped;; \
        SunOS)  $(MAKE) -e CPPFLAGS_lib="$(CPPFLAGS_lib) -DHAVE_SHADOW" \

Also subtly related is issue #6. I feel that what we may be doing here is inconsistent with what we did there (supporting separation into different subsets of flags vs. combining them together even if some are to be ignored in linker invocations).

@ldv-alt What do you think? Are you going to make changes to fix this issue or should I?

[solardiz]

In Rocky Linux SIG/Security, I ended up with these changes for now:

+++ b/SPECS/passwdqc.spec
@@ -1,7 +1,7 @@
 Summary: A password/passphrase strength checking and policy enforcement toolset
 Name: passwdqc
 Version: 2.0.3
-Release: 2%{?dist}
+Release: 2.2%{?dist}
 # Two manual pages (pam_passwdqc.8 and passwdqc.conf.5) are under the
 # 3-clause BSD-style license as specified within the files themselves.
 # The rest of the files in this package fall under the terms of
@@ -99,6 +99,7 @@ make %{?_smp_mflags} all locales \
        CPPFLAGS="-DENABLE_NLS=1 -DHAVE_LIBAUDIT=1 -DLINUX_PAM=1" \
        CFLAGS_lib="$RPM_OPT_FLAGS -W -DLINUX_PAM -fPIC" \
        CFLAGS_bin="$RPM_OPT_FLAGS -W" \
+       LDFLAGS="-pie -Wl,-z,defs -Wl,-z,relro -Wl,-z,now $RPM_OPT_FLAGS" \
        #
 
 %install
@@ -139,6 +140,9 @@ make install install_locales \
 %_mandir/man1/*.1*
 
 %changelog
+* Fri Jan 31 2025 Solar Designer <solar@openwall.com> 2.0.3-2.2
+- Pass -pie -Wl,-z,defs -Wl,-z,relro -Wl,-z,now and RPM_OPT_FLAGS to LDFLAGS
+
 * Thu Jul 20 2023 Fedora Release Engineering <releng@fedoraproject.org> - 2.0.3-2
 - Rebuilt for https://fedoraproject.org/wiki/Fedora_39_Mass_Rebuild
 

Simply passing $RPM_OPT_FLAGS wasn't enough - those hardening flags are not in there on EL9.

[solardiz]

In Rocky Linux SIG/Security, I ended up with these changes for now:

Looks like for correctness with -pie, we also need to add -fPIE to both CFLAGS_bin and LDFLAGS, but somehow on Rocky Linux 8 and 9 this builds fine without that (I guess -fPIE is implied somewhere in Red Hat's flags).

Also, for reference, here's what I just did to lkrg/logger/Makefile: lkrg-org/lkrg@67e6c83

[ldv-alt]

Alternatively, we could do:

+++ b/Makefile
@@ -128,8 +128,9 @@ default: all
 all locales pam utils install install_lib install_locales install_pam install_utils uninstall remove remove_lib remove_locales remove_pam remove_utils:
        case "`uname -s`" in \
        Linux)  $(MAKE) CPPFLAGS_lib="$(CPPFLAGS_lib) -DHAVE_SHADOW" \
-                       LDFLAGS_lib="$(LDFLAGS_lib_LINUX)" \
-                       LDFLAGS_pam="$(LDFLAGS_pam_LINUX)" \
+                       LDFLAGS="$(LDFLAGS) $(CFLAGS)" \
+                       LDFLAGS_lib="$(LDFLAGS_lib_LINUX) $(CFLAGS)" \
+                       LDFLAGS_pam="$(LDFLAGS_pam_LINUX) $(CFLAGS)" \
                        LDLIBS_pam="$(LDLIBS_pam_LINUX)" \
                        $@_wrapped;; \
        SunOS)  $(MAKE) -e CPPFLAGS_lib="$(CPPFLAGS_lib) -DHAVE_SHADOW" \

Also subtly related is issue #6. I feel that what we may be doing here is inconsistent with what we did there (supporting separation into different subsets of flags vs. combining them together even if some are to be ignored in linker invocations).

@ldv-alt What do you think? Are you going to make changes to fix this issue or should I?

I suppose we could do:

--- a/Makefile
+++ b/Makefile
@@ -128,8 +128,9 @@ default: all
 all locales pam utils install install_lib install_locales install_pam install_utils uninstall remove remove_lib remove_locales remove_pam remove_utils:
 	case "`uname -s`" in \
 	Linux)	$(MAKE) CPPFLAGS_lib="$(CPPFLAGS_lib) -DHAVE_SHADOW" \
-			LDFLAGS_lib="$(LDFLAGS_lib_LINUX)" \
-			LDFLAGS_pam="$(LDFLAGS_pam_LINUX)" \
+			LDFLAGS="$(CFLAGS_bin) $(LDFLAGS)" \
+			LDFLAGS_lib="$(CFLAGS_lib) $(LDFLAGS_lib_LINUX)" \
+			LDFLAGS_pam="$(CFLAGS_lib) $(LDFLAGS_pam_LINUX)" \
 			LDLIBS_pam="$(LDLIBS_pam_LINUX)" \
 			$@_wrapped;; \
 	SunOS)	$(MAKE) -e CPPFLAGS_lib="$(CPPFLAGS_lib) -DHAVE_SHADOW" \

[solardiz]

@ldv-alt This looks good to me, but do we want CFLAGS* included before or after LDFLAGS*? I think including them after may be needed to allow the distro to override our flags even if it only passes its custom flags as CFLAGS. Or do we want to override the distro's instead, such as because we know this package's specific linking requirements better?

[solardiz]

I am experimenting with:

+++ b/Makefile
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2000-2003,2005,2009,2010,2020 by Solar Designer
+# Copyright (c) 2000-2003,2005,2009,2010,2020,2026 by Solar Designer
 # Copyright (c) 2008,2009,2017 by Dmitry V. Levin
 # Copyright (c) 2017 by Oleg Solovyov
 # See LICENSE
@@ -125,11 +125,25 @@ OBJS_FILTER = pwqfilter.o md4.o
 
 default: all
 
+# -pie is a linker option, but let's have it in CFLAGS to ensure -fPIE and -pie
+# are ever overridden or not together.  Include CFLAGS into LDFLAGS because:
+# 1. "For predictable results, you must also specify the same set of options
+#    used for compilation (-fpie, -fPIE, or model suboptions) when you specify
+#    this linker option." (as gcc man page says about "-pie")
+# 2. In case the package only overrides CFLAGS, we want the distro's hardening
+#    or other flags to also propagate to the linker.
+# Pass the hardening flags first so that the package CFLAGS may override them.
+CFLAGS_HARDENING_LINUX = -fPIE -pie -fstack-protector-strong -D_FORTIFY_SOURCE=2
+LDFLAGS_HARDENING_LINUX = -s -Wl,-z,defs -Wl,-z,relro -Wl,-z,now
+
 all locales pam utils install install_lib install_locales install_pam install_utils uninstall remove remove_lib remove_locales remove_pam remove_utils:
        case "`uname -s`" in \
        Linux)  $(MAKE) CPPFLAGS_lib="$(CPPFLAGS_lib) -DHAVE_SHADOW" \
-                       LDFLAGS_lib="$(LDFLAGS_lib_LINUX)" \
-                       LDFLAGS_pam="$(LDFLAGS_pam_LINUX)" \
+                       CFLAGS_bin="$(CFLAGS_HARDENING_LINUX) $(CFLAGS_bin)" \
+                       CFLAGS_lib="$(CFLAGS_HARDENING_LINUX) $(CFLAGS_lib)" \
+                       LDFLAGS="$(LDFLAGS_HARDENING_LINUX) $(CFLAGS_HARDENING_LINUX) $(CFLAGS_bin) $(LDFLAGS)" \
+                       LDFLAGS_lib="$(LDFLAGS_HARDENING_LINUX) $(CFLAGS_HARDENING_LINUX) $(CFLAGS_lib) $(LDFLAGS_lib_LINUX)" \
+                       LDFLAGS_pam="$(LDFLAGS_HARDENING_LINUX) $(CFLAGS_HARDENING_LINUX) $(CFLAGS_lib) $(LDFLAGS_pam_LINUX)" \
                        LDLIBS_pam="$(LDLIBS_pam_LINUX)" \
                        $@_wrapped;; \
        SunOS)  $(MAKE) -e CPPFLAGS_lib="$(CPPFLAGS_lib) -DHAVE_SHADOW" \

[solardiz]

This broke our clang CI jobs:

clang: error: argument unused during compilation: '-pie' [-Werror,-Wunused-command-line-argument]

I may try adding -Wno-unused-command-line-argument.