[apparmor] [patch] convert serialize_parse_profile_start() to use parse_profile_start_line()

[Steve Beattie]

On Thu, Mar 05, 2015 at 09:19:01PM +0100, Christian Boltz wrote:
> Hello,
> 
> this patch converts serialize_parse_profile_start() to use 
> parse_profile_start_line(), and adjusts a test to expect an AppArmorBug 
> instead of an AttributeError exception.
> 
> The patch also adds two tests (they succeed with the old and the new code).
> Note that these tests document interesting[tm] behaviour - I tend to
> think that those cases should raise an exception, but I'm not sure about
> this because serialize_profile_from_old_profile() is a good example for
> interesting[tm] code :-/
> 
> I couldn't come up with a real-world test profile that would hit those 
> cases without erroring out aa-logprof earlier - maybe the (more 
> sane-looking) parse_profiles() / serialize_parse_profile_start() 
> protects us from hitting this interesting[tm] behaviour.
> 
> 
> I propose this patch for trunk and 2.9.

Acked-by: Steve Beattie <steve at nxnw.org>

> [ 14-convert-serialize_parse_profile_start-to-use-parse_profile_start_line.diff ]


> 
> --- utils/apparmor/aa.py        2015-03-04 23:40:36.994571322 +0100
> +++ utils/apparmor/aa.py        2015-03-05 20:55:05.702792046 +0100
> @@ -3707,17 +3707,15 @@ def serialize_profile(profile_data, name
>      return string + '\n'
>  
>  def serialize_parse_profile_start(line, file, lineno, profile, hat, prof_data_profile, prof_data_external, correct):
> -    matches = RE_PROFILE_START.search(line).groups()
> -    if profile and profile == hat and matches[3]:
> -        hat = matches[3]
> +    matches = parse_profile_start_line(line, file)
> +
> +    if profile and profile == hat and matches['profile_keyword']:
> +        hat = matches['profile']
>          in_contained_hat = True
>          if prof_data_profile:
>              pass
>      else:
> -        if matches[1]:
> -            profile = matches[1]
> -        else:
> -            profile = matches[3]
> +        profile = matches['profile']
>          if len(profile.split('//')) >= 2:
>              profile, hat = profile.split('//')[:2]
>          else:
> @@ -3728,10 +3726,7 @@ def serialize_parse_profile_start(line,
>          else:
>              hat = profile
>  
> -    flags = matches[6]
> -    profile = strip_quotes(profile)
> -    if hat:
> -        hat = strip_quotes(hat)
> +    flags = matches['flags']
>  
>      return (profile, hat, flags, in_contained_hat, correct)
>  
> --- utils/test/test-aa.py       2015-03-05 19:35:43.910167560 +0100
> +++ utils/test/test-aa.py       2015-03-05 20:40:14.951079779 +0100
> @@ -261,9 +261,19 @@ class AaTest_serialize_parse_profile_sta
>          expected = ('/foo', '/foo', None, False, True) # note that in_contained_hat == False and that profile == hat == child profile
>          self.assertEqual(result, expected)
>  
> +    def test_serialize_parse_profile_start_14(self):
> +        result = self._parse('/ext//hat {', '/bar', '/bar', True, True) # external hat inside a profile - XXX should this error out?
> +        expected = ('/ext', '/ext', None, False, True) # XXX additionally note that hat == profile, but should be 'hat'
> +        self.assertEqual(result, expected)
> +
> +    def test_serialize_parse_profile_start_15(self):
> +        result = self._parse('/ext//hat {', '/bar', '/bar', True, False) # external hat inside a profile - XXX should this error out?
> +        expected = ('/ext', 'hat', None, False, False)
> +        self.assertEqual(result, expected)

Interestingly, the parser won't accept these:

  $ echo 'profile foo { /ext//hat { /bin/true r, } /bin/false r, }'  | apparmor_parser -dd -QK
  AppArmor parser error, in stdin line 1: syntax error, unexpected TOK_OPEN, expecting TOK_MODE

but will accept it when given the profile keyword:

  $ echo 'profile foo { profile /ext//hat { /bin/true r, } /bin/false r, }'  | apparmor_parser -dd -QK
  ----- Debugging built structures -----
  Name:           foo
  Profile Mode:   Enforce
  --- Entries ---
  Mode:   r:r     Name:   (/bin/false)
  Mode:   w:      Name:   (/proc/[0-9]*/attr/current)

  Name:           /ext//hat
  Local To:       foo
  Profile Mode:   Enforce
  --- Entries ---
  Mode:   r:r     Name:   (/bin/true)

I suspect you're correct that the tools should raise an exception for
the cases you added.

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150331/57ce40d1/attachment.pgp>