[OSSTEST PATCH RFC v1 00/12] XSM test cases for OSSTest

[OSSTEST PATCH RFC v1 00/12] XSM test cases for OSSTest

[Wei Liu]

Hi all

This patch series attempts to duplicate some Debian smoke test for XSM in Xen.

Tests duplicated:
  build-{i386,amd64,armhf}-xsm
  test-amd64-{i386,amd64}-xl-xsm
  test-armhf-armhf-xl-xsm
  test-amd64-{i386,amd64}-xl-qemuu-debianhvm-amd64-xsm
  test-amd64-(i386,amd64}-xl-qemut-debianhvm-amd64-xsm

There are several loose ends, hence the RFC tag. I will list them one by one.

1. XSM policy naming convention

There's not yet an official naming convention for XSM policy. We need this to
propose changes to upstream GRUB. Daniel suggested one naming convention and it 
can be found at <1410809355-24471-1-git-send-email-wei.liu2@citrix.com>.

2. GRUB doesn't support adding in XSM policy module

This is currently being addressed by our customized patch. I will file a bug
report, with my patch attached to upstream GRUB. Ideally Debian will pick up
the change in some future release, then we can remove that patch from OSSTest
when OSSTest migrates to that release.

3. Uboot changes have not been tested, ARM related tests have not been run

I don't have ARM test infrastructure at hand.

4. In-tree default policy is too strict

For PV guest test case, it can successfully create a guest, but fails at
saving. Xen log says "permission denied".

For QEMU upstream HVM guest, QEMU segfaults with NULL pointer dereference. 

For QEMU traditional HVM guest, guest crashes with triple fault.

I have yet tried to debug HVM test cases. Presumably the failures are combined
effect of the enforced XSM policy and some QEMU bugs. It's likely to take some
time to figure out what went wrong. The bug fix and policy tuning is orthogonal
to the test case itself though.

Wei.

Wei Liu (12):
  README: list chiark-utils-bin as requirement
  gitignore: ignore images directory
  ts-xen-build-prep: install checkpolicy
  ts-xen-build: build with XSM support if requested
  mfi-common: create build-$arch-xsm job
  Debian.pm: pass in XSM configuration to bootloader setup routines
  Debian.pm: load flask policy in uboot
  ts-xen-install: install Xen with XSM support if requested
  mfi-common: use XSM build if job name contains -xsm suffix
  make-flight: create XSM test jobs
  ts-debian-install: add in seclabel if XSM is enabled
  ts-debian-hvm-install: add in seclabel if XSM is enabled

 .gitignore            |    1 +
 Osstest/Debian.pm     |   50 ++++++++++++++++++-----
 README                |    1 +
 make-flight           |   35 ++++++++++++++--
 mfi-common            |   38 ++++++++++++++++++
 ts-debian-hvm-install |   14 +++++--
 ts-debian-install     |   11 ++++-
 ts-xen-build          |    2 +
 ts-xen-build-prep     |    2 +-
 ts-xen-install        |  106 ++++++++++++++++++++++++++++++++++++++++++++++++-
 10 files changed, 238 insertions(+), 22 deletions(-)

-- 
1.7.10.4

[OSSTEST PATCH RFC v1 01/12] README: list chiark-utils-bin as requirement

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 README |    1 +
 1 file changed, 1 insertion(+)

diff --git a/README b/README
index 9a85549..ee1cd7a 100644
--- a/README
+++ b/README
@@ -204,6 +204,7 @@ To run osstest in standalone mode:
      pax rsync
      curl
      netcat
+     chiark-utils-bin
 
  - Optional: ipmitool
 
-- 
1.7.10.4

[OSSTEST PATCH RFC v1 02/12] gitignore: ignore images directory

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 .gitignore |    1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 4f2a839..8dad470 100644
--- a/.gitignore
+++ b/.gitignore
@@ -27,3 +27,4 @@ logs
 id_rsa_osstest
 id_rsa_osstest.pub
 overlay-local
+images
-- 
1.7.10.4

[OSSTEST PATCH RFC v1 03/12] ts-xen-build-prep: install checkpolicy

[Wei Liu]

This is used to complie Flask policy.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 ts-xen-build-prep |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ts-xen-build-prep b/ts-xen-build-prep
index 05a7857..edef9a9 100755
--- a/ts-xen-build-prep
+++ b/ts-xen-build-prep
@@ -178,7 +178,7 @@ sub prep () {
                                autoconf automake libtool xsltproc
                                libxml2-utils libxml2-dev libnl-dev
                                libdevmapper-dev w3c-dtd-xhtml
-			       ccache));
+			       ccache checkpolicy));
 
     target_cmd_root($ho, "chmod -R a+r /usr/share/git-core/templates");
     # workaround for Debian #595728
-- 
1.7.10.4

[OSSTEST PATCH RFC v1 04/12] ts-xen-build: build with XSM support if requested

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 ts-xen-build |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/ts-xen-build b/ts-xen-build
index 661f186..e0f0844 100755
--- a/ts-xen-build
+++ b/ts-xen-build
@@ -34,6 +34,7 @@ sub checkout () {
     build_clone($ho, 'xen', $builddir, 'xen');
 
     my $debug_build = $r{xen_build_debug} || 'y';
+    my $build_xsm = $r{enable_xsm} || 'n';
 
     # Do not set this unless you know what you are doing. This arm
     # option makes the build specific to a particular type of
@@ -47,6 +48,7 @@ sub checkout () {
         cd $builddir/xen
 	>.config
 	echo >>.config debug=$debug_build
+	echo >>.config XSM_ENABLE=$build_xsm
 	echo >>.config GIT_HTTP=y
 	echo >>.config LIBLEAFDIR_x86_64=lib
 	echo >>.config QEMU_REMOTE='$r{tree_qemu}'
-- 
1.7.10.4

[OSSTEST PATCH RFC v1 05/12] mfi-common: create build-$arch-xsm job

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 mfi-common |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/mfi-common b/mfi-common
index afb6239..cf7da4b 100644
--- a/mfi-common
+++ b/mfi-common
@@ -49,6 +49,7 @@ create_build_jobs () {
   local want_xend build_defxend build_extraxend
   local enable_ovmf
   local build_hostflags
+  local test_xsm
 
   for arch in ${BUILD_ARCHES- i386 amd64 armhf }; do
 
@@ -134,6 +135,17 @@ create_build_jobs () {
     *) enable_ovmf=true;
     esac
 
+    # Test XSM from 4.5 onwards
+    case "$xenbranch" in
+    xen-3.*-testing) test_xsm=n;;
+    xen-4.0-testing) test_xsm=n;;
+    xen-4.1-testing) test_xsm=n;;
+    xen-4.2-testing) test_xsm=n;;
+    xen-4.3-testing) test_xsm=n;;
+    xen-4.4-testing) test_xsm=n;;
+    *) test_xsm=y;
+    esac
+
     eval "
         arch_runvars=\"\$ARCH_RUNVARS_$arch\"
     "
@@ -142,6 +154,23 @@ create_build_jobs () {
 
     ./cs-job-create $flight build-$arch build                                \
                 arch=$arch enable_xend=$build_defxend enable_ovmf=$enable_ovmf\
+                enable_xsm=n                                                 \
+        tree_qemu=$TREE_QEMU                                                 \
+        tree_qemuu=$TREE_QEMU_UPSTREAM                                       \
+        tree_xen=$TREE_XEN                                                   \
+        tree_seabios=$TREE_SEABIOS                                           \
+                $RUNVARS $BUILD_RUNVARS $BUILD_XEN_RUNVARS $arch_runvars     \
+                $suite_runvars                                               \
+                host_hostflags=$build_hostflags                              \
+                revision_xen=$REVISION_XEN                                   \
+                revision_qemu=$REVISION_QEMU                                 \
+                revision_qemuu=$REVISION_QEMU_UPSTREAM                       \
+                revision_seabios=$REVISION_SEABIOS
+
+    if [ x$test_xsm = xy ]; then
+    ./cs-job-create $flight build-$arch-xsm build                            \
+                    arch=$arch enable_xend=$build_defxend enable_ovmf=$enable_ovmf\
+                    enable_xsm=$test_xsm                                     \
         tree_qemu=$TREE_QEMU                                                 \
         tree_qemuu=$TREE_QEMU_UPSTREAM                                       \
         tree_xen=$TREE_XEN                                                   \
@@ -153,6 +182,7 @@ create_build_jobs () {
                 revision_qemu=$REVISION_QEMU                                 \
                 revision_qemuu=$REVISION_QEMU_UPSTREAM                       \
                 revision_seabios=$REVISION_SEABIOS
+    fi
 
     if [ $build_extraxend = "true" ] ; then
     ./cs-job-create $flight build-$arch-xend build                           \
-- 
1.7.10.4

Re: [OSSTEST PATCH RFC v1 05/12] mfi-common: create build-$arch-xsm job

[Ian Campbell]

On Mon, 2014-09-22 at 15:11 +0100, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Could potentially reduce the duplication (assuming those two things
differ in exactly the enable_xsm runvar by assigning test_xsm to either
"n" or "n y" and then "for xsm in $test_xsm", plus a bit of logic around
the name in the loop etc?

Ian.

Re: [OSSTEST PATCH RFC v1 05/12] mfi-common: create build-$arch-xsm job

[Wei Liu]

On Wed, Sep 24, 2014 at 11:38:43AM +0100, Ian Campbell wrote:
> On Mon, 2014-09-22 at 15:11 +0100, Wei Liu wrote:
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Could potentially reduce the duplication (assuming those two things
> differ in exactly the enable_xsm runvar by assigning test_xsm to either
> "n" or "n y" and then "for xsm in $test_xsm", plus a bit of logic around
> the name in the loop etc?
> 

I will see what I can do.

Wei.

> Ian.

[OSSTEST PATCH RFC v1 06/12] Debian.pm: pass in XSM configuration to bootloader setup routines

[Wei Liu]

Change to Uboot will come in another patch. GRUB 1 is ignored, as
currently OSSTest only has Wheezy which has GRUB 2.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 Osstest/Debian.pm |   32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index ab09abb..4092351 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -45,9 +45,9 @@ BEGIN {
 
 #---------- manipulation of Debian bootloader setup ----------
 
-sub debian_boot_setup ($$$$;$) {
+sub debian_boot_setup ($$$$$;$) {
     # $xenhopt==undef => is actually a guest, do not set up a hypervisor
-    my ($ho, $want_kernver, $xenhopt, $distpath, $hooks) = @_;
+    my ($ho, $want_kernver, $want_xsm, $xenhopt, $distpath, $hooks) = @_;
 
     target_kernkind_check($ho);
     target_kernkind_console_inittab($ho,$ho,"/");
@@ -72,11 +72,11 @@ sub debian_boot_setup ($$$$;$) {
 
     my $bootloader;
     if ( $ho->{Flags}{'need-uboot-bootscr'} ) {
-	$bootloader= setupboot_uboot($ho, $want_kernver, $xenhopt, $kopt);
+	$bootloader= setupboot_uboot($ho, $want_kernver, $want_xsm, $xenhopt, $kopt);
     } elsif ($ho->{Suite} =~ m/lenny/) {
-        $bootloader= setupboot_grub1($ho, $want_kernver, $xenhopt, $kopt);
+        $bootloader= setupboot_grub1($ho, $want_kernver, $want_xsm, $xenhopt, $kopt);
     } else {
-        $bootloader= setupboot_grub2($ho, $want_kernver, $xenhopt, $kopt);
+        $bootloader= setupboot_grub2($ho, $want_kernver, $want_xsm, $xenhopt, $kopt);
     }
 
     $bootloader->{UpdateConfig}($ho);
@@ -112,8 +112,8 @@ sub bl_getmenu_open ($$$) {
     return $f;
 }
 
-sub setupboot_uboot ($$$) {
-    my ($ho,$want_kernver,$xenhopt,$xenkopt) = @_;
+sub setupboot_uboot ($$$$) {
+    my ($ho,$want_kernver,$want_xsm,$xenhopt,$xenkopt) = @_;
     my $bl= { };
 
     $bl->{UpdateConfig}= sub {
@@ -194,13 +194,17 @@ END
     return $bl;
 }
 
-sub setupboot_grub1 ($$$) {
-    my ($ho,$want_kernver,$xenhopt,$xenkopt) = @_;
+sub setupboot_grub1 ($$$$) {
+    my ($ho,$want_kernver,$want_xsm,$xenhopt,$xenkopt) = @_;
     my $bl= { };
 
     my $rmenu= "/boot/grub/menu.lst";
     my $lmenu= "$stash/$ho->{Name}--menu.lst.out";
 
+    if ($want_xsm) {
+	die "Enabling XSM with GRUB is not supported";
+    }
+
     target_editfile_root($ho, $rmenu, sub {
         while (<::EI>) {
             if (m/^## ## Start Default/ ..
@@ -274,8 +278,8 @@ sub setupboot_grub1 ($$$) {
     return $bl;
 }
 
-sub setupboot_grub2 ($$$) {
-    my ($ho,$want_kernver,$xenhopt,$xenkopt) = @_;
+sub setupboot_grub2 ($$$$) {
+    my ($ho,$want_kernver,$want_xsm,$xenhopt,$xenkopt) = @_;
     my $bl= { };
 
     my $rmenu= '/boot/grub/grub.cfg';
@@ -302,6 +306,9 @@ sub setupboot_grub2 ($$$) {
 			 $entry->{KernVer} ne $want_kernver) {
 		    logm("(skipping entry at $entry->{StartLine};".
 			 " kernel $entry->{KernVer}, not $want_kernver)");
+		} elsif ($want_xsm && !defined $entry->{Xenpolicy}) {
+		    logm("(skipping entry at $entry->{StartLine};".
+			 " XSM policy file not present)");
 		} else {
 		    # yes!
 		    last;
@@ -334,6 +341,9 @@ sub setupboot_grub2 ($$$) {
             if (m/^\s*module\s*\/(initrd\S+)/) {
                 $entry->{Initrd}= $1;
             }
+	    if (m/^\s*module\s*\/(xenpolicy\S+)/) {
+                $entry->{Xenpolicy}= $1;
+            }
         }
         die 'grub 2 bootloader entry not found' unless $entry;
 
-- 
1.7.10.4

[OSSTEST PATCH RFC v1 07/12] Debian.pm: load flask policy in uboot

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 Osstest/Debian.pm |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 4092351..5aaab83 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -123,6 +123,22 @@ sub setupboot_uboot ($$$$) {
 	my $kern = "vmlinuz-$want_kernver";
 	my $initrd = "initrd.img-$want_kernver";
 
+	my $flask_commands = "";
+	if ($want_xsm) {
+	    my $flaskpolicy = $r{flaskpoilcy};
+	    $flask_commands = <<END;
+
+setenv flask_policy_addr_r 0x1200000
+flaskpolicy=`readlink /boot/$flaskpolicy`
+ext2load scsi 0 \\\${flask_policy_addr_r} \$flaskpolicy
+fdt mknod /chosen module\@2
+fdt set /chosen/module\@2 compatible "xen,xsm-policy"
+fdt set /chosen/module\@2 reg <\\\${flask_policy_addr_r} \\\${filesize}>
+echo Loaded $flaskpolicy to \\\${flask_policy_addr_r} (\\\${filesize})
+
+END
+	}
+
 	my $root= target_guest_lv_name($ho,"root");
 
 	logm("Xen options: $xenhopt");
@@ -176,6 +192,8 @@ fdt set /chosen/module\@1 compatible "xen,linux-initrd" "xen,multiboot-module"
 fdt set /chosen/module\@1 reg <\\\${ramdisk_addr_r} \\\${filesize}>
 echo Loaded $initrd to \\\${ramdisk_addr_r} (\\\${filesize})
 
+${flask_commands}
+
 fdt print /chosen
 
 echo Booting \\\${xen_addr_r} - \\\${fdt_addr}
-- 
1.7.10.4

Re: [OSSTEST PATCH RFC v1 07/12] Debian.pm: load flask policy in uboot

[Ian Campbell]

On Mon, 2014-09-22 at 15:11 +0100, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH RFC v1 08/12] ts-xen-install: install Xen with XSM support if requested

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 ts-xen-install |  106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 105 insertions(+), 1 deletion(-)

diff --git a/ts-xen-install b/ts-xen-install
index 4d34d1f..f71b5a8 100755
--- a/ts-xen-install
+++ b/ts-xen-install
@@ -46,6 +46,8 @@ if (@ARGV and $ARGV[0] eq '--check') {
 
 our $ho;
 
+my $enable_xsm = $r{enable_xsm} =~ m/y/ ? 1 : 0;
+
 my %distpath;
 
 sub packages () {
@@ -73,6 +75,15 @@ sub extract () {
 				   $r{"${part}buildjob"}, \%distpath);
     }
     target_cmd_root($ho, '/sbin/ldconfig');
+    if ($enable_xsm) {
+        my $flaskpolicy = target_cmd_output_root($ho,
+		     'find /boot -name \'xenpolicy-*\' -exec basename {} \;');
+	# there should only be one xenpolicy file for a clean install
+	my $c = () = $flaskpolicy =~ /xenpolicy/g;
+	die "Too many XSM policy files $c" if $c > 1;
+	die "XSM policy file is required" if $c == 0;
+	store_runvar("flaskpolicy", $flaskpolicy);
+    }
 }
 
 sub adjustconfig () {
@@ -133,6 +144,86 @@ sub adjustconfig () {
     setup_cxfabric($ho);
 }
 
+
+sub grub_patch () {
+    return << 'END';
+--- /etc/grub.d/20_linux_xen.orig	2014-09-22 11:39:09.120630051 +0100
++++ /etc/grub.d/20_linux_xen	2014-09-22 11:43:07.069802099 +0100
+@@ -63,10 +63,27 @@
+   recovery="$4"
+   args="$5"
+   xen_args="$6"
+-  if ${recovery} ; then
+-    title="$(gettext_quoted "%s, with Linux %s and XEN %s (recovery mode)")"
++  xsm="$7"
++  # If user want to enable XSM support, make sure there's corresponding
++  # policy file.
++  if ${xsm} ; then
++      xenpolicy=`echo xenpolicy-$xen_version`
++      if test ! -e "${xen_dirname}/${xenpolicy}" ; then
++          return
++      fi
++      xen_args=`echo $xen_args flask_enabled=1 flask_enforcing=1`
++      if ${recovery} ; then
++          title="$(gettext_quoted "%s, with Xen %s (XSM enabled) and Linux %s (recovery mode)")"
++      else
++          title="$(gettext_quoted "%s, with Xen %s (XSM enabled) and Linux %s")"
++      fi
+   else
+-    title="$(gettext_quoted "%s, with Linux %s and XEN %s")"
++      xenpolicy=""
++      if ${recovery} ; then
++	  title="$(gettext_quoted "%s, with Linux %s and XEN %s (recovery mode)")"
++      else
++	  title="$(gettext_quoted "%s, with Linux %s and XEN %s")"
++      fi
+   fi
+   printf "menuentry '${title}' ${CLASS} {\n" "${os}" "${version}" "${xen_version}"
+   save_default_entry | sed -e "s/^/\t/"
+@@ -88,6 +105,13 @@
+ 	module	${rel_dirname}/${initrd}
+ EOF
+   fi
++  if test -n "${xenpolicy}" ; then
++    message="$(gettext_printf "Loading XSM policy ...")"
++    cat << EOF
++	echo	'$message'
++	module	${rel_dirname}/${xenpolicy}
++EOF
++  fi
+   cat << EOF
+ }
+ EOF
+@@ -98,7 +122,7 @@
+ 	version=$(echo $basename | sed -e "s,^[^0-9]*-,,g")
+         if grub_file_is_not_garbage "$i" && grep -qx 'CONFIG_XEN_\(DOM0\|PRIVILEGED_GUEST\)=y' /boot/config-${version} 2> /dev/null ; then echo -n "$i " ; fi
+       done`
+-xen_list=`for i in /boot/xen*; do
++xen_list=`for i in /boot/xen[-.]*; do
+         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
+       done`
+ prepare_boot_cache=
+@@ -137,10 +161,14 @@
+ 	fi
+ 
+ 	linux_entry "${OS}" "${version}" "${xen_version}" false \
+-	    "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" "${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT}"
++	    "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" "${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT}" false
++	linux_entry "${OS}" "${version}" "${xen_version}" false \
++	    "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" "${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT}" true
+ 	if [ "x${GRUB_DISABLE_LINUX_RECOVERY}" != "xtrue" ]; then
+ 	    linux_entry "${OS}" "${version}" "${xen_version}" true \
+-		"single ${GRUB_CMDLINE_LINUX}" "${GRUB_CMDLINE_XEN}"
++		"single ${GRUB_CMDLINE_LINUX}" "${GRUB_CMDLINE_XEN}" false
++	    linux_entry "${OS}" "${version}" "${xen_version}" true \
++		"single ${GRUB_CMDLINE_LINUX}" "${GRUB_CMDLINE_XEN}" true
+ 	fi
+ 
+ 	list=`echo $list | tr ' ' '\n' | grep -vx $linux | tr '\n' ' '`
+
+END
+}
+
 sub setupboot () {
     my $xenhopt= "conswitch=x watchdog";
 
@@ -170,8 +261,21 @@ sub setupboot () {
         };
     }
 
+    if ($enable_xsm) {
+	die if !defined($r{flaskpolicy});
+	target_putfilecontents_root_stash($ho, 10, grub_patch(), "grub.patch");
+	target_cmd_root($ho, << 'END');
+if ! grep -q -- xenpolicy /etc/grub.d/20_linux_xen ; then
+  patch -p0 /etc/grub.d/20_linux_xen  < grub.patch
+else
+  echo 'Grub script already supports XSM, not patching'
+  exit 1
+fi
+END
+    }
+
     my $want_kernver = get_runvar('kernel_ver',$r{'kernbuildjob'});
-    debian_boot_setup($ho, $want_kernver, $xenhopt, \%distpath, \@hooks);
+    debian_boot_setup($ho, $want_kernver, $enable_xsm, $xenhopt, \%distpath, \@hooks);
 
     logm("ready to boot Xen");
 }
-- 
1.7.10.4

Re: [OSSTEST PATCH RFC v1 08/12] ts-xen-install: install Xen with XSM support if requested

[Ian Campbell]

On Mon, 2014-09-22 at 15:11 +0100, Wei Liu wrote:
> +		     'find /boot -name \'xenpolicy-*\' -exec basename {} \;');

(stray tabs)

> +	# there should only be one xenpolicy file for a clean install
> +	my $c = () = $flaskpolicy =~ /xenpolicy/g;

I'm curious, what does that () do?

Does the grub patch thing work on arm (where I'm not sure if grub will
be there or not)?

Re: [OSSTEST PATCH RFC v1 08/12] ts-xen-install: install Xen with XSM support if requested

[Wei Liu]

On Wed, Sep 24, 2014 at 11:41:34AM +0100, Ian Campbell wrote:
> On Mon, 2014-09-22 at 15:11 +0100, Wei Liu wrote:
> > +		     'find /boot -name \'xenpolicy-*\' -exec basename {} \;');
> 
> (stray tabs)
> 
> > +	# there should only be one xenpolicy file for a clean install
> > +	my $c = () = $flaskpolicy =~ /xenpolicy/g;
> 
> I'm curious, what does that () do?
> 

This is called "empty parentheses" idiom. It's used to force list
context on an expression, and yet return a scalar: the count of items in
that list.

http://www.perlmonks.org/?node_id=527973

> Does the grub patch thing work on arm (where I'm not sure if grub will
> be there or not)?
> 

No. AIUI arm doesn't use grub, does it?

Wei.

Re: [OSSTEST PATCH RFC v1 08/12] ts-xen-install: install Xen with XSM support if requested

[Wei Liu]

On Wed, Sep 24, 2014 at 02:16:45PM +0100, Wei Liu wrote:
> On Wed, Sep 24, 2014 at 11:41:34AM +0100, Ian Campbell wrote:
> > On Mon, 2014-09-22 at 15:11 +0100, Wei Liu wrote:
> > > +		     'find /boot -name \'xenpolicy-*\' -exec basename {} \;');
> > 
> > (stray tabs)
> > 
> > > +	# there should only be one xenpolicy file for a clean install
> > > +	my $c = () = $flaskpolicy =~ /xenpolicy/g;
> > 
> > I'm curious, what does that () do?
> > 
> 
> This is called "empty parentheses" idiom. It's used to force list
> context on an expression, and yet return a scalar: the count of items in
> that list.
> 
> http://www.perlmonks.org/?node_id=527973
> 
> > Does the grub patch thing work on arm (where I'm not sure if grub will
> > be there or not)?
> > 
> 
> No. AIUI arm doesn't use grub, does it?
> 

That patch only applies to the grub carried in the particular version of
Wheezy we use in OSSTest. So when grub eventually comes to arm, it won't
work. But I'm kind of hoping when grub comes to arm we've already
upstreamed this patch and thus can drop it in OSSTest.

Wei

> Wei.

Re: [OSSTEST PATCH RFC v1 08/12] ts-xen-install: install Xen with XSM support if requested

[Ian Campbell]

On Wed, 2014-09-24 at 14:21 +0100, Wei Liu wrote:
> On Wed, Sep 24, 2014 at 02:16:45PM +0100, Wei Liu wrote:
> > On Wed, Sep 24, 2014 at 11:41:34AM +0100, Ian Campbell wrote:
> > > On Mon, 2014-09-22 at 15:11 +0100, Wei Liu wrote:
> > > > +		     'find /boot -name \'xenpolicy-*\' -exec basename {} \;');
> > > 
> > > (stray tabs)
> > > 
> > > > +	# there should only be one xenpolicy file for a clean install
> > > > +	my $c = () = $flaskpolicy =~ /xenpolicy/g;
> > > 
> > > I'm curious, what does that () do?
> > > 
> > 
> > This is called "empty parentheses" idiom. It's used to force list
> > context on an expression, and yet return a scalar: the count of items in
> > that list.
> > 
> > http://www.perlmonks.org/?node_id=527973

Neat!

> > > Does the grub patch thing work on arm (where I'm not sure if grub will
> > > be there or not)?
> > > 
> > 
> > No. AIUI arm doesn't use grub, does it?
> > 
> 
> That patch only applies to the grub carried in the particular version of
> Wheezy we use in OSSTest. So when grub eventually comes to arm, it won't
> work. But I'm kind of hoping when grub comes to arm we've already
> upstreamed this patch and thus can drop it in OSSTest.

My concern was that the test would work today on an ARM system without
grub (i.e. it isn't trying to patch a non-existent file etc).

When we come to support ARM systems with grub via osstest then there
will certainly be updates required, this will probably be the least of
them ;-)

Ian.

Re: [OSSTEST PATCH RFC v1 08/12] ts-xen-install: install Xen with XSM support if requested

[Wei Liu]

On Wed, Sep 24, 2014 at 02:47:18PM +0100, Ian Campbell wrote:
[...]
> > 
> > That patch only applies to the grub carried in the particular version of
> > Wheezy we use in OSSTest. So when grub eventually comes to arm, it won't
> > work. But I'm kind of hoping when grub comes to arm we've already
> > upstreamed this patch and thus can drop it in OSSTest.
> 
> My concern was that the test would work today on an ARM system without
> grub (i.e. it isn't trying to patch a non-existent file etc).
> 

Right. I should check if 20_linux_xen exists before trying to patch it.

Wei.

[OSSTEST PATCH RFC v1 09/12] mfi-common: use XSM build if job name contains -xsm suffix

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 mfi-common |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/mfi-common b/mfi-common
index cf7da4b..f016b9a 100644
--- a/mfi-common
+++ b/mfi-common
@@ -282,6 +282,14 @@ job_create_test () {
   buildjob="${bfi}build-$dom0arch"
   tsbuildjob=
 
+  # Use xsm build job if job contains -xsm suffix
+  case "$job" in
+    *-xsm) xenbuildjob="$xenbuildjob-xsm"
+           buildjob="$buildjob-xsm"
+           ;;
+    *) ;;
+  esac
+
   case "$xenbranch:$toolstack" in
     xen-3.*-testing:*) ;;
     xen-4.0-testing:*) ;;
-- 
1.7.10.4

[OSSTEST PATCH RFC v1 10/12] make-flight: create XSM test jobs

[Wei Liu]

Duplicate Debian PV and HVM test jobs for XSM testing.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 make-flight |   35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/make-flight b/make-flight
index 9963a46..b157263 100755
--- a/make-flight
+++ b/make-flight
@@ -200,11 +200,21 @@ do_hvm_win7_x64_tests () {
 do_hvm_debian_test_one () {
   testname=$1
   bios=$2
+  xsm=$3
   job_create_test test-$xenarch$kern-$dom0arch-xl$qemuu_suffix-$testname-amd64\
     test-debianhvm xl $xenarch $dom0arch $qemuu_runvar \
+    enable_xsm=n                                \
     debianhvm_image=debian-7.2.0-amd64-CD-1.iso \
     bios=$bios \
     all_hostflags=$most_hostflags,hvm
+  if [ x$xsm = xtrue ] ; then
+      job_create_test test-$xenarch$kern-$dom0arch-xl$qemuu_suffix-$testname-amd64-xsm\
+        test-debianhvm xl $xenarch $dom0arch $qemuu_runvar \
+        enable_xsm=y                                \
+        debianhvm_image=debian-7.2.0-amd64-CD-1.iso \
+        bios=$bios \
+        all_hostflags=$most_hostflags,hvm
+  fi
 }
 
 do_hvm_debian_tests() {
@@ -214,13 +224,13 @@ do_hvm_debian_tests() {
 
   # QEMU upstream supports ovmf and seabios
   if [ "x$qemuu_suffix" == "x-qemuu" ]; then
-    do_hvm_debian_test_one ovmf ovmf
-    do_hvm_debian_test_one debianhvm seabios
+    do_hvm_debian_test_one ovmf ovmf false
+    do_hvm_debian_test_one debianhvm seabios true
   fi
 
   # QEMU traditional supports rombios
   if [ "x$qemuu_suffix" == "x-qemut" ]; then
-    do_hvm_debian_test_one debianhvm rombios
+    do_hvm_debian_test_one debianhvm rombios true
   fi
 }
 
@@ -283,12 +293,31 @@ do_passthrough_tests () {
 
 test_matrix_do_one () {
 
+  # xsm tests for versions > 4.4, arch amd64 and armhf
+  case "$xenbranch:$xenarch" in
+  xen-3.*-testing:*) test_xsm=n ;;
+  xen-4.0-testing:*) test_xsm=n ;;
+  xen-4.1-testing:*) test_xsm=n ;;
+  xen-4.2-testing:*) test_xsm=n ;;
+  xen-4.3-testing:*) test_xsm=n ;;
+  xen-4.4-testing:*) test_xsm=n ;;
+  *:amd64|armhf)     test_xsm=y ;;
+  esac
+
   # Basic PV Linux test with xl
 
   job_create_test test-$xenarch$kern-$dom0arch-xl test-debian xl \
             $xenarch $dom0arch                                   \
+            enable_xsm=n                                         \
             $debian_runvars all_hostflags=$most_hostflags
 
+  if [ x$test_xsm = xy ] ; then
+      job_create_test test-$xenarch$kern-$dom0arch-xl-xsm test-debian xl \
+              $xenarch $dom0arch                                   \
+              enable_xsm=y                                         \
+              $debian_runvars all_hostflags=$most_hostflags
+  fi
+
   job_create_test test-$xenarch$kern-$dom0arch-libvirt test-debian libvirt \
             $xenarch $dom0arch                                       \
             $debian_runvars all_hostflags=$most_hostflags
-- 
1.7.10.4

Re: [OSSTEST PATCH RFC v1 10/12] make-flight: create XSM test jobs

[Ian Campbell]

On Mon, 2014-09-22 at 15:11 +0100, Wei Liu wrote:
> @@ -214,13 +224,13 @@ do_hvm_debian_tests() {
>  
>    # QEMU upstream supports ovmf and seabios
>    if [ "x$qemuu_suffix" == "x-qemuu" ]; then
> -    do_hvm_debian_test_one ovmf ovmf
> -    do_hvm_debian_test_one debianhvm seabios
> +    do_hvm_debian_test_one ovmf ovmf false
> +    do_hvm_debian_test_one debianhvm seabios true

I think it would be less repetitive to duplicate these lines as
necessary rather than duping the job creation inside these functions. So
e.g.:
+    do_hvm_debian_test_one ovmf ovmf false
+    do_hvm_debian_test_one debianhvm seabios false
+    do_hvm_debian_test_one debianhvm seabios true

>    # Basic PV Linux test with xl
>  
>    job_create_test test-$xenarch$kern-$dom0arch-xl test-debian xl \
>              $xenarch $dom0arch                                   \
> +            enable_xsm=n                                         \
>              $debian_runvars all_hostflags=$most_hostflags
>  
> +  if [ x$test_xsm = xy ] ; then
> +      job_create_test test-$xenarch$kern-$dom0arch-xl-xsm test-debian xl \
> +              $xenarch $dom0arch                                   \
> +              enable_xsm=y                                         \
> +              $debian_runvars all_hostflags=$most_hostflags
> +  fi

Refactor this into a fnction and call it a second time?

> +
>    job_create_test test-$xenarch$kern-$dom0arch-libvirt test-debian libvirt \
>              $xenarch $dom0arch                                       \
>              $debian_runvars all_hostflags=$most_hostflags

Re: [OSSTEST PATCH RFC v1 10/12] make-flight: create XSM test jobs

[Wei Liu]

On Wed, Sep 24, 2014 at 12:00:53PM +0100, Ian Campbell wrote:
> On Mon, 2014-09-22 at 15:11 +0100, Wei Liu wrote:
> > @@ -214,13 +224,13 @@ do_hvm_debian_tests() {
> >  
> >    # QEMU upstream supports ovmf and seabios
> >    if [ "x$qemuu_suffix" == "x-qemuu" ]; then
> > -    do_hvm_debian_test_one ovmf ovmf
> > -    do_hvm_debian_test_one debianhvm seabios
> > +    do_hvm_debian_test_one ovmf ovmf false
> > +    do_hvm_debian_test_one debianhvm seabios true
> 
> I think it would be less repetitive to duplicate these lines as
> necessary rather than duping the job creation inside these functions. So
> e.g.:
> +    do_hvm_debian_test_one ovmf ovmf false
> +    do_hvm_debian_test_one debianhvm seabios false
> +    do_hvm_debian_test_one debianhvm seabios true
> 

No problem.

> >    # Basic PV Linux test with xl
> >  
> >    job_create_test test-$xenarch$kern-$dom0arch-xl test-debian xl \
> >              $xenarch $dom0arch                                   \
> > +            enable_xsm=n                                         \
> >              $debian_runvars all_hostflags=$most_hostflags
> >  
> > +  if [ x$test_xsm = xy ] ; then
> > +      job_create_test test-$xenarch$kern-$dom0arch-xl-xsm test-debian xl \
> > +              $xenarch $dom0arch                                   \
> > +              enable_xsm=y                                         \
> > +              $debian_runvars all_hostflags=$most_hostflags
> > +  fi
> 
> Refactor this into a fnction and call it a second time?
> 

Ack.

Wei.

> > +
> >    job_create_test test-$xenarch$kern-$dom0arch-libvirt test-debian libvirt \
> >              $xenarch $dom0arch                                       \
> >              $debian_runvars all_hostflags=$most_hostflags
> 

[OSSTEST PATCH RFC v1 11/12] ts-debian-install: add in seclabel if XSM is enabled

[Wei Liu]

... otherwise the guest cannot be created.

Also changed cfg_xend to cfg while I was there.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 ts-debian-install |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/ts-debian-install b/ts-debian-install
index 58ea743..c42ae09 100755
--- a/ts-debian-install
+++ b/ts-debian-install
@@ -89,8 +89,15 @@ END
             $initrd_opt \\
             $archarg
 END
-    my $cfg_xend= "/etc/xen/$gho->{Name}.cfg";
-    store_runvar("$gho->{Guest}_cfgpath", $cfg_xend);
+    my $cfg= "/etc/xen/$gho->{Name}.cfg";
+
+    if ($r{enable_xsm} =~ m/y/) {
+	target_cmd_root($ho, <<"END");
+echo "seclabel='system_u:system_r:domU_t'" >> $cfg
+END
+    }
+
+    store_runvar("$gho->{Guest}_cfgpath", $cfg);
     store_runvar("$gho->{Guest}_swap_lv", "$gho->{Name}-swap");
 }
 
-- 
1.7.10.4

Re: [OSSTEST PATCH RFC v1 11/12] ts-debian-install: add in seclabel if XSM is enabled

[Ian Campbell]

On Mon, 2014-09-22 at 15:12 +0100, Wei Liu wrote:
> ... otherwise the guest cannot be created.

is this setting harmful if xsm isn't on?

> 
> Also changed cfg_xend to cfg while I was there.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  ts-debian-install |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/ts-debian-install b/ts-debian-install
> index 58ea743..c42ae09 100755
> --- a/ts-debian-install
> +++ b/ts-debian-install
> @@ -89,8 +89,15 @@ END
>              $initrd_opt \\
>              $archarg
>  END
> -    my $cfg_xend= "/etc/xen/$gho->{Name}.cfg";
> -    store_runvar("$gho->{Guest}_cfgpath", $cfg_xend);
> +    my $cfg= "/etc/xen/$gho->{Name}.cfg";
> +
> +    if ($r{enable_xsm} =~ m/y/) {
> +	target_cmd_root($ho, <<"END");
> +echo "seclabel='system_u:system_r:domU_t'" >> $cfg
> +END
> +    }
> +
> +    store_runvar("$gho->{Guest}_cfgpath", $cfg);
>      store_runvar("$gho->{Guest}_swap_lv", "$gho->{Name}-swap");
>  }
>  

Re: [OSSTEST PATCH RFC v1 11/12] ts-debian-install: add in seclabel if XSM is enabled

[Wei Liu]

On Wed, Sep 24, 2014 at 12:01:35PM +0100, Ian Campbell wrote:
> On Mon, 2014-09-22 at 15:12 +0100, Wei Liu wrote:
> > ... otherwise the guest cannot be created.
> 
> is this setting harmful if xsm isn't on?
> 

If XSM is not enabled while guest has seclabel:

libxl: warning: libxl_create.c:722:initiate_domain_create: XSM Disabled: init_seclabel not supported

Libxl ignores that field. Everything is still working.

Wei.

Re: [OSSTEST PATCH RFC v1 11/12] ts-debian-install: add in seclabel if XSM is enabled

[Ian Campbell]

On Wed, 2014-09-24 at 14:14 +0100, Wei Liu wrote:
> On Wed, Sep 24, 2014 at 12:01:35PM +0100, Ian Campbell wrote:
> > On Mon, 2014-09-22 at 15:12 +0100, Wei Liu wrote:
> > > ... otherwise the guest cannot be created.
> > 
> > is this setting harmful if xsm isn't on?
> > 
> 
> If XSM is not enabled while guest has seclabel:
> 
> libxl: warning: libxl_create.c:722:initiate_domain_create: XSM Disabled: init_seclabel not supported
> 
> Libxl ignores that field. Everything is still working.

Hrm, probably ugly enough to be worth avoiding then, IOW I agree with
making this conditional.

The reason for editing this post-hoc is that the original came from the
the xen-tools debootrap thing, so that aspect seems reasonable too.

Ian

[OSSTEST PATCH RFC v1 12/12] ts-debian-hvm-install: add in seclabel if XSM is enabled

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 ts-debian-hvm-install |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index 37eade2..3c557ef 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -152,10 +152,10 @@ sub prep () {
     target_putfilecontents_root_stash($ho, 10, preseed(),
                                       $preseed_file_path);
 
-    more_prepareguest_hvm($ho,$gho, $ram_mb, $disk_mb,
-                          OnReboot => 'preserve',
-                          Bios => $r{bios},
-                          PostImageHook => sub {
+    my $cfg = more_prepareguest_hvm($ho,$gho, $ram_mb, $disk_mb,
+				    OnReboot => 'preserve',
+				    Bios => $r{bios},
+				    PostImageHook => sub {
         my $cmds = iso_copy_content_from_image($gho, $newiso);
         $cmds .= prepare_initrd($initrddir,$newiso,$preseed_file_path);
         target_cmd_root($ho, $cmds, $isotimeout);
@@ -167,6 +167,12 @@ sub prep () {
 
         iso_create_genisoimage($ho, $gho->{Rimage}, $newiso, $isotimeout, @isogen_opts);
     });
+
+    if ($r{enable_xsm} =~ m/y/) {
+	target_cmd_root($ho, <<"END");
+echo "seclabel='system_u:system_r:domU_t'" >> $cfg
+END
+    }
 }
 
 # If host has >8G free memory, create a guest with 4G memory to catch
-- 
1.7.10.4

Re: [OSSTEST PATCH RFC v1 00/12] XSM test cases for OSSTest

[Daniel De Graaf]

On 09/22/2014 10:11 AM, Wei Liu wrote:
> Hi all
>
> This patch series attempts to duplicate some Debian smoke test for XSM in Xen.

This looks good to me, and should help shake out policy errors better than
manual testing - which, as you found, has missed a few operations.

[...]
> 4. In-tree default policy is too strict
>
> For PV guest test case, it can successfully create a guest, but fails at
> saving. Xen log says "permission denied".
>
> For QEMU upstream HVM guest, QEMU segfaults with NULL pointer dereference.
>
> For QEMU traditional HVM guest, guest crashes with triple fault.
>
> I have yet tried to debug HVM test cases. Presumably the failures are combined
> effect of the enforced XSM policy and some QEMU bugs. It's likely to take some
> time to figure out what went wrong. The bug fix and policy tuning is orthogonal
> to the test case itself though.

Fixes to the XSM policy can either be made iteratively until the tests pass, or
all at once with the hypervisor in permissive mode.  In permissive mode, the
hypervisor will remove duplicate AVCs and may also get further along successful
code paths that require more permissions.  However, the tests are probably best
done in enforcing mode (as written), since I expect most failures will be due to
a single permission missing.

-- 
Daniel De Graaf
National Security Agency