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 <tyhi...@canonical.com>
Acked-by: Seth Arnold <seth.arn...@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 <ph...@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..
> +
> + 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@lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
>
signature.asc
Description: Digital signature
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
