[apparmor] [PATCH] tests: Add dbus tests for unrequested reply messages
Tyler Hicks
tyhicks at canonical.com
Fri Sep 5 14:38:24 UTC 2014
On 2014-08-28 14:07:48, Seth Arnold wrote:
> On Thu, Aug 28, 2014 at 02:41:21AM -0500, Tyler Hicks wrote:
> > Unrequested replies are message types that are typically replies, such
> > as error and method_return message types, but have not been requested by
> > the recipient.
> >
> > The AppArmor mediation code in dbus-daemon allows requested reply
> > messages through if the original message was allowed. However,
> > unrequested reply messages should be checked against the system policy
> > to make certain that they should be allowed.
> >
> > This test verifies that the dbus-daemon is properly querying system
> > policy when it detects that a message is an unrequested reply.
> >
> > Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
>
> Acked-by: Seth Arnold <seth.arnold at canonical.com>
>
> There's some small suggestions for usability improvements inline:
>
> Thanks
>
> > ---
> > tests/regression/apparmor/Makefile | 7 +-
> > tests/regression/apparmor/dbus.inc | 22 ++
> > tests/regression/apparmor/dbus_unrequested_reply.c | 221 +++++++++++++++++++++
> > .../regression/apparmor/dbus_unrequested_reply.sh | 126 ++++++++++++
> > 4 files changed, 374 insertions(+), 2 deletions(-)
> > create mode 100644 tests/regression/apparmor/dbus_unrequested_reply.c
> > create mode 100644 tests/regression/apparmor/dbus_unrequested_reply.sh
> >
> > diff --git a/tests/regression/apparmor/Makefile b/tests/regression/apparmor/Makefile
> > index 13bc5d3..c9374f5 100644
> > --- a/tests/regression/apparmor/Makefile
> > +++ b/tests/regression/apparmor/Makefile
> > @@ -126,7 +126,7 @@ endif
> >
> > #only do dbus if proper libs are installl
> > ifneq (,$(shell pkg-config --exists dbus-1 && echo TRUE))
> > -SRC+=dbus_eavesdrop.c dbus_message.c dbus_service.c
> > +SRC+=dbus_eavesdrop.c dbus_message.c dbus_service.c dbus_unrequested_reply.c
> > else
> > $(warning ${nl}\
> > ************************************************************************${nl}\
> > @@ -190,7 +190,7 @@ TESTS=access \
> >
> > #only do dbus if proper libs are installl
> > ifneq (,$(shell pkg-config --exists dbus-1 && echo TRUE))
> > -TESTS+=dbus_eavesdrop dbus_message dbus_service
> > +TESTS+=dbus_eavesdrop dbus_message dbus_service dbus_unrequested_reply
> > endif
> >
> > # Tests that can crash the kernel should be placed here
> > @@ -224,6 +224,9 @@ dbus_message: dbus_message.c dbus_common.o
> > dbus_service: dbus_message dbus_service.c dbus_common.o
> > ${CC} ${CFLAGS} ${LDFLAGS} $(filter-out dbus_message, $^) -o $@ ${LDLIBS} $(shell pkg-config --cflags --libs dbus-1)
> >
> > +dbus_unrequested_reply: dbus_service dbus_unrequested_reply.c dbus_common.o
> > + ${CC} ${CFLAGS} ${LDFLAGS} $(filter-out dbus_service, $^) -o $@ ${LDLIBS} $(shell pkg-config --cflags --libs dbus-1)
> > +
> > tests: all
> > @if [ `whoami` = "root" ] ;\
> > then \
> > diff --git a/tests/regression/apparmor/dbus.inc b/tests/regression/apparmor/dbus.inc
> > index 539d128..57cb849 100755
> > --- a/tests/regression/apparmor/dbus.inc
> > +++ b/tests/regression/apparmor/dbus.inc
> > @@ -98,6 +98,28 @@ sendmethod()
> > send "$bus" "method_call" "$dest" "$path" "${iface}.Method"
> > }
> >
> > +# parameters: bus message_type destination
> > +#
> > +# destination must be a connection name
> > +sendunrequestedreply()
> > +{
> > + out=$(./dbus_unrequested_reply --$1 --type=$2 --name=$3 2>&1)
> > + if [ $? -ne 0 ]
> > + then
> > + fatalerror "$out"
> > + fi
> > +}
> > +
> > +sendmethodreturn()
> > +{
> > + sendunrequestedreply "$bus" "method_return" "$dest"
> > +}
> > +
> > +senderror()
> > +{
> > + sendunrequestedreply "$bus" "error" "$dest"
> > +}
> > +
> > compare_logs()
> > {
> > local msg
> > diff --git a/tests/regression/apparmor/dbus_unrequested_reply.c b/tests/regression/apparmor/dbus_unrequested_reply.c
> > new file mode 100644
> > index 0000000..143f292
> > --- /dev/null
> > +++ b/tests/regression/apparmor/dbus_unrequested_reply.c
> > @@ -0,0 +1,221 @@
> > +/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- */
> > +/* dbus_message.c Utility program to send messages from the command line
> > + *
> > + * Copyright (C) 2003 Philip Blundell <philb at gnu.org>
> > + * Copyright (C) 2014 Canonical, Ltd.
> > + *
> > + * Originally dbus-send.c from the dbus package. It has been heavily modified
> > + * to work within the regression test framework.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * 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, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + *
> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +
> > +#include "dbus_common.h"
> > +
> > +DBusConnection *connection;
> > +DBusError error;
> > +DBusBusType type = DBUS_BUS_SESSION;
> > +const char *type_str = NULL;
> > +const char *name = NULL;
> > +int message_type = DBUS_MESSAGE_TYPE_INVALID;
> > +const char *address = NULL;
> > +int session_or_system = FALSE;
> > +int log_fd = -1;
> > +
> > +static void usage(int ecode)
> > +{
> > + char *prefix = ecode ? "FAIL: " : "";
> > +
> > + fprintf(stderr,
> > + "%6sUsage: dbus_unrequested_reply [ADDRESS] --name=NAME --type=TYPE\n"
> > + " ADDRESS\t\t--system, --session (default), or --address=ADDR\n"
> > + " NAME\t\tthe message destination\n"
> > + " TYPE\t\tmethod_return or error\n",
> > + prefix);
> > + exit(ecode);
> > +}
> > +
> > +static int do_unrequested_reply(void)
> > +{
> > + DBusMessage *message;
> > +
> > + if (message_type == DBUS_MESSAGE_TYPE_METHOD_RETURN) {
> > + message = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN);
> > +
> > + if (message) {
> > + dbus_message_set_no_reply(message, TRUE);
> > +
> > + /* Make up an invalid reply_serial */
> > + if (!dbus_message_set_reply_serial(message,
> > + 123456789)) {
> > + fprintf(stderr,
> > + "FAIL: Couldn't set reply_serial\n");
> > + return 1;
> > + }
> > + }
> > + } else if (message_type == DBUS_MESSAGE_TYPE_ERROR) {
> > + message = dbus_message_new(DBUS_MESSAGE_TYPE_ERROR);
> > +
> > + if (message) {
> > + dbus_message_set_no_reply(message, TRUE);
> > +
> > + /* Make up an invalid reply_serial */
> > + if (!dbus_message_set_reply_serial(message,
> > + 123456789)) {
> > + fprintf(stderr,
> > + "FAIL: Couldn't set reply_serial\n");
> > + return 1;
> > + }
> > +
> > + /* Make up an error */
> > + if (!dbus_message_set_error_name(message,
> > + DBUS_ERROR_PROPERTY_READ_ONLY)) {
> > + fprintf(stderr,
> > + "FAIL: Couldn't set error name\n");
> > + return 1;
> > + }
> > + }
> > + } else {
> > + fprintf(stderr, "FAIL: Internal error, unknown message type\n");
> > + return 1;
> > + }
> > +
> > + if (message == NULL) {
> > + fprintf(stderr, "FAIL: Couldn't allocate D-Bus message\n");
> > + return 1;
> > + }
> > +
> > + if (!dbus_message_set_destination(message, name)) {
> > + fprintf(stderr, "FAIL: Not enough memory\n");
> > + return 1;
> > + }
> > +
> > + log_message(log_fd, "sent ", message);
> > + dbus_connection_send(connection, message, NULL);
> > + dbus_connection_flush(connection);
> > +
> > + dbus_message_unref(message);
> > +
> > + return 0;
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > + int i, rc;
> > +
> > + if (argc < 3)
> > + usage(1);
> > +
> > + for (i = 1; i < argc; i++) {
> > + char *arg = argv[i];
> > +
> > + if (strcmp(arg, "--system") == 0) {
> > + type = DBUS_BUS_SYSTEM;
> > + session_or_system = TRUE;
> > + } else if (strcmp(arg, "--session") == 0) {
> > + type = DBUS_BUS_SESSION;
> > + session_or_system = TRUE;
> > + } else if (strstr(arg, "--address") == arg) {
> > + address = strchr(arg, '=');
> > +
> > + if (address == NULL) {
> > + fprintf(stderr,
> > + "FAIL: \"--address=\" requires an ADDRESS\n");
> > + usage(1);
> > + } else {
> > + address = address + 1;
> > + }
> > + } else if (strstr(arg, "--name=") == arg)
> > + name = strchr(arg, '=') + 1;
>
> No NULL check here..
>
> > + else if (strstr(arg, "--type=") == arg)
> > + type_str = strchr(arg, '=') + 1;
>
> No NULL check here..
>
> > + else if (strstr(arg, "--log=") == arg) {
> > + char *path = strchr(arg, '=') + 1;
>
> No NULL check here..
Thanks for the review. I'll add these checks in before committing to
trunk.
Tyler
>
> > +
> > + log_fd = open(path, O_CREAT | O_TRUNC | O_WRONLY,
> > + S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP |
> > + S_IROTH | S_IWOTH);
> > + if (log_fd < 0) {
> > + fprintf(stderr,
> > + "FAIL: Couldn't open log file \"%s\": %m\n",
> > + path);
> > + exit(1);
> > + }
> > + } else if (!strcmp(arg, "--help"))
> > + usage(0);
> > + else if (arg[0] == '-')
> > + usage(1);
> > + else
> > + usage(1);
> > + }
> > +
> > + if (!name)
> > + usage(1);
> > +
> > + if (!type_str) {
> > + usage(1);
> > + } else {
> > + message_type = dbus_message_type_from_string(type_str);
> > + if (message_type != DBUS_MESSAGE_TYPE_METHOD_RETURN &&
> > + message_type != DBUS_MESSAGE_TYPE_ERROR) {
> > + fprintf(stderr,
> > + "FAIL: Message type \"%s\" is not supported\n",
> > + type_str);
> > + exit(1);
> > + }
> > + }
> > +
> > + if (session_or_system && address != NULL) {
> > + fprintf(stderr,
> > + "FAIL: \"--address\" may not be used with \"--system\" or \"--session\"\n");
> > + usage(1);
> > + }
> > +
> > + dbus_error_init(&error);
> > +
> > + if (address != NULL)
> > + connection = dbus_connection_open(address, &error);
> > + else
> > + connection = dbus_bus_get(type, &error);
> > +
> > + if (connection == NULL) {
> > + fprintf(stderr,
> > + "FAIL: Failed to open connection to \"%s\" message bus: %s\n",
> > + (address !=
> > + NULL) ? address : ((type ==
> > + DBUS_BUS_SYSTEM) ? "system" :
> > + "session"), error.message);
> > + dbus_error_free(&error);
> > + exit(1);
> > + } else if (address != NULL)
> > + dbus_bus_register(connection, &error);
> > +
> > + rc = do_unrequested_reply();
> > + dbus_connection_unref(connection);
> > + if (rc == 0)
> > + printf("PASS\n");
> > +
> > + exit(rc);
> > +}
> > diff --git a/tests/regression/apparmor/dbus_unrequested_reply.sh b/tests/regression/apparmor/dbus_unrequested_reply.sh
> > new file mode 100644
> > index 0000000..1cfd8d4
> > --- /dev/null
> > +++ b/tests/regression/apparmor/dbus_unrequested_reply.sh
> > @@ -0,0 +1,126 @@
> > +#! /bin/bash
> > +# Copyright (C) 2013 Canonical, Ltd.
> > +#
> > +# This program is free software; you can redistribute it and/or
> > +# modify it under the terms of the GNU General Public License as
> > +# published by the Free Software Foundation, version 2 of the
> > +# License.
> > +
> > +#=NAME dbus_unrequested_reply
> > +#=DESCRIPTION
> > +# This test verifies that unrequested reply messages are not allowed through.
> > +#=END
> > +
> > +pwd=`dirname $0`
> > +pwd=`cd $pwd ; /bin/pwd`
> > +
> > +bin=$pwd
> > +
> > +. $bin/prologue.inc
> > +requires_features dbus
> > +. $bin/dbus.inc
> > +
> > +service="--$bus --name=$dest $path $iface"
> > +unconfined_log="${tmpdir}/unconfined.log"
> > +confined_log="${tmpdir}/confined.log"
> > +
> > +ur_runtestbg()
> > +{
> > + local lock=${tmpdir}/lock
> > + local lockfd=-1
> > + local args=$service
> > +
> > + if [ $# -gt 2 ]
> > + then
> > + args="--log=$3 $args"
> > + fi
> > +
> > + exec {lockfd}>$lock
> > + flock -n $lockfd
> > + args="--lock-fd=$lockfd $args"
> > +
> > + runtestbg "$1" "$2" $args
> > +
> > + exec {lockfd}>&-
> > + flock -w 30 $lock true
> > + rm $lock
> > +}
> > +
> > +ur_checktestbg()
> > +{
> > + kill -SIGTERM $_pid 2>/dev/null
> > + checktestbg "$@"
> > +}
> > +
> > +ur_runchecktest()
> > +{
> > + ur_runtestbg "$@"
> > + ur_checktestbg
> > +}
> > +
> > +ur_gendbusprofile()
> > +{
> > + gendbusprofile "$confined_log w,
> > + dbus bind bus=$bus name=$dest,
> > + $@"
> > +}
> > +
> > +start_bus
> > +
> > +settest dbus_service
> > +
> > +# Start a dbus service and send unrequested method_return and error messages to
> > +# the service. The service should always start and stop just fine. The test
> > +# results hinge on comparing the message log from confined services to the
> > +# message log from the initial unconfined run.
> > +
> > +# Do an unconfined run to get an "expected" log for comparisons
> > +ur_runtestbg "unrequested_reply (method_return, unconfined)" pass $unconfined_log
> > +sendmethodreturn
> > +ur_checktestbg
> > +
> > +# All dbus perms are granted so the logs should be equal
> > +ur_gendbusprofile "dbus,"
> > +ur_runtestbg "unrequested_reply (method_return, dbus allowed)" pass $confined_log
> > +sendmethodreturn
> > +ur_checktestbg "compare_logs $unconfined_log eq $confined_log"
> > +
> > +# Only send perm is granted so the confined service should not be able to
> > +# receive unrequested replies from the client
> > +ur_gendbusprofile "dbus send,"
> > +ur_runtestbg "unrequested_reply (method_return, send allowed)" pass $confined_log
> > +sendmethodreturn
> > +ur_checktestbg "compare_logs $unconfined_log ne $confined_log"
> > +
> > +# Send and receive perms are granted so the logs should be equal
> > +ur_gendbusprofile "dbus (send receive),"
> > +ur_runtestbg "unrequested_reply (method_return, send receive allowed)" pass $confined_log
> > +sendmethodreturn
> > +ur_checktestbg "compare_logs $unconfined_log eq $confined_log"
> > +
> > +# Now test unrequested error replies
> > +
> > +# Do an unconfined run to get an "expected" log for comparisons
> > +removeprofile
> > +ur_runtestbg "unrequested_reply (error, unconfined)" pass $unconfined_log
> > +senderror
> > +ur_checktestbg
> > +
> > +# All dbus perms are granted so the logs should be equal
> > +ur_gendbusprofile "dbus,"
> > +ur_runtestbg "unrequested_reply (error, dbus allowed)" pass $confined_log
> > +senderror
> > +ur_checktestbg "compare_logs $unconfined_log eq $confined_log"
> > +
> > +# Only send perm is granted so the confined service should not be able to
> > +# receive unrequested replies from the client
> > +ur_gendbusprofile "dbus send,"
> > +ur_runtestbg "unrequested_reply (error, send allowed)" pass $confined_log
> > +senderror
> > +ur_checktestbg "compare_logs $unconfined_log ne $confined_log"
> > +
> > +# Send and receive perms are granted so the logs should be equal
> > +ur_gendbusprofile "dbus (send receive),"
> > +ur_runtestbg "unrequested_reply (error, send receive allowed)" pass $confined_log
> > +senderror
> > +ur_checktestbg "compare_logs $unconfined_log eq $confined_log"
> > --
> > 2.1.0
> >
> >
> > --
> > AppArmor mailing list
> > AppArmor at lists.ubuntu.com
> > Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
> >
> --
> 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: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140905/db9ad1f7/attachment.pgp>
More information about the AppArmor
mailing list