[apparmor] [patch 14/XX] convert af_unix rules to support name= rather than path=

Fri Aug 22 20:17:41 UTC 2014

On 08/22/2014 12:55 PM, Steve Beattie wrote:
> This patch converts the path= modifier to the af_unix rules to use
> name= instead. The reasoning here is that the audit log messages will
> be using the keyword name instead of path, to be consistent with the
> file log messages which report as name.
> 
> I'm... ambivalent about this patch, as the unix(7) documentation refers
> to paths as well as netstat -x output. The file rejection logs are the
> outlier here by referring to paths as names, but the keyword for them
> doesn't get used in apparmor policy, unlike for the new unix socket
> mediation. I think our options are:
> 
>   1) Take this patch and make the kernel rejection messages for af_unix
>      refer to 'name', and accept the inconsistency with unix(7)
>      documentation.
> 
we could

>   2) Keep using the path keyword in the userspace tools while leaving
>      the logging consistent by using the name keyword there, and
>      documenting the inconsistency between the log messages and the
>      language.
> 
No

>   3) Use the path keyword in userspace and in af_unix log messages, and
>      accept the inconsistency between af_unix and file log messages
>      (with perhaps a long term plan to move the file log keyword to
>      path).
> 
better

>   4) Keep the name keyword in the af_unix log messages to be consistent
>      with file messages, but modify the parser to accept either the
>      file= or path= keywords, as alternate ways of specifying the same
>      thing, and accept the implementation complexity in the userspace
>      tools, as well as potential user confusion over the keywords.
> 
not file= perhaps you meant name=

So just to reiterate the abstract socket and file path name auditing
take different paths. There is no need to have them be the same
except for consistency in audit log naming.

However are we going to use name= for ipv4 and ipv6? Should the
abstract address be more consistent with those?

laddr and faddr? For the auditing providing by lsm audit? Not that
that really fits our logging either but ...

> This patch applies on top of the patch series that John posted.
> 
> Signed-off-by: Steve Beattie <steve at nxnw.org>
> ---
>  parser/af_unix.cc                            |    2 
>  parser/apparmor.d.pod                        |   56 +++++++++++++--------------
>  parser/tst/simple_tests/unix/bad_bind_1.sd   |    2 
>  parser/tst/simple_tests/unix/bad_bind_2.sd   |    2 
>  parser/tst/simple_tests/unix/bad_peer_1.sd   |    4 -
>  parser/tst/simple_tests/unix/bad_regex_01.sd |    4 -
>  parser/tst/simple_tests/unix/bad_regex_02.sd |    2 
>  parser/tst/simple_tests/unix/bad_regex_04.sd |    4 -
>  parser/tst/simple_tests/unix/ok_bind_1.sd    |    2 
>  parser/tst/simple_tests/unix/ok_msg_7.sd     |    2 
>  parser/tst/simple_tests/unix/ok_msg_8.sd     |    2 
>  parser/tst/simple_tests/unix/ok_msg_9.sd     |    2 
>  12 files changed, 43 insertions(+), 41 deletions(-)
> 
> Index: b/parser/af_unix.cc
> ===================================================================
> --- a/parser/af_unix.cc
> +++ b/parser/af_unix.cc
> @@ -37,7 +37,7 @@ int parse_unix_mode(const char *str_mode
>  
>  
>  static struct supported_cond supported_conds[] = {
> -	{ "path", true, false, false, either_cond },
> +	{ "name", true, false, false, either_cond },
>  	{ NULL, false, false, false, local_cond },	/* sentinal */
>  };
>  
> Index: b/parser/apparmor.d.pod
> ===================================================================
> --- a/parser/apparmor.d.pod
> +++ b/parser/apparmor.d.pod
> @@ -175,13 +175,13 @@ B<TYPE COND> = 'type' '='  ( <AARE> | '(
>  
>  B<PROTO COND> = 'protocol' '='  ( <AARE> | '(' ( '"' <AARE> '"' | <AARE> )+ ')' )
>  
> -B<UNIX LOCAL EXPR> = ( I<UNIX PATH COND> | I<UNIX LABEL COND> | I<UNIX ATTR COND> | I<UNIX OPT COND> )*
> +B<UNIX LOCAL EXPR> = ( I<UNIX NAME COND> | I<UNIX LABEL COND> | I<UNIX ATTR COND> | I<UNIX OPT COND> )*
>   each cond can appear at most once
>  
> -B<UNIX PEER EXPR> = 'peer' '=' ( I<UNIX PATH COND> | I<UNIX LABEL COND> )+
> +B<UNIX PEER EXPR> = 'peer' '=' ( I<UNIX NAME COND> | I<UNIX LABEL COND> )+
>   each cond can appear at most once
>  
> -B<UNIX PATH COND> 'path' '=' ( <AARE> | '(' '"' <AARE> '"' | <AARE> ')' )
> +B<UNIX NAME COND> 'name' '=' ( <AARE> | '(' '"' <AARE> '"' | <AARE> ')' )
>  
>  B<UNIX LABEL COND> 'label' '=' ( <AARE> | '(' '"' <AARE> '"' | <AARE> ')' )
>  
> @@ -898,25 +898,27 @@ domain sockets, see man 7 unix for more
>  =head3 Unix socket paths
>  
>  The path component of a unix domain socket is specified by the
> -  path=
> -conditional. If a path conditional is not specified as part of a rule
> +  name=
> +
> +conditional. If a name conditional is not specified as part of a rule
>  then the rule matches both abstract and anonymous sockets.
>  
> -In apparmor the path of an abstract unix domain socket begins with the
> -I<@> character, similar to how they are reported by netstat -x. The name
> -then follows and may contain pattern matching and any characters including
> -the null character. In apparmor null characters must be specified by using
> -an escape sequence I<\000> or I<\x00>. The pattern matching is the same
> -as is used by path matching so * will not match I</> even though it
> -has no special meaning with in an abstract socket name. Eg.
> -  unix path=@*,
> -
> -Anonymous unix domain sockets have no path associated with them, however
> -it can be specified with the special I<none> keyword to indicate the
> -rule only applies to anonymous unix domain sockets. Eg.
> -  unix path=none,
> +In apparmor the name of an abstract unix domain socket begins with
> +the I<@> character, similar to how they are reported (as paths) by
> +netstat -x. The name then follows and may contain pattern matching
> +and any characters including the null character. In apparmor null
> +characters must be specified by using an escape sequence I<\000> or
> +I<\x00>. The pattern matching is the same as is used by path matching
> +so * will not match I</> even though it has no special meaning with
> +in an abstract socket name. Eg.
> +  unix name=@*,
> +
> +Anonymous unix domain sockets have no path name associated with
> +them, however it can be specified with the special I<none> keyword
> +to indicate the rule only applies to anonymous unix domain sockets. Eg.
> +  unix name=none,
>  
> -If the path component of a rule is not specified then the rule applies
> +If the name component of a rule is not specified then the rule applies
>  to both abstract and anonymous sockets.
>  
>  =head3 Unix socket permissions
> @@ -961,20 +963,20 @@ create, bind, listen, shutdown, getattr,
>  
>    unix type=dgram,
>  
> -  unix path=none
> +  unix name=none
>  
> -  unix path=@foo,
> +  unix name=@foo,
>  
> -  unix type=stream path=@foo,
> +  unix type=stream name=@foo,
>  
> -  unix server path=@foo,
> +  unix server name=@foo,
>  
> -  unix accept path=@foo peer=(label=/bar),
> +  unix accept name=@foo peer=(label=/bar),
>  
> -  unix receive path=@foo peer=(label=/bar),
> +  unix receive name=@foo peer=(label=/bar),
>  
>  
> -  unix path=none
> +  unix name=none
>  
>  
>  =head3 Abstract unix domain sockets autobind
> @@ -1000,7 +1002,7 @@ Eg.
>  Fine grained mediation rules however can not be lossly converted back
>  to the coarse grained network rule. Eg
>  
> -   unix bind path=@example,
> +   unix bind name=@example,
>  
>  Has no exact match under coarse grained network rules, the closest match is
>  the much wider permission rule of.
> Index: b/parser/tst/simple_tests/unix/bad_bind_1.sd
> ===================================================================
> --- a/parser/tst/simple_tests/unix/bad_bind_1.sd
> +++ b/parser/tst/simple_tests/unix/bad_bind_1.sd
> @@ -4,5 +4,5 @@
>  #
>  
>  profile foo {
> -  unix bind peer=(path=@foo ),
> +  unix bind peer=(name=@foo ),
>  }
> Index: b/parser/tst/simple_tests/unix/bad_bind_2.sd
> ===================================================================
> --- a/parser/tst/simple_tests/unix/bad_bind_2.sd
> +++ b/parser/tst/simple_tests/unix/bad_bind_2.sd
> @@ -4,5 +4,5 @@
>  #
>  
>  profile foo {
> -  unix bind label=foo path=@bar,
> +  unix bind label=foo name=@bar,
>  }
> Index: b/parser/tst/simple_tests/unix/bad_peer_1.sd
> ===================================================================
> --- a/parser/tst/simple_tests/unix/bad_peer_1.sd
> +++ b/parser/tst/simple_tests/unix/bad_peer_1.sd
> @@ -3,7 +3,7 @@
>  #=EXRESULT FAIL
>  #
>  
> -# path must be none for anonymous or start with @ for abstract
> +# path name must be none for anonymous or start with @ for abstract
>  profile foo {
> -  unix send peer(path=wat),
> +  unix send peer(name=wat),
>  }
> Index: b/parser/tst/simple_tests/unix/bad_regex_01.sd
> ===================================================================
> --- a/parser/tst/simple_tests/unix/bad_regex_01.sd
> +++ b/parser/tst/simple_tests/unix/bad_regex_01.sd
> @@ -1,8 +1,8 @@
>  #
> -#=DESCRIPTION unix rule with a bad path regex expansion
> +#=DESCRIPTION unix rule with a bad name regex expansion
>  #=EXRESULT FAIL
>  #
>  
>  profile foo {
> -  unix send path=@foo{one,two peer=(label=splat),
> +  unix send name=@foo{one,two peer=(label=splat),
>  }
> Index: b/parser/tst/simple_tests/unix/bad_regex_02.sd
> ===================================================================
> --- a/parser/tst/simple_tests/unix/bad_regex_02.sd
> +++ b/parser/tst/simple_tests/unix/bad_regex_02.sd
> @@ -4,5 +4,5 @@
>  #
>  
>  profile foo {
> -  unix bind path=abcd]efg,
> +  unix bind name=abcd]efg,
>  }
> Index: b/parser/tst/simple_tests/unix/bad_regex_04.sd
> ===================================================================
> --- a/parser/tst/simple_tests/unix/bad_regex_04.sd
> +++ b/parser/tst/simple_tests/unix/bad_regex_04.sd
> @@ -1,8 +1,8 @@
>  #
> -#=DESCRIPTION unix rule with a bad path regex expansion
> +#=DESCRIPTION unix rule with a bad path name regex expansion
>  #=EXRESULT FAIL
>  #
>  
>  profile foo {
> -  unix send path=/some/random/{path peer=(label=splat),
> +  unix send name=/some/random/{path peer=(label=splat),
>  }
> Index: b/parser/tst/simple_tests/unix/ok_bind_1.sd
> ===================================================================
> --- a/parser/tst/simple_tests/unix/ok_bind_1.sd
> +++ b/parser/tst/simple_tests/unix/ok_bind_1.sd
> @@ -3,5 +3,5 @@
>  #=EXRESULT PASS
>  
>  profile a_profile {
> -  unix path=@SomeService,
> +  unix name=@SomeService,
>  }
> Index: b/parser/tst/simple_tests/unix/ok_msg_7.sd
> ===================================================================
> --- a/parser/tst/simple_tests/unix/ok_msg_7.sd
> +++ b/parser/tst/simple_tests/unix/ok_msg_7.sd
> @@ -3,5 +3,5 @@
>  #=EXRESULT PASS
>  
>  profile a_profile {
> -  unix (send) path=none,
> +  unix (send) name=none,
>  }
> Index: b/parser/tst/simple_tests/unix/ok_msg_8.sd
> ===================================================================
> --- a/parser/tst/simple_tests/unix/ok_msg_8.sd
> +++ b/parser/tst/simple_tests/unix/ok_msg_8.sd
> @@ -3,5 +3,5 @@
>  #=EXRESULT PASS
>  
>  profile a_profile {
> -  unix (send) path=@foo,
> +  unix (send) name=@foo,
>  }
> Index: b/parser/tst/simple_tests/unix/ok_msg_9.sd
> ===================================================================
> --- a/parser/tst/simple_tests/unix/ok_msg_9.sd
> +++ b/parser/tst/simple_tests/unix/ok_msg_9.sd
> @@ -3,5 +3,5 @@
>  #=EXRESULT PASS
>  
>  profile a_profile {
> -  unix (send) peer=(path=@foo),
> +  unix (send) peer=(name=@foo),
>  }
> 
> 
> 