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