[apparmor] [PATCH 7/6] libapparmor: Strip a trailing newline character in aa_splitcon(3)

Tyler Hicks tyhicks at canonical.com
Tue May 19 14:32:45 UTC 2015


Adjust the internal splitcon() function to strip a single trailing
newline character when the bool strip_newline argument is true.

aa_getprocattr_raw(2) needs to set strip_newline to true since the
kernel appends a newline character to the end of the AppArmor contexts
read from /proc/>PID>/attr/current.

aa_splitcon(3) also sets strip_newline to true since it is unknown
whether the context is originated from a location that appends a newline
or not.

aa_getpeercon_raw(2) does not set strip_newline to true since it is
unexpected for the kernel to append a newline to the the buffer returned
from getsockopt(2).

This patch also creates tests specifically for splitcon() and updates
the aa_splitcon(3) man page.

Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
---
 libraries/libapparmor/doc/aa_splitcon.pod |   7 ++
 libraries/libapparmor/src/kernel.c        |  50 ++++++----
 libraries/libapparmor/src/tst_kernel.c    | 161 ++++++++++++++++++++++++------
 3 files changed, 166 insertions(+), 52 deletions(-)

diff --git a/libraries/libapparmor/doc/aa_splitcon.pod b/libraries/libapparmor/doc/aa_splitcon.pod
index a85b619..2a46dd2 100644
--- a/libraries/libapparmor/doc/aa_splitcon.pod
+++ b/libraries/libapparmor/doc/aa_splitcon.pod
@@ -40,6 +40,11 @@ terminated. The enforcement mode is also NUL terminated and the parenthesis
 surrounding the mode are removed. If @mode is non-NULL, it will point to the
 first character in the enforcement mode string on success.
 
+The Linux kernel's /proc/<PID>/attr/current interface appends a trailing
+newline character to AppArmor contexts that are read from that file. If @con
+contains a single trailing newline character, it will be stripped by
+aa_splitcon() prior to all other processing.
+
 =head1 RETURN VALUE
 
 Returns a pointer to the first character in the label string. NULL is returned
@@ -50,7 +55,9 @@ on error.
  Context                        Label               Mode 
  -----------------------------  ------------------  -------
  unconfined                     unconfined          NULL
+ unconfined\n                   unconfined          NULL
  /bin/ping (enforce)            /bin/ping           enforce
+ /bin/ping (enforce)\n          /bin/ping           enforce
  /usr/sbin/rsyslogd (complain)  /usr/sbin/rsyslogd  complain
 
 =head1 BUGS
diff --git a/libraries/libapparmor/src/kernel.c b/libraries/libapparmor/src/kernel.c
index 5f2d835..07bc9b4 100644
--- a/libraries/libapparmor/src/kernel.c
+++ b/libraries/libapparmor/src/kernel.c
@@ -171,19 +171,31 @@ static bool parse_unconfined(char *con, int size)
  * splitcon - split the confinement context into a label and mode
  * @con: the confinement context
  * @size: size of the confinement context (not including the NUL terminator)
+ * @strip_newline: true if a trailing newline character should be stripped
  * @mode: if non-NULL and a mode is present, will point to mode string in @con
  *  on success
  *
  * Modifies the @con string to split it into separate label and mode strings.
- * The @mode argument is optional. If @mode is NULL, @con will still be split
- * between the label and mode (if present) but @mode will not be set.
+ * If @strip_newline is true and @con contains a single trailing newline, it
+ * will be stripped on success (it will not be stripped on error). The @mode
+ * argument is optional. If @mode is NULL, @con will still be split between the
+ * label and mode (if present) but @mode will not be set.
  *
  * Returns: a pointer to the label string or NULL on error
  */
-static char *splitcon(char *con, int size, char **mode)
+static char *splitcon(char *con, int size, bool strip_newline, char **mode)
 {
 	char *label = NULL;
 	char *mode_str = NULL;
+	char *newline = NULL;
+
+	if (size == 0)
+		goto out;
+
+	if (strip_newline && con[size - 1] == '\n') {
+		newline = &con[size - 1];
+		size--;
+	}
 
 	if (parse_unconfined(con, size)) {
 		label = con;
@@ -203,6 +215,8 @@ static char *splitcon(char *con, int size, char **mode)
 		}
 	}
 out:
+	if (label && strip_newline && newline)
+		*newline = 0; /* overwrite '\n', if requested, on success */
 	if (mode)
 		*mode = mode_str;
 	return label;
@@ -214,15 +228,16 @@ out:
  * @mode: if non-NULL and a mode is present, will point to mode string in @con
  *  on success
  *
- * Modifies the @con string to split it into separate label and mode strings.
- * The @mode argument is optional. If @mode is NULL, @con will still be split
+ * Modifies the @con string to split it into separate label and mode strings. A
+ * single trailing newline character will be stripped from @con, if found. The
+ * @mode argument is optional. If @mode is NULL, @con will still be split
  * between the label and mode (if present) but @mode will not be set.
  *
  * Returns: a pointer to the label string or NULL on error
  */
 char *aa_splitcon(char *con, char **mode)
 {
-	return splitcon(con, strlen(con), mode);
+	return splitcon(con, strlen(con), true, mode);
 }
 
 /**
@@ -282,21 +297,18 @@ int aa_getprocattr_raw(pid_t tid, const char *attr, char *buf, int len,
 		errno = saved;
 		goto out;
 	} else if (size > 0 && buf[size - 1] != 0) {
-		char *nul;
-
 		/* check for null termination */
-		if (buf[size - 1] == '\n') {
-			nul = &buf[size - 1];
-		} else if (len == 0) {
-			errno = ERANGE;
-			goto out2;
-		} else {
-			nul = &buf[size];
-			size++;
+		if (buf[size - 1] != '\n') {
+			if (len == 0) {
+				errno = ERANGE;
+				goto out2;
+			} else {
+				buf[size] = 0;
+				size++;
+			}
 		}
 
-		*nul = 0;
-		if (splitcon(buf, nul - buf, mode) != buf) {
+		if (splitcon(buf, size, true, mode) != buf) {
 			errno = EINVAL;
 			goto out2;
 		}
@@ -663,7 +675,7 @@ int aa_getpeercon_raw(int fd, char *buf, int *len, char **mode)
 		}
 	}
 
-	if (splitcon(buf, optlen - 1, mode) != buf) {
+	if (splitcon(buf, optlen - 1, false, mode) != buf) {
 		rc = -1;
 		errno = EINVAL;
 		goto out;
diff --git a/libraries/libapparmor/src/tst_kernel.c b/libraries/libapparmor/src/tst_kernel.c
index 8a8e70e..a383774 100644
--- a/libraries/libapparmor/src/tst_kernel.c
+++ b/libraries/libapparmor/src/tst_kernel.c
@@ -19,7 +19,7 @@
 #include <stdio.h>
 #include <string.h>
 
-#include "features.c"
+#include "kernel.c"
 
 static int nullcmp_and_strcmp(const void *s1, const void *s2)
 {
@@ -30,6 +30,30 @@ static int nullcmp_and_strcmp(const void *s1, const void *s2)
 	return strcmp(s1, s2);
 }
 
+static int do_test_splitcon(char *con, int size, bool strip_nl, char **mode,
+			    const char *expected_label,
+			    const char *expected_mode, const char *error)
+{
+	char *label;
+	int rc = 0;
+
+	label = splitcon(con, size, strip_nl, mode);
+
+	if (nullcmp_and_strcmp(label, expected_label)) {
+		fprintf(stderr, "FAIL: %s: label \"%s\" != \"%s\"\n",
+			error, label, expected_label);
+		rc = 1;
+	}
+
+	if (mode && nullcmp_and_strcmp(*mode, expected_mode)) {
+		fprintf(stderr, "FAIL: %s: mode \"%s\" != \"%s\"\n",
+			error, *mode, expected_mode);
+		rc = 1;
+	}
+
+	return rc;
+}
+
 static int do_test_aa_splitcon(char *con, char **mode,
 			       const char *expected_label,
 			       const char *expected_mode, const char *error)
@@ -54,69 +78,136 @@ static int do_test_aa_splitcon(char *con, char **mode,
 	return rc;
 }
 
-#define TEST_SPLITCON(con, expected_label, expected_mode, error)	\
+#define TEST_SPLITCON(con, size, strip_nl, expected_label,		\
+		      expected_mode, error)				\
+	do {								\
+		char c1[] = con;					\
+		char c2[] = con;					\
+		size_t sz = size < 0 ? strlen(con) : size;		\
+		char *mode;						\
+									\
+		if (do_test_splitcon(c1, sz, strip_nl, &mode,		\
+				expected_label, expected_mode,		\
+				"splitcon: " error)) {			\
+			rc = 1;						\
+		} else if (do_test_splitcon(c2, sz, strip_nl, NULL,	\
+				expected_label, NULL,			\
+				"splitcon: " error " (NULL mode)")) {	\
+			rc = 1;						\
+		}							\
+	} while (0)
+
+#define TEST_AA_SPLITCON(con, expected_label, expected_mode, error)	\
 	do {								\
 		char c1[] = con;					\
 		char c2[] = con;					\
+		char c3[] = con "\n";					\
 		char *mode;						\
 									\
 		if (do_test_aa_splitcon(c1, &mode, expected_label,	\
-					expected_mode, error)) {	\
+				expected_mode, "aa_splitcon: " error)) {\
 			rc = 1;						\
 		} else if (do_test_aa_splitcon(c2, NULL, expected_label,\
-					       NULL,			\
-					       error " (NULL mode)")) {	\
+				NULL,					\
+				"aa_splitcon: " error " (NULL mode)")) {\
+			rc = 1;						\
+		} else if (do_test_aa_splitcon(c3, &mode,		\
+				expected_label, expected_mode,		\
+				"aa_splitcon: " error " (newline)")) {	\
 			rc = 1;						\
 		}							\
 	} while (0)
 
+static int test_splitcon(void)
+{
+	int rc = 0;
+
+	/**
+	 * NOTE: the TEST_SPLITCON() macro automatically generates
+	 * corresponding tests with a NULL mode pointer.
+	 */
+
+	TEST_SPLITCON("", 0, true, NULL, NULL, "empty string test #1");
+	TEST_SPLITCON("", 0, false, NULL, NULL, "empty string test #2");
+
+	TEST_SPLITCON("unconfined", -1, true, "unconfined", NULL,
+		      "unconfined #1");
+	TEST_SPLITCON("unconfined", -1, false, "unconfined", NULL,
+		      "unconfined #2");
+	TEST_SPLITCON("unconfined\n", -1, true, "unconfined", NULL,
+		      "unconfined #3");
+	TEST_SPLITCON("unconfined\n", -1, false, NULL, NULL,
+		      "unconfined #4");
+
+	TEST_SPLITCON("label (mode)", -1, true, "label", "mode",
+		      "basic split #1");
+	TEST_SPLITCON("label (mode)", -1, false, "label", "mode",
+		      "basic split #2");
+	TEST_SPLITCON("label (mode)\n", -1, true, "label", "mode",
+		      "basic split #3");
+	TEST_SPLITCON("label (mode)\n", -1, false, NULL, NULL,
+		      "basic split #4");
+
+	TEST_SPLITCON("/a/b/c (enforce)", -1, true, "/a/b/c", "enforce",
+		      "path enforce split #1");
+	TEST_SPLITCON("/a/b/c (enforce)", -1, false, "/a/b/c", "enforce",
+		      "path enforce split #2");
+	TEST_SPLITCON("/a/b/c (enforce)\n", -1, true, "/a/b/c", "enforce",
+		      "path enforce split #3");
+	TEST_SPLITCON("/a/b/c (enforce)\n", -1, false, NULL, NULL,
+		      "path enforce split #4");
+
+	return rc;
+}
+
 
 static int test_aa_splitcon(void)
 {
 	int rc = 0;
 
-	TEST_SPLITCON("label (mode)", "label", "mode", "basic split");
+	/**
+	 * NOTE: the TEST_AA_SPLITCON() macro automatically generates
+	 * corresponding tests with a NULL mode pointer and contexts with
+	 * trailing newline characters.
+	 */
 
-	TEST_SPLITCON("/a/b/c (enforce)", "/a/b/c", "enforce",
-		      "path enforce split");
+	TEST_AA_SPLITCON("label (mode)", "label", "mode", "basic split");
 
-	TEST_SPLITCON("/a/b/c (complain)", "/a/b/c", "complain",
-		      "path complain split");
+	TEST_AA_SPLITCON("/a/b/c (enforce)", "/a/b/c", "enforce",
+			 "path enforce split");
 
-	TEST_SPLITCON("profile_name (enforce)", "profile_name", "enforce",
-		      "name enforce split");
+	TEST_AA_SPLITCON("/a/b/c (complain)", "/a/b/c", "complain",
+			 "path complain split");
 
-	TEST_SPLITCON("profile_name (complain)", "profile_name", "complain",
-		      "name complain split");
+	TEST_AA_SPLITCON("profile_name (enforce)", "profile_name", "enforce",
+			 "name enforce split");
 
-	TEST_SPLITCON("unconfined", "unconfined", NULL, "unconfined");
+	TEST_AA_SPLITCON("profile_name (complain)", "profile_name", "complain",
+			 "name complain split");
 
-	TEST_SPLITCON("(odd) (enforce)", "(odd)", "enforce",
-		      "parenthesized label #1");
+	TEST_AA_SPLITCON("unconfined", "unconfined", NULL, "unconfined");
 
-	TEST_SPLITCON("(odd) (enforce) (enforce)", "(odd) (enforce)", "enforce",
-		      "parenthesized label #2");
+	TEST_AA_SPLITCON("(odd) (enforce)", "(odd)", "enforce",
+			 "parenthesized label #1");
 
-	TEST_SPLITCON("/usr/bin/😺 (enforce)", "/usr/bin/😺", "enforce",
-		      "non-ASCII path");
+	TEST_AA_SPLITCON("(odd) (enforce) (enforce)", "(odd) (enforce)",
+			 "enforce", "parenthesized label #2");
 
-	TEST_SPLITCON("👍 (enforce)", "👍", "enforce", "non-ASCII profile name");
+	TEST_AA_SPLITCON("/usr/bin/😺 (enforce)", "/usr/bin/😺", "enforce",
+			 "non-ASCII path");
 
-	/* Negative tests */
+	TEST_AA_SPLITCON("👍 (enforce)", "👍", "enforce",
+			 "non-ASCII profile name");
 
-	TEST_SPLITCON("", NULL, NULL, "empty string test");
-
-	TEST_SPLITCON("/a/b/c (complain)\n", NULL, NULL,
-		      "path split w/ invalid trailing newline");
+	/* Negative tests */
 
-	TEST_SPLITCON("unconfined\n", NULL, NULL,
-		      "unconfined w/ invalid trailing newline");
+	TEST_AA_SPLITCON("", NULL, NULL, "empty string test");
 
-	TEST_SPLITCON("profile\t(enforce)", NULL, NULL,
-		      "invalid tab separator");
+	TEST_AA_SPLITCON("profile\t(enforce)", NULL, NULL,
+			 "invalid tab separator");
 
-	TEST_SPLITCON("profile(enforce)", NULL, NULL,
-		      "invalid missing separator");
+	TEST_AA_SPLITCON("profile(enforce)", NULL, NULL,
+			 "invalid missing separator");
 
 	return rc;
 }
@@ -125,6 +216,10 @@ int main(void)
 {
 	int retval, rc = 0;
 
+	retval = test_splitcon();
+	if (retval)
+		rc = retval;
+
 	retval = test_aa_splitcon();
 	if (retval)
 		rc = retval;
-- 
2.1.4




More information about the AppArmor mailing list