[apparmor] [PATCH 7/6] libapparmor: Strip a trailing newline character in aa_splitcon(3)
Seth Arnold
seth.arnold at canonical.com
Tue May 19 22:04:10 UTC 2015
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 <tyhicks at canonical.com>
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 <seth.arnold 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
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150519/d6851a78/attachment.pgp>
More information about the AppArmor
mailing list