[apparmor] [patch] aa.py: let parse_audit_allow also match comments

[Kshitij Gupta]

Hello,

On Sat, Nov 8, 2014 at 3:56 AM, Christian Boltz <apparmor at cboltz.de> wrote:

> Hello,
>
> Am Freitag, 7. November 2014 schrieb Christian Boltz:
> > This patch changes parse_audit_allow() in aa.py to also return
> > comments at the end of the line.
> >
> > This comment is not yet stored or used (except in the TODO note ;-)
> > but I'd like to have it available from the beginning while writing
> > the rule classes.
>
> Here's v2 which strip()s whitespace at the end of the comment.
>
>
> === modified file 'utils/apparmor/aa.py'
> --- utils/apparmor/aa.py        2014-10-20 20:40:42 +0000
> +++ utils/apparmor/aa.py        2014-11-07 22:24:06 +0000
> @@ -2752,8 +2752,8 @@
>              if not profile:
>                  raise AppArmorException(_('Syntax Error: Unexpected
> capability entry found in file: %(file)s line: %(line)s') % { 'file': file,
> 'line': lineno + 1 })
>
> -            audit, allow, allow_keyword = parse_audit_allow(matches)
> -            # TODO: honor allow_keyword
> +            audit, allow, allow_keyword, comment =
> parse_audit_allow(matches)
> +            # TODO: honor allow_keyword and comment
>
>              capability = ALL
>              if matches.group('capability'):
> @@ -2870,8 +2870,8 @@
>              if not profile:
>                  raise AppArmorException(_('Syntax Error: Unexpected bare
> file rule found in file: %(file)s line: %(line)s') % { 'file': file,
> 'line': lineno + 1 })
>
> -            audit, allow, allow_keyword = parse_audit_allow(matches)
> -            # TODO: honor allow_keyword
> +            audit, allow, allow_keyword, comment =
> parse_audit_allow(matches)
> +            # TODO: honor allow_keyword and comment
>
>              mode = apparmor.aamode.AA_BARE_FILE_MODE
>              if not matches.group('owner'):
> @@ -3222,7 +3222,12 @@
>          if allow != 'allow' and allow != 'deny':  # should never happen
>              raise AppArmorException(_("Invalid allow/deny keyword %s" %
> allow))
>
> -    return (audit, allow, allow_keyword)
> +    comment = ''
> +    if matches.group('comment'):
> +        # include a space so that we don't need to add it everywhere when
> writing the rule
>
+        comment = ' %s' % matches.group('comment').strip()
>

Would the superfluous spaces be better dealt in the regex RE_EOL by
ignoring them from comment group?
That would save us many such calls.

+
> +    return (audit, allow, allow_keyword, comment)
>
>  # RE_DBUS_ENTRY = re.compile('^dbus\s*()?,\s*$')
>  #   use stuff like '(?P<action>(send|write|w|receive|read|r|rw))'
>
>
>
Thanks for the patch.

Looks good and doesn't seem it would break anything.

If above question is answered to true then ack is for v1 (with a followup
on the regex) else v2 is good.

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

Regards,

Kshitij Gupta

>
>
> Regards,
>
> Christian Boltz
> --
> > Und bin grad auch zu faul nachzuschauen. Das hab ich schlicht
> > aus den autoconf-Makefiles... ;)
> Automake-Makefiles, autoconf-Makefile's gibt es nicht ;)
> [> David Haller und Ralf Corsepius in suse-programming]
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20141108/7403c9b2/attachment.html>