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 <[email protected]> --- 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 -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
