From 0a52cf81e39e535eb6e1db761bc6a362d7282f8b Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 10 Sep 2020 13:51:59 -0700 Subject: [PATCH] parser: add support for autobind sockets af_unix allows for sockets to be bound to a name that is autogenerated. Currently this type of binding is only supported by a very generic rule. unix (bind) type=dgram, but this allows both sockets with specified names and anonymous sockets. Extend unix rule syntax to support specifying just an auto bind socket by specifying addr=auto eg. unix (bind) addr=auto, It is important to note that addr=auto only works for the bind permission as once the socket is bound to an autogenerated address, the addr with have a valid unique value that can be matched against with a regular addr=@name expression Fixes: https://bugs.launchpad.net/apparmor/+bug/1867216 MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/521 Signed-off-by: John Johansen --- parser/af_unix.cc | 44 +++++- parser/apparmor.d.pod | 27 +++- parser/tst/simple_tests/unix/bad_attr_5.sd | 7 + parser/tst/simple_tests/unix/bad_opt_5.sd | 7 + parser/tst/simple_tests/unix/bad_peer_2.sd | 9 ++ .../tst/simple_tests/unix/bad_shutdown_3.sd | 7 + parser/tst/simple_tests/unix/ok_attr_7.sd | 7 + parser/tst/simple_tests/unix/ok_attr_8.sd | 7 + parser/tst/simple_tests/unix/ok_create_4.sd | 7 + parser/tst/simple_tests/unix/ok_msg_20.sd | 7 + parser/tst/simple_tests/unix/ok_opt_7.sd | 7 + tests/regression/apparmor/Makefile | 1 + tests/regression/apparmor/unix_socket.c | 21 ++- tests/regression/apparmor/unix_socket.inc | 2 +- .../apparmor/unix_socket_autobind.sh | 128 ++++++++++++++++++ .../regression/apparmor/unix_socket_client.c | 4 +- utils/test/test-parser-simple-tests.py | 4 + 17 files changed, 290 insertions(+), 6 deletions(-) create mode 100644 parser/tst/simple_tests/unix/bad_attr_5.sd create mode 100644 parser/tst/simple_tests/unix/bad_opt_5.sd create mode 100644 parser/tst/simple_tests/unix/bad_peer_2.sd create mode 100644 parser/tst/simple_tests/unix/bad_shutdown_3.sd create mode 100644 parser/tst/simple_tests/unix/ok_attr_7.sd create mode 100644 parser/tst/simple_tests/unix/ok_attr_8.sd create mode 100644 parser/tst/simple_tests/unix/ok_create_4.sd create mode 100644 parser/tst/simple_tests/unix/ok_msg_20.sd create mode 100644 parser/tst/simple_tests/unix/ok_opt_7.sd create mode 100644 tests/regression/apparmor/unix_socket_autobind.sh diff --git a/parser/af_unix.cc b/parser/af_unix.cc index a62663ae0..bd1972145 100644 --- a/parser/af_unix.cc +++ b/parser/af_unix.cc @@ -29,6 +29,9 @@ #include "profile.h" #include "af_unix.h" +/* See unix(7) for autobind address definiation */ +#define autobind_address_pattern "\\x00[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]"; + int parse_unix_mode(const char *str_mode, int *mode, int fail) { return parse_X_mode("unix", AA_VALID_NET_PERMS, str_mode, mode, fail); @@ -54,7 +57,9 @@ void unix_rule::move_conditionals(struct cond_entry *conds) } if (strcmp(ent->name, "addr") == 0) { move_conditional_value("unix socket", &addr, ent); - if (addr[0] != '@' && strcmp(addr, "none") != 0) + if (addr[0] != '@' && + !(strcmp(addr, "none") == 0 || + strcmp(addr, "auto") == 0)) yyerror("unix rule: invalid value for addr='%s'\n", addr); } @@ -82,7 +87,9 @@ void unix_rule::move_peer_conditionals(struct cond_entry *conds) } if (strcmp(ent->name, "addr") == 0) { move_conditional_value("unix", &peer_addr, ent); - if (peer_addr[0] != '@' && strcmp(peer_addr, "none") != 0) + if ((peer_addr[0] != '@') && + !(strcmp(peer_addr, "none") == 0 || + strcmp(peer_addr, "auto") == 0)) yyerror("unix rule: invalid value for addr='%s'\n", peer_addr); } } @@ -222,6 +229,12 @@ bool unix_rule::write_addr(std::ostringstream &buffer, const char *addr) if (strcmp(addr, "none") == 0) { /* anonymous */ buffer << "\\x01"; + } else if (strcmp(addr, "auto") == 0) { + /* autobind - special autobind rule written already + * just generate pattern that matches autobind + * generated addresses. + */ + buffer << autobind_address_pattern; } else { /* skip leading @ */ ptype = convert_aaregex_to_pcre(addr + 1, 0, glob_null, buf, &pos); @@ -323,6 +336,33 @@ int unix_rule::gen_policy_re(Profile &prof) mask &= ~AA_NET_CREATE; } + /* write special pattern for autobind? Will not grant bind + * on any specific address + */ + if ((mask & AA_NET_BIND) && (!addr || (strcmp(addr, "auto") == 0))) { + std::ostringstream tmp; + + tmp << buffer.str(); + /* todo: change to out of band separator */ + /* skip addr, its 0 length */ + tmp << "\\x00"; + /* local label option */ + if (!write_label(tmp, label)) + goto fail; + /* seperator */ + tmp << "\\x00"; + + buf = tmp.str(); + if (!prof.policy.rules->add_rule(buf.c_str(), deny, + map_perms(AA_NET_BIND), + map_perms(audit & AA_NET_BIND), + dfaflags)) + goto fail; + /* clear if auto, else generic need to generate addr below */ + if (addr) + mask &= ~AA_NET_BIND; + } + if (mask) { /* local addr */ if (!write_addr(buffer, addr)) diff --git a/parser/apparmor.d.pod b/parser/apparmor.d.pod index ecbbecf3b..a5424aa18 100644 --- a/parser/apparmor.d.pod +++ b/parser/apparmor.d.pod @@ -1263,6 +1263,31 @@ in an abstract socket name. Eg. unix addr=@*, +Autobound unix domain sockets have a unix sun_path assigned to them +by the kernel, as such specifying a policy based address is not possible. +The autobinding of sockets can be controlled by specifying the special +I keyword. Eg. + + unix addr=auto, + +To indicate that the rule only applies to auto binding of unix domain +sockets. It is important to note this only applies to the I +permission as once the socket is bound to an address it is +indistiguishable from a socket that have an addr bound with a +specified name. When the I keyword is used with other permissions +or as part of a peer addr it will be replaced with a pattern that +can match an autobound socket. Eg. For some kernels + + unix rw addr=auto, + +is transformed to + + unix rw addr=@[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9], + +It is important to note, this pattern may match abstract sockets that +were not autobound but have an addr that fits what is generated by +the kernel when autobinding a socket. + Anonymous unix domain sockets have no sun_path associated with the socket address, however it can be specified with the special I keyword to indicate the rule only applies to anonymous unix domain sockets. Eg. @@ -1270,7 +1295,7 @@ 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. +to autobind, abstract and anonymous sockets. =head3 Unix socket permissions diff --git a/parser/tst/simple_tests/unix/bad_attr_5.sd b/parser/tst/simple_tests/unix/bad_attr_5.sd new file mode 100644 index 000000000..7ddb675cf --- /dev/null +++ b/parser/tst/simple_tests/unix/bad_attr_5.sd @@ -0,0 +1,7 @@ +# +#=DESCRIPTION simple unix getattr w/peer modifier +#=EXRESULT FAIL + +profile a_profile { + unix getattr peer=(addr=auto), +} diff --git a/parser/tst/simple_tests/unix/bad_opt_5.sd b/parser/tst/simple_tests/unix/bad_opt_5.sd new file mode 100644 index 000000000..9ebc547d7 --- /dev/null +++ b/parser/tst/simple_tests/unix/bad_opt_5.sd @@ -0,0 +1,7 @@ +# +#=DESCRIPTION simple unix getopt w/peer addr test +#=EXRESULT FAIL + +profile a_profile { + unix getopt peer=(addr=auto), +} diff --git a/parser/tst/simple_tests/unix/bad_peer_2.sd b/parser/tst/simple_tests/unix/bad_peer_2.sd new file mode 100644 index 000000000..114fb6be8 --- /dev/null +++ b/parser/tst/simple_tests/unix/bad_peer_2.sd @@ -0,0 +1,9 @@ +# +#=Description unix rule with bad 'peer' +#=EXRESULT FAIL +# + +# path address must be none for anonymous or start with @ for abstract +profile foo { + unix send peer(addr=auto), +} diff --git a/parser/tst/simple_tests/unix/bad_shutdown_3.sd b/parser/tst/simple_tests/unix/bad_shutdown_3.sd new file mode 100644 index 000000000..537963336 --- /dev/null +++ b/parser/tst/simple_tests/unix/bad_shutdown_3.sd @@ -0,0 +1,7 @@ +# +#=DESCRIPTION simple unix shutdown w/peer test +#=EXRESULT FAIL + +profile a_profile { + unix shutdown peer=(addr=auto), +} diff --git a/parser/tst/simple_tests/unix/ok_attr_7.sd b/parser/tst/simple_tests/unix/ok_attr_7.sd new file mode 100644 index 000000000..28788c4a9 --- /dev/null +++ b/parser/tst/simple_tests/unix/ok_attr_7.sd @@ -0,0 +1,7 @@ +# +#=DESCRIPTION simple unix getattr w/addr acceptance test +#=EXRESULT PASS + +profile a_profile { + unix getattr addr=auto, +} diff --git a/parser/tst/simple_tests/unix/ok_attr_8.sd b/parser/tst/simple_tests/unix/ok_attr_8.sd new file mode 100644 index 000000000..4331480eb --- /dev/null +++ b/parser/tst/simple_tests/unix/ok_attr_8.sd @@ -0,0 +1,7 @@ +# +#=DESCRIPTION simple unix setattr w/addr acceptance test +#=EXRESULT PASS + +profile a_profile { + unix setattr addr=auto, +} diff --git a/parser/tst/simple_tests/unix/ok_create_4.sd b/parser/tst/simple_tests/unix/ok_create_4.sd new file mode 100644 index 000000000..e76a8ef24 --- /dev/null +++ b/parser/tst/simple_tests/unix/ok_create_4.sd @@ -0,0 +1,7 @@ +# +#=DESCRIPTION simple unix create w/addr acceptance test +#=EXRESULT PASS + +profile a_profile { + unix create addr=auto, +} diff --git a/parser/tst/simple_tests/unix/ok_msg_20.sd b/parser/tst/simple_tests/unix/ok_msg_20.sd new file mode 100644 index 000000000..abfa19304 --- /dev/null +++ b/parser/tst/simple_tests/unix/ok_msg_20.sd @@ -0,0 +1,7 @@ +# +#=DESCRIPTION simple unix msg test +#=EXRESULT PASS + +profile a_profile { + unix (send) addr=auto, +} diff --git a/parser/tst/simple_tests/unix/ok_opt_7.sd b/parser/tst/simple_tests/unix/ok_opt_7.sd new file mode 100644 index 000000000..af3edbe49 --- /dev/null +++ b/parser/tst/simple_tests/unix/ok_opt_7.sd @@ -0,0 +1,7 @@ +# +#=DESCRIPTION simple unix setopt w/addr acceptance test +#=EXRESULT PASS + +profile a_profile { + unix setopt addr=auto, +} diff --git a/tests/regression/apparmor/Makefile b/tests/regression/apparmor/Makefile index 94b42829a..15b990c17 100644 --- a/tests/regression/apparmor/Makefile +++ b/tests/regression/apparmor/Makefile @@ -242,6 +242,7 @@ TESTS=aa_exec \ unix_socket_pathname \ unix_socket_abstract \ unix_socket_unnamed \ + unix_socket_autobind \ unlink\ xattrs\ xattrs_profile\ diff --git a/tests/regression/apparmor/unix_socket.c b/tests/regression/apparmor/unix_socket.c index bd43a9e8b..cb8610ac1 100644 --- a/tests/regression/apparmor/unix_socket.c +++ b/tests/regression/apparmor/unix_socket.c @@ -29,6 +29,7 @@ #define MSG_BUF_MAX 1024 #define PATH_FOR_UNNAMED "none" +#define PATH_FOR_AUTOBIND "auto" static int connection_based_messaging(int sock, int sock_is_peer_sock, char *msg_buf, size_t msg_buf_len) @@ -99,7 +100,7 @@ int main (int argc, char *argv[]) size_t sun_path_len; pid_t pid; int sock, peer_sock, type, rc; - int unnamed = 0; + int unnamed = 0, autobind = 0; if (argc != 5) { fprintf(stderr, @@ -124,6 +125,9 @@ int main (int argc, char *argv[]) addr.sun_path[0] = '\0'; } else if (!strcmp(sun_path, PATH_FOR_UNNAMED)) { unnamed = 1; + } else if (!strcmp(sun_path, PATH_FOR_AUTOBIND)) { + sun_path_len = 0; + autobind = 1; } else { /* include the nul terminator for pathname addr types */ sun_path_len++; @@ -195,6 +199,21 @@ int main (int argc, char *argv[]) exit(1); } } + + if (autobind) { + unsigned int len = sizeof(addr); + rc = getsockname(sock, (struct sockaddr *) &addr, &len); + if (rc < 0) { + perror("FAIL - getsockname"); + exit(1); + } + if (len > sizeof(addr)) { + perror("FAIL - getsockname: address too long"); + exit(1); + } + addr.sun_path[0] = '@'; + sun_path = addr.sun_path; + } } rc = get_sock_io_timeo(sock); diff --git a/tests/regression/apparmor/unix_socket.inc b/tests/regression/apparmor/unix_socket.inc index cd38e8849..a8bacb667 100644 --- a/tests/regression/apparmor/unix_socket.inc +++ b/tests/regression/apparmor/unix_socket.inc @@ -18,7 +18,7 @@ message=4a0c83d87aaa7afa2baab5df3ee4df630f0046d5bfb7a3080c550b721f401b3b\ do_test() { - local addr_type="$1" # abstract or unnamed + local addr_type="$1" # abstract, auto or unnamed local test_prog="$2" # server or client local l_u_access="$3" # optional local unbound perms local l_b_access="$4" # local bound perms diff --git a/tests/regression/apparmor/unix_socket_autobind.sh b/tests/regression/apparmor/unix_socket_autobind.sh new file mode 100644 index 000000000..3a183d753 --- /dev/null +++ b/tests/regression/apparmor/unix_socket_autobind.sh @@ -0,0 +1,128 @@ +#! /bin/bash +# +# Copyright (C) 2014 Canonical, Ltd. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of version 2 of the GNU General Public +# License published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, contact Canonical Ltd. + +#=NAME unix_socket_autobind abstract sockets +#=DESCRIPTION +# This tests access to autobinding abstract unix domain sockets. The +# server opens a socket, forks a client with it's own profile, passes +# an fd across exec, sends a message to the client over the socket, and +# sees what happens. +#=END +# +# TODO: peer_addr auto, just generates a pattern it would be better if we +# could extract the bound socket name and pass that in to the profile +# generation + +pwd=`dirname $0` +pwd=`cd $pwd ; /bin/pwd` + +bin=$pwd + +. $bin/prologue.inc +. $bin/unix_socket.inc +requires_kernel_features policy/versions/v7 +requires_kernel_features network/af_unix +requires_parser_support "unix," + +settest unix_socket + +addr=auto +#TODO: replace client_addr pattern with actual autobound address +client_addr=@[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f].client + +# Test autobind stream server and client +do_test "autobind" \ + "server" \ + "create,setopt" \ + "bind,listen,getopt,shutdown,getattr" \ + stream \ + "$addr" \ + "accept,read,write" \ + "unconfined" \ + "" \ + dgram \ + "@autoXXX" \ + "${test}XXX" \ + "" +do_test "autobind" \ + "client" \ + "" \ + "create,getopt,setopt,getattr" \ + stream \ + "" \ + "connect,write,read" \ + "$test" \ + "$addr" \ + seqpacket \ + "" \ + "${test}XXX" \ + "@autoXXX" + +# Test autobind dgram server and client +do_test "autobind" \ + "server" \ + "create,setopt" \ + "bind,getopt,shutdown,getattr" \ + dgram \ + "$addr" \ + "read,write" \ + "unconfined" \ + "$client_addr" \ + seqpacket \ + "@autoXXX" \ + "${test}XXX" \ + "${client_addr}XXX" +do_test "autobind" \ + "client" \ + "create,setopt,getattr" \ + "bind,getopt,getattr" \ + dgram \ + "$client_addr" \ + "write,read" \ + "$test" \ + "$addr" \ + stream \ + "${client_addr}XXX" \ + "${test}XXX" \ + "@autoXXX" + +# Test autobind seqpacket server and client +do_test "autobind" \ + "server" \ + "create,setopt" \ + "bind,listen,getopt,shutdown,getattr" \ + seqpacket \ + "$addr" \ + "accept,read,write" \ + "unconfined" \ + "" \ + stream \ + "@autoXXX" \ + "${test}XXX" \ + "" +do_test "autobind" \ + "client" \ + "" \ + "create,getopt,setopt,getattr" \ + seqpacket \ + "" \ + "connect,write,read" \ + "$test" \ + "$addr" \ + dgram \ + "" \ + "${test}XXX" \ + "@autoXXX" diff --git a/tests/regression/apparmor/unix_socket_client.c b/tests/regression/apparmor/unix_socket_client.c index 7619aaf0d..33423ca1a 100644 --- a/tests/regression/apparmor/unix_socket_client.c +++ b/tests/regression/apparmor/unix_socket_client.c @@ -44,7 +44,9 @@ static int connection_based_messaging(int sock, struct sockaddr_un *peer_addr, if (peer_addr) { rc = connect(sock, (struct sockaddr *)peer_addr, peer_addr_len); if (rc < 0) { - perror("FAIL CLIENT - connect"); + if (peer_addr_len > 0 && peer_addr->sun_path[0] == 0) + peer_addr->sun_path[0] = '@'; + fprintf(stderr, "FAIL CLIENT - connect '%s'(%d): %m", peer_addr->sun_path, peer_addr_len); exit(1); } } diff --git a/utils/test/test-parser-simple-tests.py b/utils/test/test-parser-simple-tests.py index 5e228e4ed..9a2550065 100644 --- a/utils/test/test-parser-simple-tests.py +++ b/utils/test/test-parser-simple-tests.py @@ -168,6 +168,10 @@ exception_not_raised = [ 'unix/bad_regex_04.sd', 'unix/bad_shutdown_1.sd', 'unix/bad_shutdown_2.sd', + 'unix/bad_peer_2.sd', + 'unix/bad_attr_5.sd', + 'unix/bad_opt_5.sd', + 'unix/bad_shutdown_3.sd', 'vars/boolean/boolean_bad_2.sd', 'vars/boolean/boolean_bad_3.sd', 'vars/boolean/boolean_bad_4.sd',