Blob Blame Raw
commit dd648b44760b9041d5ea4279e78ee7fb4e5aa13d
Author: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date:   Tue May 13 11:52:21 2014 +0100

    Fix case insensitive matching in compiled regular expressions

diff --git a/src/include/map.h b/src/include/map.h
index ed73093..59732a1 100644
--- a/src/include/map.h
+++ b/src/include/map.h
@@ -155,11 +155,14 @@ typedef struct value_pair_tmpl_t {
 
 	union {
 		struct {
-			value_data_t const	*value;	 //!< actual data
-			size_t			length;  //!< of the vpd data
+			value_data_t const	*value;		//!< actual data
+			size_t			length;		//!< of the vpd data
 		} literal;
 		xlat_exp_t	*xlat;	 //!< pre-parsed xlat_exp_t
-		regex_t		*preg;	 //!< pre-parsed regex_t
+		struct {
+			regex_t			*comp;		//!< pre-parsed regex_t
+			bool			iflag;		//!< Case insensitive
+		} preg;
 	} data;
 } value_pair_tmpl_t;
 
@@ -170,7 +173,9 @@ typedef struct value_pair_tmpl_t {
 #define vpt_tag		attribute.tag
 
 #define vpt_xlat	data.xlat
-#define vpt_preg	data.preg
+
+#define vpt_preg	data.preg.comp
+#define vpt_iflag	data.preg.iflag
 
 #define vpt_value	data.literal.value
 #define vpt_length	data.literal.length
diff --git a/src/include/parser.h b/src/include/parser.h
index 5126edd..6aa858a 100644
--- a/src/include/parser.h
+++ b/src/include/parser.h
@@ -73,7 +73,6 @@ struct fr_cond_t {
 		fr_cond_t  	*child;
 	} data;
 
-	int		regex_i;
 	int		negate;
 	int		pass2_fixup;
 
diff --git a/src/main/evaluate.c b/src/main/evaluate.c
index 60351c9..c21ed4f 100644
--- a/src/main/evaluate.c
+++ b/src/main/evaluate.c
@@ -240,16 +240,13 @@ int radius_evaluate_tmpl(REQUEST *request, int modreturn, UNUSED int depth,
 }
 
 
-static int do_regex(REQUEST *request, value_pair_map_t const *map, bool iflag)
+static int do_regex(REQUEST *request, value_pair_map_t const *map)
 {
 	int compare, rcode, ret;
-	int cflags = REG_EXTENDED;
 	regex_t reg, *preg;
 	char *lhs, *rhs;
 	regmatch_t rxmatch[REQUEST_MAX_REGEX + 1];
 
-	if (iflag) cflags |= REG_ICASE;
-
 	/*
 	 *  Expand and then compile it.
 	 */
@@ -262,7 +259,7 @@ static int do_regex(REQUEST *request, value_pair_map_t const *map, bool iflag)
 		}
 		rad_assert(rhs != NULL);
 
-		compare = regcomp(&reg, rhs, cflags);
+		compare = regcomp(&reg, rhs, REG_EXTENDED | (map->src->vpt_iflag ? REG_ICASE : 0));
 		if (compare != 0) {
 			if (debug_flag) {
 				char errbuf[128];
@@ -635,7 +632,7 @@ int radius_evaluate_map(REQUEST *request, UNUSED int modreturn, UNUSED int depth
 	 */
 	if ((map->src->type == VPT_TYPE_REGEX) ||
 	    (map->src->type == VPT_TYPE_REGEX_STRUCT)) {
-		return do_regex(request, map, c->regex_i);
+		return do_regex(request, map);
 	}
 #endif
 
diff --git a/src/main/modcall.c b/src/main/modcall.c
index 22ab00b..915c370 100644
--- a/src/main/modcall.c
+++ b/src/main/modcall.c
@@ -2829,7 +2829,7 @@ static bool pass2_regex_compile(CONF_ITEM const *ci, value_pair_tmpl_t *vpt)
 	talloc_set_destructor(preg, _free_compiled_regex);
 	if (!preg) return false;
 
-	rcode = regcomp(preg, vpt->name, REG_EXTENDED);
+	rcode = regcomp(preg, vpt->name, REG_EXTENDED | (vpt->vpt_iflag ? REG_ICASE : 0));
 	if (rcode != 0) {
 		char buffer[256];
 		regerror(rcode, preg, buffer, sizeof(buffer));
diff --git a/src/main/parser.c b/src/main/parser.c
index fcfd16e..738ca7f 100644
--- a/src/main/parser.c
+++ b/src/main/parser.c
@@ -340,7 +340,8 @@ static ssize_t condition_tokenize_cast(char const *start, DICT_ATTR const **pda,
  *  @param[out] error the parse error (if any)
  *  @return length of the string skipped, or when negative, the offset to the offending error
  */
-static ssize_t condition_tokenize(TALLOC_CTX *ctx, CONF_ITEM *ci, char const *start, int brace, fr_cond_t **pcond, char const **error, int flags)
+static ssize_t condition_tokenize(TALLOC_CTX *ctx, CONF_ITEM *ci, char const *start, int brace,
+				  fr_cond_t **pcond, char const **error, int flags)
 {
 	ssize_t slen;
 	char const *p = start;
@@ -477,7 +478,8 @@ static ssize_t condition_tokenize(TALLOC_CTX *ctx, CONF_ITEM *ci, char const *st
 			rad_assert(c->data.vpt->type != VPT_TYPE_REGEX);
 
 		} else { /* it's an operator */
-			int regex;
+			bool regex;
+			bool i_flag = false;
 
 			/*
 			 *	The next thing should now be a comparison operator.
@@ -613,7 +615,7 @@ static ssize_t condition_tokenize(TALLOC_CTX *ctx, CONF_ITEM *ci, char const *st
 				 *	Allow /foo/i
 				 */
 				if (p[slen] == 'i') {
-					c->regex_i = true;
+					i_flag = true;
 					slen++;
 				}
 
@@ -637,6 +639,10 @@ static ssize_t condition_tokenize(TALLOC_CTX *ctx, CONF_ITEM *ci, char const *st
 				return_0("Syntax error");
 			}
 
+			if (c->data.map->src->type == VPT_TYPE_REGEX) {
+				c->data.map->src->vpt_iflag = i_flag;
+			}
+
 			/*
 			 *	Could have been a reference to an attribute which is registered later.
 			 *	Mark it as being checked in pass2.
@@ -1074,7 +1080,6 @@ done:
 			rcode = radius_evaluate_map(NULL, 0, 0, c);
 			TALLOC_FREE(c->data.map);
 			c->cast = NULL;
-			c->regex_i = false;
 			if (rcode) {
 				c->type = COND_TYPE_TRUE;
 			} else {
@@ -1097,7 +1102,6 @@ done:
 			int rcode;
 
 			rad_assert(c->cast == NULL);
-			rad_assert(c->regex_i == false);
 
 			rcode = radius_evaluate_map(NULL, 0, 0, c);
 			if (rcode) {
diff --git a/src/tests/keywords/if-regex-match b/src/tests/keywords/if-regex-match
index f15e74f..f3e6aa9 100644
--- a/src/tests/keywords/if-regex-match
+++ b/src/tests/keywords/if-regex-match
@@ -1,16 +1,96 @@
 # PRE: if
 #
-# May as well exercise the regular expression engine
-if (User-Name !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) {
-	reject
+
+# Non matching on attribute ref
+if (User-Name !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])%{Tmp-String-0}/) {
+	update reply {
+		Filter-Id += 'Fail 0'
+	}
 }
 
-if ("%{User-Name}" !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) {
-	reject
+# Matching on xlat expanded value
+if ("%{User-Name}" !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])%{Tmp-String-0}/) {
+	update reply {
+		Filter-Id += 'Fail 1'
+	}
 }
 
-if (User-Name =~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) {
+# Matching on attribute ref with capture groups
+if (User-Name =~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])%{Tmp-String-0}/) {
+	# Test all the capture groups
 	update {
 		reply:User-Name := "%{7}_%{6}_%{5}_%{4}_%{3}_%{2}_%{1}_%{0}"
 	}
 }
+else {
+	update reply {
+		Filter-Id += 'Fail 2'
+	}
+}
+
+# Checking capture groups are cleared out correctly
+if (User-Name =~ /^([0-9])_%{Tmp-String-0}/) {
+	if ("%{0}%{1}%{2}%{3}%{4}%{5}%{6}%{7}" != '1_1') {
+		update reply {
+			Filter-Id += 'Fail 3'
+		}
+	}
+}
+else {
+	update reply {
+		Filter-Id += 'Fail 3.5'
+	}
+}
+
+# Checking capture groups are cleared out correctly when there are no matches
+if (User-Name =~ /^.%{Tmp-String-0}/) {
+	if ("%{0}%{1}%{2}%{3}%{4}%{5}%{6}%{7}" != '1') {
+		update reply {
+			Filter-Id += 'Fail 4'
+		}
+	}
+}
+else {
+	update reply {
+		Filter-Id += 'Fail 4.5'
+	}
+}
+
+# compiled - ref - insensitive
+if (Calling-Station-Id !~ /:roamyroam%{Tmp-String-0}$/i) {
+	update reply {
+		Filter-Id += 'Fail 5'
+	}
+}
+
+# compiled - expansion - insensitive
+if ("%{Calling-Station-Id}" !~ /:roamyroam%{Tmp-String-0}$/i) {
+	update reply {
+		Filter-Id += 'Fail 6'
+	}
+}
+
+# compiled - enum - ref - insensitive
+if (Service-Type !~ /^framed-user%{Tmp-String-0}$/i) {
+	update reply {
+		Filter-Id += 'Fail 7'
+	}
+}
+
+# compiled - enum - expansion - insensitive
+if ("%{Service-Type}" !~ /^framed-user%{Tmp-String-0}$/i) {
+	update reply {
+		Filter-Id += 'Fail 8'
+	}
+}
+
+# compiled - enum - ref
+if (Service-Type =~ /^framed-user%{Tmp-String-0}$/) {
+	update reply {
+		Filter-Id += 'Fail 9'
+	}
+}
+
+
+
+
diff --git a/src/tests/keywords/if-regex-match-comp b/src/tests/keywords/if-regex-match-comp
new file mode 100644
index 0000000..8d68142
--- /dev/null
+++ b/src/tests/keywords/if-regex-match-comp
@@ -0,0 +1,94 @@
+# PRE: if
+#
+
+# Non matching on attribute ref
+if (User-Name !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) {
+	update reply {
+		Filter-Id += 'Fail 0'
+	}
+}
+
+# Matching on xlat expanded value
+if ("%{User-Name}" !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) {
+	update reply {
+		Filter-Id += 'Fail 1'
+	}
+}
+
+# Matching on attribute ref with capture groups
+if (User-Name =~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) {
+	# Test all the capture groups
+	update {
+		reply:User-Name := "%{7}_%{6}_%{5}_%{4}_%{3}_%{2}_%{1}_%{0}"
+	}
+}
+else {
+	update reply {
+		Filter-Id += 'Fail 2'
+	}
+}
+
+# Checking capture groups are cleared out correctly
+if (User-Name =~ /^([0-9])_/) {
+	if ("%{0}%{1}%{2}%{3}%{4}%{5}%{6}%{7}" != '1_1') {
+		update reply {
+			Filter-Id += 'Fail 3'
+		}
+	}
+}
+else {
+	update reply {
+		Filter-Id += 'Fail 3.5'
+	}
+}
+
+# Checking capture groups are cleared out correctly when there are no matches
+if (User-Name =~ /^./) {
+	if ("%{0}%{1}%{2}%{3}%{4}%{5}%{6}%{7}" != '1') {
+		update reply {
+			Filter-Id += 'Fail 4'
+		}
+	}
+}
+else {
+	update reply {
+		Filter-Id += 'Fail 4.5'
+	}
+}
+
+# compiled - ref - insensitive
+if (Calling-Station-Id !~ /:roamyroam$/i) {
+	update reply {
+		Filter-Id += 'Fail 5'
+	}
+}
+
+# compiled - expansion - insensitive
+if ("%{Calling-Station-Id}" !~ /:roamyroam$/i) {
+	update reply {
+		Filter-Id += 'Fail 6'
+	}
+}
+
+# compiled - enum - ref - insensitive
+if (Service-Type !~ /^framed-user$/i) {
+	update reply {
+		Filter-Id += 'Fail 7'
+	}
+}
+
+# compiled - enum - expansion - insensitive
+if ("%{Service-Type}" !~ /^framed-user$/i) {
+	update reply {
+		Filter-Id += 'Fail 8'
+	}
+}
+
+# compiled - enum - ref
+if (Service-Type =~ /^framed-user$/) {
+	update reply {
+		Filter-Id += 'Fail 9'
+	}
+}
+
+
diff --git a/src/tests/keywords/if-regex-match-comp.attrs b/src/tests/keywords/if-regex-match-comp.attrs
new file mode 100644
index 0000000..ba7188d
--- /dev/null
+++ b/src/tests/keywords/if-regex-match-comp.attrs
@@ -0,0 +1,7 @@
+User-Name = '1_2_3_4_5_6_7'
+User-Password = 'hello'
+Service-Type := 'Framed-User'
+Calling-Station-ID := '00:11:22:33:44:55:66:ROAMYROAM'
+
+Response-Packet-Type == Access-Accept
+User-Name == '7_6_5_4_3_2_1_1_2_3_4_5_6_7'
diff --git a/src/tests/keywords/if-regex-match.attrs b/src/tests/keywords/if-regex-match.attrs
index ab03050..ba7188d 100644
--- a/src/tests/keywords/if-regex-match.attrs
+++ b/src/tests/keywords/if-regex-match.attrs
@@ -1,5 +1,7 @@
 User-Name = '1_2_3_4_5_6_7'
 User-Password = 'hello'
+Service-Type := 'Framed-User'
+Calling-Station-ID := '00:11:22:33:44:55:66:ROAMYROAM'
 
 Response-Packet-Type == Access-Accept
 User-Name == '7_6_5_4_3_2_1_1_2_3_4_5_6_7'