libapparmor: Simplify aa_policy_cache API

This patch changes the aa_policy_cache_new() prototype and gets rid of
aa_policy_cache_is_valid() and aa_policy_cache_create().

The create bool of aa_policy_cache_new() is replaced with a 16 bit
unsigned int used to specify the maximum number of caches that should be
present in the specified cache directory. If the number is exceeded, the
old cache directories are reaped. The definition of "old" is private to
libapparmor and only 1 cache directory is currently supported. However,
that will change in the near future and multiple cache directories will
be supported.

If 0 is specified for the max_caches parameter, no new caches can be
created and only an existing, valid cache can be used. An error is
returned if no valid caches exist in that case.

If UINT16_MAX is specified, an unlimited amount of caches can be created
and reaping is disabled.

This means that 0 to (2^16)-2, or infinite, caches will be supported in
the future.

This change allows for the parser to continue to support the
--skip-bad-cache (by passing 0 for max_caches) and the --write-cache
option (by passing 1 or more for max_caches) without confusing
libapparmor users with the aa_policy_cache_{is_valid,create}()
functions.

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>

libraries/libapparmor/include/sys/apparmor.h

@@ -175,12 +175,10 @@ int aa_kernel_interface_write_policy(int fd, const char *buffer, size_t size);
 typedef struct aa_policy_cache aa_policy_cache;
 int aa_policy_cache_new(aa_policy_cache **policy_cache,
 			aa_features *kernel_features, const char *path,
-			bool create);
+			uint16_t max_caches);
 aa_policy_cache *aa_policy_cache_ref(aa_policy_cache *policy_cache);
 void aa_policy_cache_unref(aa_policy_cache *policy_cache);
 
-bool aa_policy_cache_is_valid(aa_policy_cache *policy_cache);
-int aa_policy_cache_create(aa_policy_cache *policy_cache);
 int aa_policy_cache_remove(const char *path);
 int aa_policy_cache_replace_all(aa_policy_cache *policy_cache,
 				aa_kernel_interface *kernel_interface);

libraries/libapparmor/src/libapparmor.map

@@ -80,8 +80,6 @@ APPARMOR_2.10 {
         aa_policy_cache_new;
         aa_policy_cache_ref;
         aa_policy_cache_unref;
-        aa_policy_cache_is_valid;
-        aa_policy_cache_create;
         aa_policy_cache_remove;
         aa_policy_cache_replace_all;
         aa_splitcon;

libraries/libapparmor/src/policy_cache.c

@@ -85,16 +85,29 @@ error:
 static int init_cache_features(aa_policy_cache *policy_cache,
 			       aa_features *kernel_features, bool create)
 {
+	bool call_create_cache = false;
+
 	if (aa_features_new(&policy_cache->features,
 			    policy_cache->features_path)) {
 		policy_cache->features = NULL;
 		if (!create || errno != ENOENT)
 			return -1;
 
-		return create_cache(policy_cache, kernel_features);
+		/* The cache directory needs to be created */
+		call_create_cache = true;
+	} else if (!aa_features_is_equal(policy_cache->features,
+					 kernel_features)) {
+		if (!create) {
+			errno = ENOENT;
+			return -1;
 		}
 
-	return 0;
+		/* The cache directory needs to be refreshed */
+		call_create_cache = true;
+	}
+
+	return call_create_cache ?
+		create_cache(policy_cache, kernel_features) : 0;
 }
 
 struct replace_all_cb_data {
@@ -131,16 +144,21 @@ static int replace_all_cb(DIR *dir unused, const char *name, struct stat *st,
  *                aa_policy_cache_new object upon success
  * @kernel_features: features representing the currently running kernel
  * @path: path to the policy cache
- * @create: true if the cache should be created if it doesn't already exist
+ * @max_caches: The maximum number of policy caches, one for each unique set of
+ *              kernel features, before older caches are auto-reaped. 0 means
+ *              that no new caches should be created (existing, valid caches
+ *              will be used) and auto-reaping is disabled. UINT16_MAX means
+ *              that a cache can be created and auto-reaping is disabled.
  *
  * Returns: 0 on success, -1 on error with errno set and *@policy_cache
  *          pointing to NULL
  */
 int aa_policy_cache_new(aa_policy_cache **policy_cache,
 			aa_features *kernel_features, const char *path,
-			bool create)
+			uint16_t max_caches)
 {
 	aa_policy_cache *pc;
+	bool create = max_caches > 0;
 
 	*policy_cache = NULL;
 
@@ -149,6 +167,11 @@ int aa_policy_cache_new(aa_policy_cache **policy_cache,
 		return -1;
 	}
 
+	if (max_caches > 1) {
+		errno = ENOTSUP;
+		return -1;
+	}
+
 	pc = calloc(1, sizeof(*pc));
 	if (!pc) {
 		errno = ENOMEM;
@@ -211,31 +234,6 @@ void aa_policy_cache_unref(aa_policy_cache *policy_cache)
 	}
 }
 
-/**
- * aa_policy_cache_is_valid - checks if the policy_cache is valid for the currently running kernel
- * @policy_cache: the policy_cache
- *
- * Returns: true if the policy_cache is valid for the currently running kernel,
- *          false if not
- */
-bool aa_policy_cache_is_valid(aa_policy_cache *policy_cache)
-{
-	return aa_features_is_equal(policy_cache->features,
-				    policy_cache->kernel_features);
-}
-
-/**
- * aa_policy_cache_create - creates a valid policy_cache for the currently running kernel
- * @policy_cache: the policy_cache
- *
- * Returns: 0 on success, -1 on error with errno set and features pointing to
- *          NULL
- */
-int aa_policy_cache_create(aa_policy_cache *policy_cache)
-{
-	return create_cache(policy_cache, policy_cache->kernel_features);
-}
-
 /**
  * aa_policy_cache_remove - removes all policy cache files under a path
  * @path: the path to a policy cache directory

parser/parser_main.c

@@ -899,6 +899,8 @@ int main(int argc, char *argv[])
 
 	if ((!skip_cache && (write_cache || !skip_read_cache)) ||
 	    force_clear_cache) {
+		uint16_t max_caches = write_cache && cond_clear_cache ? 1 : 0;
+
 		if (!cacheloc && asprintf(&cacheloc, "%s/cache", basedir) == -1) {
 			PERROR(_("Memory allocation error."));
 			return 1;
@@ -918,7 +920,7 @@ int main(int argc, char *argv[])
 			pwarn(_("The --create-cache-dir option is deprecated. Please use --write-cache.\n"));
 
 		retval = aa_policy_cache_new(&policy_cache, features, cacheloc,
-					     write_cache);
+					     max_caches);
 		if (retval) {
 			if (errno != ENOENT) {
 				PERROR(_("Failed setting up policy cache (%s): %s\n"),
@@ -926,24 +928,18 @@ int main(int argc, char *argv[])
 				return 1;
 			}
 
-			write_cache = 0;
-			skip_read_cache = 0;
-		} else if (!aa_policy_cache_is_valid(policy_cache)) {
-			if (write_cache && cond_clear_cache &&
-			    aa_policy_cache_create(policy_cache)) {
-				if (show_cache)
+			if (show_cache) {
+				if (max_caches > 0)
 					PERROR("Cache write disabled: Cannot create cache '%s': %m\n",
 					       cacheloc);
-				write_cache = 0;
-				skip_read_cache = 1;
-			} else if (!write_cache || !cond_clear_cache) {
-				if (show_cache)
+				else
 					PERROR("Cache read/write disabled: Policy cache is invalid\n");
+			}
+
 			write_cache = 0;
 			skip_read_cache = 1;
 		}
 	}
-	}
 
 	retval = last_error = 0;
 	for (i = optind; i <= argc; i++) {

tests/regression/apparmor/aa_policy_cache.c

@@ -16,36 +16,33 @@
 
 #include <errno.h>
 #include <fcntl.h>
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
 #include <sys/apparmor.h>
 
-#define OPT_CREATE		"create"
-#define OPT_IS_VALID		"is-valid"
 #define OPT_NEW			"new"
-#define OPT_NEW_CREATE		"new-create"
 #define OPT_REMOVE		"remove"
 #define OPT_REMOVE_POLICY	"remove-policy"
 #define OPT_REPLACE_ALL		"replace-all"
+#define OPT_FLAG_MAX_CACHES	"--max-caches"
 
 static void usage(const char *prog)
 {
 	fprintf(stderr,
-		"FAIL - usage: %s %s <PATH>\n"
-		"              %s %s <PATH>\n"
-		"              %s %s <PATH>\n"
-		"              %s %s <PATH>\n"
+		"FAIL - usage: %s %s [%s N] <PATH>\n"
 		"              %s %s <PATH>\n"
 		"              %s %s <PROFILE_NAME>\n"
-		"              %s %s <PATH>\n",
-		prog, OPT_CREATE, prog, OPT_IS_VALID, prog, OPT_NEW,
-		prog, OPT_NEW_CREATE, prog, OPT_REMOVE, prog, OPT_REMOVE_POLICY,
-		prog, OPT_REPLACE_ALL);
+		"              %s %s [%s N] <PATH>\n",
+		prog, OPT_NEW, OPT_FLAG_MAX_CACHES,
+		prog, OPT_REMOVE,
+		prog, OPT_REMOVE_POLICY,
+		prog, OPT_REPLACE_ALL, OPT_FLAG_MAX_CACHES);
 }
 
-static int test_create(const char *path)
+static int test_new(const char *path, uint16_t max_caches)
 {
 	aa_features *features = NULL;
 	aa_policy_cache *policy_cache = NULL;
@@ -56,64 +53,7 @@ static int test_create(const char *path)
 		goto out;
 	}
 
-	if (aa_policy_cache_new(&policy_cache, features, path, false)) {
-		perror("FAIL - aa_policy_cache_new");
-		goto out;
-	}
-
-	if (aa_policy_cache_create(policy_cache)) {
-		perror("FAIL - aa_policy_cache_create");
-		goto out;
-	}
-
-	rc = 0;
-out:
-	aa_features_unref(features);
-	aa_policy_cache_unref(policy_cache);
-	return rc;
-}
-
-static int test_is_valid(const char *path)
-{
-	aa_features *features = NULL;
-	aa_policy_cache *policy_cache = NULL;
-	int rc = 1;
-
-	if (aa_features_new_from_kernel(&features)) {
-		perror("FAIL - aa_features_new_from_kernel");
-		goto out;
-	}
-
-	if (aa_policy_cache_new(&policy_cache, features, path, false)) {
-		perror("FAIL - aa_policy_cache_new");
-		goto out;
-	}
-
-	if (!aa_policy_cache_is_valid(policy_cache)) {
-		errno = EINVAL;
-		perror("FAIL - aa_policy_cache_is_valid");
-		goto out;
-	}
-
-	rc = 0;
-out:
-	aa_features_unref(features);
-	aa_policy_cache_unref(policy_cache);
-	return rc;
-}
-
-static int test_new(const char *path, bool create)
-{
-	aa_features *features = NULL;
-	aa_policy_cache *policy_cache = NULL;
-	int rc = 1;
-
-	if (aa_features_new_from_kernel(&features)) {
-		perror("FAIL - aa_features_new_from_kernel");
-		goto out;
-	}
-
-	if (aa_policy_cache_new(&policy_cache, features, path, create)) {
+	if (aa_policy_cache_new(&policy_cache, features, path, max_caches)) {
 		perror("FAIL - aa_policy_cache_new");
 		goto out;
 	}
@@ -167,7 +107,7 @@ out:
 	return rc;
 }
 
-static int test_replace_all(const char *path)
+static int test_replace_all(const char *path, uint16_t max_caches)
 {
 	aa_features *features = NULL;
 	aa_policy_cache *policy_cache = NULL;
@@ -178,7 +118,7 @@ static int test_replace_all(const char *path)
 		goto out;
 	}
 
-	if (aa_policy_cache_new(&policy_cache, features, path, false)) {
+	if (aa_policy_cache_new(&policy_cache, features, path, max_caches)) {
 		perror("FAIL - aa_policy_cache_new");
 		goto out;
 	}
@@ -197,27 +137,45 @@ out:
 
 int main(int argc, char **argv)
 {
+	uint16_t max_caches = 0;
+	const char *str = NULL;
 	int rc = 1;
 
-	if (argc != 3) {
+	if (!(argc == 3 || argc == 5)) {
 		usage(argv[0]);
 		exit(1);
 	}
 
-	if (strcmp(argv[1], OPT_CREATE) == 0) {
-		rc = test_create(argv[2]);
-	} else if (strcmp(argv[1], OPT_IS_VALID) == 0) {
-		rc = test_is_valid(argv[2]);
-	} else if (strcmp(argv[1], OPT_NEW) == 0) {
-		rc = test_new(argv[2], false);
-	} else if (strcmp(argv[1], OPT_NEW_CREATE) == 0) {
-		rc = test_new(argv[2], true);
-	} else if (strcmp(argv[1], OPT_REMOVE) == 0) {
-		rc = test_remove(argv[2]);
-	} else if (strcmp(argv[1], OPT_REMOVE_POLICY) == 0) {
-		rc = test_remove_policy(argv[2]);
+	str = argv[argc - 1];
+
+	if (argc == 5) {
+		unsigned long tmp;
+
+		errno = 0;
+		tmp = strtoul(argv[3], NULL, 10);
+		if ((errno == ERANGE && tmp == ULONG_MAX) ||
+		    (errno && tmp == 0)) {
+			perror("FAIL - strtoul");
+			exit(1);
+		}
+
+		if (tmp > UINT16_MAX) {
+			fprintf(stderr, "FAIL - %lu larger than UINT16_MAX\n",
+				tmp);
+			exit(1);
+		}
+
+		max_caches = tmp;
+	}
+
+	if (strcmp(argv[1], OPT_NEW) == 0) {
+		rc = test_new(str, max_caches);
+	} else if (strcmp(argv[1], OPT_REMOVE) == 0 && argc == 3) {
+		rc = test_remove(str);
+	} else if (strcmp(argv[1], OPT_REMOVE_POLICY) == 0 && argc == 3) {
+		rc = test_remove_policy(str);
 	} else if (strcmp(argv[1], OPT_REPLACE_ALL) == 0) {
-		rc = test_replace_all(argv[2]);
+		rc = test_replace_all(str, max_caches);
 	} else {
 		usage(argv[0]);
 	}

tests/regression/apparmor/aa_policy_cache.sh

@@ -36,7 +36,7 @@ remove_cachedir()
 
 create_empty_cache()
 {
-	$test new-create "$cachedir" > /dev/null
+	$test new --max-caches 1 "$cachedir" > /dev/null
 }
 
 create_cache_files()
@@ -105,28 +105,18 @@ runchecktest "AA_POLICY_CACHE new (no cachedir)" fail new "$cachedir"
 create_cachedir
 runchecktest "AA_POLICY_CACHE new (no .features)" fail new "$cachedir"
 remove_cachedir
-runchecktest "AA_POLICY_CACHE new-create (no cachedir)" pass new-create "$cachedir"
-runchecktest "AA_POLICY_CACHE new-create (existing cache)" pass new-create "$cachedir"
+runchecktest "AA_POLICY_CACHE new create (no cachedir)" pass new --max-caches 1 "$cachedir"
+runchecktest "AA_POLICY_CACHE new create (existing cache)" pass new --max-caches 1 "$cachedir"
 runchecktest "AA_POLICY_CACHE new (existing cache)" pass new "$cachedir"
 
-runchecktest "AA_POLICY_CACHE is-valid (good .features)" pass is-valid "$cachedir"
 install_bad_features_file
-runchecktest "AA_POLICY_CACHE is-valid (bad .features)" fail is-valid "$cachedir"
-remove_cachedir
-runchecktest "AA_POLICY_CACHE is-valid (no cachedir)" fail is-valid "$cachedir"
-
-create_cachedir
-install_bad_features_file
-runchecktest "AA_POLICY_CACHE create (bad .features)" pass create "$cachedir"
-runchecktest "AA_POLICY_CACHE create (good .features)" pass create "$cachedir"
-remove_features_file
-runchecktest "AA_POLICY_CACHE create (no .features)" fail create "$cachedir"
-remove_cachedir
-runchecktest "AA_POLICY_CACHE create (no cachedir)" fail create "$cachedir"
+runchecktest "AA_POLICY_CACHE new (bad .features)" fail new "$cachedir"
+runchecktest "AA_POLICY_CACHE new create (bad .features)" pass new --max-caches 1 "$cachedir"
 
 # Make sure that no test policies are already loaded
 verify_policies_are_not_loaded
 
+remove_cachedir
 runchecktest "AA_POLICY_CACHE replace-all (no cachedir)" fail replace-all "$cachedir"
 create_cachedir
 runchecktest "AA_POLICY_CACHE replace-all (no .features)" fail replace-all "$cachedir"