Open wallrj opened 8 years ago
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 implementation details :-). Specifically how to install something like hardening-wrapper on centos5, and how this will with the old and slightly odd devtoolset toolchain. . Have you tried building Python packages on the manylinux image with hardening enabled? Any luck in getting it to work? On Apr 28, 2016 5:29 PM, "Robert T. McGibbon" notifications@github.com wrote:
I'm not too knowledgeable about hardening features, but this strikes me as a very good idea.
— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/pypa/manylinux/issues/59#issuecomment-215601184
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 hardening-check
performed from an Ubuntu 14.04 container after compilation inside manylinux.
# hardening-check /pwd/build/lib.linux-x86_64-2.7/msgpack/*.so
/pwd/build/lib.linux-x86_64-2.7/msgpack/_packer.so:
Position Independent Executable: no, regular shared library (ignored)
Stack protected: yes
Fortify Source functions: no, only unprotected functions found!
Read-only relocations: yes
Immediate binding: no, not found!
/pwd/build/lib.linux-x86_64-2.7/msgpack/_unpacker.so:
Position Independent Executable: no, regular shared library (ignored)
Stack protected: yes
Fortify Source functions: no, only unprotected functions found!
Read-only relocations: yes
Immediate binding: no, not found!
And here's how I compiled:
[root@e26a34328bc6 msgpack-python]# export CFLAGS="-g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security"
[root@e26a34328bc6 msgpack-python]# export CPPFLAGS="-D_FORTIFY_SOURCE=2"
[root@e26a34328bc6 msgpack-python]# export CXXFLAGS="-g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security"
[root@e26a34328bc6 msgpack-python]# export FFLAGS="-g -O2 -fstack-protector --param=ssp-buffer-size=4"
[root@e26a34328bc6 msgpack-python]# export GCJFLAGS="-g -O2 -fstack-protector --param=ssp-buffer-size=4"
[root@e26a34328bc6 msgpack-python]# export LDFLAGS="-Wl,-Bsymbolic-functions -Wl,-z,relro"
[root@e26a34328bc6 msgpack-python]# /opt/python/cp27-cp27m/bin/python setup.py bdist_wheel
running bdist_wheel
running build
running build_py
running build_ext
cythonize: 'msgpack/_packer.pyx'
building 'msgpack._packer' extension
creating build/temp.linux-x86_64-2.7
creating build/temp.linux-x86_64-2.7/msgpack
gcc -pthread -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -fPIC -D__LITTLE_ENDIAN__=1 -I. -I/opt/python/cp27-cp27m/include/python2.7 -c msgpack/_packer.cpp -o build/temp.linux-x86_64-2.7/msgpack/_packer.o
cc1plus: warning: command line option ‘-Wstrict-prototypes’ is valid for C/ObjC but not for C++ [enabled by default]
g++ -pthread -shared -Wl,-Bsymbolic-functions -Wl,-z,relro -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 build/temp.linux-x86_64-2.7/msgpack/_packer.o -o build/lib.linux-x86_64-2.7/msgpack/_packer.so
cythonize: 'msgpack/_unpacker.pyx'
building 'msgpack._unpacker' extension
gcc -pthread -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -fPIC -D__LITTLE_ENDIAN__=1 -I. -I/opt/python/cp27-cp27m/include/python2.7 -c msgpack/_unpacker.cpp -o build/temp.linux-x86_64-2.7/msgpack/_unpacker.o
cc1plus: warning: command line option ‘-Wstrict-prototypes’ is valid for C/ObjC but not for C++ [enabled by default]
In file included from /opt/python/cp27-cp27m/include/python2.7/Python.h:80:0,
from msgpack/_unpacker.cpp:4:
msgpack/unpack.h: In function ‘int unpack_callback_true(unpack_user*, PyObject**)’:
/opt/python/cp27-cp27m/include/python2.7/object.h:769:22: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
((PyObject*)(op))->ob_refcnt++)
^
msgpack/unpack.h:129:3: note: in expansion of macro ‘Py_INCREF’
{ Py_INCREF(Py_True); *o = Py_True; return 0; }
^
msgpack/unpack.h: In function ‘int unpack_callback_false(unpack_user*, PyObject**)’:
/opt/python/cp27-cp27m/include/python2.7/object.h:769:22: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
((PyObject*)(op))->ob_refcnt++)
^
msgpack/unpack.h:132:3: note: in expansion of macro ‘Py_INCREF’
{ Py_INCREF(Py_False); *o = Py_False; return 0; }
^
In file included from msgpack/_unpacker.cpp:277:0:
msgpack/unpack.h: In function ‘int unpack_callback_ext(unpack_user*, const char*, const char*, unsigned int, PyObject**)’:
msgpack/unpack.h:268:77: warning: deprecated conversion from string constant to ‘char*’ [-Wwrite-strings]
py = PyObject_CallFunction(u->ext_hook, "(is#)", typecode, pos, length-1);
^
In file included from /opt/python/cp27-cp27m/include/python2.7/Python.h:80:0,
from msgpack/_unpacker.cpp:4:
msgpack/_unpacker.pyx: In function ‘PyObject* __pyx_pf_7msgpack_9_unpacker_4unpack(PyObject*, PyObject*, PyObject*, PyObject*, int, PyObject*, PyObject*, PyObject*, PyObject*, Py_ssize_t, Py_ssize_t, Py_ssize_t, Py_ssize_t, Py_ssize_t)’:
/opt/python/cp27-cp27m/include/python2.7/object.h:769:22: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
((PyObject*)(op))->ob_refcnt++)
^
msgpack/_unpacker.cpp:376:28: note: in expansion of macro ‘Py_INCREF’
#define __Pyx_NewRef(obj) (Py_INCREF(obj), obj)
^
msgpack/_unpacker.cpp:378:41: note: in expansion of macro ‘__Pyx_NewRef’
#define __Pyx_PyBool_FromLong(b) ((b) ? __Pyx_NewRef(Py_True) : __Pyx_NewRef(Py_False))
^
msgpack/_unpacker.pyx:164:15: note: in expansion of macro ‘__Pyx_PyBool_FromLong’
return unpackb(stream.read(), use_list=use_list,
^
/opt/python/cp27-cp27m/include/python2.7/object.h:769:22: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
((PyObject*)(op))->ob_refcnt++)
^
msgpack/_unpacker.cpp:376:28: note: in expansion of macro ‘Py_INCREF’
#define __Pyx_NewRef(obj) (Py_INCREF(obj), obj)
^
msgpack/_unpacker.cpp:378:65: note: in expansion of macro ‘__Pyx_NewRef’
#define __Pyx_PyBool_FromLong(b) ((b) ? __Pyx_NewRef(Py_True) : __Pyx_NewRef(Py_False))
^
msgpack/_unpacker.pyx:164:15: note: in expansion of macro ‘__Pyx_PyBool_FromLong’
return unpackb(stream.read(), use_list=use_list,
^
g++ -pthread -shared -Wl,-Bsymbolic-functions -Wl,-z,relro -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 build/temp.linux-x86_64-2.7/msgpack/_unpacker.o -o build/lib.linux-x86_64-2.7/msgpack/_unpacker.so
installing to build/bdist.linux-x86_64/wheel
running install
running install_lib
creating build/bdist.linux-x86_64/wheel
creating build/bdist.linux-x86_64/wheel/msgpack
copying build/lib.linux-x86_64-2.7/msgpack/__init__.py -> build/bdist.linux-x86_64/wheel/msgpack
copying build/lib.linux-x86_64-2.7/msgpack/_version.py -> build/bdist.linux-x86_64/wheel/msgpack
copying build/lib.linux-x86_64-2.7/msgpack/exceptions.py -> build/bdist.linux-x86_64/wheel/msgpack
copying build/lib.linux-x86_64-2.7/msgpack/fallback.py -> build/bdist.linux-x86_64/wheel/msgpack
copying build/lib.linux-x86_64-2.7/msgpack/_packer.so -> build/bdist.linux-x86_64/wheel/msgpack
copying build/lib.linux-x86_64-2.7/msgpack/_unpacker.so -> build/bdist.linux-x86_64/wheel/msgpack
running install_egg_info
running egg_info
writing msgpack_python.egg-info/PKG-INFO
writing top-level names to msgpack_python.egg-info/top_level.txt
writing dependency_links to msgpack_python.egg-info/dependency_links.txt
reading manifest file 'msgpack_python.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no files found matching '*.c' under directory 'msgpack'
writing manifest file 'msgpack_python.egg-info/SOURCES.txt'
Copying msgpack_python.egg-info to build/bdist.linux-x86_64/wheel/msgpack_python-0.4.7-py2.7.egg-info
running install_scripts
creating build/bdist.linux-x86_64/wheel/msgpack_python-0.4.7.dist-info/WHEEL
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
On Apr 29, 2016, at 2:31 PM, Richard Wall notifications@github.com wrote:
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:
hardening-check /pwd/build/lib.linux-x86_64-2.7/msgpack/*.so
/pwd/build/lib.linux-x86_64-2.7/msgpack/_packer.so: Position Independent Executable: no, regular shared library (ignored) Stack protected: yes Fortify Source functions: no, only unprotected functions found! Read-only relocations: yes Immediate binding: no, not found! /pwd/build/lib.linux-x86_64-2.7/msgpack/_unpacker.so: Position Independent Executable: no, regular shared library (ignored) Stack protected: yes Fortify Source functions: no, only unprotected functions found! Read-only relocations: yes Immediate binding: no, not found! And here's how I compiled:
[root@e26a34328bc6 msgpack-python]# export CFLAGS="-g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security" [root@e26a34328bc6 msgpack-python]# export CPPFLAGS="-D_FORTIFY_SOURCE=2" [root@e26a34328bc6 msgpack-python]# export CXXFLAGS="-g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security" [root@e26a34328bc6 msgpack-python]# export FFLAGS="-g -O2 -fstack-protector --param=ssp-buffer-size=4" [root@e26a34328bc6 msgpack-python]# export GCJFLAGS="-g -O2 -fstack-protector --param=ssp-buffer-size=4" [root@e26a34328bc6 msgpack-python]# export LDFLAGS="-Wl,-Bsymbolic-functions -Wl,-z,relro" [root@e26a34328bc6 msgpack-python]# /opt/python/cp27-cp27m/bin/python setup.py bdist_wheel
running bdist_wheel running build running build_py running build_ext cythonize: 'msgpack/_packer.pyx' building 'msgpack._packer' extension creating build/temp.linux-x86_64-2.7 creating build/temp.linux-x86_64-2.7/msgpack gcc -pthread -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -fPIC -DLITTLE_ENDIAN__=1 -I. -I/opt/python/cp27-cp27m/include/python2.7 -c msgpack/_packer.cpp -o build/temp.linux-x86_64-2.7/msgpack/_packer.o cc1plus: warning: command line option ‘-Wstrict-prototypes’ is valid for C/ObjC but not for C++ [enabled by default] g++ -pthread -shared -Wl,-Bsymbolic-functions -Wl,-z,relro -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 build/temp.linux-x86_64-2.7/msgpack/_packer.o -o build/lib.linux-x86_64-2.7/msgpack/_packer.so cythonize: 'msgpack/_unpacker.pyx' building 'msgpack._unpacker' extension gcc -pthread -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -fPIC -DLITTLE_ENDIAN=1 -I. -I/opt/python/cp27-cp27m/include/python2.7 -c msgpack/_unpacker.cpp -o build/temp.linux-x86_64-2.7/msgpack/_unpacker.o cc1plus: warning: command line option ‘-Wstrict-prototypes’ is valid for C/ObjC but not for C++ [enabled by default] In file included from /opt/python/cp27-cp27m/include/python2.7/Python.h:80:0, from msgpack/_unpacker.cpp:4: msgpack/unpack.h: In function ‘int unpack_callback_true(unpackuser, PyObject)’: /opt/python/cp27-cp27m/include/python2.7/object.h:769:22: warning: dereferencing type-punned pointer will break strict-aliasing rules -Wstrict-aliasing->ob_refcnt++) ^ msgpack/unpack.h:129:3: note: in expansion of macro ‘Py_INCREF’ { Py_INCREF(Py_True); *o = Py_True; return 0; } ^ msgpack/unpack.h: In function ‘int unpack_callback_false(unpackuser, PyObject*)’: /opt/python/cp27-cp27m/include/python2.7/object.h:769:22: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing](%28PyObject%29%28op%29)->ob_refcnt++) ^ msgpack/unpack.h:132:3: note: in expansion of macro ‘Py_INCREF’ { Py_INCREF(Py_False); _o = Py_False; return 0; } ^ In file included from msgpack/_unpacker.cpp:277:0: msgpack/unpack.h: In function ‘int unpack_callback_ext(unpackuser, const char, const char, unsigned int, PyObject*)’: msgpack/unpack.h:268:77: warning: deprecated conversion from string constant to ‘char_’ [-Wwrite-strings] py = PyObject_CallFunction(u->ext_hook, "(is#)", typecode, pos, length-1); ^ In file included from /opt/python/cp27-cp27m/include/python2.7/Python.h:80:0, from msgpack/_unpacker.cpp:4: msgpack/unpacker.pyx: In function ‘PyObject __pyx_pf_7msgpack_9_unpacker4unpack(PyObject, PyObject, PyObject, PyObject, int, PyObject, PyObject, PyObject, PyObject_, Py_ssize_t, Py_ssize_t, Py_ssize_t, Py_ssize_t, Py_ssize_t)’: /opt/python/cp27-cp27m/include/python2.7/object.h:769:22: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing](%28PyObject%29%28op%29)->ob_refcnt++) ^ msgpack/_unpacker.cpp:376:28: note: in expansion of macro ‘Py_INCREF’
define __Pyx_NewRef(obj) (Py_INCREF(obj), obj)
^
msgpack/_unpacker.cpp:378:41: note: in expansion of macro ‘__Pyx_NewRef’
define Pyx_PyBool_FromLong(b) ((b) ? Pyx_NewRef(Py_True) : __Pyx_NewRef(Py_False))
^
msgpack/_unpacker.pyx:164:15: note: in expansion of macro ‘__Pyx_PyBool_FromLong’ return unpackb(stream.read(), use_list=use_list, ^ /opt/python/cp27-cp27m/include/python2.7/object.h:769:22: warning: dereferencing type-punned pointer will break strict-aliasing rules -Wstrict-aliasing->ob_refcnt++) ^ msgpack/_unpacker.cpp:376:28: note: in expansion of macro ‘Py_INCREF’
define __Pyx_NewRef(obj) (Py_INCREF(obj), obj)
^
msgpack/_unpacker.cpp:378:65: note: in expansion of macro ‘__Pyx_NewRef’
define Pyx_PyBool_FromLong(b) ((b) ? Pyx_NewRef(Py_True) : __Pyx_NewRef(Py_False))
^
msgpack/_unpacker.pyx:164:15: note: in expansion of macro ‘__Pyx_PyBool_FromLong’ return unpackb(stream.read(), use_list=use_list, ^ g++ -pthread -shared -Wl,-Bsymbolic-functions -Wl,-z,relro -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 build/temp.linux-x86_64-2.7/msgpack/_unpacker.o -o build/lib.linux-x86_64-2.7/msgpack/_unpacker.so installing to build/bdist.linux-x86_64/wheel running install running install_lib creating build/bdist.linux-x86_64/wheel creating build/bdist.linux-x86_64/wheel/msgpack copying build/lib.linux-x86_64-2.7/msgpack/init.py -> build/bdist.linux-x86_64/wheel/msgpack copying build/lib.linux-x86_64-2.7/msgpack/_version.py -> build/bdist.linux-x86_64/wheel/msgpack copying build/lib.linux-x86_64-2.7/msgpack/exceptions.py -> build/bdist.linux-x86_64/wheel/msgpack copying build/lib.linux-x86_64-2.7/msgpack/fallback.py -> build/bdist.linux-x86_64/wheel/msgpack copying build/lib.linux-x86_64-2.7/msgpack/_packer.so -> build/bdist.linux-x86_64/wheel/msgpack copying build/lib.linux-x86_64-2.7/msgpack/_unpacker.so -> build/bdist.linux-x86_64/wheel/msgpack running install_egg_info running egg_info writing msgpack_python.egg-info/PKG-INFO writing top-level names to msgpack_python.egg-info/top_level.txt writing dependency_links to msgpack_python.egg-info/dependency_links.txt reading manifest file 'msgpack_python.egg-info/SOURCES.txt' reading manifest template 'MANIFEST.in' warning: no files found matching '*.c' under directory 'msgpack' writing manifest file 'msgpack_python.egg-info/SOURCES.txt' Copying msgpack_python.egg-info to build/bdist.linux-x86_64/wheel/msgpack_python-0.4.7-py2.7.egg-info running install_scripts creating build/bdist.linux-x86_64/wheel/msgpack_python-0.4.7.dist-info/WHEEL — You are receiving this because you commented. Reply to this email directly or view it on GitHub
Would there be any disadvantages of setting those environment variables globally in the manylinux docker container, that you're aware of?
@rmcgibbo I'm not sure. I'd need to investigate and chat to my colleagues about it. @sarum90, @moshez, @tomprince, @exarkun any thoughts?
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:
-D
flags. Seems like this is probably a remote chance, but technically possible. Similarly, ensure CFLAGS and friends aren't already set to something special in manylinux before changing them.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 hardened-wrapper
approach is that it basically hacks the compiler defaults directly, so it works with any build system.
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:
source /opt/disable_hardening_envvars.sh
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.
Yes. But that preference shouldn't stop solutions that are not consistently applied, in the absence of those that are
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