Open hughsie opened 7 years ago
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.
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.
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.
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.
Ah, it makes sense as a build argument true. Perhaps we can even default to it. My comment was only for kwargs.
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.
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
.
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.
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.
As this issue is old, has a workaround and proably isn’t up-to-date with the latest developments, I suggest we can close it now
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.
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.
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.
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.
O, I'm sorry for my copy and paste error. :-(
I update the link URL, please see again.
Thanks!
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.
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
The blhc test is pass now. https://salsa.debian.org/chinese-team/lunar-date/-/jobs/2217180
Thanks you very much!
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 asrelro : "full"
or something high level likehardening: "full"
would be fine, the latter could also enable a different set of stack protection options too.Thanks again.