[apparmor] [PATCH 6/6] tests: Update unix_socket to test for unnamed sockets
Seth Arnold
seth.arnold at canonical.com
Sat Sep 6 07:48:53 UTC 2014
On Thu, Sep 04, 2014 at 06:55:46AM -0500, Tyler Hicks wrote:
> Update the test server program to use socketpair() and pass the socket
> fd to the client program.
>
> Make the test script aware of the special "none" keyword for unnamed
> UNIX domain sockets.
>
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
Acked-by: Seth Arnold <seth.arnold at canonical.com>
This is fine as it is but I've got a few suggestions inline:
Thanks
> ---
> tests/regression/apparmor/unix_socket.c | 88 +++++++++++++++++++-------
> tests/regression/apparmor/unix_socket.sh | 48 +++++++++++---
> tests/regression/apparmor/unix_socket_client.c | 78 +++++++++++++++++------
> 3 files changed, 161 insertions(+), 53 deletions(-)
>
> diff --git a/tests/regression/apparmor/unix_socket.c b/tests/regression/apparmor/unix_socket.c
> index 50ae8e8..0e495a2 100644
> --- a/tests/regression/apparmor/unix_socket.c
> +++ b/tests/regression/apparmor/unix_socket.c
> @@ -14,6 +14,8 @@
> * along with this program; if not, contact Canonical Ltd.
> */
>
> +#define _GNU_SOURCE
> +
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -21,18 +23,24 @@
> #include <sys/types.h>
> #include <sys/un.h>
> #include <unistd.h>
> +#include <fcntl.h>
>
> -#define MSG_BUF_MAX 1024
> +#define MSG_BUF_MAX 1024
> +#define PATH_FOR_UNNAMED "none"
>
> -static int connection_based_messaging(int sock, char *msg_buf,
> - size_t msg_buf_len)
> +static int connection_based_messaging(int sock, int sock_is_peer_sock,
> + char *msg_buf, size_t msg_buf_len)
> {
> int peer_sock, rc;
>
> - peer_sock = accept(sock, NULL, NULL);
> - if (peer_sock < 0) {
> - perror("FAIL - accept");
> - return 1;
> + if (sock_is_peer_sock) {
> + peer_sock = sock;
> + } else {
> + peer_sock = accept(sock, NULL, NULL);
> + if (peer_sock < 0) {
> + perror("FAIL - accept");
> + return 1;
> + }
> }
>
> rc = write(peer_sock, msg_buf, msg_buf_len);
> @@ -118,7 +126,8 @@ int main (int argc, char *argv[])
> const char *sun_path;
> size_t sun_path_len;
> pid_t pid;
> - int sock, type, rc;
> + int sock, peer_sock, type, rc;
> + int unnamed = 0;
>
> if (argc != 5) {
> fprintf(stderr,
> @@ -138,6 +147,8 @@ int main (int argc, char *argv[])
> memcpy(addr.sun_path, sun_path, sun_path_len);
> addr.sun_path[0] = '\0';
> sun_path_len = sizeof(addr.sun_path);
> + } else if (!strcmp(sun_path, PATH_FOR_UNNAMED)) {
> + unnamed = 1;
> } else {
> memcpy(addr.sun_path, sun_path, sun_path_len + 1);
> }
> @@ -160,25 +171,43 @@ int main (int argc, char *argv[])
> }
> memcpy(msg_buf, argv[3], msg_buf_len);
>
> - sock = socket(AF_UNIX, type | SOCK_CLOEXEC, 0);
> - if (sock == -1) {
> - perror("FAIL - socket");
> - exit(1);
> - }
> + if (unnamed) {
> + int sv[2];
>
> - rc = bind(sock, (struct sockaddr *)&addr,
> - sun_path_len + sizeof(addr.sun_family));
> - if (rc < 0) {
> - perror("FAIL - bind");
> - exit(1);
> - }
> + rc = socketpair(AF_UNIX, type, 0, sv);
> + if (rc == -1) {
> + perror("FAIL - socketpair");
> + exit(1);
> + }
> + sock = sv[0];
> + peer_sock = sv[1];
>
> - if (type & SOCK_STREAM || type & SOCK_SEQPACKET) {
> - rc = listen(sock, 2);
> + rc = fcntl(sock, F_SETFD, FD_CLOEXEC);
> + if (rc == -1) {
> + perror("FAIL - fcntl");
> + exit(1);
> + }
> + } else {
> + sock = socket(AF_UNIX, type | SOCK_CLOEXEC, 0);
> + if (sock == -1) {
> + perror("FAIL - socket");
> + exit(1);
> + }
> +
> + rc = bind(sock, (struct sockaddr *)&addr,
> + sun_path_len + sizeof(addr.sun_family));
> if (rc < 0) {
> - perror("FAIL - listen");
> + perror("FAIL - bind");
> exit(1);
> }
> +
> + if (type & SOCK_STREAM || type & SOCK_SEQPACKET) {
> + rc = listen(sock, 2);
> + if (rc < 0) {
> + perror("FAIL - listen");
> + exit(1);
> + }
> + }
> }
>
> pid = fork();
> @@ -186,7 +215,18 @@ int main (int argc, char *argv[])
> perror("FAIL - fork");
> exit(1);
> } else if (!pid) {
> - execl(argv[4], argv[4], sun_path, argv[2], NULL);
> + char *fd_number = NULL;
> +
> + if (unnamed) {
> + rc = asprintf(&fd_number, "%d", peer_sock);
> + if (rc == -1) {
> + perror("FAIL - asprintf");
> + exit(1);
> + }
> + }
> +
> + /* fd_number will be NULL for pathname and abstract sockets */
> + execl(argv[4], argv[4], sun_path, argv[2], fd_number, NULL);
> exit(0);
If the execl() fails this should print a FAIL and exit(1).
> }
>
> @@ -195,7 +235,7 @@ int main (int argc, char *argv[])
> exit(1);
>
> rc = (type & SOCK_STREAM || type & SOCK_SEQPACKET) ?
> - connection_based_messaging(sock, msg_buf, msg_buf_len) :
> + connection_based_messaging(sock, unnamed, msg_buf, msg_buf_len) :
> connectionless_messaging(sock, msg_buf, msg_buf_len);
> if (rc)
> exit(1);
> diff --git a/tests/regression/apparmor/unix_socket.sh b/tests/regression/apparmor/unix_socket.sh
> index 7c23464..3878728 100755
> --- a/tests/regression/apparmor/unix_socket.sh
> +++ b/tests/regression/apparmor/unix_socket.sh
> @@ -34,6 +34,8 @@ sockpath_pathname=${tmpdir}/unix_socket.sock
> bad_sockpath_pathname="${sockpath_pathname}XXX"
> sockpath_abstract="@apparmor_unix_socket"
> bad_sockpath_abstract="${sockpath_abstract}XXX"
> +sockpath_unnamed="none"
> +bad_sockpath_unnamed="$sockpath_abstract"
> message=4a0c83d87aaa7afa2baab5df3ee4df630f0046d5bfb7a3080c550b721f401b3b\
> 8a738e1435a3b77aa6482a70fb51c44f20007221b85541b0184de66344d46a4c
>
> @@ -42,9 +44,14 @@ isabstract()
> [ "${1:0:1}" == "@" ]
> }
>
> +isunnamed()
> +{
> + [ "$1" == "none" ]
> +}
> +
> removesocket()
> {
> - if ! isabstract "$1"; then
> + if ! isabstract "$1" && ! isunnamed "$1" ; then
> rm -f "$1"
> fi
> }
> @@ -63,7 +70,7 @@ testsocktype()
> local okclients
> local badclients
>
> - if isabstract $sockpath; then
> + if isabstract "$sockpath" || isunnamed "$sockpath"; then
> local ls_access # local server accesses
> local ps_access # peer server accesses
> local s_access # combined server accesses
> @@ -74,14 +81,38 @@ testsocktype()
>
> local access # used for iterating accesses
>
> + local client_create # set if client create perms are needed
> + local ps_sock_label # peer server socket label
> +
> + if isabstract "$sockpath"; then
> + # unix_socket_client creates the socket which means:
> + # 1) the create access is required
> + # 2) the socket will be labeled as unconfined during
> + # the confined server tests
> + client_create="create,"
> + ps_sock_label="unconfined"
> + else # unnamed
> + # unix_socket_client inherits the socket which means:
> + # 1) the create access is not needed
> + # 2) the socket will be labeled as $test during the
> + # confined server tests
> + client_create=""
> + ps_sock_label="$test"
> + fi
> +
> if [ "$socktype" == "dgram" ]; then
> # Connectionless
> # Server doesn't listen() or accept()
> ls_access="create,bind,getopt,setopt"
> ps_access="read,write"
>
> - # Client calls bind()
> - lc_access="${client_create}bind,getopt,setopt"
> + lc_access="$client_create"
> + # Client calls bind() if a connectionless pathname or
> + # abstract socket is used
> + if ! isunnamed "$sockpath"; then
> + lc_access+="bind,"
> + fi
> + lc_access+="getopt,setopt"
> pc_access="connect,write,read"
> else # stream or seqpacket
> # Connection based
> @@ -101,11 +132,11 @@ testsocktype()
> "unix:($s_access)" \
> "unix:addr=$sockpath" \
> "unix:type=$socktype" \
> - "unix:peer=(label=unconfined)" \
> + "unix:peer=(label=$ps_sock_label)" \
> "unix:($s_access):addr=$sockpath" \
> - "unix:($ls_access):addr=$sockpath unix:($ps_access):addr=$sockpath:peer=(label=unconfined)" \
> - "unix:($ls_access):type=$socktype:addr=$sockpath unix:($ps_access):type=$socktype:addr=$sockpath:peer=(label=unconfined)" \
> - "unix:type=$socktype:addr=$sockpath:peer=(label=unconfined)" \
> + "unix:($ls_access):addr=$sockpath unix:($ps_access):addr=$sockpath:peer=(label=$ps_sock_label)" \
> + "unix:($ls_access):type=$socktype:addr=$sockpath unix:($ps_access):type=$socktype:addr=$sockpath:peer=(label=$ps_sock_label)" \
> + "unix:type=$socktype:addr=$sockpath:peer=(label=$ps_sock_label)" \
> )
> # Start with no accessess, then remove each access one-by-one
> # from the list of server accesses, and then test bad
> @@ -218,4 +249,5 @@ testsockpath()
> testsockpath "pathname socket" "$sockpath_pathname" "$bad_sockpath_pathname"
> if [ "$(have_features network/af_unix)" == "true" ] ; then
> testsockpath "abstract socket" "$sockpath_abstract" "$bad_sockpath_abstract"
> + testsockpath "unnamed socket" "$sockpath_unnamed" "$bad_sockpath_unnamed"
> fi
> diff --git a/tests/regression/apparmor/unix_socket_client.c b/tests/regression/apparmor/unix_socket_client.c
> index c0892cf..e7bdbed 100644
> --- a/tests/regression/apparmor/unix_socket_client.c
> +++ b/tests/regression/apparmor/unix_socket_client.c
> @@ -22,7 +22,8 @@
> #include <sys/un.h>
> #include <unistd.h>
>
> -#define MSG_BUF_MAX 1024
> +#define MSG_BUF_MAX 1024
> +#define PATH_FOR_UNNAMED "none"
>
> static int connection_based_messaging(int sock)
> {
> @@ -44,17 +45,26 @@ static int connection_based_messaging(int sock)
> return 0;
> }
>
> -static int connectionless_messaging(int sock)
> +static int connectionless_messaging(int sock, int unnamed)
> {
> struct sockaddr_un addr;
> char msg_buf[MSG_BUF_MAX];
> int rc;
>
> - addr.sun_family = AF_UNIX;
> - rc = bind(sock, (struct sockaddr *)&addr, sizeof(sa_family_t));
> - if (rc < 0) {
> - perror("FAIL CLIENT - bind");
> - return 1;
> + /**
> + * Calling bind() on an unnamed socket results in autobinding in the
> + * abstract namespace. That's not wanted since we're testing unnamed
> + * sockets and not abstract sockets. Finally, there's no need to call
> + * bind() since the two unnamed sockets from socketpair() are already
> + * connected and read for IO.
> + */
> + if (!unnamed) {
> + addr.sun_family = AF_UNIX;
> + rc = bind(sock, (struct sockaddr *)&addr, sizeof(sa_family_t));
Does addr need to be zeroed before use?
> + if (rc < 0) {
> + perror("FAIL CLIENT - bind");
> + return 1;
> + }
> }
>
> rc = write(sock, NULL, 0);
> @@ -108,17 +118,25 @@ static int get_set_sock_io_timeo(int sock)
> return 0;
> }
>
> +static void usage(const char *name)
> +{
This should probably have a FAIL output here, just to cover all bases.
> + fprintf(stderr, "Usage: %s <socket> <type> [<fd_number>]\n\n"
> + " type\t\tstream, dgram, or seqpacket\n"
> + " fd_number\t\tfd number for inherited unnamed socket\n",
> + name);
> +}
> +
> int main(int argc, char *argv[])
> {
> struct sockaddr_un peer_addr;
> const char *sun_path;
> size_t sun_path_len;
> int sock, type, rc;
> + int unnamed = 0;
> + const char *fd_number = NULL;
>
> - if (argc != 3) {
> - fprintf(stderr, "Usage: %s <socket> <type>\n\n"
> - " type\t\tstream, dgram, or seqpacket\n",
> - argv[0]);
> + if (argc < 3 || argc > 4) {
> + usage(argv[0]);
> exit(1);
> }
>
> @@ -131,6 +149,13 @@ int main(int argc, char *argv[])
> memcpy(peer_addr.sun_path, sun_path, sun_path_len);
> peer_addr.sun_path[0] = '\0';
> sun_path_len = sizeof(peer_addr.sun_path);
> + } else if (!strcmp(sun_path, PATH_FOR_UNNAMED)) {
> + unnamed = 1;
> + if (argc != 4) {
> + usage(argv[0]);
> + exit(1);
> + }
> + fd_number = argv[3];
> } else {
> memcpy(peer_addr.sun_path, sun_path, sun_path_len + 1);
> }
> @@ -146,26 +171,37 @@ int main(int argc, char *argv[])
> exit(1);
> }
>
> - sock = socket(AF_UNIX, type, 0);
> - if (sock < 0) {
> - perror("FAIL CLIENT - socket");
> - exit(1);
> + if (unnamed) {
> + rc = sscanf(fd_number, "%d", &sock);
> + if (rc != 1) {
> + perror("FAIL CLIENT - sscanf");
> + usage(argv[0]);
> + exit(1);
> + }
> + } else {
> + sock = socket(AF_UNIX, type, 0);
> + if (sock < 0) {
> + perror("FAIL CLIENT - socket");
> + exit(1);
> + }
> }
>
> rc = get_set_sock_io_timeo(sock);
> if (rc)
> exit(1);
>
> - rc = connect(sock, (struct sockaddr *)&peer_addr,
> - sun_path_len + sizeof(peer_addr.sun_family));
> - if (rc < 0) {
> - perror("FAIL CLIENT - connect");
> - exit(1);
> + if (!unnamed) {
> + rc = connect(sock, (struct sockaddr *)&peer_addr,
> + sun_path_len + sizeof(peer_addr.sun_family));
> + if (rc < 0) {
> + perror("FAIL CLIENT - connect");
> + exit(1);
> + }
> }
>
> rc = (type == SOCK_STREAM || type == SOCK_SEQPACKET) ?
> connection_based_messaging(sock) :
> - connectionless_messaging(sock);
> + connectionless_messaging(sock, unnamed);
> if (rc)
> exit(1);
>
> --
> 2.1.0
>
>
> --
> 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/20140906/542d6c12/attachment-0001.pgp>
More information about the AppArmor
mailing list