[apparmor] [patch] merge script handling into get_interpreter_and_abstraction()

Kshitij Gupta kgupta8592 at gmail.com
Sun Oct 18 19:42:03 UTC 2015


Hello,

On Sun, Oct 18, 2015 at 8:17 PM, Christian Boltz <apparmor at cboltz.de> wrote:

> Hello,
>
> 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.
>
>
> [ 94-merge-script-handling-into-get_interpreter_and_abstraction.diff ]
>
Ahh, a patch a just reviewed was based on the above. I was wondering why I
couldn't find it on my local bzr.


>
> --- utils/apparmor/aa.py        2015-10-18 16:31:12.192164267 +0200
> +++ utils/apparmor/aa.py        2015-10-18 16:37:09.794351825 +0200
> @@ -397,6 +397,42 @@
>          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
>
Add a new line here.

> +    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)
>
Please look for some suggestions in my reply for patch 95 (*wow*, close to
100 patches!)

+    interpreter = re.sub('^(/usr)?/bin/', '', interpreter_path)
>
Consider moving the regex outside and compiling it beforehand.

+
> +    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 +475,9 @@
>      local_profile[localfile]['include']['abstractions/base'] = 1
>
>      if os.path.exists(localfile) and os.path.isfile(localfile):
> -        hashbang = head(localfile)
> -        if hashbang.startswith('#!'):
> -            interpreter_path =
> get_full_path(hashbang.lstrip('#!').strip())
> -
> -            interpreter = re.sub('^(/usr)?/bin/', '', interpreter_path)
> +        interpreter_path, abstraction =
> get_interpreter_and_abstraction(localfile)
>
> +        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 +486,9 @@
>
>
>  local_profile[localfile]['allow']['path'][interpreter_path]['audit'] =
> local_profile[localfile]['allow']['path'][interpreter_path].get('audit',
> set())
>
> -            if interpreter == 'perl':
> -                local_profile[localfile]['include']['abstractions/perl']
> = 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
> +            if abstraction:
> +                local_profile[localfile]['include'][abstraction] = True
> +
>              handle_binfmt(local_profile[localfile], interpreter_path)
>          else:
>
> @@ -1407,24 +1435,15 @@
>                              changed[profile] = True
>
>                              if exec_mode & str_to_mode('i'):
> -                                #if 'perl' in 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)
> +                                interpreter_path, abstraction =
> get_interpreter_and_abstraction(exec_target)
>
> +                                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':
> -
> aa[profile][hat]['include']['abstractions/perl'] = True
> -                                    elif interpreter in ['bash', 'dash',
> 'sh']:
> -
> aa[profile][hat]['include']['abstractions/bash'] = True
> +                                    if abstraction:
> +
> aa[profile][hat]['include'][abstraction] = True
>
>                      # Update tracking info based on kind of change
>
> --- utils/test/test-aa.py       2015-10-18 16:19:32.501921777 +0200
> +++ utils/test/test-aa.py       2015-10-18 16:18:51.129325349 +0200
> @@ -13,7 +13,9 @@
>  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 @@
>          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!
>
lol

+        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)
>
> Thanks for the patch.

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


> Regards,
>
> Christian Boltz
> --
> Disclaimer: In case you are either 1) a complete idiot; or 2) a lawyer;
> or 3) both, please be aware that [...]   [from fixubuntu.com]
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
>



-- 
Regards,

Kshitij Gupta
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20151019/b782002c/attachment-0001.html>


More information about the AppArmor mailing list