merge script handling into get_interpreter_and_abstraction()

Both create_new_profile() and handle_children() check if the given exec
target is a script and add permissions for the interpreter and a
matching abstraction.

This patch merges that into the get_interpreter_and_abstraction()
function and changes create_new_profile() and handle_children() to use
this function.

A nice side effect is that handle_children() now knows more abstractions
(its original list was incomplete).
The behaviour of create_new_profile() doesn't change.

Also add tests for get_interpreter_and_abstraction() to make sure it
does what we expect.


Acked-by: Kshitij Gupta <kgupta8592@gmail.com>

Bug: https://launchpad.net/bugs/1505775

utils/apparmor/aa.py

@@ -400,6 +400,43 @@ def handle_binfmt(profile, path):
         profile['allow']['path'][library]['mode'] = profile['allow']['path'][library].get('mode', set()) | str_to_mode('mr')
         profile['allow']['path'][library]['audit'] |= profile['allow']['path'][library].get('audit', set())
 
+def get_interpreter_and_abstraction(exec_target):
+    '''Check if exec_target is a script.
+       If a hashbang is found, check if we have an abstraction for it.
+
+       Returns (interpreter_path, abstraction)
+       - interpreter_path is none if exec_target is not a script or doesn't have a hashbang line
+       - abstraction is None if no matching abstraction exists'''
+
+    if not os.path.exists(exec_target):
+        aaui.UI_Important(_('Execute target %s does not exist!') % exec_target)
+        return None, None
+
+    if not os.path.isfile(exec_target):
+        aaui.UI_Important(_('Execute target %s is not a file!') % exec_target)
+        return None, None
+
+    hashbang = head(exec_target)
+    if not hashbang.startswith('#!'):
+        return None, None
+
+    interpreter = hashbang[2:].strip()
+    interpreter_path = get_full_path(interpreter)
+    interpreter = re.sub('^(/usr)?/bin/', '', interpreter_path)
+
+    if interpreter in ['bash', 'dash', 'sh']:
+        abstraction = 'abstractions/bash'
+    elif interpreter == 'perl':
+        abstraction = 'abstractions/perl'
+    elif re.search('^python([23]|[23]\.[0-9]+)?$', interpreter):
+        abstraction = 'abstractions/python'
+    elif interpreter == 'ruby':
+        abstraction = 'abstractions/ruby'
+    else:
+        abstraction = None
+
+    return interpreter_path, abstraction
+
 def get_inactive_profile(local_profile):
     if extras.get(local_profile, False):
         return {local_profile: extras[local_profile]}
@@ -439,12 +476,9 @@ def create_new_profile(localfile, is_stub=False):
     local_profile[localfile]['include']['abstractions/base'] = 1
 
     if os.path.exists(localfile) and os.path.isfile(localfile):
-        hashbang = head(localfile)
+        interpreter_path, abstraction = get_interpreter_and_abstraction(localfile)
-        if hashbang.startswith('#!'):
-            interpreter_path = get_full_path(hashbang.lstrip('#!').strip())
-
-            interpreter = re.sub('^(/usr)?/bin/', '', interpreter_path)
 
+        if interpreter_path:
             local_profile[localfile]['allow']['path'][localfile]['mode'] = local_profile[localfile]['allow']['path'][localfile].get('mode', str_to_mode('r')) | str_to_mode('r')
 
             local_profile[localfile]['allow']['path'][localfile]['audit'] = local_profile[localfile]['allow']['path'][localfile].get('audit', set())
@@ -453,14 +487,9 @@ def create_new_profile(localfile, is_stub=False):
 
             local_profile[localfile]['allow']['path'][interpreter_path]['audit'] = local_profile[localfile]['allow']['path'][interpreter_path].get('audit', set())
 
-            if interpreter == 'perl':
+            if abstraction:
-                local_profile[localfile]['include']['abstractions/perl'] = True
+                local_profile[localfile]['include'][abstraction] = True
-            elif re.search('^python([23]|[23]\.[0-9]+)?$', interpreter):
+
-                local_profile[localfile]['include']['abstractions/python'] = True
-            elif interpreter == 'ruby':
-                local_profile[localfile]['include']['abstractions/ruby'] = True
-            elif interpreter in ['bash', 'dash', 'sh']:
-                local_profile[localfile]['include']['abstractions/bash'] = True
             handle_binfmt(local_profile[localfile], interpreter_path)
         else:
 
@@ -1409,24 +1438,15 @@ def handle_children(profile, hat, root):
                             changed[profile] = True
 
                             if exec_mode & str_to_mode('i'):
-                                #if 'perl' in exec_target:
+                                interpreter_path, abstraction = get_interpreter_and_abstraction(exec_target)
-                                #    aa[profile][hat]['include']['abstractions/perl'] = True
-                                #elif '/bin/bash' in exec_target or '/bin/sh' in exec_target:
-                                #    aa[profile][hat]['include']['abstractions/bash'] = True
-                                hashbang = head(exec_target)
-                                if hashbang.startswith('#!'):
-                                    interpreter = hashbang[2:].strip()
-                                    interpreter_path = get_full_path(interpreter)
-                                    interpreter = re.sub('^(/usr)?/bin/', '', interpreter_path)
 
+                                if interpreter_path:
                                     aa[profile][hat]['allow']['path'][interpreter_path]['mode'] = aa[profile][hat]['allow']['path'][interpreter_path].get('mode', str_to_mode('ix')) | str_to_mode('ix')
 
                                     aa[profile][hat]['allow']['path'][interpreter_path]['audit'] = aa[profile][hat]['allow']['path'][interpreter_path].get('audit', set())
 
-                                    if interpreter == 'perl':
+                                    if abstraction:
-                                        aa[profile][hat]['include']['abstractions/perl'] = True
+                                        aa[profile][hat]['include'][abstraction] = True
-                                    elif interpreter in ['bash', 'dash', 'sh']:
-                                        aa[profile][hat]['include']['abstractions/bash'] = True
 
                     # Update tracking info based on kind of change
 

utils/test/test-aa.py

@@ -13,7 +13,9 @@ import unittest
 from common_test import AATest, setup_all_loops
 from common_test import read_file, write_file
 
-from apparmor.aa import (check_for_apparmor, create_new_profile,
+import os
+
+from apparmor.aa import (check_for_apparmor, get_interpreter_and_abstraction, create_new_profile,
      get_profile_flags, set_profile_flags, is_skippable_file, is_skippable_dir,
      parse_profile_start, parse_profile_data, separate_vars, store_list_var, write_header, serialize_parse_profile_start)
 from apparmor.common import AppArmorException, AppArmorBug
@@ -97,6 +99,47 @@ class AaTest_create_new_profile(AATest):
         else:
             self.assertEqual(profile[program][program]['include'].keys(), {'abstractions/base'})
 
+class AaTest_get_interpreter_and_abstraction(AATest):
+    tests = [
+        ('#!/bin/bash',             ('/bin/bash',           'abstractions/bash')),
+        ('#!/bin/dash',             ('/bin/dash',           'abstractions/bash')),
+        ('#!/bin/sh',               ('/bin/sh',             'abstractions/bash')),
+        ('#!  /bin/sh  ',           ('/bin/sh',             'abstractions/bash')),
+        ('#!/usr/bin/perl',         ('/usr/bin/perl',       'abstractions/perl')),
+        ('#!/usr/bin/python',       ('/usr/bin/python',     'abstractions/python')),
+        ('#!/usr/bin/python2',      ('/usr/bin/python2',    'abstractions/python')),
+        ('#!/usr/bin/python2.7',    ('/usr/bin/python2.7',  'abstractions/python')),
+        ('#!/usr/bin/python3',      ('/usr/bin/python3',    'abstractions/python')),
+        ('#!/usr/bin/python4',      ('/usr/bin/python4',    None)),  # python abstraction is only applied to py2 and py3
+        ('#!/usr/bin/ruby',         ('/usr/bin/ruby',       'abstractions/ruby')),
+        ('#!/usr/bin/foobarbaz',    ('/usr/bin/foobarbaz',  None)),  # we don't have an abstraction for "foobarbaz"
+        ('foo',                     (None,                  None)),  # no hashbang - not a script
+    ]
+
+    def _run_test(self, params, expected):
+        exp_interpreter_path, exp_abstraction = expected
+
+        program = self.writeTmpfile('program', "%s\nfoo\nbar" % params)
+        interpreter_path, abstraction = get_interpreter_and_abstraction(program)
+
+        # damn symlinks!
+        if exp_interpreter_path and os.path.islink(exp_interpreter_path):
+            dirname = os.path.dirname(exp_interpreter_path)
+            exp_interpreter_path = os.readlink(exp_interpreter_path)
+            if not exp_interpreter_path.startswith('/'):
+                exp_interpreter_path = os.path.join(dirname, exp_interpreter_path)
+
+        self.assertEqual(interpreter_path, exp_interpreter_path)
+        self.assertEqual(abstraction, exp_abstraction)
+
+    def test_special_file(self):
+        self.assertEqual((None, None), get_interpreter_and_abstraction('/dev/null'))
+
+    def test_file_not_found(self):
+        self.createTmpdir()
+        self.assertEqual((None, None), get_interpreter_and_abstraction('%s/file-not-found' % self.tmpdir))
+
+
 class AaTest_get_profile_flags(AaTestWithTempdir):
     def _test_get_flags(self, profile_header, expected_flags):
         file = write_file(self.tmpdir, 'profile', '%s {\n}\n' % profile_header)