[apparmor] [patch v3] tests: readdir - test both getdents() and getdents64() if available
Steve Beattie
steve at nxnw.org
Wed Apr 5 23:48:34 UTC 2017
On Wed, Apr 05, 2017 at 04:09:15PM -0500, Tyler Hicks wrote:
> > +#if defined(SYS_getdents) && defined(SYS_getdents64)
> > + if (rc != rc64) {
> > + printf("FAIL - getdents and getdents64 returned different values: %d vs %d\n", rc, rc64);
>
> I feel like this should be ERROR instead of FAIL. If we use FAIL here,
> the test will "pass" in an expected fail test case when getdents()
> returns something different than getdents64(). I don't see a way around
> that other than printing a different (unexpected) output than FAIL.
>
> swap.sh looks to be the only other test using ERROR. prologue.inc
> doesn't know about ERROR and handles it by calling testerror().
>
> We're getting into extreme corner case territory here. As before, I'm
> prepared to declare "good enough". Let me know your thoughts.
Hrm, yeah, you're right on that. Though, in the read access not
permitted case, we should be getting rejected on the open() call,
not the getdents()/getdents64() calls.
Anyway, we're probably into over-engineered testcase area here, but what
follows is v3 of the patch.
Changes since v2:
- Convert to passing the expected return value to the readdir test, and
only fail if something unexpected is generated.
- Change the readdir.sh test to pass the expected return values,
and expect a PASS result for all tests.
- Add a testcase that confirms that granting write access to a
directory does not allow reading the directory's contents.
- Add a debug print function to readdir.c and use it in a few cases
when DEBUG is defined at compile time.
Subject: tests: readdir - test both getdents() and getdents64() if available
In commit 3649, Colin King fixed the readdir test build issue where
aarch64 only supports getdetns64(), not getdents(). Realistically,
however, we want to ensure mediation occurs on both syscalls where
they exist. This patch changes the test to attempt performing both
versions of getdents(). Because we want to catch the situation where
the result of getdents differs from getdents64, we now pass in the
expected result.
Also add a test to verify that having write access does not grant
the ability to read a directory's contents.
Bug: https://bugs.launchpad.net/bugs/1674245
Signed-off-by: Steve Beattie <steve at nxnw.org>
---
tests/regression/apparmor/readdir.c | 129 ++++++++++++++++++++++++++++-------
tests/regression/apparmor/readdir.sh | 21 ++++-
2 files changed, 120 insertions(+), 30 deletions(-)
Index: b/tests/regression/apparmor/readdir.c
===================================================================
--- a/tests/regression/apparmor/readdir.c
+++ b/tests/regression/apparmor/readdir.c
@@ -1,14 +1,20 @@
/*
* Copyright (C) 2002-2006 Novell/SUSE
+ * Copyright (C) 2017 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.
+ *
+ * We attempt to test both getdents() and getdents64() here, but
+ * some architectures like aarch64 only support getdents64().
*/
#define _GNU_SOURCE
#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
#include <unistd.h>
#include <errno.h>
#include <fcntl.h>
@@ -18,45 +24,118 @@
#include <string.h>
#include <sys/syscall.h>
-int main(int argc, char *argv[])
+/* error if neither SYS_getdents or SYS_getdents64 is defined */
+#if !defined(SYS_getdents) && !defined(SYS_getdents64)
+ #error "Neither SYS_getdents or SYS_getdents64 is defined, something has gone wrong!"
+#endif
+
+/* define DEBUG to enable debugging statements i.e. gcc -DDEBUG */
+#ifdef DEBUG
+void pdebug(const char *format, ...)
{
- int fd;
-#if defined(SYS_getdents64)
- struct dirent64 dir;
+ va_list arg;
+
+ va_start(arg, format);
+ vprintf(format, arg);
+ va_end(arg);
+}
#else
- struct dirent dir;
+void pdebug(const char *format, ...) { return; }
#endif
- if (argc != 2){
- fprintf(stderr, "usage: %s dir\n",
- argv[0]);
- return 1;
+#ifdef SYS_getdents
+int my_readdir(char *dirname)
+{
+ int fd;
+ struct dirent dir;
+
+ fd = open(dirname, O_RDONLY, 0);
+ if (fd == -1) {
+ pdebug("open failed: %s\n", strerror(errno));
+ return errno;
}
- fd = open(argv[1], O_RDONLY, 0);
- if (fd == -1){
- printf("FAIL - open %s\n", strerror(errno));
- return 1;
+ /* getdents isn't exported by glibc, so must use syscall() */
+ if (syscall(SYS_getdents, fd, &dir, sizeof(dir)) == -1){
+ pdebug("getdents failed: %s\n", strerror(errno));
+ close(fd);
+ return errno;
}
- /*
- if (fchdir(fd) == -1){
- printf("FAIL - fchdir %s\n", strerror(errno));
- return 1;
+ close(fd);
+ return 0;
+}
+#endif
+
+#ifdef SYS_getdents64
+int my_readdir64(char *dirname)
+{
+ int fd;
+ struct dirent64 dir;
+
+ fd = open(dirname, O_RDONLY, 0);
+ if (fd == -1) {
+ pdebug("open failed: %s\n", strerror(errno));
+ return errno;
}
- */
/* getdents isn't exported by glibc, so must use syscall() */
-#if defined(SYS_getdents64)
- if (syscall(SYS_getdents64, fd, &dir, sizeof(struct dirent64)) == -1){
-#else
- if (syscall(SYS_getdents, fd, &dir, sizeof(struct dirent)) == -1){
+ if (syscall(SYS_getdents64, fd, &dir, sizeof(dir)) == -1){
+ pdebug("getdents64 failed: %s\n", strerror(errno));
+ close(fd);
+ return errno;
+ }
+
+ close(fd);
+ return 0;
+}
#endif
- printf("FAIL - getdents %s\n", strerror(errno));
- return 1;
+
+int main(int argc, char *argv[])
+{
+ int rc = 0, err = 0;
+ char *dirpath, *endptr;
+ int expected;
+
+ if (argc != 3) {
+ fprintf(stderr, "usage: %s [dir] [expected retval]\n",
+ argv[0]);
+ err = 1;
+ goto err;
}
+ dirpath = argv[1];
+
+ errno = 0;
+ expected = (int) strtol(argv[2], &endptr, 10);
+ if (errno != 0 || endptr == argv[2]) {
+ fprintf(stderr, "ERROR: couldn't convert '%s' to an integer\n",
+ argv[2]);
+ err = 1;
+ goto err;
+ }
+ pdebug("expected = %d\n", expected);
+
+#ifdef SYS_getdents
+ rc = my_readdir(dirpath);
+ if (rc != expected) {
+ printf("FAIL - my_readdir returned %d, expected %d\n", rc, expected);
+ err = rc;
+ goto err;
+ }
+#endif
+
+#ifdef SYS_getdents64
+ rc = my_readdir64(argv[1]);
+ if (rc != expected) {
+ printf("FAIL - my_readdir64 returned %d, expected %d\n", rc, expected);
+ err = rc;
+ goto err;
+ }
+#endif
+
printf("PASS\n");
- return 0;
+err:
+ return err;
}
Index: b/tests/regression/apparmor/readdir.sh
===================================================================
--- a/tests/regression/apparmor/readdir.sh
+++ b/tests/regression/apparmor/readdir.sh
@@ -1,5 +1,6 @@
#! /bin/bash
# Copyright (C) 2002-2005 Novell/SUSE
+# Copyright (C) 2017 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
@@ -8,7 +9,7 @@
#=NAME readdir
#=DESCRIPTION
-# AppArmor requires 'r' permission on a directory in order for a confined task
+# AppArmor requires 'r' permission on a directory in order for a confined task
# to be able to read the directory contents. This test verifies this.
#=END
@@ -26,20 +27,30 @@ badperm=ix
mkdir $dir
+# The readdir utility expects the return value to be passed as the
+# second argument and returns success if the succeeding or failing calls
+# match the expected value. It will fail the test if they don't, so for
+# example the result differs acrorss getdents() and getdents64() this
+# will be detected.
+
# READDIR TEST
genprofile $dir/:$okperm
-runchecktest "READDIR" pass $dir
+runchecktest "READDIR" pass $dir 0
# READDIR TEST (no perm)
genprofile $dir/:$badperm
-runchecktest "READDIR (no perm)" fail $dir
+runchecktest "READDIR (no perm)" pass $dir 13
+
+# READDIR TEST (write perm) - ensure write perm isn't sufficient
+genprofile $dir/:w
+runchecktest "READDIR (write perm)" pass $dir 13
# this test is to make sure the raw 'file' rule allows access
# to directories
genprofile file
-runchecktest "READDIR 'file' dir" pass $dir
+runchecktest "READDIR 'file' dir" pass $dir 0
# this test is to make sure the raw 'file' rule allows access
# to '/'
genprofile file
-runchecktest "READDIR 'file' '/'" pass '/'
+runchecktest "READDIR 'file' '/'" pass '/' 0
--
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170405/a5910f88/attachment.pgp>
More information about the AppArmor
mailing list