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 <tyhi...@canonical.com>
> ---
> 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
<sbeat...@ubuntu.com>
http://NxNW.org/~steve/
signature.asc
Description: Digital signature
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
