On 2014-09-16 16:52:12, Steve Beattie wrote: > On Mon, Sep 15, 2014 at 02:55:59PM -0500, Tyler Hicks wrote: > > The unix_socket_client test program was using an abstract socket, which > > was set up using the autobind feature, when testing any socket address > > types. > > > > To more accurately test a specific address type, this patch changes the > > client code to use whatever address type that the server is using. The > > string ".client" will be added to the end of the server's address. > > > > Signed-off-by: Tyler Hicks <[email protected]> > > --- > > tests/regression/apparmor/unix_socket_client.c | 29 ++++++++++++++-- > > tests/regression/apparmor/unix_socket_pathname.sh | 40 > > +++++++++++++---------- > > 2 files changed, 49 insertions(+), 20 deletions(-) > > > > diff --git a/tests/regression/apparmor/unix_socket_client.c > > b/tests/regression/apparmor/unix_socket_client.c > > index bda7db6..30e5ee7 100644 > > --- a/tests/regression/apparmor/unix_socket_client.c > > +++ b/tests/regression/apparmor/unix_socket_client.c > > @@ -24,6 +24,9 @@ > > > > #define MSG_BUF_MAX 1024 > > > > +#define SUN_PATH_SUFFIX ".client" > > +#define SUN_PATH_SUFFIX_LEN strlen(SUN_PATH_SUFFIX) > > + > > static int connection_based_messaging(int sock) > > { > > char msg_buf[MSG_BUF_MAX]; > > @@ -44,14 +47,33 @@ static int connection_based_messaging(int sock) > > return 0; > > } > > > > -static int connectionless_messaging(int sock) > > +static int connectionless_messaging(int sock, struct sockaddr_un > > *peer_addr, > > + socklen_t peer_addr_len) > > { > > struct sockaddr_un addr; > > + size_t peer_path_len = peer_addr_len - sizeof(addr.sun_family); > > + size_t path_len = peer_path_len + SUN_PATH_SUFFIX_LEN; > > char msg_buf[MSG_BUF_MAX]; > > int rc; > > > > + if (path_len > sizeof(addr.sun_path)) { > > + fprintf(stderr, "FAIL CLIENT - path_len too big\n"); > > + return 1; > > + } > > + > > + /** > > + * Subtract 1 to get rid of nul-terminator in pathname address types. > > + * We're essentially moving the nul char so path_len stays the same. > > + */ > > + if (peer_addr->sun_path[0]) > > + peer_path_len--; > > + > > addr.sun_family = AF_UNIX; > > - rc = bind(sock, (struct sockaddr *)&addr, sizeof(sa_family_t)); > > + memcpy(addr.sun_path, peer_addr->sun_path, peer_path_len); > > + strcpy(addr.sun_path + peer_path_len, SUN_PATH_SUFFIX); > > + > > + rc = bind(sock, (struct sockaddr *)&addr, > > + path_len + sizeof(addr.sun_family)); > > if (rc < 0) { > > perror("FAIL CLIENT - bind"); > > return 1; > > @@ -174,7 +196,8 @@ int main(int argc, char *argv[]) > > > > rc = (type == SOCK_STREAM || type == SOCK_SEQPACKET) ? > > connection_based_messaging(sock) : > > - connectionless_messaging(sock); > > + connectionless_messaging(sock, &peer_addr, > > + sun_path_len + sizeof(peer_addr.sun_family)); > > Is there a reason to add sizeof(peer_addr.sun_family) to sun_path_len > to use as the peer_addr_len argument to connectionless_messaging(), when > the only use of peer_addr_len is to immediately subtract it back off? > Or would it just make sense to have connectionless_messaging() take the > just the sun_path and path len as arguments?
I did it that way because the next patch in the series moves the call to
connect() into connection_based_messaging(), which means that we must
pass in (sun_path_len + sizeof(peer_addr.sun_family)) to
connection_based_messaging().
Instead of having connection_based_messaging() take (sun_path_len +
sizeof(peer_addr.sun_family)) and connectionless_messaging() take
sun_path_len, I decided to make them both take (sun_path_len +
sizeof(peer_addr.sun_family)).
>
> > if (rc)
> > exit(1);
> >
> > diff --git a/tests/regression/apparmor/unix_socket_pathname.sh
> > b/tests/regression/apparmor/unix_socket_pathname.sh
> > index 17ed855..d089d09 100755
> > --- a/tests/regression/apparmor/unix_socket_pathname.sh
> > +++ b/tests/regression/apparmor/unix_socket_pathname.sh
> > @@ -32,7 +32,8 @@ requires_features policy/versions/v6
> > settest unix_socket
> >
> > client=$bin/unix_socket_client
> > -sockpath=${tmpdir}/unix_socket.sock
> > +sockpath=${tmpdir}/aa_sock
> > +client_sockpath=${tmpdir}/aa_sock.client
> > message=4a0c83d87aaa7afa2baab5df3ee4df630f0046d5bfb7a3080c550b721f401b3b\
> > 8a738e1435a3b77aa6482a70fb51c44f20007221b85541b0184de66344d46a4c
> >
> > @@ -57,11 +58,15 @@ okclient=rw
> > badclient1=r
> > badclient2=w
> >
> > -removesocket()
> > +removesockets()
> > {
> > - if [ -S "$1" ]; then
> > - rm -f "$1"
> > - fi
> > + local sock
> > +
> > + for sock in "$@"; do
> > + if [ -S "$sock" ]; then
> > + rm -f "$sock"
> > + fi
> > + done
> > }
> >
> > testsocktype()
> > @@ -70,30 +75,30 @@ testsocktype()
> > local testdesc="AF_UNIX pathname socket ($socktype)"
> > local args="$sockpath $socktype $message $client"
> >
> > - removesocket $sockpath
> > + removesockets $sockpath $client_sockpath
> >
> > # PASS - unconfined
> >
> > runchecktest "$testdesc; unconfined" pass $args
> > - removesocket $sockpath
> > + removesockets $sockpath $client_sockpath
> >
> > # PASS - server w/ access to the file
> >
> > genprofile $sockpath:$okserver $af_unix $client:Ux
> > runchecktest "$testdesc; confined server w/ access ($okserver)" pass
> > $args
> > - removesocket $sockpath
> > + removesockets $sockpath $client_sockpath
> >
> > # FAIL - server w/o access to the file
> >
> > genprofile $af_unix $client:Ux
> > runchecktest "$testdesc; confined server w/o access" fail $args
> > - removesocket $sockpath
> > + removesockets $sockpath $client_sockpath
> >
> > # FAIL - server w/ bad access to the file
> >
> > genprofile $sockpath:$badserver1 $af_unix $client:Ux
> > runchecktest "$testdesc; confined server w/ bad access ($badserver1)"
> > fail $args
> > - removesocket $sockpath
> > + removesockets $sockpath $client_sockpath
> >
> > # $badserver2 is set to non-null at the top of the test script if the
> > # kernel advertises ABI v7 or newer
> > @@ -102,7 +107,8 @@ testsocktype()
> >
> > genprofile $sockpath:$badserver2 $af_unix $client:Ux
> > runchecktest "$testdesc; confined server w/ bad access
> > ($badserver2)" fail $args
> > - removesocket $sockpath
> > + removesockets $sockpath $client_sockpath
> > +
> > fi
> >
> > if [ -n "$af_unix" ] ; then
> > @@ -113,38 +119,38 @@ testsocktype()
> > removesockets $sockpath
>
> Ah, this is where removesockets leaked in to patch 04. Presumably
> $client_sockpath needs to be added there as well?
I moved some of these patches around between v1 and v2. I thought I had
caught all of the removesocket -> removesockets changes but obviously
not. Thanks! :)
Tyler
>
> > fi
> >
> > - server="$sockpath:$okserver $af_unix $client:px"
> > + server="$sockpath:$okserver $client_sockpath:$okserver $af_unix
> > $client:px"
> >
> > # PASS - client w/ access to the file
> >
> > genprofile $server -- image=$client $sockpath:$okclient $af_unix
> > runchecktest "$testdesc; confined client w/ access ($okclient)" pass
> > $args
> > - removesocket $sockpath
> > + removesockets $sockpath $client_sockpath
> >
> > # FAIL - client w/o access to the file
> >
> > genprofile $server -- image=$client $af_unix
> > runchecktest "$testdesc; confined client w/o access" fail $args
> > - removesocket $sockpath
> > + removesockets $sockpath $client_sockpath
> >
> > # FAIL - client w/ bad access to the file
> >
> > genprofile $server -- image=$client $sockpath:$badclient1 $af_unix
> > runchecktest "$testdesc; confined client w/ bad access ($badclient1)"
> > fail $args
> > - removesocket $sockpath
> > + removesockets $sockpath $client_sockpath
> >
> > # FAIL - client w/ bad access to the file
> >
> > genprofile $server -- image=$client $sockpath:$badclient2
> > runchecktest "$testdesc; confined client w/ bad access ($badclient2)"
> > fail $args
> > - removesocket $sockpath
> > + removesockets $sockpath $client_sockpath
> >
> > if [ -n "$af_unix" ] ; then
> > # FAIL - client w/o af_unix access
> >
> > genprofile $server -- image=$client $sockpath:$okclient
> > runchecktest "$testdesc; confined client w/o af_unix" fail $args
> > - removesocket $sockpath
> > + removesockets $sockpath $client_sockpath
> > fi
> >
> > removeprofile
>
> --
> Steve Beattie
> <[email protected]>
> http://NxNW.org/~steve/
> --
> AppArmor mailing list
> [email protected]
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
