[apparmor] [PATCH 2/3] apparmor.d.pod: refactor profile file, profile, subprofile, hat patterns

[Christian Boltz]

Hello,

Am Mittwoch, 25. März 2015 schrieb John Johansen:
> Signed-off-by: John Johansen <john.johansen at canonical.com>
> ---
>  parser/apparmor.d.pod | 46
> ++++++++++++++++++++++++++++++---------------- 1 file changed, 30
> insertions(+), 16 deletions(-)
> 
> diff --git a/parser/apparmor.d.pod b/parser/apparmor.d.pod
> index 3b4e4e9..9d2664d 100644
> --- a/parser/apparmor.d.pod
> +++ b/parser/apparmor.d.pod
> @@ -44,6 +44,12 @@ to the policy; this behaviour is modelled after
> cpp(1).
> 
>  =over 4
> 
> +B<PROFILE FILE> = I<PREAMBLE> I<PROFILES>
> +
> +B<PREAMBLE> = ( I<COMMENT> | I<VARIABLE ASSIGNMENT> | I<INCLUDE> )*
> +
> +B<PROFILES> = ( B<PROFILE> )*

Strictly speaking, this is not 100% correct. INCLUDE can also appear
between two profiles, not only in the PREAMBLE.

In practise, this is corner-case enough to ignore it here ;-)

The same applies for comments between profiles, which is probably used 
more often - OTOH I'm sure nobody will consult the manpage to find out 
where adding a comment is allowed ;-)

> -B<PROFILE> = [ I<COMMENT> ... ] [ I<VARIABLE ASSIGNMENT> ... ] ( '"'
> I<PROGRAM> '"' | I<PROGRAM> ) [ 'flags=(complain)' ]'{' ( I<RULES> )*
> '}'
> +B<PROFILE> = ( I<FILEGLOB> | I<PROFILE NAME> ) [ I<ATTACHMENT
> SPECIFICATION> ] [ <PROFILE FLAG CONDS> ] I<BLOCK> 
> +
> +B<PROFILE NAME> = 'profile' I<AARE>

Even with FILEGLOB, the 'profile' keyword is allowed (but optional), so 
this should be
    B<PROFILE> = ( [ 'profile '] I<FILEGLOB> | I<PROFILE NAME> ) ...

The more readable solution is probably:
    B<PROFILE> = I<PROFILE NAME> ...
    B<PROFILE NAME> = [ 'profile ' ] I<FILEGLOB> | 'profile' I<AARE>

BTW: I don't like BLOCK too much and would prefer to keep 
    '{' ( I<RULES> )* '}'
even if we have to repeat that for hats and subprofiles. In this case, 
readability should win over de-duplication ;-)

> +B<ATTACHMENT SPECIFICATION> = I<FILEGLOB>
> +
> +B<PROFILE FLAG CONDS> = 'flags=(' comma or white space separated list
> of I<PROFILE FLAGS> ')'

"flags=" is optional ;-)

> +B<PROFILE FLAGS> = 'complain' | 'enforce' | 'mediate_deleted' |
> attach_disconnected'

That list is incomplete, see create-apparmor.vim.py ;-)

aa_flags = ['complain',
            'audit',
            'attach_disconnected',
            'no_attach_disconnected',
            'chroot_attach',
            'chroot_no_attach',
            'chroot_relative',
            'namespace_relative',
            'mediate_deleted',
            'delegate_deleted']

> +B<BLOCK> = '{' ( I<RULES> )* '}'

See above for my opinion about BLOCK ;-)

> +B<SUBPROFILE> = I<PROFILE NAME> [ I<ATTACHMENT SPECIFICATION> ] [
> <PROFILE FLAG CONDS> ] I<BLOCK> 
> +
> +B<HAT> = ('hat' | '^') I<HATNAME> [ <PROFILE FLAG CONDS> ] I<BLOCK>

Should be a space after the hat keyword? -> 'hat '

(same question for some other keywords like 'profile' ;-)

> +B<HATNAME> = '^'  (non-whitespace characters; see aa_change_hat(2)

A quick test shows that "non-whitespace" isn't correct - for example,
^!foo {}  causes a parser error ;-)  Please specify what exactly is 
allowed.


Regards,

Christian Boltz
-- 
Always file a bug: if it's not in Bugzilla, then it's not there ;)
[Pascal Bleser in opensuse-factory]