Bug 958014 - mongodb package should be built with PIE flags
Summary: mongodb package should be built with PIE flags
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: mongodb
Version: 22
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Marek Skalický
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-04-30 06:58 UTC by Dhiru Kholia
Modified: 2015-02-26 09:15 UTC (History)
10 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-02-26 08:56:19 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Dhiru Kholia 2013-04-30 06:58:43 UTC
Description of problem:

http://fedoraproject.org/wiki/Packaging:Guidelines#PIE says that "you MUST
enable the PIE compiler flags if your package is long running ...".

However, currently mongodb is not being built with PIE flags. This is a
clear violation of the packaging guidelines.

This issue (in its wider scope) is being discussed at,

https://fedorahosted.org/fesco/ticket/1104

https://lists.fedoraproject.org/pipermail/devel/2013-March/180827.html

Version-Release number of selected component (if applicable):

mongodb-server-2.2.3-4.fc19.x86_64.rpm

How reproducible:

You can use following programs to check if a package is hardened:

http://people.redhat.com/sgrubb/files/rpm-chksec

OR

https://github.com/kholia/checksec

Steps to Reproduce:

Get scanner.py from https://github.com/kholia/checksec

$ ./scanner.py mongodb-server-2.2.3-4.fc19.x86_64.rpm
mongodb-server,mongodb-server-2.2.3-4.fc19.x86_64.rpm,/usr/bin/mongod,NX=Enabled,CANARY=Disabled,RELRO=Partial,PIE=Disabled,RPATH=Disabled,RUNPATH=Disabled,FORTIFY=Disabled,CATEGORY=network-ip
mongodb-server,mongodb-server-2.2.3-4.fc19.x86_64.rpm,/usr/bin/mongos,NX=Enabled,CANARY=Disabled,RELRO=Partial,PIE=Disabled,RPATH=Disabled,RUNPATH=Disabled,FORTIFY=Disabled,CATEGORY=network-ip

Comment 1 Dhiru Kholia 2013-04-30 07:05:59 UTC
For enabling PIE I have uploaded a patch + modified .spec file at https://github.com/kholia/unSPEC/tree/master/mongodb

That link also has a small benchmarking script (stuffer.py).

In my limited testing, I couldn't find any performance difference between 
the PIE and no-PIE builds.

Comment 2 Troy Dawson 2013-04-30 13:27:38 UTC
Thank you for not only providing a patch, but testing that it builds, and benchmarking it.

I'll make sure it gets in ASAP.

Comment 3 Dhiru Kholia 2013-04-30 19:41:04 UTC
My patch may not be the best. I recommend asking in #fedora-devel about this. Thanks!

Comment 4 Troy Dawson 2013-05-02 22:28:31 UTC
Questions about your patch.
I'd really like to not change things other than harden it.

Your LINKFLAG change looks good.

-    env.Append( LINKFLAGS=["-fPIC", "-pthread",  "-rdynamic"] )
+    env.Append( LINKFLAGS=["-fPIC", "-pthread",  "-rdynamic", "-Wl,-z,relro", "-specs=/usr/lib/rpm/redhat/redhat-hardened-ld"])

But it looks like you made lots of extra changes to the CCFLAGS

-        env.Append( CCFLAGS=["-O3"] )
+        env.Append( CCFLAGS=["-O3", "-Wp,-D_FORTIFY_SOURCE=2", "-fexceptions", "-fstack-protector", "--param=ssp-buffer-size=4", "-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1", "-m64" ,"-mtune=generic"] )

If we trimmed those down to just the stuff needed to harden mongodb what would it be?

        env.Append( CCFLAGS=["-O3", "-Wp,-D_FORTIFY_SOURCE=2", "-fstack-protector", "-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1"] )

Does that look right?

Comment 5 Dhiru Kholia 2013-05-03 05:25:09 UTC
My patch is a hack. Ideally, we should not be using hard coded flags.

MongoDB's build system doesn't use the existing passed (exported) flags and it overrides them. This should be fixed (upstream?).

Also,

$ rpm --define '_hardened_build 1' -E '%{?__global_cflags}'                                                          
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1

(These flags can change in the future).

Is it possible to modify MongoDB's build environment so that it uses existing exported flags? Something like?

-        env.Append( CCFLAGS=["-O3"] )
+        env.Append( CCFLAGS=env.get("CFLAGS"))

This assumes that CFLAGS was correctly set earlier.

Comment 6 Troy Dawson 2013-05-03 13:47:45 UTC
You are changing the subject and didn't answer my question.
According to the guidelines[1] we just need to put "%global _hardened_build 1" at the top.
Obviously that doesn't work because that expects a normal build environment.  So the next thing to do is add -fPIC to the compiler flags and -z to the linker flags.

To the compiler flags you added
"-Wp,-D_FORTIFY_SOURCE=2", "-fexceptions", "-fstack-protector", "--param=ssp-buffer-size=4", "-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1", "-m64" ,"-mtune=generic"

To the linker flags you added
"-Wl,-z,relro", "-specs=/usr/lib/rpm/redhat/redhat-hardened-ld"

Regardless of how we get them to the compiler, how much of that extra stuff is for hardening, and how much of it is to add options the way you like it?

[1] - http://fedoraproject.org/wiki/Packaging:Guidelines#PIE

Comment 7 Dhiru Kholia 2013-05-04 09:03:13 UTC
>> "...how much of it is to add options the way you like it?"

I don't think I have added something extra.

See the output of the following commands,

✗ rpm --define '_hardened_build 1' -E '%{?__global_cflags}'
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1

✗ rpm --define '_hardened_build 1' -E '%{?__global_ldflags}'
-Wl,-z,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld

All I am saying is that instead of hard-coding the output of above commands *selectively* into the build system, just pass the required flags from the .SPEC to the build system dynamically.

...

I am not a packager / maintainer. So, I would recommend asking about this on the mailing list or / add #fedora-devel.

Comment 8 Dhiru Kholia 2013-05-04 09:08:21 UTC
Sometime in the near future, "-fstack-protector" might be replaced by "-fstack-protector-strong". We don't want to keep modifying the build system.


So, I won't recommend selectively picking a sub-set of hardening flags and hard coding them into the build system.

Comment 9 Troy Dawson 2013-05-04 12:47:01 UTC
OK, the little light went on in my head, I see what you are saying.  That does make sense.

The only thing I'm worried about is that for 2.2.x that -O3 has been hard coded in, and if we change it, that might change things.  It might even make things faster for all I know, but what I don't know is why upstream has it that way.

How about this.
For 2.2.x (currently 2.2.4) we do the patch/hack, where we hardcode it.

For 2.4.x (which I am currently working on) we do the new/correct way.  And put in a bug with upstream.  They haven't been the most responsive to bugs from us, but maybe they'll say why they chose -O3.
That way any increase/decrease in speed and/or functionality can be attributed to it being the new 2.4.x.

Comment 10 Dhiru Kholia 2013-05-05 12:51:48 UTC
>> "For 2.2.x (currently 2.2.4) we do the patch/hack, where we hard code it."

Sounds like a good enough solution for now. Thanks!

Comment 11 Troy Dawson 2013-06-06 18:47:38 UTC
Sorry for the delay, I got swamped.
It looks like putting your patch in isn't an option.  It might build on x86_64, but it fails on other arches.  So I implemented your second idea, which is to set the various flag fields.
By using the Append command, I was able to add the various system env variables, without taking away what 10Gen has already set in the Sconfig file.  There is some duplication but it seems to work well.
I am still doing some testing, but the src.rpm is here.

http://tdawson.fedorapeople.org/mongodb/mongodb-2.2.4-3.fc18.src.rpm

Comment 12 Dhiru Kholia 2013-06-07 08:25:30 UTC
Thanks for the update Troy. 

I am happy to hear that you have found a better way to fix the problem.

It would be great if you could independently benchmark the PIE build against the non-PIE build.

Comment 13 Troy Dawson 2013-06-07 14:18:35 UTC
Just to let you know, my version failed my harden tests.  While your version (on x86_64) passed them.  I am reworking my fix.

Comment 14 Troy Dawson 2013-06-07 21:39:31 UTC
http://tdawson.fedorapeople.org/mongodb/mongodb-2.2.4-4.fc18.src.rpm

OK, this time I properly fixed the SConstruct file.  Giving it two options that we can now pass it for ccflags and linkflags.  This passed the hardening tests.  I haven't done any time tests.
It's building on everything but EPEL6.  For some reason it is not longer able to see snappy during the build on EPEL6.  I need to make sure that's due to a possible snappy change and not due to my changes.

Anyway, we're very close.

Comment 16 Björn 'besser82' Esser 2014-12-05 15:30:28 UTC
Any new progress on this?  I've just checked the build log of the most recent rawhide-build of mongodb and saw whether the system-flags nor fully hardening are properly applied during the build-process…

Comment 17 Troy Dawson 2014-12-05 15:44:38 UTC
Sorry, it looks like I didn't comment when I was done testing.

It appears that hardening only worked for x86_64.  From my investigation at the time, it appears that hardening doesn't work on ARM at all.  (Meaning it doesn't matter what the package is)

Also from my investigation at the time it is my recommendation that mongodb not be hardend.  The process is too invasive and caused more bugs than I felt comfortable making.

Comment 18 Marek Skalický 2015-01-12 14:32:30 UTC
I have done scratch-build for rawhide https://koji.fedoraproject.org/koji/taskinfo?taskID=8568190. Please test...

For x86 and amd64 it works quite well (there  run tests which completed successfully ). What does not work on ARM? What to test?

Comment 19 Marek Skalický 2015-02-26 08:56:19 UTC
MongoDB version 2.6.7-5 enables hardened build.
https://koji.fedoraproject.org/koji/buildinfo?buildID=614283

Please test. And feel free to reopen...


Note You need to log in before you can comment on or make changes to this bug.