[gentoo-hardened] Update on selinux-policy-2 eclass

[gentoo-hardened] Update on selinux-policy-2 eclass

[Sven Vermeulen]

Hi *

To allow for more manageable patching on our selinux policies (since Matthew
will bombard me anyhow with things to fix ;-) and not to clutter the files/
directory in the Portage tree, I've been meaning to update the
selinux-policy-2 eclass to support reusing the patchbundles offered by the
selinux-base-policy releases. The result can currently be seen at
http://bit.ly/owGhAs but, unless people object, I'll be pushing this eclass
to the hardened-dev overlay tomorrow (in an eclass/ directory) without
changing the eclass name ("bumping") since it doesn't change anything for
existing ebuilds.

The changes made to the eclass are:
- support for the BASEPOL version (reuse of patch bundles)
- (fix) apply patches before copying sources
- add the necessary eclass documentation comments
- define the eclass variables (including POLICY_TYPES)
- support higher-level EAPIs (0 - 4 currently)

Below more information about these changes for those interested.

This change is part of a larger change coming up, namely to update the
SELinux policy packages to 2.20110726. Since I'll make these be EAPI=4 this
eclass update is a prerequisite.

Wkr,
	Sven Vermeulen

Support for the BASEPOL version (reuse of patch bundles)
========================================================

We introduce a new eclass variable called BASEPOL which can be used by an
ebuild to declare that the module depends on a particular
selinux-base-policy as well as needs to be patched with the patches in the
patchbundle (that is available with the selinux-base-policy).

If BASEPOL isn't set, the old behaviour is kept (i.e. not applying the
patchbundle). Also, the POLICY_PATCH variable is still used so no changes
there. The main difference is that, if BASEPOL is used, then the
POLICY_PATCH provided patches need to be relative to this BASEPOL version
(and not the main upstream version).

An example:
	MODS="gpg"
	BASEPOL="2.20110726-r1"

	inherit selinux-policy-2

Previously, we had to do something like this:
	MODS="gpg"
	DEPEND=">=sec-policy/selinux-base-policy-2.20110726-r1"
	POLICY_PATCH="${FILESDIR}/fix-apps-gpg-r1.patch"

	inherit selinux-policy-2

where the fix in POLICY_PATCH was still available in the patchbundle as
well. This led to duplicate patch management efforts and increased the
number of files we had in our various "files/" locations.


(Fix) Apply patches before copying sources
==========================================

In our current selinux-policy-2 eclass, we copy the reference policy sources
to several source directories, labeled after their target policy (targeted,
strict, mcs, mls) after which we applied the (same) patches to each source
directory. This we can of course optimize, so the new eclass patches the
sources before copying them to the respective source directories.

Add the necessary eclass documentation comments
===============================================

Gentoo requires that the eclasses are properly documented using specific
tags in the eclass comments, allowing for automated eclass documentation
generation. An example of such automatically generated document can be found
at http://devmanual.gentoo.org/eclass-reference/mysql.eclass/index.html

For the selinux-policy-2 eclass, no such document exists yet since our
eclass wasn't properly documented. The new eclass contains the proper
documentation tags.


Define the eclass variables (including POLICY_TYPES)
====================================================

Part of the eclass documentation effort is to streamline the variable
declarations. One variable that we currently use is POLICY_TYPES, where we
did many of the following calls:
	[ -z "${POLICY_TYPES} ] && local POLICY_TYPES="strict targeted mls mcs"

By declaring the variables with a default fallback value, all these calls
aren't necessary anymore. 


Support higher-level EAPIs (0 - 4 currently)
============================================

Higher level EAPIs (more than 1 ;-) introduce specific phase functions to
streamline the build process (src_prepare & src_configure). The new eclass
update supports these, but if the EAPI isn't sufficiently high, the old
behaviour is retained (for instance, src_unpack then calls src_prepare
itself).

This also allows for sec-policy/* to be fully EAPI=4 defined, which will be
the case for the 2.20110726 version(s) of the policies.

Re: [gentoo-hardened] Update on selinux-policy-2 eclass

[Peter Volkov]

Hi Sven.

В Втр, 02/08/2011 в 09:19 +0200, Sven Vermeulen пишет:
> To allow for more manageable patching on our selinux policies (since Matthew
> will bombard me anyhow with things to fix ;-) and not to clutter the files/
> directory in the Portage tree, I've been meaning to update the
> selinux-policy-2 eclass to support reusing the patchbundles offered by the
> selinux-base-policy releases. The result can currently be seen at
> http://bit.ly/owGhAs but, unless people object, I'll be pushing this eclass
> to the hardened-dev overlay tomorrow (in an eclass/ directory) without
> changing the eclass name ("bumping") since it doesn't change anything for
> existing ebuilds.

First of all thank you for all this job you are doing. Here are just
general comments on how to clean code a bit:

1. 
: ${BASEPOL:="0"}
later is checked with [[ "${BASEPOL}" == "0" ]];
I guess it's better to make this consistent with : ${POLICY_PATCH:=""}
(and [[ -n ${POLICY_PATCH} ]]).

2. if [[ "${BASEPOL}" == "0" ]];
Here and in many other places, you don't need to quote variables inside
bash checks [[ ]]. (while it's good idea to quote strings).

3. if [[ ${EAPI:-0} -le 1 ]];
EAPI is a string and not a number. Please use 
has "${EAPI:-0}" 0 1 && selinux-policy-2_src_prepare

4. [ -n "${POLICY_PATCH}" ]
generally it's better to use bash tests [[ ]] and avoid quotation.

5. epatch "${POLPATCH}" || die 
drop || die. epatch dies on its own.

6. modfiles="`find ${S}/refpolicy/policy/modules -iname $i.te`
$modfiles"
It's better to use $() instead of backtics ``
http://mywiki.wooledge.org/BashFAQ/082

7. cp "${S}"/refpolicy/doc/Makefile.example ...
add || die for this command and similar commands below.

8. 
selinux-policy-2_src_compile() {
	for i in ${POLICY_TYPES}; do
		make NAME=$i -C "${S}"/${i} || die "${i} compile failed"
Is parallel build unsupported here? May be emake?

9. echo "Installing ${i} ${j} policy package"
It's better to use einfo here as it was implemented exactly for this
cases :)

10.
 			insinto ${BASEDIR}/${i}
			doins "${S}"/${i}/${j}.pp
Until eclass supports only EAPI>=4 "|| die" should be appended here.

With best regards,
--
Peter.

Re: [gentoo-hardened] Update on selinux-policy-2 eclass

[Sven Vermeulen]

On Wed, Aug 03, 2011 at 02:59:14PM +0400, Peter Volkov wrote:
> В Втр, 02/08/2011 в 09:19 +0200, Sven Vermeulen пишет:
> > To allow for more manageable patching on our selinux policies (since Matthew
> > will bombard me anyhow with things to fix ;-) and not to clutter the files/
> > directory in the Portage tree, I've been meaning to update the
> > selinux-policy-2 eclass to support reusing the patchbundles offered by the
> > selinux-base-policy releases. The result can currently be seen at
> > http://bit.ly/owGhAs but, unless people object, I'll be pushing this eclass
> > to the hardened-dev overlay tomorrow (in an eclass/ directory) without
> > changing the eclass name ("bumping") since it doesn't change anything for
> > existing ebuilds.
> 
> First of all thank you for all this job you are doing. Here are just
> general comments on how to clean code a bit:

Thanks for the feedback. I've incorporated most of the changes you
suggested. The eclass is currently in the hardened-dev overlay (you can view
it at http://bit.ly/oJdMVz) with the changes included.

I'd like to reply to a few of your suggestions (to show you I didn't ignore
them and to solicit some reaction or suggestions too ;-)

> 4. [ -n "${POLICY_PATCH}" ]
> generally it's better to use bash tests [[ ]] and avoid quotation.

For POLICY_PATCH, I'll keep the quotation(s) because it can contain multiple
patches (space-separated).

> 8. 
> selinux-policy-2_src_compile() {
> 	for i in ${POLICY_TYPES}; do
> 		make NAME=$i -C "${S}"/${i} || die "${i} compile failed"
> Is parallel build unsupported here? May be emake?

It isn't supported out-of-the-box. I would have to create a Makefile here
(from within the eclass) to allow parallel builds, but I think that would
make it less obvious of what is going on here.

Also, the make operation takes about 2 seconds on a moderate CPU and I
assume that most users set their POLICY_TYPES to the type they use (and not
more), so the gain here is minimal.

Also, emake fails here, it complains about a missing file:

	/bin/sh: tmp/ldap.mod.fc: No such file or directory

whereas with just "make" it works well. The Makefile used is probably the
fault here, but I'm no wizard in these things and since "just" make works,
I'll stick with that ;)

Wkr,
	Sven Vermeulen

Re: [gentoo-hardened] Update on selinux-policy-2 eclass

[Peter Volkov]

В Срд, 03/08/2011 в 15:29 +0200, Sven Vermeulen пишет:
> > 4. [ -n "${POLICY_PATCH}" ]
> > generally it's better to use bash tests [[ ]] and avoid quotation.
> 
> For POLICY_PATCH, I'll keep the quotation(s) because it can contain multiple
> patches (space-separated).

If you use [[ -n ${POLICY_PATCH} ]] then quotation is not needed even in
case it contains multiple values. E.g. here:
if [[ -n "${POLICY_PATCH}" ]];

quotation is not required - bash will understand this correctly.

BTW,
		for POLPATCH in "${POLICY_PATCH}";
		do
			cd "${S}/refpolicy/policy/modules"
			epatch "${POLPATCH}"
		done

It looks like quotation is not necessary around "${POLICY_PATCH}"?
Independently of how many values has "${POLICY_PATCH}" values for cycle
will iterate only once.

Also it looks like it's better use bash array for POLICY_PATCH. This way
you'll allow path to patch to have spaces and still correct iteration.
For example take a look at PATCHES variable in base.eclass. It has code
that allows you to make such changes in eclass backward compatible. But
still it's better to use arrays so probably it's good idea for eclass
just die in case user uses POLICY_PATCH as a variable and not as a bash
array:

POLICY_PATCH=( "${FILESDIR}/mypatch.patch" "${FILESDIR}/patches_folder/"
)

[[ "$(declare -p POLICY_PATCH 2>/dev/null 2>&1)" == "declare -a"* ]] ||
die
for x in "${POLICY_PATCH[@]}"; do
            epatch "${x}"
done

> > 8. 
> > selinux-policy-2_src_compile() {
> > 	for i in ${POLICY_TYPES}; do
> > 		make NAME=$i -C "${S}"/${i} || die "${i} compile failed"
> > Is parallel build unsupported here? May be emake?
> 
> emake fails here

It's good idea to document this within comments above and use emake -j1.

--
Peter.