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 <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>

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 */

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();

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;
 

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);

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

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) {
+					if (*s == '/' || !*s)
-						pcre.append("[^/\\x00]");
+						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;
 					}
-
+					error = append_glob(pcre, glob, "[^\\x00]*", ".*");
-					pcre.append("[^\\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
+				const char *pos = sptr;
-				 * does not need to be quoted; give a warning */
+				int c;
-				pwarn("Character %c was quoted unnecessarily, "
+				if ((c = str_escseq(&pos, "")) != -1) {
-				      "dropped preceding quote ('\\') character\n", *sptr);
+					/* valid escape we don't want to
-			}
+					 * interpret here */
-			pcre.append(1, *sptr);
+					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,
+								glob_default,
-								&len);
+								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);				\
+		ptype = convert_aaregex_to_pcre((input), 0, glob, tbuf2, &pos); \
-		asprintf(&output_string, "simple regex conversion for '%s'\texpected = '%s'\tresult = '%s'", \
+		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;
 }
 

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;

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;

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,
+}
+

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,
+}
+

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,
+}
+

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,
+}

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,
+}

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,
+}