ubuntu-bugcontrol team mailing list archive

[Maciej Kisielewski <maciej.kisielewski@xxxxxxxxxxxxx>]

Maciej Kisielewski has proposed merging lp:~kissiel/qa-regression-testing/kernel-sec-py3-friendly into lp:qa-regression-testing.

Requested reviews:
  Ubuntu Bug Control (ubuntu-bugcontrol)

For more details, see:
https://code.launchpad.net/~kissiel/qa-regression-testing/kernel-sec-py3-friendly/+merge/302017

This MR makes all python code in kernel-security suite python3 friendly as well as testlib.py
print conversions were done using 2to3, other issues were done manually.

This helps significantly when porting the security suite to snappy.

It would be even better if I'd be allowed to make all those tests explicitly run python3 (by changing shebangs).

Comments welcome!

Changes made:
* print as a function
* explicitly decoding output from std{out,err}
* s/self.assertNotEquals/self.assertNotEqual/
* explicitly closing open files
* bytes literals instead of strings when used with conjuction with the raw output of std{out,err}
* 0o0000 notation for octal literals
* 'as' syntax for catching exceptions
* string.ascii_letters instead of locale-dependent string.letters
* /usr/bin/env python instead of absolute /usr/bin/python in shebangs


-- 
Your team Ubuntu Bug Control is requested to review the proposed merge of lp:~kissiel/qa-regression-testing/kernel-sec-py3-friendly into lp:qa-regression-testing.

=== modified file 'scripts/kernel-security/errno-read.py'
--- scripts/kernel-security/errno-read.py	2009-04-03 00:53:35 +0000
+++ scripts/kernel-security/errno-read.py	2016-08-04 12:39:13 +0000
@@ -1,9 +1,12 @@
-#!/usr/bin/python
+#!/usr/bin/env python
+from __future__ import print_function
 import sys
 try:
-    file(sys.argv[1]).read(int(sys.argv[2]))
-except IOError, e:
-    print >>sys.stderr, "%s: %s" % (sys.argv[1], e.strerror)
+    f = open(sys.argv[1])
+    f.read(int(sys.argv[2]))
+    f.close()
+except IOError as e:
+    print("%s: %s" % (sys.argv[1], e.strerror), file=sys.stderr)
     sys.exit(e.errno)
-print "%s: Success" % (sys.argv[1])
+print("%s: Success" % (sys.argv[1]))
 sys.exit(0)

=== modified file 'scripts/kernel-security/proc-leaks/dac-bypass.py'
--- scripts/kernel-security/proc-leaks/dac-bypass.py	2012-03-21 14:49:14 +0000
+++ scripts/kernel-security/proc-leaks/dac-bypass.py	2016-08-04 12:39:13 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 # Demonstrates DAC bypass on /proc/$pid file descriptors across setuid exec
 # Author: Kees Cook <kees@xxxxxxxxxx>
 # License: GPLv2
@@ -21,9 +21,9 @@
     auxv = struct.unpack('@%dL' % (len(blob)/len(struct.pack('@L',0))), blob)
     while auxv[0] != 0:
         if auxv[0] == 7:
-            print "AT_BASE:   0x%x" % (auxv[1])
+            print("AT_BASE:   0x%x" % (auxv[1]))
         if auxv[0] == 25:
-            print "AT_RANDOM: 0x%x" % (auxv[1])
+            print("AT_RANDOM: 0x%x" % (auxv[1]))
         auxv = auxv[2:]
 
 # Allow child/parent synchronization to avoid start-up race.
@@ -55,9 +55,9 @@
                     if name == 'auxv':
                         dump_auxv(saw)
                     else:
-                        print saw
+                        print(saw)
                     last[name] = saw
-    except Exception, o:
+    except Exception as o:
         if o.errno == errno.EEXIST or o.errno == errno.EPERM:
             # Target quit or wasn't permitted to access the file to
             # begin with, so okay.

=== modified file 'scripts/kernel-security/proc-maps/maps-protection.py'
--- scripts/kernel-security/proc-maps/maps-protection.py	2013-11-27 23:58:15 +0000
+++ scripts/kernel-security/proc-maps/maps-protection.py	2016-08-04 12:39:13 +0000
@@ -81,7 +81,7 @@
 
         info = os.stat('maps-helper-setuid')
         self.assertEqual(info.st_uid, 0, "maps-helper-setuid not owned by root")
-        self.assertEqual(info.st_mode & 04000, 04000, "maps-helper-setuid not setuid")
+        self.assertEqual(info.st_mode & 0o4000, 0o4000, "maps-helper-setuid not setuid")
 
     def test_30_setuid_before_dropping_privs(self):
         '''Can not read setuid process before priv-dropping to our uid'''

=== modified file 'scripts/test-kernel-security.py'
--- scripts/test-kernel-security.py	2016-06-27 16:06:54 +0000
+++ scripts/test-kernel-security.py	2016-08-04 12:39:13 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 #
 #    kernel-security.py regression testing script for kernel and
 #    security features

=== modified file 'scripts/testlib.py'
--- scripts/testlib.py	2016-06-26 15:39:53 +0000
+++ scripts/testlib.py	2016-08-04 12:39:13 +0000
@@ -19,6 +19,7 @@
 
 '''Common classes and functions for package tests.'''
 
+from __future__ import print_function
 import string, random, crypt, subprocess, pwd, grp, time, unittest, tempfile, shutil, os, os.path, re, glob
 import sys, socket, gzip
 
@@ -179,20 +180,20 @@
 
 def require_nonroot():
     if os.geteuid() == 0:
-        print >>sys.stderr, "This series of tests should be run as a regular user with sudo access, not as root."
+        print("This series of tests should be run as a regular user with sudo access, not as root.", file=sys.stderr)
         sys.exit(1)
 
 def require_root():
     if os.geteuid() != 0:
-        print >>sys.stderr, "This series of tests should be run with root privileges (e.g. via sudo)."
+        print("This series of tests should be run with root privileges (e.g. via sudo).", file=sys.stderr)
         sys.exit(1)
 
 def require_sudo():
     if os.geteuid() != 0 or os.environ.get('SUDO_USER', None) is None:
-        print >>sys.stderr, "This series of tests must be run under sudo."
+        print("This series of tests must be run under sudo.", file=sys.stderr)
         sys.exit(1)
     if os.environ['SUDO_USER'] == 'root':
-        print >>sys.stderr, 'Please run this test using sudo from a regular user. (You ran sudo from root.)'
+        print('Please run this test using sudo from a regular user. (You ran sudo from root.)', file=sys.stderr)
         sys.exit(1)
 
 def random_string(length,lower=False):
@@ -200,7 +201,7 @@
     length.'''
 
     s = ''
-    selection = string.letters
+    selection = string.ascii_letters
     if lower:
         selection = string.lowercase
     maxind = len(selection)-1
@@ -402,11 +403,11 @@
     out, outerr = sp.communicate(input)
     # Handle redirection of stdout
     if out is None:
-        out = ''
+        out = b''
     # Handle redirection of stderr
     if outerr is None:
-        outerr = ''
-    return [sp.returncode, out + outerr]
+        outerr = b''
+    return [sp.returncode, out.decode(sys.stdout.encoding) + outerr.decode(sys.stderr.encoding)]
 
 def cmd_pipe(command1, command2, input = None, stderr = subprocess.STDOUT, stdin = None):
     '''Try to pipe command1 into command2.'''
@@ -594,7 +595,7 @@
         rc, report = cmd(['apt-get','source',source])
         assert (rc == 0)
         shutil.copytree(build_src, cached_src)
-        print "(unpacked %s)" % os.path.basename(glob.glob('%s_*.dsc' % source)[0]),
+        print("(unpacked %s)" % os.path.basename(glob.glob('%s_*.dsc' % source)[0]), end=' ')
 
 
     unpacked_dir = os.path.join(build_src, glob.glob('%s-*' % source)[0])
@@ -720,7 +721,7 @@
     # check for it.
     rc, report = cmd(['ps', 'x'])
     if 'kdeinit4 Running' not in report:
-        print >>sys.stderr, ("kdeinit not running (you may start/stop any KDE application then run this script again)")
+        print(("kdeinit not running (you may start/stop any KDE application then run this script again)"), file=sys.stderr)
         return False
     return True
 
@@ -730,7 +731,7 @@
     rc, pkg_config = cmd(['pkg-config', '--cflags', '--libs'] + libs)
     expected = 0
     if rc != expected:
-        print >>sys.stderr, 'Got exit code %d, expected %d\n' % (rc, expected)
+        print('Got exit code %d, expected %d\n' % (rc, expected), file=sys.stderr)
     assert(rc == expected)
     return pkg_config.split()
 
@@ -739,7 +740,7 @@
     rc, output = cmd(['capsh', '--decode=%x' % cap_num])
     expected = 0
     if rc != expected:
-        print >>sys.stderr, 'capsh: got exit code %d, expected %d\n' % (rc, expected)
+        print('capsh: got exit code %d, expected %d\n' % (rc, expected), file=sys.stderr)
     cap_name = output.strip().split('=')[1]
     return cap_name
 
@@ -856,7 +857,7 @@
         if not os.path.exists('/usr/bin/lsb_release') and not os.path.exists('/bin/lsb_release'):
             raise OSError("Please install 'lsb-release'")
         for line in subprocess.Popen(['lsb_release','-a'],stdout=subprocess.PIPE,stderr=subprocess.PIPE).communicate()[0].splitlines():
-            field, value = line.split(':',1)
+            field, value = line.decode(sys.stdout.encoding).split(':',1)
             value=value.strip()
             field=field.strip()
             # Convert numerics
@@ -933,14 +934,14 @@
             kernel = self.kernel_version_ubuntu
             if kernel != self.kernel_version_signature:
                 kernel += " (%s)" % (self.kernel_version_signature)
-            print >>sys.stdout, "Running test: '%s' distro: '%s %.2f' kernel: '%s' arch: '%s' uid: %d/%d SUDO_USER: '%s')" % ( \
+            print("Running test: '%s' distro: '%s %.2f' kernel: '%s' arch: '%s' uid: %d/%d SUDO_USER: '%s')" % ( \
                 sys.argv[0],
                 self.lsb_release['Distributor ID'],
                 self.lsb_release['Release'],
                 kernel,
                 self.dpkg_arch,
                 os.geteuid(), os.getuid(),
-                os.environ.get('SUDO_USER', ''))
+                os.environ.get('SUDO_USER', '')), file=sys.stdout)
             sys.stdout.flush()
 
         # Additional heuristics
@@ -952,7 +953,7 @@
         #    time.sleep(0.5)
 
     def hello(self, msg):
-        print >>sys.stderr, "Hello from %s" % (msg)
+        print("Hello from %s" % (msg), file=sys.stderr)
 # The central instance
 manager = TestlibManager()
 
@@ -1000,7 +1001,7 @@
             # accept if the beginning of the line matches
             filetype = '^%s' % (filetype)
         result = 'File type reported by file: [%s], expected regex: [%s]\n' % (out, filetype)
-        self.assertNotEquals(None, re.search(filetype, out), result)
+        self.assertNotEqual(None, re.search(filetype, out), result)
 
     def yank_commonname_from_cert(self, certfile):
         '''Extract the commonName from a given PEM'''
@@ -1016,7 +1017,7 @@
 
     def announce(self, text):
         if self.my_verbosity:
-            print >>sys.stdout, "(%s) " % (text),
+            print("(%s) " % (text), end=' ', file=sys.stdout)
             sys.stdout.flush()
 
     def make_clean(self):
@@ -1078,7 +1079,7 @@
         '''Test a shell command doesn't match a specific exit code'''
         rc, report, out = self._testlib_shell_cmd(args, stdin=stdin, stdout=stdout, stderr=stderr)
         result = 'Got (unwanted) exit code %d\n' % rc
-        self.assertNotEquals(unwanted, rc, msg + result + report)
+        self.assertNotEqual(unwanted, rc, msg + result + report)
 
     def assertShellOutputContains(self, text, args, stdin=None, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, msg="", invert=False, expected=None):
         '''Test a shell command contains a specific output'''
@@ -1099,7 +1100,7 @@
         if not invert:
             self.assertEqual(text, out, msg + result + report)
         else:
-            self.assertNotEquals(text, out, msg + result + report)
+            self.assertNotEqual(text, out, msg + result + report)
         if expected is not None:
             result = 'Got exit code %d. Expected %d (%s)\n' % (rc, expected, " ".join(args))
             self.assertEqual(rc, expected, msg + result + report)
@@ -1118,17 +1119,21 @@
         self.assertEqual(exists, os.path.exists(sysctl), sysctl)
         value = None
         if exists:
-            value = int(open(sysctl).read())
+            f = open(sysctl)
+            value = int(f.read())
             report = "%s is not %d: %d" % (sysctl, expected, value)
             if msg:
                 report += " (%s)" % (msg)
             self.assertEqual(value, expected, report)
+            f.close()
         return value
 
     def set_sysctl_value(self, path, desired):
         sysctl = '/proc/sys/%s' % (path)
         self.assertTrue(os.path.exists(sysctl),"%s does not exist" % (sysctl))
-        open(sysctl, 'w').write(str(desired))
+        f = open(sysctl, 'w')
+        f.write(str(desired))
+        f.close()
         self._test_sysctl_value(path, desired)
 
     def kernel_at_least(self, introduced):