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?
> 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?
> 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/
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
