[apparmor] [patch 04/12] And this version actually implements it. Le sigh. But hurrah for having testcases so that it was possible to discover that this was the case.
Seth Arnold
seth.arnold at canonical.com
Tue Aug 26 23:28:58 UTC 2014
On Mon, Aug 25, 2014 at 05:06:09PM -0700, john.johansen at canonical.com wrote:
> Signed-off-by: Steve Beattie <steve at nxnw.org>
Acked-by: Seth Arnold <seth.arnold at canonical.com>
Minor suggestions inline.
Thanks
> ---
> parser/af_unix.cc | 46 +++++++++----------
> parser/af_unix.h | 10 ++--
> parser/apparmor.d.pod | 64 +++++++++++++--------------
> 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
> 13 files changed, 74 insertions(+), 72 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 },
> + { "addr", true, false, false, either_cond },
> { NULL, false, false, false, local_cond }, /* sentinal */
> };
>
> @@ -53,10 +53,10 @@ void unix_rule::move_conditionals(struct
> ent->name);
> continue;
> }
> - if (strcmp(ent->name, "path") == 0) {
> - move_conditional_value("unix socket", &path, ent);
> - if (path[0] != '@' && strcmp(path, "none") != 0)
> - yyerror("unix rule: invalid value for path='%s'\n", path);
> + if (strcmp(ent->name, "addr") == 0) {
> + move_conditional_value("unix socket", &addr, ent);
> + if (addr[0] != '@' && strcmp(addr, "none") != 0)
> + yyerror("unix rule: invalid value for addr='%s'\n", addr);
> }
>
> /* TODO: add conditionals for
> @@ -81,16 +81,16 @@ void unix_rule::move_peer_conditionals(s
> ent->name);
> continue;
> }
> - if (strcmp(ent->name, "path") == 0) {
> - move_conditional_value("unix", &peer_path, ent);
> - if (peer_path[0] != '@' && strcmp(path, "none") != 0)
> - yyerror("unix rule: invalid value for path='%s'\n", peer_path);
> + if (strcmp(ent->name, "addr") == 0) {
> + move_conditional_value("unix", &peer_addr, ent);
> + if (peer_addr[0] != '@' && strcmp(addr, "none") != 0)
> + yyerror("unix rule: invalid value for addr='%s'\n", peer_addr);
> }
> }
> }
>
> unix_rule::unix_rule(unsigned int type_p, bool audit_p, bool denied):
> - af_rule("unix"), path(NULL), peer_path(NULL)
> + af_rule("unix"), addr(NULL), peer_addr(NULL)
> {
> if (type_p != 0xffffffff) {
> sock_type_n = type_p;
> @@ -105,7 +105,7 @@ unix_rule::unix_rule(unsigned int type_p
>
> unix_rule::unix_rule(int mode_p, struct cond_entry *conds,
> struct cond_entry *peer_conds):
> - af_rule("unix"), path(NULL), peer_path(NULL)
> + af_rule("unix"), addr(NULL), peer_addr(NULL)
> {
> move_conditionals(conds);
> move_peer_conditionals(peer_conds);
> @@ -138,16 +138,16 @@ unix_rule::unix_rule(int mode_p, struct
> ostream &unix_rule::dump_local(ostream &os)
> {
> af_rule::dump_local(os);
> - if (path)
> - os << "path='" << path << "'";
> + if (addr)
> + os << "addr='" << addr << "'";
> return os;
> }
>
> ostream &unix_rule::dump_peer(ostream &os)
> {
> af_rule::dump_peer(os);
> - if (peer_path)
> - os << "path='" << peer_path << "'";
> + if (peer_addr)
> + os << "addr='" << peer_addr << "'";
> return os;
> }
>
> @@ -157,10 +157,10 @@ int unix_rule::expand_variables(void)
> int error = af_rule::expand_variables();
> if (error)
> return error;
> - error = expand_entry_variables(&path);
> + error = expand_entry_variables(&addr);
> if (error)
> return error;
> - error = expand_entry_variables(&peer_path);
> + error = expand_entry_variables(&peer_addr);
> if (error)
> return error;
>
> @@ -274,12 +274,12 @@ int unix_rule::gen_policy_re(Profile &pr
> }
>
> /* local addr */
> - if (path) {
> - if (strcmp(path, "none") == 0) {
> + if (addr) {
> + if (strcmp(addr, "none") == 0) {
> buffer << "\\x01";
> } else {
> /* skip leading @ */
> - ptype = convert_aaregex_to_pcre(path + 1, 0, buf, &pos);
> + ptype = convert_aaregex_to_pcre(addr + 1, 0, buf, &pos);
> if (ptype == ePatternInvalid)
> goto fail;
> /* kernel starts abstract with \0 */
> @@ -357,12 +357,12 @@ int unix_rule::gen_policy_re(Profile &pr
> buffer << "\\x" << std::setfill('0') << std::setw(2) << std::hex << CMD_ADDR;
>
> /* peer addr */
> - if (peer_path) {
> - if (strcmp(peer_path, "none") == 0) {
> + if (peer_addr) {
> + if (strcmp(peer_addr, "none") == 0) {
> buffer << "\\x01";
> } else {
> /* skip leading @ */
> - ptype = convert_aaregex_to_pcre(peer_path + 1, 0, buf, &pos);
> + ptype = convert_aaregex_to_pcre(peer_addr + 1, 0, buf, &pos);
> if (ptype == ePatternInvalid)
> goto fail;
> /* kernel starts abstract with \0 */
> 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 ADDRESS 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 ADDRESS COND> | I<UNIX LABEL COND> )+
> each cond can appear at most once
>
> -B<UNIX PATH COND> 'path' '=' ( <AARE> | '(' '"' <AARE> '"' | <AARE> ')' )
> +B<UNIX ADDRESS COND> 'addr' '=' ( <AARE> | '(' '"' <AARE> '"' | <AARE> ')' )
>
> B<UNIX LABEL COND> 'label' '=' ( <AARE> | '(' '"' <AARE> '"' | <AARE> ')' )
>
> @@ -897,26 +897,28 @@ 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
> -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,
> +The path address component of a unix domain socket is specified by the
> + addr=
"path address component" reads awkwardly, "address" would make more sense
to me.
>
> -If the path component of a rule is not specified then the rule applies
> +conditional. If an address conditional is not specified as part of
> +a rule then the rule matches both abstract and anonymous sockets.
> +
> +In apparmor the address of an abstract unix domain socket begins with
> +the I<@> character, similar to how they are reported (as paths) by
> +netstat -x. The address 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 addr=@*,
This should probably mention that "unix addr=@*," is liable to
occasionally fail if the abstract socket names are randomly generated.
"unix addr=@**," should be mentioned here too.
> +
> +Anonymous unix domain sockets have no address 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 addr=none,
> +
> +If the address component of a rule is not specified then the rule applies
> to both abstract and anonymous sockets.
>
> =head3 Unix socket permissions
> @@ -925,7 +927,7 @@ socket permissions are the union of all
>
> Unix domain socket rules are broad and general and become more restrictive
> as further information is specified. Policy may be specified down to
> -the path and label level. The content of the communication is not
> +the address and label level. The content of the communication is not
> examined.
>
> Unix socket rule permissions are implied when a rule does not explicitly
> @@ -961,20 +963,20 @@ create, bind, listen, shutdown, getattr,
>
> unix type=dgram,
>
> - unix path=none
> + unix addr=none
Not introduced here, but this is missing the trailing comma.
>
> - unix path=@foo,
> + unix addr=@foo,
>
> - unix type=stream path=@foo,
> + unix type=stream addr=@foo,
>
> - unix server path=@foo,
> + unix server addr=@foo,
>
> - unix accept path=@foo peer=(label=/bar),
> + unix accept addr=@foo peer=(label=/bar),
>
> - unix receive path=@foo peer=(label=/bar),
> + unix receive addr=@foo peer=(label=/bar),
>
>
> - unix path=none
> + unix addr=none
Not introduced here, but this is missing the trailing comma.
>
>
> =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 addr=@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=(addr=@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 addr=@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 address must be none for anonymous or start with @ for abstract
> profile foo {
> - unix send peer(path=wat),
> + unix send peer(addr=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 addr regex expansion
> #=EXRESULT FAIL
> #
>
> profile foo {
> - unix send path=@foo{one,two peer=(label=splat),
> + unix send addr=@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 addr=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 address regex expansion
> #=EXRESULT FAIL
> #
>
> profile foo {
> - unix send path=/some/random/{path peer=(label=splat),
> + unix send addr=/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 addr=@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) addr=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) addr=@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=(addr=@foo),
> }
> Index: b/parser/af_unix.h
> ===================================================================
> --- a/parser/af_unix.h
> +++ b/parser/af_unix.h
> @@ -31,8 +31,8 @@ class unix_rule: public af_rule {
> void move_peer_conditionals(struct cond_entry *conds);
> void downgrade_rule(Profile &prof);
> public:
> - char *path;
> - char *peer_path;
> + char *addr;
> + char *peer_addr;
> int mode;
> int audit;
> bool deny;
> @@ -42,12 +42,12 @@ public:
> struct cond_entry *peer_conds);
> virtual ~unix_rule()
> {
> - free(path);
> - free(peer_path);
> + free(addr);
> + free(peer_addr);
> };
>
> virtual bool has_peer_conds(void) {
> - return af_rule::has_peer_conds() || peer_path;
> + return af_rule::has_peer_conds() || peer_addr;
> }
>
> virtual ostream &dump_local(ostream &os);
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140826/55460fca/attachment.pgp>
More information about the AppArmor
mailing list