On Tue, May 19, 2015 at 09:32:45AM -0500, Tyler Hicks wrote: > 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]>
Very nice update, I like the table and the doubled tests, I think this is a more robust API for userspace as a result Acked-by: Seth Arnold <[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
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
