RE: Re: SQL Module: Inconsistent behavior dealing with the escaping backslash
Great, I just tried out your suggestion and it worked as expected and resolved the issue I had. I also agree that we should use "binary" value for the secrets not the "printed form" string value. Given that, Do you think can this fix be part of the next freeradius 3.x.x release? Thanks for the clarification. Best Regards, Wookjong -----Original Message----- From: Freeradius-Users [mailto:freeradius-users-bounces+wookjong.kwak=gemalto.com@lists.freeradius.org] On Behalf Of Alan DeKok Sent: Thursday, January 25, 2018 3:30 PM To: FreeRadius users mailing list <freeradius-users@lists.freeradius.org> Subject: [+SPAM+]: Re: SQL Module: Inconsistent behavior dealing with the escaping backslash On Jan 25, 2018, at 3:02 PM, Kwak Wookjong <Wookjong.Kwak@gemalto.com> wrote:
Yes, in the SQL DB, double slashes "\\" are presented.
OK. That's good.
... (0) EXPAND %{sql: SELECT secret FROM nas WHERE nasname = '%{Packet-Src-IP-Address}'} (0) --> test\\123 (0) FreeRADIUS-Client-Secret = test\123
***** up until here, everything seems good as "\\" becomes to "\"
That's fine.
***** problem below when it is converting control list to client field, "\" becomes to "\\", again.
(0) Converting control list to client fields (0) ipv4addr = 192.168.99.1 (0) secret = test\\123 (0) shortname = Radius Local Test (0) nas_type = other Adding client 192.168.99.1/32 with shared secret "test\\123"
***** So, when it is adding the client, it is double backslashes.
Hmm... the issue is that there are multiple representations of a string. The "binary" value, where the characters just mean themselves, and the "printed" form, where things are escaped, e.g. \n, \\, \t, etc. For various reasons, the dynamic clients have to print the attributes you defined. But, it should just use the "binary" value for secrets, and not use the "printed" form. I think this change should fix it: --- a/src/main/client.c +++ b/src/main/client.c @@ -1341,7 +1341,7 @@ RADCLIENT *client_afrom_request(RADCLIENT_LIST *clients, REQUEST *request) * We could reuse the CONF_PAIR buff, this just keeps things * consistent between client_afrom_cs, and client_afrom_query. */ - *p = talloc_strdup(c, strvalue); + *p = talloc_strdup(c, vp->vp_strvalue); Hopefully that won't break it for anyone else... Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html ________________________________ This message and any attachments are intended solely for the addressees and may contain confidential information. Any unauthorized use or disclosure, either whole or partial, is prohibited. E-mails are susceptible to alteration. Our company shall not be liable for the message if altered, changed or falsified. If you are not the intended recipient of this message, please delete it and notify the sender. Although all reasonable efforts have been made to keep this transmission free from viruses, the sender will not be liable for damages caused by a transmitted virus.
On Jan 25, 2018, at 5:07 PM, Kwak Wookjong <Wookjong.Kwak@gemalto.com> wrote:
Great, I just tried out your suggestion and it worked as expected and resolved the issue I had.
OK.
I also agree that we should use "binary" value for the secrets not the "printed form" string value.
The question is whether it breaks peoples existing configuration. I suspect not, given the nature of the issue.
Given that, Do you think can this fix be part of the next freeradius 3.x.x release?
I've pushed a fix. Alan DeKok.
participants (2)
-
Alan DeKok -
Kwak Wookjong