launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/delete-virt-check into lp:launchpad-buildd.

Commit message:
Remove virtualization check from buildrecipe.  It was a rather futile
security check as escaping chroots is trivial, and it will fail when the
PPA builder pool is converted to scalingstack.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/delete-virt-check/+merge/198242

The virtualization check in buildrecipe is (a) not forward-compatible with PPAs in scalingstack and (b) a pointless piece of security theatre as escaping chroots is well-known to be a trivial exercise.  I discussed this with William on IRC and he suggested just deleting it.
-- 
https://code.launchpad.net/~cjwatson/launchpad-buildd/delete-virt-check/+merge/198242
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/delete-virt-check into lp:launchpad-buildd.

=== modified file 'buildrecipe'
--- buildrecipe	2013-10-21 20:33:48 +0000
+++ buildrecipe	2013-12-09 12:27:27 +0000
@@ -27,13 +27,6 @@
 RETCODE_FAILURE_BUILD_SOURCE_PACKAGE = 203
 
 
-class NotVirtualized(Exception):
-    """Exception raised when not running in a virtualized environment."""
-
-    def __init__(self):
-        Exception.__init__(self, 'Not running under Xen.')
-
-
 def call_report_rusage(args, env):
     """Run a subprocess.
 
@@ -92,12 +85,6 @@
         As a side-effect, sets self.source_dir_relative.
         :return: a retcode from `bzr dailydeb`.
         """
-        try:
-            ensure_virtualized()
-        except NotVirtualized, e:
-            sys.stderr.write('Aborting on failed virtualization check:\n')
-            sys.stderr.write(str(e))
-            return 1
         assert not os.path.exists(self.tree_path)
         recipe_path = os.path.join(self.work_dir, 'recipe')
         manifest_path = os.path.join(self.tree_path, 'manifest')
@@ -208,15 +195,6 @@
         os.environ["HOME"], "build-" + build_id, *extra)
 
 
-def ensure_virtualized():
-    """Raise an exception if not running in a virtualized environment.
-
-    Raises if not running under Xen.
-    """
-    if not os.path.isdir('/proc/xen') or os.path.exists('/proc/xen/xsd_kva'):
-        raise NotVirtualized()
-
-
 if __name__ == '__main__':
     setrlimit(RLIMIT_AS, (1000000000, -1))
     builder = RecipeBuilder(*sys.argv[1:])

=== modified file 'debian/changelog'
--- debian/changelog	2013-12-09 11:52:37 +0000
+++ debian/changelog	2013-12-09 12:27:27 +0000
@@ -7,6 +7,9 @@
   * Make the status XML-RPC method a synonym for status_dict.
   * Add a new "livefs" build manager, based on livecd-rootfs/BuildLiveCD
     (LP: #1247461).
+  * Remove virtualization check from buildrecipe.  It was a rather futile
+    security check as escaping chroots is trivial, and it will fail when the
+    PPA builder pool is converted to scalingstack.
 
   [ Adam Conrad ]
   * update-debian-chroot: Allow arm64-on-x86 builds with qemu-arm64-static.