SQL-User-Name in %{sql:..} expansion
Hi, I am currently in the process of migrating our 2.2.9 setup to 3.0.11 and, naturally, stumbled over a few things (i.e. more messages will follow). It seems that SQL-User-Name is not usable in expansions any more, only when calling the module (we use authorize_check_query and authorize_reply_query). When I put this (for testing only) in my authorize section ... update { request:Stripped-User-Name := "bla" } update { control:Tmp-String-0 := "%{sql:SELECT '%{SQL-User-Name}'}" } ... I get this in the debug output: (0) authorize { (0) update { (0) request:Stripped-User-Name := "bla" (0) } # update = noop (0) update { (0) EXPAND %{Stripped-User-Name} (0) --> bla (0) SQL-User-Name set to 'bla' rlm_sql (sql): Reserved connection (1) (0) Executing select query: SELECT '' rlm_sql (sql): Released connection (1) (0) EXPAND %{sql:SELECT '%{SQL-User-Name}'} (0) --> (0) control:Tmp-String-0 := (0) } # update = noop ... So, it says that Stripped-User-Name will contain "bla", but in the query it is the empty string. Is this intended, a glitch, or am I doing something wrong? My current workaround is to escape myself (with %{escape:%{Stripped-User-Name}} and proper escape_characters in the expr config). Regards Jakob
On 18/05/16 11:51, Jakob Hirsch wrote:
Hi,
I am currently in the process of migrating our 2.2.9 setup to 3.0.11 and, naturally, stumbled over a few things (i.e. more messages will follow).
It seems that SQL-User-Name is not usable in expansions any more, only when calling the module (we use authorize_check_query and authorize_reply_query).
Yeah, we ran into that. Annoying. I did this at the top-level of radiusd.conf: sqlusername = "%{%{Stripped-User-Name}:-%{User-Name}}" ...and then did this everywhere: update { blah = "${sqlusername}" } ...to save on typing.
On Wed, May 18, 2016 at 12:05:37PM +0100, Phil Mayers wrote:
On 18/05/16 11:51, Jakob Hirsch wrote:
I am currently in the process of migrating our 2.2.9 setup to 3.0.11 and, naturally, stumbled over a few things (i.e. more messages will follow).
It seems that SQL-User-Name is not usable in expansions any more, only when calling the module (we use authorize_check_query and authorize_reply_query).
Yeah, we ran into that. Annoying.
Hmm. Bug, but I can't see an easy way to fix it. The sql xlat does correctly add SQL-User-Name to the request with the right value. The problem is that xlat has already expanded all attributes before passing the string through to the sql module xlat. And of course at that time SQL-User-Name didn't exist. You can show this by doing # User-Name = "bob" # (SQL-User-Name here doesn't exist) update request { Tmp-String-1 := "%{sql:select '%{SQL-User-Name}'}" # this expands to "select ''" } # (SQL-User-Name here is "bob") update request { User-Name := 'jimbo' } update request { Tmp-String-1 := "%{sql:select '%{SQL-User-Name}'}" # this expands to "select 'bob'" } # SQL-User-Name here is "jimbo" So SQL-User-Name lags behind by one xlat.
I did this at the top-level of radiusd.conf:
sqlusername = "%{%{Stripped-User-Name}:-%{User-Name}}"
...and then did this everywhere:
update { blah = "${sqlusername}" }
...to save on typing.
Probably the easiest way without radius_xlat calling some sort of module "pre-xlat" function before doing the xlat. Or having a "delayed expansion" flag which tells radius_xlat not to expand anything and to let the module do it. But I guess that's what happened before; it was probably fixing all the \\\\\\\\ escaping madness that broke this... Matthew -- Matthew Newton, Ph.D. <mcn4@le.ac.uk> Systems Specialist, Infrastructure Services, I.T. Services, University of Leicester, Leicester LE1 7RH, United Kingdom For IT help contact helpdesk extn. 2253, <ithelp@le.ac.uk>
Probably the easiest way without radius_xlat calling some sort of module "pre-xlat" function before doing the xlat. Or having a "delayed expansion" flag which tells radius_xlat not to expand anything and to let the module do it. But I guess that's what happened before; it was probably fixing all the \\\\\\\\ escaping madness that broke this...
SQL-User-Name is only useful because it expands to the group being processed. For everything else the xlat escape function will prevent injection attacks. -Arran Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2
On Wed, May 18, 2016 at 10:18:01AM -0400, Arran Cudbard-Bell wrote:
Probably the easiest way without radius_xlat calling some sort of module "pre-xlat" function before doing the xlat. Or having a "delayed expansion" flag which tells radius_xlat not to expand anything and to let the module do it. But I guess that's what happened before; it was probably fixing all the \\\\\\\\ escaping madness that broke this...
SQL-User-Name is only useful because it expands to the group being processed. For everything else the xlat escape function will prevent injection attacks.
OK. So is it worth removing the sql_set_user() call from sql_xlat so that the xlat doesn't add SQL-User-Name? As it's not available to use in the actual xlat it seems like it's just a side effect that's confusing. It's still available in other sql calls of course. Matthew -- Matthew Newton, Ph.D. <mcn4@le.ac.uk> Systems Specialist, Infrastructure Services, I.T. Services, University of Leicester, Leicester LE1 7RH, United Kingdom For IT help contact helpdesk extn. 2253, <ithelp@le.ac.uk>
On Wed, May 18, 2016 at 07:43:17PM -0400, Arran Cudbard-Bell wrote:
So is it worth removing the sql_set_user() call from sql_xlat so that the xlat doesn't add SQL-User-Name?
Yes, that'd make sense.
Done in 3.1.x. Didn't touch in 3.0.x in case someone's config is (probably unbeknown to them) depending on an xlat creating the attribute. Matthew -- Matthew Newton, Ph.D. <mcn4@le.ac.uk> Systems Specialist, Infrastructure Services, I.T. Services, University of Leicester, Leicester LE1 7RH, United Kingdom For IT help contact helpdesk extn. 2253, <ithelp@le.ac.uk>
Arran Cudbard-Bell wrote on 2016-05-18 16:18:
SQL-User-Name is only useful because it expands to the group being processed. For everything else the xlat escape function will prevent injection attacks.
Oh, ok. I thought the main use is that it's safe against SQL injections (because of the escaping). Of course, if one uses other attributes in the query, expr's escape would be needed for that. I tend to think that using the native escaping function would be better (like mysql_real_escape), but then you had to make sure that it uses the same context/connection as the query in which it will be used, which might be tricky. Anyway, I digress. So I will just use escape. Thanks for the input!
participants (4)
-
Arran Cudbard-Bell -
Jakob Hirsch -
Matthew Newton -
Phil Mayers