[apparmor] [PATCH 08/31] split routines for loading binary policy into its own file

Tyler Hicks tyhicks at canonical.com
Sat Dec 6 00:22:07 UTC 2014


From: John Johansen <john.johansen at canonical.com>

Signed-off-by: John Johansen <john.johansen at canonical.com>
[tyhicks: Handle inverted return from find_subdomainfs_mountpoint()]
[tyhicks: Link test progs to libapparmor to fix make check build fail]
[tyhicks: Migrate from opendir() to open() for opening apparmorfs]
[tyhicks: Make some of the split out functions static]
Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
---
 parser/Makefile           |  12 ++-
 parser/kernel_interface.c | 206 ++++++++++++++++++++++++++++++++++++++++++++++
 parser/kernel_interface.h |  25 ++++++
 parser/lib.c              |   2 +-
 parser/lib.h              |   2 +-
 parser/parser_interface.c |  83 -------------------
 parser/parser_main.c      |  24 +++---
 7 files changed, 252 insertions(+), 102 deletions(-)
 create mode 100644 parser/kernel_interface.c
 create mode 100644 parser/kernel_interface.h

diff --git a/parser/Makefile b/parser/Makefile
index f85b1fd..b95b9da 100644
--- a/parser/Makefile
+++ b/parser/Makefile
@@ -81,10 +81,10 @@ SRCS = parser_common.c parser_include.c parser_interface.c parser_lex.c \
        parser_yacc.c parser_regex.c parser_variable.c parser_policy.c \
        parser_alias.c common_optarg.c lib.c network.c \
        mount.cc dbus.cc profile.cc rule.cc signal.cc ptrace.cc \
-       af_rule.cc af_unix.cc features.c policy_cache.c
+       af_rule.cc af_unix.cc features.c policy_cache.c kernel_interface.c
 HDRS = parser.h parser_include.h immunix.h mount.h dbus.h lib.h profile.h \
        rule.h common_optarg.h signal.h ptrace.h network.h af_rule.h af_unix.h \
-       features.h policy_cache.h
+       features.h policy_cache.h kernel_interface.h
 TOOLS = apparmor_parser
 
 OBJECTS = $(patsubst %.cc, %.o, $(SRCS:.c=.o))
@@ -125,6 +125,7 @@ TEST_OBJECTS = $(filter-out \
 			policy_cache.o, ${OBJECTS}) \
                $(AAREOBJECTS)
 TEST_LDFLAGS = $(AARE_LDFLAGS)
+TEST_LDLIBS = $(AALIB)
 
 ifdef V
   VERBOSE = 1
@@ -248,6 +249,9 @@ features.o: features.c features.h parser.h libapparmor_re/apparmor_re.h
 policy_cache.o: policy_cache.c policy_cache.h parser.h
 	$(CXX) $(EXTRA_CFLAGS) -c -o $@ $<
 
+kernel_interface.o: kernel_interface.c kernel_interface.h
+	$(CXX) $(EXTRA_CFLAGS) -c -o $@ $<
+
 lib.o: lib.c lib.h parser.h
 	$(CXX) $(EXTRA_CFLAGS) -c -o $@ $<
 
@@ -293,9 +297,9 @@ cap_names.h: /usr/include/linux/capability.h
 	echo "$(CAPABILITIES)" | LC_ALL=C sed -n -e "s/[ \\t]\\?CAP_\\([A-Z0-9_]\\+\\)/\{\"\\L\\1\", \\UCAP_\\1\},\\n/pg" > $@
 
 tst_lib: lib.c parser.h $(filter-out lib.o, ${TEST_OBJECTS})
-	$(CXX) $(TEST_CFLAGS) -o $@ $< $(filter-out $(<:.c=.o), ${TEST_OBJECTS}) $(TEST_LDFLAGS)
+	$(CXX) $(TEST_CFLAGS) -o $@ $< $(filter-out $(<:.c=.o), ${TEST_OBJECTS}) $(TEST_LDFLAGS) $(TEST_LDLIBS)
 tst_%: parser_%.c parser.h $(filter-out parser_%.o, ${TEST_OBJECTS})
-	$(CXX) $(TEST_CFLAGS) -o $@ $< $(filter-out $(<:.c=.o), ${TEST_OBJECTS}) $(TEST_LDFLAGS)
+	$(CXX) $(TEST_CFLAGS) -o $@ $< $(filter-out $(<:.c=.o), ${TEST_OBJECTS}) $(TEST_LDFLAGS) $(TEST_LDLIBS)
 
 .SILENT: check
 .PHONY: check
diff --git a/parser/kernel_interface.c b/parser/kernel_interface.c
new file mode 100644
index 0000000..5585249
--- /dev/null
+++ b/parser/kernel_interface.c
@@ -0,0 +1,206 @@
+/*
+ *   Copyright (c) 2014
+ *   Canonical, Ltd. (All rights reserved)
+ *
+ *   This program is free software; you can redistribute it and/or
+ *   modify it under the terms of version 2 of the GNU General Public
+ *   License published by the Free Software Foundation.
+ *
+ *   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, contact Novell, Inc. or Canonical
+ *   Ltd.
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/apparmor.h>
+#include <unistd.h>
+
+#include "kernel_interface.h"
+#include "lib.h"
+#include "parser.h"
+
+#define DEFAULT_APPARMORFS "/sys/kernel/security/apparmor"
+
+/**
+ * aa_find_iface_dir - find where the apparmor interface is located
+ * @dir - RETURNs: stored location of interface director
+ *
+ * Returns: 0 on success, -1 with errno set if there is an error
+ */
+int aa_find_iface_dir(char **dir)
+{
+	if (aa_find_mountpoint(dir) == -1) {
+		struct stat buf;
+		if (stat(DEFAULT_APPARMORFS, &buf) == -1) {
+			return -1;
+		} else {
+			*dir = strdup(DEFAULT_APPARMORFS);
+			if (*dir == NULL)
+				return -1;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * open_iface_dir - open the apparmor interface dir
+ *
+ * Returns: opened file descriptor, or -1 with errno on error
+ */
+static int open_iface_dir(void)
+{
+	autofree char *dir = NULL;
+
+	if (aa_find_iface_dir(&dir) == -1)
+		return -1;
+
+	return open(dir, O_RDONLY | O_CLOEXEC | O_DIRECTORY);
+}
+
+
+/* bleah the kernel should just loop and do multiple load, but to support
+ * older systems we need to do this
+ */
+#define PROFILE_HEADER_SIZE
+static char header_version[] = "\x04\x08\x00version";
+
+static const char *next_profile_buffer(const char *buffer, int size)
+{
+	const char *b = buffer;
+
+	for (; size - sizeof(header_version); b++, size--) {
+		if (memcmp(b, header_version, sizeof(header_version)) == 0) {
+			return b;
+		}
+	}
+	return NULL;
+}
+
+static int write_buffer(int fd, const char *buffer, int size, int set)
+{
+	const char *err_str = set ? "profile set" : "profile";
+	int wsize = write(fd, buffer, size);
+	if (wsize < 0) {
+		PERROR(_("%s: Unable to write %s\n"), progname, err_str);
+		return -1;
+	} else if (wsize < size) {
+		PERROR(_("%s: Unable to write %s\n"), progname, err_str);
+		errno = EPROTO;
+		return -1;
+	}
+	return 0;
+}
+
+/**
+ * write_policy_buffer - load compiled policy into the kernel
+ * @fd: kernel iterface to write to
+ * @atomic: whether to load all policy in buffer atomically (true)
+ * @buffer: buffer of policy to load
+ * @size: the size of the data in the buffer
+ *
+ * Returns: 0 if the buffer loaded correctly
+ *         -1 if the load failed with errno set to the error
+ *
+ * @atomic should only be set to true if the kernel supports atomic profile
+ * set loads, otherwise only the 1st profile in the buffer will be loaded
+ * (older kernels only support loading one profile at a time).
+ */
+static int write_policy_buffer(int fd, int atomic,
+			       const char *buffer, size_t size)
+{
+	size_t bsize;
+	int rc;
+
+	if (atomic) {
+		rc = write_buffer(fd, buffer, size, true);
+	} else {
+		const char *b, *next;
+
+		rc = 0;	/* in case there are no profiles */
+		for (b = buffer; b; b = next, size -= bsize) {
+			next = next_profile_buffer(b + sizeof(header_version),
+						   size);
+			if (next)
+				bsize = next - b;
+			else
+				bsize = size;
+			if (write_buffer(fd, b, bsize, false) == -1)
+				return -1;
+		}
+	}
+
+	if (rc)
+		return -1;
+
+	return 0;
+}
+
+/**
+ * open_option_iface - open the interface file for @option
+ * @aadir: apparmorfs dir
+ * @option: load command option
+ *
+ * Returns: fd to interface or -1 on error, with errno set.
+ */
+static int open_option_iface(int aadir, int option)
+{
+	const char *name;
+
+	switch (option) {
+	case OPTION_ADD:
+		name = ".load";
+		break;
+	case OPTION_REPLACE:
+		name = ".replace";
+		break;
+	case OPTION_REMOVE:
+		name = ".remove";
+		break;
+	default:
+		errno = EINVAL;
+		return -1;
+	}
+
+	return openat(aadir, name, O_WRONLY);
+
+	/* TODO: push up */
+	/*
+	if (fd < 0) {
+		PERROR(_("Unable to open %s - %s\n"), filename,
+		       strerror(errno));
+		return -errno;
+	}
+	*/
+}
+
+int aa_load_buffer(int option, char *buffer, int size)
+{
+	autoclose int dirfd = -1;
+	autoclose int fd = -1;
+
+	/* TODO: push backup into caller */
+	if (!kernel_load)
+		return 0;
+
+	dirfd = open_iface_dir();
+	if (dirfd == -1)
+		return -1;
+
+	fd = open_option_iface(dirfd, option);
+	if (fd == -1)
+		return -1;
+
+	return write_policy_buffer(fd, kernel_supports_setload, buffer, size);
+}
diff --git a/parser/kernel_interface.h b/parser/kernel_interface.h
new file mode 100644
index 0000000..13e5996
--- /dev/null
+++ b/parser/kernel_interface.h
@@ -0,0 +1,25 @@
+/*
+ *   Copyright (c) 2014
+ *   Canonical, Ltd. (All rights reserved)
+ *
+ *   This program is free software; you can redistribute it and/or
+ *   modify it under the terms of version 2 of the GNU General Public
+ *   License published by the Free Software Foundation.
+ *
+ *   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, contact Novell, Inc. or Canonical
+ *   Ltd.
+ */
+
+#ifndef __AA_KERNEL_INTERFACE_H
+#define __AA_KERNEL_INTERFACE_H
+
+int aa_find_iface_dir(char **dir);
+int aa_load_buffer(int option, char *buffer, int size);
+
+#endif /* __AA_KERNEL_INTERFACE_H */
diff --git a/parser/lib.c b/parser/lib.c
index 38b4342..602848c 100644
--- a/parser/lib.c
+++ b/parser/lib.c
@@ -167,7 +167,7 @@ fail:
  *
  * Returns: true if an octal digit, else false
  */
-bool isodigit(char c)
+int isodigit(char c)
 {
 	return (c >= '0' && c <= '7') ? true : false;
 }
diff --git a/parser/lib.h b/parser/lib.h
index f047fd8..4cdf428 100644
--- a/parser/lib.h
+++ b/parser/lib.h
@@ -13,7 +13,7 @@ void __autofclose(FILE **f);
 int dirat_for_each(DIR *dir, const char *name, void *data,
 		   int (* cb)(DIR *, const char *, struct stat *, void *));
 
-bool isodigit(char c);
+int isodigit(char c);
 long strntol(const char *str, const char **endptr, int base, long maxval,
 	     size_t n);
 int strn_escseq(const char **pos, const char *chrs, size_t n);
diff --git a/parser/parser_interface.c b/parser/parser_interface.c
index 7829b22..895b2bc 100644
--- a/parser/parser_interface.c
+++ b/parser/parser_interface.c
@@ -604,86 +604,3 @@ exit:
 	return error;
 }
 
-/* bleah the kernel should just loop and do multiple load, but to support
- * older systems we need to do this
- */
-#define PROFILE_HEADER_SIZE
-static char header_version[] = "\x04\x08\x00version";
-
-static char *next_profile_buffer(char *buffer, int size)
-{
-	char *b = buffer;
-
-	for (; size - sizeof(header_version); b++, size--) {
-		if (memcmp(b, header_version, sizeof(header_version)) == 0) {
-			return b;
-		}
-	}
-	return NULL;
-}
-
-static int write_buffer(int fd, char *buffer, int size, bool set)
-{
-	const char *err_str = set ? "profile set" : "profile";
-	int wsize = write(fd, buffer, size);
-	if (wsize < 0) {
-		PERROR(_("%s: Unable to write %s\n"), progname, err_str);
-		return -errno;
-	} else if (wsize < size) {
-		PERROR(_("%s: Unable to write %s\n"), progname, err_str);
-		return -EPROTO;
-	}
-	return 0;
-}
-
-int sd_load_buffer(int option, char *buffer, int size)
-{
-	autoclose int fd = -1;
-	int error, bsize;
-	autofree char *filename = NULL;
-
-	/* TODO: push backup into caller */
-	if (!kernel_load)
-		return 0;
-
-	switch (option) {
-	case OPTION_ADD:
-		if (asprintf(&filename, "%s/.load", subdomainbase) == -1)
-			return -ENOMEM;
-		break;
-	case OPTION_REPLACE:
-		if (asprintf(&filename, "%s/.replace", subdomainbase) == -1)
-			return -ENOMEM;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	fd = open(filename, O_WRONLY);
-	if (fd < 0) {
-		PERROR(_("Unable to open %s - %s\n"), filename,
-		       strerror(errno));
-		return -errno;
-	}
-
-	if (kernel_supports_setload) {
-		error = write_buffer(fd, buffer, size, true);
-	} else {
-		char *b, *next;
-
-		error = 0;	/* in case there are no profiles */
-		for (b = buffer; b; b = next, size -= bsize) {
-			next = next_profile_buffer(b + sizeof(header_version),
-						   size);
-			if (next)
-				bsize = next - b;
-			else
-				bsize = size;
-			error = write_buffer(fd, b, bsize, false);
-			if (error)
-				break;
-		}
-	}
-
-	return error;
-}
diff --git a/parser/parser_main.c b/parser/parser_main.c
index 7ef74e7..09625c3 100644
--- a/parser/parser_main.c
+++ b/parser/parser_main.c
@@ -41,6 +41,7 @@
 
 #include "lib.h"
 #include "features.h"
+#include "kernel_interface.h"
 #include "parser.h"
 #include "parser_version.h"
 #include "parser_include.h"
@@ -50,7 +51,6 @@
 
 #define OLD_MODULE_NAME "subdomain"
 #define PROC_MODULES "/proc/modules"
-#define DEFAULT_APPARMORFS "/sys/kernel/security/" MODULE_NAME
 #define MOUNTED_FS "/proc/mounts"
 #define AADFA "pattern=aadfa"
 
@@ -512,18 +512,14 @@ static int process_config_file(const char *name)
 
 int find_subdomainfs_mountpoint(void)
 {
-	if (aa_find_mountpoint(&subdomainbase) == -1) {
-		struct stat buf;
-		if (stat(DEFAULT_APPARMORFS, &buf) == -1) {
+	if (aa_find_iface_dir(&subdomainbase) == -1) {
 		PERROR(_("Warning: unable to find a suitable fs in %s, is it "
 			 "mounted?\nUse --subdomainfs to override.\n"),
 		       MOUNTED_FS);
-		} else {
-			subdomainbase = strdup(DEFAULT_APPARMORFS);
-		}
+		return false;
 	}
 
-	return (subdomainbase == NULL);
+	return true;
 }
 
 int have_enough_privilege(void)
@@ -624,9 +620,11 @@ int process_binary(int option, const char *profilename)
 			size += rsize;
 	} while (rsize > 0);
 
-	if (rsize == 0)
-		retval = sd_load_buffer(option, buffer, size);
-	else
+	if (rsize == 0) {
+		retval = aa_load_buffer(option, buffer, size);
+		if (retval == -1)
+			retval = -errno;
+	} else
 		retval = rsize;
 
 	if (conf_verbose) {
@@ -969,8 +967,8 @@ int main(int argc, char *argv[])
 
 	/* Check to make sure there is an interface to load policy */
 	if (!(UNPRIVILEGED_OPS) && (subdomainbase == NULL) &&
-	    (retval = find_subdomainfs_mountpoint())) {
-		return retval;
+	    !find_subdomainfs_mountpoint()) {
+		return 1;
 	}
 
 	if (!binary_input) parse_default_paths();
-- 
2.1.0




More information about the AppArmor mailing list