From a0706d3a4688a145379985753749dfe6c1932bab Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 12 Feb 2015 10:19:16 -0800 Subject: [PATCH] And the related patch to fix globbing for af_unix abstract names Abstract af_unix socket names can contain a null character, however the aare to pcre conversion explicitly disallows null characters because they are not valid characters for pathnames. Fix this so that they type of globbing is selectable. this is a partial fix for Bug: http://bugs.launchpad.net/bugs/1413410 Signed-off-by: John Johansen Acked-by: Steve Beattie --- parser/af_unix.cc | 4 +- parser/dbus.cc | 12 +- parser/mount.cc | 2 +- parser/parser.h | 4 +- parser/parser_misc.c | 5 +- parser/parser_regex.c | 113 +++++++++++++----- parser/ptrace.cc | 2 +- parser/signal.cc | 2 +- .../tst/simple_tests/file/bad_embedded_0_1.sd | 9 ++ .../tst/simple_tests/file/bad_embedded_0_2.sd | 9 ++ .../tst/simple_tests/file/bad_embedded_0_3.sd | 9 ++ .../tst/simple_tests/unix/ok_embedded_0_1.sd | 8 ++ .../tst/simple_tests/unix/ok_embedded_0_2.sd | 8 ++ .../tst/simple_tests/unix/ok_embedded_0_3.sd | 8 ++ 14 files changed, 155 insertions(+), 40 deletions(-) create mode 100644 parser/tst/simple_tests/file/bad_embedded_0_1.sd create mode 100644 parser/tst/simple_tests/file/bad_embedded_0_2.sd create mode 100644 parser/tst/simple_tests/file/bad_embedded_0_3.sd create mode 100644 parser/tst/simple_tests/unix/ok_embedded_0_1.sd create mode 100644 parser/tst/simple_tests/unix/ok_embedded_0_2.sd create mode 100644 parser/tst/simple_tests/unix/ok_embedded_0_3.sd diff --git a/parser/af_unix.cc b/parser/af_unix.cc index 11fc137f2..b8eea55ed 100644 --- a/parser/af_unix.cc +++ b/parser/af_unix.cc @@ -243,7 +243,7 @@ bool unix_rule::write_addr(std::ostringstream &buffer, const char *addr) buffer << "\\x01"; } else { /* skip leading @ */ - ptype = convert_aaregex_to_pcre(addr + 1, 0, buf, &pos); + ptype = convert_aaregex_to_pcre(addr + 1, 0, glob_null, buf, &pos); if (ptype == ePatternInvalid) return false; /* kernel starts abstract with \0 */ @@ -267,7 +267,7 @@ bool unix_rule::write_label(std::ostringstream &buffer, const char *label) if (label) { int pos; - ptype = convert_aaregex_to_pcre(label, 0, buf, &pos); + ptype = convert_aaregex_to_pcre(label, 0, glob_default, buf, &pos); if (ptype == ePatternInvalid) return false; /* kernel starts abstract with \0 */ diff --git a/parser/dbus.cc b/parser/dbus.cc index 9cdb19d93..5f10a11c0 100644 --- a/parser/dbus.cc +++ b/parser/dbus.cc @@ -228,7 +228,7 @@ int dbus_rule::gen_policy_re(Profile &prof) busbuf.append(buffer.str()); if (bus) { - ptype = convert_aaregex_to_pcre(bus, 0, busbuf, &pos); + ptype = convert_aaregex_to_pcre(bus, 0, glob_default, busbuf, &pos); if (ptype == ePatternInvalid) goto fail; } else { @@ -238,7 +238,7 @@ int dbus_rule::gen_policy_re(Profile &prof) vec[0] = busbuf.c_str(); if (name) { - ptype = convert_aaregex_to_pcre(name, 0, namebuf, &pos); + ptype = convert_aaregex_to_pcre(name, 0, glob_default, namebuf, &pos); if (ptype == ePatternInvalid) goto fail; vec[1] = namebuf.c_str(); @@ -248,7 +248,7 @@ int dbus_rule::gen_policy_re(Profile &prof) } if (peer_label) { - ptype = convert_aaregex_to_pcre(peer_label, 0, + ptype = convert_aaregex_to_pcre(peer_label, 0, glob_default, peer_labelbuf, &pos); if (ptype == ePatternInvalid) goto fail; @@ -259,7 +259,7 @@ int dbus_rule::gen_policy_re(Profile &prof) } if (path) { - ptype = convert_aaregex_to_pcre(path, 0, pathbuf, &pos); + ptype = convert_aaregex_to_pcre(path, 0, glob_default, pathbuf, &pos); if (ptype == ePatternInvalid) goto fail; vec[3] = pathbuf.c_str(); @@ -269,7 +269,7 @@ int dbus_rule::gen_policy_re(Profile &prof) } if (interface) { - ptype = convert_aaregex_to_pcre(interface, 0, ifacebuf, &pos); + ptype = convert_aaregex_to_pcre(interface, 0, glob_default, ifacebuf, &pos); if (ptype == ePatternInvalid) goto fail; vec[4] = ifacebuf.c_str(); @@ -279,7 +279,7 @@ int dbus_rule::gen_policy_re(Profile &prof) } if (member) { - ptype = convert_aaregex_to_pcre(member, 0, memberbuf, &pos); + ptype = convert_aaregex_to_pcre(member, 0, glob_default, memberbuf, &pos); if (ptype == ePatternInvalid) goto fail; vec[5] = memberbuf.c_str(); diff --git a/parser/mount.cc b/parser/mount.cc index f525d6786..2181ccc0e 100644 --- a/parser/mount.cc +++ b/parser/mount.cc @@ -554,7 +554,7 @@ static int build_mnt_opts(std::string& buffer, struct value_list *opts) } list_for_each(opts, ent) { - ptype = convert_aaregex_to_pcre(ent->value, 0, buffer, &pos); + ptype = convert_aaregex_to_pcre(ent->value, 0, glob_default, buffer, &pos); if (ptype == ePatternInvalid) return FALSE; diff --git a/parser/parser.h b/parser/parser.h index c20109dce..da1b93d1a 100644 --- a/parser/parser.h +++ b/parser/parser.h @@ -334,7 +334,9 @@ extern const char *basedir; #define default_match_pattern "[^\\000]*" #define anyone_match_pattern "[^\\000]+" -extern pattern_t convert_aaregex_to_pcre(const char *aare, int anchor, +#define glob_default 0 +#define glob_null 1 +extern pattern_t convert_aaregex_to_pcre(const char *aare, int anchor, int glob, std::string& pcre, int *first_re_pos); extern int build_list_val_expr(std::string& buffer, struct value_list *list); extern int convert_entry(std::string& buffer, char *entry); diff --git a/parser/parser_misc.c b/parser/parser_misc.c index 97a2103de..d0b1f8dab 100644 --- a/parser/parser_misc.c +++ b/parser/parser_misc.c @@ -243,7 +243,10 @@ char *processunquoted(const char *string, int len) * pass it through to be handled by the backend * pcre conversion */ - if (strchr("*?[]{}^,\\", c) != NULL) { + if (c == 0) { + strncpy(s, string, pos - string); + s += pos - string; + } else if (strchr("*?[]{}^,\\", c) != NULL) { *s++ = '\\'; *s++ = c; } else diff --git a/parser/parser_regex.c b/parser/parser_regex.c index 77ed6c913..45f7f3ec6 100644 --- a/parser/parser_regex.c +++ b/parser/parser_regex.c @@ -29,6 +29,7 @@ /* #define DEBUG */ +#include "lib.h" #include "parser.h" #include "profile.h" #include "libapparmor_re/apparmor_re.h" @@ -83,9 +84,27 @@ static void filter_slashes(char *path) *dptr = 0; } +static error_type append_glob(std::string &pcre, int glob, + const char *default_glob, const char *null_glob) +{ + switch (glob) { + case glob_default: + pcre.append(default_glob); + break; + case glob_null: + pcre.append(null_glob); + break; + default: + PERROR(_("%s: Invalid glob type %d\n"), progname, glob); + return e_parse_error; + break; + } + return e_no_error; +} + /* converts the apparmor regex in aare and appends pcre regex output * to pcre string */ -pattern_t convert_aaregex_to_pcre(const char *aare, int anchor, +pattern_t convert_aaregex_to_pcre(const char *aare, int anchor, int glob, std::string& pcre, int *first_re_pos) { #define update_re_pos(X) if (!(*first_re_pos)) { *first_re_pos = (X); } @@ -170,9 +189,8 @@ pattern_t convert_aaregex_to_pcre(const char *aare, int anchor, const char *s = sptr; while (*s == '*') s++; - if (*s == '/' || !*s) { - pcre.append("[^/\\x00]"); - } + if (*s == '/' || !*s) + error = append_glob(pcre, glob, "[^/\\x00]", "[^/]"); } if (*(sptr + 1) == '*') { /* is this the first regex form we @@ -188,13 +206,12 @@ pattern_t convert_aaregex_to_pcre(const char *aare, int anchor, } else { ptype = ePatternRegex; } - - pcre.append("[^\\x00]*"); + error = append_glob(pcre, glob, "[^\\x00]*", ".*"); sptr++; } else { update_re_pos(sptr - aare); ptype = ePatternRegex; - pcre.append("[^/\\x00]*"); + error = append_glob(pcre, glob, "[^/\\x00]*", "[^/]*"); } /* *(sptr+1) == '*' */ } /* bEscape */ @@ -342,12 +359,26 @@ pattern_t convert_aaregex_to_pcre(const char *aare, int anchor, default: if (bEscape) { - /* quoting mark used for something that - * does not need to be quoted; give a warning */ - pwarn("Character %c was quoted unnecessarily, " - "dropped preceding quote ('\\') character\n", *sptr); - } - pcre.append(1, *sptr); + const char *pos = sptr; + int c; + if ((c = str_escseq(&pos, "")) != -1) { + /* valid escape we don't want to + * interpret here */ + pcre.append("\\"); + pcre.append(sptr, pos - sptr); + sptr += (pos - sptr) - 1; + } else { + /* quoting mark used for something that + * does not need to be quoted; give a + * warning */ + pwarn("Character %c was quoted " + "unnecessarily, dropped preceding" + " quote ('\\') character\n", + *sptr); + pcre.append(1, *sptr); + } + } else + pcre.append(1, *sptr); break; } /* switch (*sptr) */ @@ -412,7 +443,7 @@ static int process_profile_name_xmatch(Profile *prof) name = prof->attachment; else name = local_name(prof->name); - ptype = convert_aaregex_to_pcre(name, 0, tbuf, + ptype = convert_aaregex_to_pcre(name, 0, glob_default, tbuf, &prof->xmatch_len); if (ptype == ePatternBasic) prof->xmatch_len = strlen(name); @@ -440,8 +471,8 @@ static int process_profile_name_xmatch(Profile *prof) int len; tbuf.clear(); ptype = convert_aaregex_to_pcre(alt->name, 0, - tbuf, - &len); + glob_default, + tbuf, &len); if (ptype == ePatternBasic) len = strlen(alt->name); if (len < prof->xmatch_len) @@ -473,7 +504,7 @@ static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry) if (entry->mode & ~AA_CHANGE_PROFILE) filter_slashes(entry->name); - ptype = convert_aaregex_to_pcre(entry->name, 0, tbuf, &pos); + ptype = convert_aaregex_to_pcre(entry->name, 0, glob_default, tbuf, &pos); if (ptype == ePatternInvalid) return FALSE; @@ -511,7 +542,7 @@ static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry) int pos; vec[0] = tbuf.c_str(); if (entry->link_name) { - ptype = convert_aaregex_to_pcre(entry->link_name, 0, lbuf, &pos); + ptype = convert_aaregex_to_pcre(entry->link_name, 0, glob_default, lbuf, &pos); if (ptype == ePatternInvalid) return FALSE; if (entry->subset) @@ -534,7 +565,7 @@ static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry) if (entry->ns) { int pos; - ptype = convert_aaregex_to_pcre(entry->ns, 0, lbuf, &pos); + ptype = convert_aaregex_to_pcre(entry->ns, 0, glob_default, lbuf, &pos); vec[index++] = lbuf.c_str(); } vec[index++] = tbuf.c_str(); @@ -616,13 +647,13 @@ int build_list_val_expr(std::string& buffer, struct value_list *list) buffer.append("("); - ptype = convert_aaregex_to_pcre(list->value, 0, buffer, &pos); + ptype = convert_aaregex_to_pcre(list->value, 0, glob_default, buffer, &pos); if (ptype == ePatternInvalid) goto fail; list_for_each(list->next, ent) { buffer.append("|"); - ptype = convert_aaregex_to_pcre(ent->value, 0, buffer, &pos); + ptype = convert_aaregex_to_pcre(ent->value, 0, glob_default, buffer, &pos); if (ptype == ePatternInvalid) goto fail; } @@ -639,7 +670,7 @@ int convert_entry(std::string& buffer, char *entry) int pos; if (entry) { - ptype = convert_aaregex_to_pcre(entry, 0, buffer, &pos); + ptype = convert_aaregex_to_pcre(entry, 0, glob_default, buffer, &pos); if (ptype == ePatternInvalid) return FALSE; } else { @@ -790,7 +821,7 @@ static int test_filter_slashes(void) return rc; } -#define MY_REGEX_TEST(input, expected_str, expected_type) \ +#define MY_REGEX_EXT_TEST(glob, input, expected_str, expected_type) \ do { \ std::string tbuf; \ std::string tbuf2 = "testprefix"; \ @@ -799,7 +830,7 @@ static int test_filter_slashes(void) pattern_t ptype; \ int pos; \ \ - ptype = convert_aaregex_to_pcre((input), 0, tbuf, &pos); \ + ptype = convert_aaregex_to_pcre((input), 0, glob, tbuf, &pos); \ asprintf(&output_string, "simple regex conversion for '%s'\texpected = '%s'\tresult = '%s'", \ (input), (expected_str), tbuf.c_str()); \ MY_TEST(strcmp(tbuf.c_str(), (expected_str)) == 0, output_string); \ @@ -808,21 +839,25 @@ static int test_filter_slashes(void) /* ensure convert_aaregex_to_pcre appends only to passed ref string */ \ expected_str2 = tbuf2; \ expected_str2.append((expected_str)); \ - ptype = convert_aaregex_to_pcre((input), 0, tbuf2, &pos); \ - asprintf(&output_string, "simple regex conversion for '%s'\texpected = '%s'\tresult = '%s'", \ + ptype = convert_aaregex_to_pcre((input), 0, glob, tbuf2, &pos); \ + asprintf(&output_string, "simple regex conversion %sfor '%s'\texpected = '%s'\tresult = '%s'", \ + glob == glob_null ? "with null allowed in glob " : "",\ (input), expected_str2.c_str(), tbuf2.c_str()); \ MY_TEST((tbuf2 == expected_str2), output_string); \ free(output_string); \ } \ while (0) +#define MY_REGEX_TEST(input, expected_str, expected_type) MY_REGEX_EXT_TEST(glob_default, input, expected_str, expected_type) + + #define MY_REGEX_FAIL_TEST(input) \ do { \ std::string tbuf; \ pattern_t ptype; \ int pos; \ \ - ptype = convert_aaregex_to_pcre((input), 0, tbuf, &pos); \ + ptype = convert_aaregex_to_pcre((input), 0, glob_default, tbuf, &pos); \ MY_TEST(ptype == ePatternInvalid, "simple regex conversion invalid type check for '" input "'"); \ } \ while (0) @@ -927,6 +962,9 @@ static int test_aaregex_to_pcre(void) MY_REGEX_TEST("\\\\|", "\\\\\\|", ePatternBasic); MY_REGEX_TEST("\\\\(", "\\\\\\(", ePatternBasic); MY_REGEX_TEST("\\\\)", "\\\\\\)", ePatternBasic); + MY_REGEX_TEST("\\000", "\\000", ePatternBasic); + MY_REGEX_TEST("\\x00", "\\x00", ePatternBasic); + MY_REGEX_TEST("\\d000", "\\d000", ePatternBasic); /* more complicated character class tests */ /* -- embedded alternations */ @@ -940,6 +978,27 @@ static int test_aaregex_to_pcre(void) MY_REGEX_TEST("{alpha,b[\\{a,b\\}]t,gamma}", "(alpha|b[\\{a,b\\}]t|gamma)", ePatternRegex); MY_REGEX_TEST("{alpha,b[\\{a\\,b\\}]t,gamma}", "(alpha|b[\\{a\\,b\\}]t|gamma)", ePatternRegex); + /* test different globbing behavior conversion */ + MY_REGEX_EXT_TEST(glob_default, "/foo/**", "/foo/[^/\\x00][^\\x00]*", ePatternTailGlob); + MY_REGEX_EXT_TEST(glob_null, "/foo/**", "/foo/[^/].*", ePatternTailGlob); + MY_REGEX_EXT_TEST(glob_default, "/foo/f**", "/foo/f[^\\x00]*", ePatternTailGlob); + MY_REGEX_EXT_TEST(glob_null, "/foo/f**", "/foo/f.*", ePatternTailGlob); + + MY_REGEX_EXT_TEST(glob_default, "/foo/*", "/foo/[^/\\x00][^/\\x00]*", ePatternRegex); + MY_REGEX_EXT_TEST(glob_null, "/foo/*", "/foo/[^/][^/]*", ePatternRegex); + MY_REGEX_EXT_TEST(glob_default, "/foo/f*", "/foo/f[^/\\x00]*", ePatternRegex); + MY_REGEX_EXT_TEST(glob_null, "/foo/f*", "/foo/f[^/]*", ePatternRegex); + + MY_REGEX_EXT_TEST(glob_default, "/foo/**.ext", "/foo/[^\\x00]*\\.ext", ePatternRegex); + MY_REGEX_EXT_TEST(glob_null, "/foo/**.ext", "/foo/.*\\.ext", ePatternRegex); + MY_REGEX_EXT_TEST(glob_default, "/foo/f**.ext", "/foo/f[^\\x00]*\\.ext", ePatternRegex); + MY_REGEX_EXT_TEST(glob_null, "/foo/f**.ext", "/foo/f.*\\.ext", ePatternRegex); + + MY_REGEX_EXT_TEST(glob_default, "/foo/*.ext", "/foo/[^/\\x00]*\\.ext", ePatternRegex); + MY_REGEX_EXT_TEST(glob_null, "/foo/*.ext", "/foo/[^/]*\\.ext", ePatternRegex); + MY_REGEX_EXT_TEST(glob_default, "/foo/f*.ext", "/foo/f[^/\\x00]*\\.ext", ePatternRegex); + MY_REGEX_EXT_TEST(glob_null, "/foo/f*.ext", "/foo/f[^/]*\\.ext", ePatternRegex); + return rc; } diff --git a/parser/ptrace.cc b/parser/ptrace.cc index 6f2891a26..bc7122b46 100644 --- a/parser/ptrace.cc +++ b/parser/ptrace.cc @@ -139,7 +139,7 @@ int ptrace_rule::gen_policy_re(Profile &prof) buffer << "\\x" << std::setfill('0') << std::setw(2) << std::hex << AA_CLASS_PTRACE; if (peer_label) { - ptype = convert_aaregex_to_pcre(peer_label, 0, buf, &pos); + ptype = convert_aaregex_to_pcre(peer_label, 0, glob_default, buf, &pos); if (ptype == ePatternInvalid) goto fail; buffer << buf; diff --git a/parser/signal.cc b/parser/signal.cc index 7f9aa26da..c823871d5 100644 --- a/parser/signal.cc +++ b/parser/signal.cc @@ -294,7 +294,7 @@ int signal_rule::gen_policy_re(Profile &prof) buffer << ")"; } if (peer_label) { - ptype = convert_aaregex_to_pcre(peer_label, 0, buf, &pos); + ptype = convert_aaregex_to_pcre(peer_label, 0, glob_default, buf, &pos); if (ptype == ePatternInvalid) goto fail; buffer << buf; diff --git a/parser/tst/simple_tests/file/bad_embedded_0_1.sd b/parser/tst/simple_tests/file/bad_embedded_0_1.sd new file mode 100644 index 000000000..34e3b3959 --- /dev/null +++ b/parser/tst/simple_tests/file/bad_embedded_0_1.sd @@ -0,0 +1,9 @@ +# +#=DESCRIPTION embedded \000 in file path +#=EXRESULT PASS +# currently do NOT have semantic checks for \000 in file path +# +/usr/bin/foo { + /foo\000bar w, +} + diff --git a/parser/tst/simple_tests/file/bad_embedded_0_2.sd b/parser/tst/simple_tests/file/bad_embedded_0_2.sd new file mode 100644 index 000000000..c93bc5024 --- /dev/null +++ b/parser/tst/simple_tests/file/bad_embedded_0_2.sd @@ -0,0 +1,9 @@ +# +#=DESCRIPTION embedded \x00 in file path +#=EXRESULT PASS +# currently do NOT have semantic checks for \000 in file path +# +/usr/bin/foo { + /foo\x00bar w, +} + diff --git a/parser/tst/simple_tests/file/bad_embedded_0_3.sd b/parser/tst/simple_tests/file/bad_embedded_0_3.sd new file mode 100644 index 000000000..d0f21eded --- /dev/null +++ b/parser/tst/simple_tests/file/bad_embedded_0_3.sd @@ -0,0 +1,9 @@ +# +#=DESCRIPTION embedded \d00 in file path +#=EXRESULT PASS +# currently do NOT have semantic checks for \000 in file path +# +/usr/bin/foo { + /foo\d00bar w, +} + diff --git a/parser/tst/simple_tests/unix/ok_embedded_0_1.sd b/parser/tst/simple_tests/unix/ok_embedded_0_1.sd new file mode 100644 index 000000000..651be4d02 --- /dev/null +++ b/parser/tst/simple_tests/unix/ok_embedded_0_1.sd @@ -0,0 +1,8 @@ +# +#=DESCRIPTION unix rule with embedded \000 +#=EXRESULT PASS +# + +profile foo { + unix addr=@foo\000bar, +} diff --git a/parser/tst/simple_tests/unix/ok_embedded_0_2.sd b/parser/tst/simple_tests/unix/ok_embedded_0_2.sd new file mode 100644 index 000000000..c20d87020 --- /dev/null +++ b/parser/tst/simple_tests/unix/ok_embedded_0_2.sd @@ -0,0 +1,8 @@ +# +#=DESCRIPTION unix rule with embedded \x00 +#=EXRESULT PASS +# + +profile foo { + unix addr=@foo\x00bar, +} diff --git a/parser/tst/simple_tests/unix/ok_embedded_0_3.sd b/parser/tst/simple_tests/unix/ok_embedded_0_3.sd new file mode 100644 index 000000000..7d370d43a --- /dev/null +++ b/parser/tst/simple_tests/unix/ok_embedded_0_3.sd @@ -0,0 +1,8 @@ +# +#=DESCRIPTION unix rule with embedded \d00 +#=EXRESULT PASS +# + +profile foo { + unix addr=@foo\d00bar, +}