Bug 501228 - Review Request: mod_selinux - An apache module to launch web applications with restrictive privileges
Summary: Review Request: mod_selinux - An apache module to launch web applications wit...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jochen Schmitt
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-05-18 07:16 UTC by KaiGai Kohei
Modified: 2009-05-28 08:16 UTC (History)
5 users (show)

Fixed In Version: 2.2.1938-1.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-05-28 08:16:12 UTC
Type: ---
Embargoed:
jochen: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description KaiGai Kohei 2009-05-18 07:16:13 UTC
Spec URL: http://sepgsql.googlecode.com/files/mod_selinux.spec_v2.2.1903
SRPM URL: http://sepgsql.googlecode.com/files/mod_selinux-2.2.1903-1.fc11.src.rpm

Description:
The Apache/SELinux plus is an extra module (mod_selinux.so) which enables
to launch contents-handler (it means both of references to static contents
and invocations of web applications) with individual and restrictive
privileges set, based on http authentication.
The mod_selinux.so generates a one-time worker thread for each request,
and it assigns the worker restrictive domain based on the authentication
prior to launching contents handlers.
It means we can apply valid access controls on web-applications, and
makes assurance operating system can prevent violated accesses, even if
web application contains security bugs or vulnerabilities.

Comment 1 KaiGai Kohei 2009-05-18 08:03:22 UTC
The rpmlint says as follows:

[kaigai@masu ~]$ rpmlint /usr/src/redhat/RPMS/i586/mod_selinux-2.2.1903-1.fc11.i586.rpm
mod_selinux.i586: E: explicit-lib-dependency libselinux
1 packages and 0 specfiles checked; 1 errors, 0 warnings.

The mod_selinux requires libselinux but I didn't note an explicit earliest version number because it is now unclear when getcon_raw()/setcon_raw() is included into libselinux package.
(At least, it was already merged in the period of Fedora *Core*.)

Is it allowed to restrict it on somewhere enough new version (e.g libselinux >= 2.0.0)?

Comment 2 Jochen Schmitt 2009-05-18 16:44:36 UTC
Good:
+ Basename of the SPEC file matches with package name.
+ Package name fullfill naming guidelines
+ URL tag show on proper project home page.
+ Could download upstream tar ball via spectool -g
+ Package contains valid License tag
+ License tag state ASL 2.0 as a valid OSS license
+ Package contains verbatin copy of the license tag
+ License in the source file header matches with license tag
+ Package tar ball matches with upstream
(md5sum: 855b8b05fd71b39277f2ffbe4c7ae376)
+ Rpmlint is quiete on source rpm
+ Package contains smp-enabled build step
+ Package contains no subpackages
+ Package has proper defintion of Buildroot
+ Buildroot will be cleaned on the start of %clean and %install
+ %doc stanza is small, so we need no extra doc subpackage
+ %files stanza have proper %defattr statemend
+ %files standza haven't duplicated file entries
+ All package files are owned by the package
+ No package files belong to another package
+ Package has proper %Changelog 

Bad:
- Package fails on koji (pleas see: http://koji.fedoraproject.org/koji/taskinfo?taskID=1361107)
  This happens only for 64-bit architectures
- Package could no build localy on F-10 because of dependencies

Comment 3 KaiGai Kohei 2009-05-19 02:17:56 UTC
Jochen, Thanks for your detailed reviewing.

I uploaded the revised Spec and SRPM at:
 Spec URL: http://sepgsql.googlecode.com/files/mod_selinux.spec_v2.2.1904
 SRPM URL: http://sepgsql.googlecode.com/files/mod_selinux-2.2.1904-1.fc11.src.rpm

(In reply to comment #2)
> - Package fails on koji (pleas see:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=1361107)
>   This happens only for 64-bit architectures

Its Makefile assumed an external file provided by httpd-devel is deployed at
/usr/lib/httpd/build/special.mk, but it was /usr/lib64/httpd/build/special.mk
in x86_64 and ppc64.

So, I updated it as follows:

- top_srcdir=/etc/httpd		
- top_builddir=/usr/lib/httpd		
- include /usr/lib/httpd/build/special.mk
+ top_srcdir=/etc/httpd
+ top_builddir=$(shell $(APXS) -q libdir)/httpd
+ include $(top_builddir)/build/special.mk

The /usr/sbin/apxs (provided by httpd-devel) can return a correct path for the
target environment, so the Makefile new gets being portable.

at x86_64:
  [kaigai@masu ~]$ /usr/sbin/apxs -q libdir
  /usr/lib64

at i386:
  [kaigai@saba ~]$ /usr/sbin/apxs -q libdir
  /usr/lib

> - Package could no build localy on F-10 because of dependencies  

Yes, this package uses a new feature in linux-2.6.28 will be available in F-11.

The mod_selinux switches the security context on the worker thread to handle
a http request prior to invocations of contents handler.
But it was not available at linux-2.6.27 or older.

Fortunately, it was sumarized at SELinux-ML yesterday:
  http://marc.info/?l=selinux&m=124265539924989&w=2

Comment 4 KaiGai Kohei 2009-05-22 04:49:20 UTC
(In reply to comment #1)
> The rpmlint says as follows:
> 
> [kaigai@masu ~]$ rpmlint
> /usr/src/redhat/RPMS/i586/mod_selinux-2.2.1903-1.fc11.i586.rpm
> mod_selinux.i586: E: explicit-lib-dependency libselinux
> 1 packages and 0 specfiles checked; 1 errors, 0 warnings.
> 
> The mod_selinux requires libselinux but I didn't note an explicit earliest
> version number because it is now unclear when getcon_raw()/setcon_raw() is
> included into libselinux package.
> (At least, it was already merged in the period of Fedora *Core*.)
> 
> Is it allowed to restrict it on somewhere enough new version
> (e.g libselinux >= 2.0.0)?  

http://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires

Hmm, it says as follows:
| Packages must not contain explicit Requires on libraries except
| when absolutely necessary. When explicit library Requires are necessary,
| there should be a spec file comment justifying it.

I fixed the mod_selinux.spec to remove explicit dependency to libselinux
(without specific version number), as follows:

Spec: http://sepgsql.googlecode.com/files/mod_selinux.spec_v2.2.1930
SRPM: http://sepgsql.googlecode.com/files/mod_selinux-2.2.1930-1.fc11.src.rpm

Thanks,

Comment 5 Jochen Schmitt 2009-05-24 19:42:33 UTC
Good:
+ Could download package via spectool -g
+ Packaged sources matches with upstream
(md5sum: aadee8b6e5c7d99a6ff0a66fca8032dd)
+ Scratch build on koni works fine.
+ No complaints from rpmlint for source rpm
+ No complaints from rpmlint for binary rpm
+ No complaints from rpmlint for debuginfo rpm
+ Debuginfo package contains sources

I will APPROVE this package, but keep in mind to request only branches for devel and F-11.

Comment 6 KaiGai Kohei 2009-05-25 03:54:11 UTC
Thanks for your reviewing.

New Package CVS Request
=======================
Package Name: mod_selinux
Short Description: Apache/SELinux plus module
Owners: kaigai
Branches: F-11
InitialCC: kaigai.nec.com

Comment 7 Jason Tibbitts 2009-05-26 22:10:36 UTC
CVS done, with the caveat that I did not add an InitialCC because we can only CC FAS accounts (or group accounts), not addresses.

Comment 8 Fedora Update System 2009-05-26 23:45:07 UTC
mod_selinux-2.2.1930-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/mod_selinux-2.2.1930-1.fc11

Comment 9 KaiGai Kohei 2009-05-27 00:06:05 UTC
(In reply to comment #7)
> CVS done, with the caveat that I did not add an InitialCC because we can only
> CC FAS accounts (or group accounts), not addresses.  

Sorry, I misunderstood it.

Comment 10 Fedora Update System 2009-05-27 02:17:50 UTC
mod_selinux-2.2.1938-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/mod_selinux-2.2.1938-1.fc11

Comment 11 Fedora Update System 2009-05-28 08:16:06 UTC
mod_selinux-2.2.1938-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.


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