Hi, while I've been staring at debug output of 2.x.x I noticed that%{rand:x} doesn't seem to do what it should: ++? if (true ) -> TRUE ++- entering if (true ) {...} expand: %{rand:1000000}%Y%m%d%H%M%S -> 201307150802013-07-15 08:17:08 expand: %{rand:1000000}%Y%m%d%H%M%S -> 201307150802013-07-15 08:17:08 expand: %{rand:1000000}%Y%m%d%H%M%S -> 201307150802013-07-15 08:17:08 As you see, the expression %{rand:1000000} gets expanded into an empty string. In my particular case, this leads to a lot less entropy than I thought. The expansion is there three times alright, that's as per config, but I would have expected the results to be different. Now that I knew where to look, I went back to 2.2.0 mainstream release code - and saw the same :-( I didn't go back to 2.1.12. I'm "fairly sure" it worked back in the day. (Yes, I also want a %s insatead of %S in my config, got it. But the %{rand:} is really a bit more important than that. ) Stefan -- Stefan WINTER Ingenieur de Recherche Fondation RESTENA - Réseau Téléinformatique de l'Education Nationale et de la Recherche 6, rue Richard Coudenhove-Kalergi L-1359 Luxembourg Tel: +352 424409 1 Fax: +352 422473
On 15 Jul 2013, at 07:41, Stefan Winter <stefan.winter@restena.lu> wrote:
Hi,
while I've been staring at debug output of 2.x.x I noticed that%{rand:x} doesn't seem to do what it should:
++? if (true ) -> TRUE ++- entering if (true ) {...} expand: %{rand:1000000}%Y%m%d%H%M%S -> 201307150802013-07-15 08:17:08 expand: %{rand:1000000}%Y%m%d%H%M%S -> 201307150802013-07-15 08:17:08 expand: %{rand:1000000}%Y%m%d%H%M%S -> 201307150802013-07-15 08:17:08
As you see, the expression %{rand:1000000} gets expanded into an empty string. In my particular case, this leads to a lot less entropy than I thought. The expansion is there three times alright, that's as per config, but I would have expected the results to be different.
Now that I knew where to look, I went back to 2.2.0 mainstream release code - and saw the same :-(
Works in 3.0.0, but yes, broken in v2.x.x HEAD. Odd seeing as the xlat code is very similar. -Arran
On 15 Jul 2013, at 08:20, Arran Cudbard-Bell <a.cudbardb@freeradius.org> wrote:
On 15 Jul 2013, at 07:41, Stefan Winter <stefan.winter@restena.lu> wrote:
Hi,
while I've been staring at debug output of 2.x.x I noticed that%{rand:x} doesn't seem to do what it should:
++? if (true ) -> TRUE ++- entering if (true ) {...} expand: %{rand:1000000}%Y%m%d%H%M%S -> 201307150802013-07-15 08:17:08 expand: %{rand:1000000}%Y%m%d%H%M%S -> 201307150802013-07-15 08:17:08 expand: %{rand:1000000}%Y%m%d%H%M%S -> 201307150802013-07-15 08:17:08
As you see, the expression %{rand:1000000} gets expanded into an empty string. In my particular case, this leads to a lot less entropy than I thought. The expansion is there three times alright, that's as per config, but I would have expected the results to be different.
Now that I knew where to look, I went back to 2.2.0 mainstream release code - and saw the same :-(
Works in 3.0.0, but yes, broken in v2.x.x HEAD. Odd seeing as the xlat code is very similar.
So after investigating this a bit more, it seems the correct way to pass arguments to xlat functions in 2.x.x is with a space after the colon. This is because xlat functions which take numerical arguments, may get mistaken for attributes with tag selectors by the xlat parser code (in 2.x.x at least). The behaviour change which caused your issue was introduced by https://github.com/FreeRADIUS/freeradius-server/commit/33e9e1f7fbf5e63baa0fb..., but it's not a bug. The man pages for xlat, show a space after the ':' for module calls. The parser is 3.0.0 is smarter, and uses the table of registered xlat functions to determine whether it's an xlat module or an attribute. We don't do this in 2.x.x because of the potential performance hit. Though as compilation and evaluation is still done at runtime... um never mind, that'll be fixed in 3.1 :) The difficult work was splitting it into two stages, which is now complete. Anyway %{rand: 1000000} should work fine. +- entering group authorize {...} expand: 500 -> 500 expand: %{rand: 500} -> 309 expand: ccccccccccccc -> ccccccccccccc expand: %{randstr:ccccccccccccc} -> aodcqryxbsgpg Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team
Hi,
So after investigating this a bit more, it seems the correct way to pass arguments to xlat functions in 2.x.x is with a space after the colon.
This is because xlat functions which take numerical arguments, may get mistaken for attributes with tag selectors by the xlat parser code (in 2.x.x at least).
The behaviour change which caused your issue was introduced by https://github.com/FreeRADIUS/freeradius-server/commit/33e9e1f7fbf5e63baa0fb..., but it's not a bug. The man pages for xlat, show a space after the ':' for module calls.
The parser is 3.0.0 is smarter, and uses the table of registered xlat functions to determine whether it's an xlat module or an attribute. We don't do this in 2.x.x because of the potential performance hit. Though as compilation and evaluation is still done at runtime... um never mind, that'll be fixed in 3.1 :) The difficult work was splitting it into two stages, which is now complete.
Anyway %{rand: 1000000} should work fine.
+- entering group authorize {...} expand: 500 -> 500 expand: %{rand: 500} -> 309 expand: ccccccccccccc -> ccccccccccccc expand: %{randstr:ccccccccccccc} -> aodcqryxbsgpg
Rrrright. You know, I would almost buy your "RTFM", except that you admitted that this is way too unexpected and confusing for a normal users to "get it" - by writing "it's broken" yourself ;-) Anyway, this works indeed, I changed my config and see actual randomness. Greetings, Stefan Winter -- Stefan WINTER Ingenieur de Recherche Fondation RESTENA - Réseau Téléinformatique de l'Education Nationale et de la Recherche 6, rue Richard Coudenhove-Kalergi L-1359 Luxembourg Tel: +352 424409 1 Fax: +352 422473
On 15 Jul 2013, at 13:38, Stefan Winter <stefan.winter@restena.lu> wrote:
Hi,
So after investigating this a bit more, it seems the correct way to pass arguments to xlat functions in 2.x.x is with a space after the colon.
This is because xlat functions which take numerical arguments, may get mistaken for attributes with tag selectors by the xlat parser code (in 2.x.x at least).
The behaviour change which caused your issue was introduced by https://github.com/FreeRADIUS/freeradius-server/commit/33e9e1f7fbf5e63baa0fb..., but it's not a bug. The man pages for xlat, show a space after the ':' for module calls.
The parser is 3.0.0 is smarter, and uses the table of registered xlat functions to determine whether it's an xlat module or an attribute. We don't do this in 2.x.x because of the potential performance hit. Though as compilation and evaluation is still done at runtime... um never mind, that'll be fixed in 3.1 :) The difficult work was splitting it into two stages, which is now complete.
Anyway %{rand: 1000000} should work fine.
+- entering group authorize {...} expand: 500 -> 500 expand: %{rand: 500} -> 309 expand: ccccccccccccc -> ccccccccccccc expand: %{randstr:ccccccccccccc} -> aodcqryxbsgpg
Rrrright. You know, I would almost buy your "RTFM", except that you admitted that this is way too unexpected and confusing for a normal users to "get it" - by writing "it's broken" yourself ;-)
No comment :)
Anyway, this works indeed, I changed my config and see actual randomness.
Good. Any more last minute oddities or can we release 2.2.1 now? Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team
Hi,
Anyway, this works indeed, I changed my config and see actual randomness.
Good. Any more last minute oddities or can we release 2.2.1 now?
Well, there's still the SEGV on TERM - Alan's code fix a few days ago fixed one instance of the SEGV, but my server crashes at a later point of de-init now. I'm in touch with Alan for further debugging. Greetings, Stefan Winter -- Stefan WINTER Ingenieur de Recherche Fondation RESTENA - Réseau Téléinformatique de l'Education Nationale et de la Recherche 6, rue Richard Coudenhove-Kalergi L-1359 Luxembourg Tel: +352 424409 1 Fax: +352 422473
participants (2)
-
Arran Cudbard-Bell -
Stefan Winter