-
Notifications
You must be signed in to change notification settings - Fork 219
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
Enable hardening of binaries included in the wheels built for manylinux #59
Comments
I'm not too knowledgeable about hardening features, but this strikes me as a very good idea. |
Seems like a sensible idea in principle, I just have no idea about the
|
It seems to be possible to enable the "Stack protected" and "Read-only relocations" features using the build tools built into the current manylinux image: Here are the results of
And here's how I compiled:
|
Would there be any disadvantages of setting those environment variables globally in the manylinux docker container, that you're aware of? Sent from my iPhone
|
@rmcgibbo I'm not sure. I'd need to investigate and chat to my colleagues about it. |
My 2 cents: As with most security compiler flags this will have some performance cost on the binaries built with these flags. From what I can tell from https://wiki.debian.org/Hardening all of these flags seem like reasonable compromises between performance and security. Other than that, it is possible that adding some of these flags might break compilation of some C files (-Werror=format-security might), but it's probably the case that those C files should be fixed so that static analysis can more easily verify that they don't have security vulnerabilities. Obviously you should make sure the buildchain in manylinux accepts those compiler flags, but you probably would notice if that was a problem in your pre-merge tests or developer tests. Disadvantages:
Advantages:
|
Once thing I'm wondering about is whether Python package build systems are actually consistent about respecting those envvars. (See for example the lots of notes on how to hack different build systems to enable these on the Debian wiki.) The nice thing about the |
One thing to keep in mind is that it will typically be the package authors that will be building manylinux wheels, in which case, they will have complete control over the build. I do think that enabling hardening is the right default. I think most modern distributions enable or are moving towards enabling hardening of various forms. Despite being based on an ancient distro, manylinux is part of the modern ecosystem and so should enable hardening by default. |
Right, but most package authors will probably just go with whatever defaults we set, so we should try to prefer solutions where the defaults are well-chosen and consistently applied :-) |
+1 for enabling hardening environment variables by default along with a tool to easily disable them to make it easy for package maintainers to check whether those variables are the cause of unexpected build errors. For instance we could provide a shell script such as:
|
Yes. But that preference shouldn't stop solutions that are not consistently applied, in the absence of those that are |
* Use hardening for building all tools & libraries This does not affect the wheels that are produced by end users as proposed in #59 but mitigates potential security issues in the tools used by manylinux images as mentioned in #1005 * Always update system packages in the final step Since docker cache is used, system packages are not updated when cache is present. Always update them in the final step.
We package our Python software and all its dependencies as a Debian package for easy installation and distribution.
We run
lintian
on the deb package files and recently it began issuing the following warning:(Our ref https://clusterhq.atlassian.net/browse/FLOC-4383)
We think it's because we recently updated to
pip==8.1.1
which installs manylinux binary wheel files.And the binaries in these wheels are not compiled with the hardening features that are required of binaries in Debian packages:
Perhaps the manylinux build environment could set the necessary environment variables e.g:
Or use http://manpages.ubuntu.com/manpages/wily/man1/hardening-wrapper.1.html
Maybe related to #46
The text was updated successfully, but these errors were encountered: