I have an issue with FreeRadius 2.1.12. If I make use of a check/control regexp match in either rlm_sql or rlm_files, and the match succeeds, then a spurious attribute is added onto the control list containing the regexp which was matched. To demonstrate: authorize { update reply { Reply-Message += "Before: %{control:User-Name}" } testuser update reply { Reply-Message += "After: %{control:User-Name}" } ... testuser is an instance of rlm_files which contains: DEFAULT User-Name =~ "@example\\.com$" Reply-Message += "Matched" Then send a test packet: # radtest test@example.com secret localhost 1 testing123 The response includes: Reply-Message = "Before: " Reply-Message = "Matched" Reply-Message = "After: @example\\.com$" Debug output: rad_recv: Access-Request packet from host 127.0.0.1 port 49269, id=27, length=86 User-Name = "test@example.com" User-Password = "secret" NAS-IP-Address = 192.168.56.101 NAS-Port = 1 Message-Authenticator = 0x476a68f002b8b1f49df07064f8e1411a # Executing section authorize from file /etc/freeradius/sites-enabled/stratRadius +- entering group authorize {...} expand: Before: %{control:User-Name} -> Before: ++[reply] returns notfound [testuser] expand: %{Called-Station-Id} -> [testuser] expand: %{User-Name} -> test@example.com [testuser] users: Matched entry DEFAULT at line 1 ++[testuser] returns ok expand: After: %{control:User-Name} -> After: @example\.com$ ++[reply] returns ok This is a problem because I was using the attribute in the control list for something else. It only seems to be a side-effect of the regexp match. If instead I use: DEFAULT User-Name == "test@example.com" Reply-Message += "Matched" then the control:User-Name is not updated. Regards, Brian.
On Fri, Jun 15, 2012 at 06:45:26PM +0100, Brian Candler wrote:
I have an issue with FreeRadius 2.1.12.
If I make use of a check/control regexp match in either rlm_sql or rlm_files, and the match succeeds, then a spurious attribute is added onto the control list containing the regexp which was matched.
Starts at src/modules/rlm_files/rlm_files.c:473, unchanged since 2002. It's basically if the pairs to match are found in the incoming request then add the reply pairs from the users file to the reply list add the matched non-comparison check items to the control list fi
It only seems to be a side-effect of the regexp match. If instead I use:
DEFAULT User-Name == "test@example.com" Reply-Message += "Matched"
then the control:User-Name is not updated.
That comes from src/lib/valuepair.c:435 ish, in pairmove(). It doesn't copy comparison operators. Given valuepair.c:495 ("Very bad code. Barely working, if at all.") and the case T_OP_REG_EQ section that is ifdef'd out, I'd suggest the following patch might be sensible. diff --git a/src/lib/valuepair.c b/src/lib/valuepair.c index 150c311..98169ab 100644 --- a/src/lib/valuepair.c +++ b/src/lib/valuepair.c @@ -443,6 +443,8 @@ void pairmove(VALUE_PAIR **to, VALUE_PAIR **from) case T_OP_CMP_TRUE: case T_OP_CMP_FALSE: case T_OP_CMP_EQ: + case T_OP_REG_EQ: + case T_OP_REG_NE: tailfrom = i; continue; Totally untested of course :). The 'case T_OP_REG_EQ' has been removed in the master branch. There may be other cases where it's assumed =~ is more like := than ==. If so, the documentation at http://wiki.freeradius.org/Operators needs fixing instead. Matthew -- Matthew Newton, Ph.D. <mcn4@le.ac.uk> Systems Architect (UNIX and Networks), Network Services, I.T. Services, University of Leicester, Leicester LE1 7RH, United Kingdom For IT help contact helpdesk extn. 2253, <ithelp@le.ac.uk>
On Sat, Jun 16, 2012 at 12:56:54AM +0100, Matthew Newton wrote:
Given valuepair.c:495 ("Very bad code. Barely working, if at all.") and the case T_OP_REG_EQ section that is ifdef'd out, I'd suggest the following patch might be sensible.
diff --git a/src/lib/valuepair.c b/src/lib/valuepair.c index 150c311..98169ab 100644 --- a/src/lib/valuepair.c +++ b/src/lib/valuepair.c @@ -443,6 +443,8 @@ void pairmove(VALUE_PAIR **to, VALUE_PAIR **from) case T_OP_CMP_TRUE: case T_OP_CMP_FALSE: case T_OP_CMP_EQ: + case T_OP_REG_EQ: + case T_OP_REG_NE: tailfrom = i; continue;
Totally untested of course :).
That looks plausible, thank you. I'll leave Alan to decide whether to commit it. I have worked around the issue now as I needed the config to be deployable with stock 2.1.12.
The 'case T_OP_REG_EQ' has been removed in the master branch.
That makes sense for the control list: I think it was a legacy crutch so that Attribute = "value" would behave as Attribute == "value". Regards, Brian.
participants (2)
-
Brian Candler -
Matthew Newton