Ovirt devel - Sep 2008 - [PATCH node] add ovirt semodule in Node

For now, it is only to allow qemu to access disk partitions directly,
required in order to use iSCSI storage pools with SELinux enabled.

Signed-off-by: Alan Pevec <apevec at redhat.com>

Moved from ovirt-node-image repository as it should be in the node
RPM since that RPM is used for creating nodes from stock Fedora
installs and this policy needs to be set there as well.  Added check
for selinuxenabled before making the change.

This is necessary to make clalance's patches for allowing the appliance
to manage the host it is running on as a Node.

FIXME: This patch is going in to fix the problem, but we should be using
http://fedoraproject.org/wiki/PackagingDrafts/SELinux/PolicyModules

Signed-off-by: Perry Myers <pmyers at redhat.com>
---
 ovirt-listen-awake/ovirt-install-node |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/ovirt-listen-awake/ovirt-install-node
b/ovirt-listen-awake/ovirt-install-node
index 84c7b14..68f5b37 100644
--- a/ovirt-listen-awake/ovirt-install-node
+++ b/ovirt-listen-awake/ovirt-install-node
@@ -25,7 +25,7 @@ add_if_not_exist() {
     file="$2"
 
     grep -qE "^[[:space:]]*$string($|#|[[:space:]])"
"$file" \
-	|| echo "$string" >> "$file"
+        || echo "$string" >> "$file"
 }
 
 if [ "$1" = "stateless" ]; then
@@ -70,7 +70,7 @@ elif [ "$1" = "stateful" ]; then
     read yesno
 
     if [ "$yesno" != "y" -a "$yesno" !=
"Y" ]; then
-	exit 2
+        exit 2
     fi
 
     mkdir -p $OVIRT_BACKUP_DIR
@@ -100,3 +100,21 @@ elif [ "$1" = "stateful" ]; then
 else
     usage
 fi
+
+# Common to both stateless and stateful Nodes
+
+if $(selinuxenabled) ; then
+    # make disks available to VMs
+    cat > /tmp/ovirt.te <<EOF
+module ovirt 1.0.0;
+require {
+    type fixed_disk_device_t;
+    type qemu_t;
+    class blk_file { ioctl getattr setattr read write };
+}
+allow qemu_t fixed_disk_device_t:blk_file { ioctl getattr setattr read write };
+EOF
+    checkmodule -M -m -o /tmp/ovirt.mod /tmp/ovirt.te
+    semodule_package -o /tmp/ovirt.pp -m /tmp/ovirt.mod
+    semodule -i /tmp/ovirt.pp
+fi
-- 
1.5.5.1

Perry Myers wrote:
> FIXME: This patch is going in to fix the problem, but we should be using
> http://fedoraproject.org/wiki/PackagingDrafts/SELinux/PolicyModules
yes, +1 for this FIXME

WS-only changes as separate NFS commit
> -	|| echo "$string" >> "$file"
> +        || echo "$string" >> "$file"
> -	exit 2
> +        exit 2
> +
> +# Common to both stateless and stateful Nodes
> +
> +if $(selinuxenabled) ; then
no need to subshell:
if selinuxenabled ; then
> +    # make disks available to VMs
> +    cat > /tmp/ovirt.te <<EOF
as Jim mentioned, <<\EOF to ease sore reviewer eyes :)

ACK otherwise

Perry Myers <pmyers at redhat.com> wrote:
> For now, it is only to allow qemu to access disk partitions directly,
> required in order to use iSCSI storage pools with SELinux enabled.
>
> Signed-off-by: Alan Pevec <apevec at redhat.com>
>
> Moved from ovirt-node-image repository as it should be in the node
> RPM since that RPM is used for creating nodes from stock Fedora
> installs and this policy needs to be set there as well.  Added check
> for selinuxenabled before making the change.
>
> This is necessary to make clalance's patches for allowing the appliance
> to manage the host it is running on as a Node.
>
> FIXME: This patch is going in to fix the problem, but we should be using
> http://fedoraproject.org/wiki/PackagingDrafts/SELinux/PolicyModules
>
> Signed-off-by: Perry Myers <pmyers at redhat.com>
> ---
>  ovirt-listen-awake/ovirt-install-node |   22 ++++++++++++++++++++--
>  1 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/ovirt-listen-awake/ovirt-install-node
b/ovirt-listen-awake/ovirt-install-node
> index 84c7b14..68f5b37 100644
> --- a/ovirt-listen-awake/ovirt-install-node
> +++ b/ovirt-listen-awake/ovirt-install-node
> @@ -25,7 +25,7 @@ add_if_not_exist() {
>      file="$2"
>
>      grep -qE "^[[:space:]]*$string($|#|[[:space:]])"
"$file" \
> -	|| echo "$string" >> "$file"
> +        || echo "$string" >> "$file"
>  }
>
>  if [ "$1" = "stateless" ]; then
> @@ -70,7 +70,7 @@ elif [ "$1" = "stateful" ]; then
>      read yesno
>
>      if [ "$yesno" != "y" -a "$yesno" !=
"Y" ]; then
> -	exit 2
> +        exit 2
>      fi
>
>      mkdir -p $OVIRT_BACKUP_DIR
> @@ -100,3 +100,21 @@ elif [ "$1" = "stateful" ]; then
>  else
>      usage
>  fi
> +
> +# Common to both stateless and stateful Nodes
> +
> +if $(selinuxenabled) ; then
No need for a subshell: you can use this syntax:

   if selinuxenabled ; then
> +    # make disks available to VMs
> +    cat > /tmp/ovirt.te <<EOF
It's good to get into the habit of using quoted here scripts
when nothing in the body needs to be $var or ``-expanded, i.e.,

       cat > /tmp/ovirt.te <<\EOF
> +module ovirt 1.0.0;
> +require {
> +    type fixed_disk_device_t;
> +    type qemu_t;
> +    class blk_file { ioctl getattr setattr read write };
> +}
> +allow qemu_t fixed_disk_device_t:blk_file { ioctl getattr setattr read
write };
> +EOF
> +    checkmodule -M -m -o /tmp/ovirt.mod /tmp/ovirt.te
> +    semodule_package -o /tmp/ovirt.pp -m /tmp/ovirt.mod
> +    semodule -i /tmp/ovirt.pp
> +fi
It probably isn't worth adding 3 uses of mktemp
but factoring out 6 uses of
/tmp/ovirt might win points:

----------------------
        t=/tmp/ovirt
        cat > $t.te <<\EOF
        ...
EOF
    checkmodule -M -m -o $t.mod $t.te
    semodule_package -o $t.pp -m $t.mod
    semodule -i $t.pp
----------------------

At first I was going to say no symlink-in-/tmp attack is possible,
because no one else is around...  But if some bad guy with shell access
to a stateful node knows this script will be running at next boot,
and /tmp will persist (I don't know about that), creating a symlink,
/tmp/ovirt.te, pointing to /etc/passwd would cause trouble.

So using mktemp might be a good idea after all:

        t=$(mktemp -d)
        cat > $t/te <<\EOF
        ...
EOF
    checkmodule -M -m -o $t/mod $t/te
    semodule_package -o $t/pp -m $t/mod
    semodule -i $t/pp