Bug 965482

Summary: slock package should be built with PIE flags
Product: [Fedora] Fedora Reporter: Dhiru Kholia <dkholia>
Component: slockAssignee: Petr Šabata <psabata>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 17CC: bressers, psabata
Target Milestone: ---Keywords: Reopened
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: slock-1.1-3.fc17 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-07-01 12:30:00 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
this should enable PIE
none
this should enable PIE
none
patch to enable PIE
none
patch to enable PIE none

Description Dhiru Kholia 2013-05-21 09:53:46 UTC
Description of problem:

http://fedoraproject.org/wiki/Packaging:Guidelines#PIE says that "you MUST
enable the PIE compiler flags if your package has suid binaries...".

However, currently slock 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):

slock-1.1-2.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 slock-1.1-2.fc19.x86_64.rpm
slock,slock-1.1-2.fc19.x86_64.rpm,/usr/bin/slock,mode=0104755,NX=Enabled,CANARY=Enabled,RELRO=Enabled,PIE=Disabled,RPATH=Disabled,RUNPATH=Disabled,FORTIFY=Enabled,CATEGORY=None

Comment 1 Petr Šabata 2013-05-21 11:05:30 UTC
This also affects older Fedoras.  Switching component version to 17.

Comment 2 Petr Šabata 2013-05-21 11:46:36 UTC
We actually have -fPIC and -z now in the flags, see slock-1.0-config.patch.

The guidelines state the following:
This adds -fPIC (if -fPIE is not already present) to the compiler flags, and adds -z now to the linker flags. 

I suppose the checksec tool should somehow reflect that.

Comment 3 Dhiru Kholia 2013-05-22 06:21:20 UTC
Created attachment 751507 [details]
this should enable PIE

Comment 4 Dhiru Kholia 2013-05-22 06:22:12 UTC
Created attachment 751508 [details]
this should enable PIE

Comment 5 Dhiru Kholia 2013-05-22 06:26:47 UTC
Petr,

After using the attached patches, I re-built slock and got the following output from checksec-ng.

✗ ./scanner.py ~/rpmbuild/RPMS/x86_64/slock-1.1-2.fc18.x86_64.rpm
/bin/slock,mode=0104755,NX=Enabled,CANARY=Enabled,RELRO=Enabled,PIE=Enabled,RPATH=Disabled,RUNPATH=Disabled,FORTIFY=Enabled,CATEGORY=None

*Notice* how PIE has value "Enabled" now (and this is what it should be for setuid programs).

Please re-open this bug for tracking purposes.

Comment 6 Dhiru Kholia 2013-05-22 06:28:41 UTC
Don't mind the "fc18" part. 

The SRPM file I used was http://dl.fedoraproject.org/pub/fedora/linux/development/19/source/SRPMS/s/slock-1.1-2.fc19.src.rpm

Comment 7 Petr Šabata 2013-05-22 08:45:43 UTC
I'm sorry if I'm missing something but the attachments you've posted are just the spec and the patch present in git (and used in all Fedora branches).

I don't know what you did or didn't change.  All current Fedora builds (f17+) use flags in line with packaging guidelines.  Indeed, the -fPIE flag is not used but -fPIC is.

What do you want to track by having this bug open?  There's no issue here.

Comment 8 Dhiru Kholia 2013-05-22 12:54:24 UTC
Petr,

The attachments are slightly modified (in order to enable PIE). I should have renamed them before uploading.

Comment 9 Dhiru Kholia 2013-05-22 12:55:02 UTC
Created attachment 751719 [details]
patch to enable PIE

Comment 10 Dhiru Kholia 2013-05-22 12:55:32 UTC
Created attachment 751720 [details]
patch to enable PIE

Comment 11 Dhiru Kholia 2013-05-22 12:57:15 UTC
I have attached patches (in addition to attaching the whole modified files).

Let me know if you run into problems and thanks for promptly handling this.

Comment 12 Petr Šabata 2013-05-23 13:31:51 UTC
I see the linker flags expand to something else than the guidelines state.
I still think using -fPIC was sufficient but I've changed the spec and the patch a bit.

pyelftools aren't in Fedora and I don't want to use pip on my system, therefore I can't run your checksec tool to verify the output.  Please, see the koji scratch build [1] and let me know if this is sufficient for you.

[1] http://koji.fedoraproject.org/koji/taskinfo?taskID=5413824

Comment 13 Dhiru Kholia 2013-05-23 23:23:17 UTC
>> I see the linker flags expand to something else than the guidelines state.

You are right. The guidelines need to updated.

>> pyelftools aren't in Fedora and I don't want to use pip on my system...

I use virtualenv all the time for developing and testing stuff. 

At some point, I am planning to integrate my checksec stuff with Koji.

>>  Please, see the koji scratch build and let me know if this is sufficient for you.

Yes, the bug has been fixed.

Comment 14 Petr Šabata 2013-05-24 08:16:36 UTC
Thank you for the confirmation.

I'll have to do similar changes in some of my other packages so they respect LDFLAGS properly.  Especially with the upcoming amd64 Fedora PIE changes...

Comment 15 Fedora Update System 2013-05-24 10:41:29 UTC
slock-1.1-3.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/slock-1.1-3.fc19

Comment 16 Fedora Update System 2013-05-24 10:51:41 UTC
slock-1.1-3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/slock-1.1-3.fc18

Comment 17 Fedora Update System 2013-05-24 11:03:58 UTC
slock-1.1-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/slock-1.1-3.fc17

Comment 18 Fedora Update System 2013-06-04 21:05:22 UTC
slock-1.1-3.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 19 Fedora Update System 2013-06-04 21:08:28 UTC
slock-1.1-3.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 20 Fedora Update System 2013-06-05 03:24:09 UTC
slock-1.1-3.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.