Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No easy way to enable full RELRO on security sensitive binaries #1140

Open
hughsie opened this issue Dec 5, 2016 · 17 comments
Open

No easy way to enable full RELRO on security sensitive binaries #1140

hughsie opened this issue Dec 5, 2016 · 17 comments

Comments

@hughsie
Copy link
Contributor

hughsie commented Dec 5, 2016

It seems meson defaults to "partial RELRO" + PIE for executables which seems a good default. For security sensitive things we need to have "full RELRO" which is more secure, but does have a small runtime cost. Full RELRO is usually set for daemons running as root or things that are taking untrusted binary things from the Internet. More info here: http://tk-blog.blogspot.co.uk/2009/02/relro-not-so-well-known-memory.html

I know you can do this for all exectuables using something like https://github.com/GNOME/sysprof/blob/52bc856be44e52480eb91dca922abf4444e30ac4/meson.build#L62-L65 but it makes sense to me to have some kind of executable() argument to specify the security level. Either something as low level as relro : "full" or something high level like hardening: "full" would be fine, the latter could also enable a different set of stack protection options too.

Thanks again.

@hughsie
Copy link
Contributor Author

hughsie commented Dec 5, 2016

As @TingPing pointed out, the penalty for "full RELRO" isn't huge for most binaries so perhaps we could just change the project defaults allowing projects that really need that fast startup time over the (pretty large) security benefits could opt-out.

@nirbheek
Copy link
Member

nirbheek commented Dec 5, 2016

The only question I have is whether there are equivalents for other platforms: OS X and Windows that we should enable when that option is turned on. Otherwise it'll be misleading for people if it's a no-op on other platforms. If there's no equivalents, we can give it a linux-specific name that makes it clear that it is only available on Linux.

@TingPing
Copy link
Member

TingPing commented Dec 5, 2016

For stack protector (wikipedia):

The compiler suite from Microsoft implements buffer overflow protection since version 2003 through the /GS command-line switch, which is enabled by default since version 2005.[30] Using /GS- disables the protection.

@jpakkane
Copy link
Member

Otherwise it'll be misleading for people if it's a no-op on other platforms. If there's no equivalents, we can give it a linux-specific name that makes it clear that it is only available on Linux.

We already do this. For example lundef is not available on OSX so we don't add that to the list of options when compiling to it. Similarly asan is only available on GCC + Clang, not MSVC, etc.

@nirbheek
Copy link
Member

Ah, it makes sense as a build argument true. Perhaps we can even default to it. My comment was only for kwargs.

@nirbheek nirbheek added the gnome label Feb 6, 2017
@nirbheek nirbheek added this to the 0.39.0 milestone Feb 6, 2017
@nirbheek nirbheek modified the milestones: 0.39.0, 0.40.0 Mar 9, 2017
@nirbheek nirbheek modified the milestone: 0.40.0 Apr 22, 2017
@hughsie
Copy link
Contributor Author

hughsie commented Aug 29, 2018

This has caused problems in RHEL where applications porting from automake to meson are not being built with full RELRO, which causes them to fail the automated security tests Red Hat does. I guess one workaround is to copy around the fwupd "workaround" https://github.com/hughsie/fwupd/blob/master/meson.build#L108 to all projects although this is kind of nasty. I'm not sure I understand mesons codebase enough to attempt something like executable(hardening: "full") but with enough pointers I could give it a go.

@QuLogic
Copy link
Member

QuLogic commented Aug 29, 2018

RedHat packages? They should be setting buildtype=plain and then grabbing the relevant flags from CC, etc., like Fedora does. This is automatic without having to mess with meson.build.

@nirbheek
Copy link
Member

buildtype=plain is indeed the solution for now. We can add support for this to Meson, but it should be as a builtin-option not a kwarg on a target. It can be alongside the new optimization builtin-option.

@QuLogic
Copy link
Member

QuLogic commented Aug 29, 2018

Well, what I mean is, if we're trying to satisfy packaging requirements, they should be satisfied by fixing the package in preference to messing with the meson.build. Whether meson wants to add a flag or not is orthogonal, but enabling full RELRO is achievable with existing settings and is already done for any meson-using package in Fedora, so the same should be possible in RHEL.

@pwnorbitals

This comment has been minimized.

@hughsie
Copy link
Contributor Author

hughsie commented Oct 3, 2020

and then grabbing the relevant flags from CC

Well, that works when building packages in something that's setting the CC flags, but for a developer on a workstation they're going to be blank... which is why I thought we should provide some kind of toggle to enable/disable it.

@dcbaker
Copy link
Member

dcbaker commented Oct 4, 2020

I think we should have a toggle for this. Different compilers have different options for this, and having a generic way to turn this kind of stuff on from the command line is very mesonic.

@atzlinux
Copy link

atzlinux commented Nov 25, 2021

Debian blhc Failed:
https://salsa.debian.org/chinese-team/lunar-date/-/jobs/2215458

There is "LDFLAGS missing (-Wl,-z,now)" error.

Is it cause by this issue?

The build log warnning info is:
https://salsa.debian.org/chinese-team/lunar-date/-/jobs/2215449#L1467
Please see Line 1467.

@nirbheek
Copy link
Member

Is it cause by this issue?

It seems to be caused by an assertion failure in the test of the project you are building:

Bail out! ERROR:../tests/testing.c:226:test_lunar_date: assertion failed (lunar_array[i].raw_value == value): ("\345\233\275\345\272\206\350\212\202" == "")

Seems unrelated to this issue.

@atzlinux
Copy link

O, I'm sorry for my copy and paste error. :-(

I update the link URL, please see again.

Thanks!

@nirbheek
Copy link
Member

nirbheek commented Nov 25, 2021

WARNING: -Wl,-z,relro looks like a linker argument, but has_argument and other similar methods only support checking compiler arguments. Using them to check linker arguments are never supported, and results are likely to be wrong regardless of the compiler you are using. has_link_argument or other similar method can be used instead.
Compiler for C supports arguments -Wl,-z,relro: YES 
WARNING: -Wl,-z,now looks like a linker argument, but has_argument and other similar methods only support checking compiler arguments. Using them to check linker arguments are never supported, and results are likely to be wrong regardless of the compiler you are using. has_link_argument or other similar method can be used instead.
Compiler for C supports arguments -Wl,-z,now: YES 

This can be fixed with:

--- a/meson.build
+++ b/meson.build
@@ -97,7 +97,7 @@ test_link_args = [
   '-Wl,-z,now',
   ]
 foreach arg: test_link_args
-  if cc.has_argument(arg)
+  if cc.has_link_argument(arg)
     global_link_args += arg
   endif
 endforeach

But it's not causing the build error:

$ blhc --debian --line-numbers --color ${SALSA_CI_BLHC_ARGS} ${WORKING_DIR}/*.build || [ $? -eq 1 ]
152:LDFLAGS missing (-Wl,-z,now): g-ir-scanner: link: x86_64-linux-gnu-gcc -pthread -o /builds/chinese-team/lunar-date/debian/output/source_dir/obj-x86_64-linux-gnu/tmp-introspectboo2g7h5/LunarDate-3.0 -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -ffile-prefix-map=/builds/chinese-team/lunar-date/debian/output/source_dir=. -fstack-protector-strong -Wformat -Werror=format-security /builds/chinese-team/lunar-date/debian/output/source_dir/obj-x86_64-linux-gnu/tmp-introspectboo2g7h5/LunarDate-3.0.o -L. -Wl,-rpath,. -Wl,--no-as-needed -L/builds/chinese-team/lunar-date/debian/output/source_dir/obj-x86_64-linux-gnu/lunar-date -Wl,-rpath,/builds/chinese-team/lunar-date/debian/output/source_dir/obj-x86_64-linux-gnu/lunar-date -L/builds/chinese-team/lunar-date/debian/output/source_dir/obj-x86_64-linux-gnu/lunar-date -Wl,-rpath,/builds/chinese-team/lunar-date/debian/output/source_dir/obj-x86_64-linux-gnu/lunar-date -llunar-date-3.0 -llunar-date-3.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lgirepository-1.0 -lgio-2.0 -lgobject-2.0 -Wl,--export-dynamic -lgmodule-2.0 -pthread -lglib-2.0 -Wl,-z,relro
Cleaning up file based variables 00:01
ERROR: Job failed: exit code 1

Basically, the only way link args can be passed to g-ir-scanner is by setting the LDFLAGS env var. You might have that set in your CI script, and it doesn't contain -Wl,-z,now.

And yes, this issue will be fixed once we have a meson-wide option to set relro.

@atzlinux
Copy link

This can be fixed with:

--- a/meson.build
+++ b/meson.build
@@ -97,7 +97,7 @@ test_link_args = [
   '-Wl,-z,now',
   ]
 foreach arg: test_link_args
-  if cc.has_argument(arg)
+  if cc.has_link_argument(arg)
     global_link_args += arg
   endif
 endforeach

add this patch in
https://salsa.debian.org/chinese-team/lunar-date/-/blob/4d9a787939e796fc5c8a35161bac135a6506bc23/debian/patches/meson.build.patch

$ blhc --debian --line-numbers --color ${SALSA_CI_BLHC_ARGS} ${WORKING_DIR}/*.build || [ $? -eq 1 ]
152:LDFLAGS missing (-Wl,-z,now): g-ir-scanner: link: x86_64-linux-gnu-gcc -pthread -o /builds
Basically, the only way link args can be passed to g-ir-scanner is by setting the `LDFLAGS` env var. You might have that set in your CI script, and it doesn't contain `-Wl,-z,now`.

add it in debian/rules:

export DEB_LDFLAGS_MAINT_APPEND = -Wl,-z,now

please see:
https://salsa.debian.org/chinese-team/lunar-date/-/blob/4d9a787939e796fc5c8a35161bac135a6506bc23/debian/rules#L4

The blhc test is pass now.
https://salsa.debian.org/chinese-team/lunar-date/-/jobs/2217180

Thanks you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants