Re: Version 2.2.2 has been released
Hi, sorry, I found another unlang oddity regarding return codes. A simple auth section like this:
Auth-Type Test { updated ok }
reject the user:
Found Auth-Type = Test # Executing group from file [...] +group Test { ++[updated] = updated ++[ok] = ok +} # group Test = updated Failed to authenticate the user.
So the "ok" gets processed, but the final return code is "updated", which makes FreeRADIUS reject the request, where it should be accepted (to my understanding). Inserting another "ok" before the "updated" won't help:
Found Auth-Type = Test # Executing group from file [...] +group Test { ++[ok] = ok ++[updated] = updated ++[ok] = ok +} # group Test = updated Failed to authenticate the user.
Regards Jakob
Jakob Hirsch wrote:
sorry, I found another unlang oddity regarding return codes.
This shouldn't be different from 2.2.0.
So the "ok" gets processed, but the final return code is "updated", which makes FreeRADIUS reject the request, where it should be accepted (to my understanding).
This has to do with the return code priorities. The higher priority return code is the one which is kept. Here, "updated" has higher priority than "ok". It's been that way since 0.9, IIRC. Alan DeKok.
On Mon, Nov 04, 2013 at 12:55:18PM -0800, Alan DeKok wrote:
Jakob Hirsch wrote:
sorry, I found another unlang oddity regarding return codes.
This shouldn't be different from 2.2.0.
So the "ok" gets processed, but the final return code is "updated", which makes FreeRADIUS reject the request, where it should be accepted (to my understanding).
This has to do with the return code priorities. The higher priority return code is the one which is kept.
Here, "updated" has higher priority than "ok". It's been that way since 0.9, IIRC.
In authenticate, they are all set to '1': https://github.com/FreeRADIUS/freeradius-server/blob/master/src/main/modcall... I would have though from that that the OK should immediately return (and therefore the updated=1 should be irrelevant)? 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>
Matthew Newton wrote:
In authenticate, they are all set to '1':
https://github.com/FreeRADIUS/freeradius-server/blob/master/src/main/modcall...
For the modules in authenticate. It's a little weird. The sub-sections of authenticate use the "authorize" priorities and codes.
I would have though from that that the OK should immediately return (and therefore the updated=1 should be irrelevant)?
No because you want to be able to use unlang inside of Auth-Type Foo { ... } Alan DeKok.
On Mon, Nov 04, 2013 at 01:33:36PM -0800, Alan DeKok wrote:
Matthew Newton wrote:
In authenticate, they are all set to '1':
https://github.com/FreeRADIUS/freeradius-server/blob/master/src/main/modcall...
For the modules in authenticate. It's a little weird.
The sub-sections of authenticate use the "authorize" priorities and codes.
OK thanks, that clarifies it. Weird gives me an excuse to be confused :) Out of interest, having just stared at that code, priority is set to -1 on line 448: https://github.com/FreeRADIUS/freeradius-server/blob/release_2_2_2/src/main/... and then not touched (apart from setting to -1 again on line 732) until it is tested on line 806: https://github.com/FreeRADIUS/freeradius-server/blob/release_2_2_2/src/main/... Is there any way that condition (priority <= 0) can ever be false? It's subsequenty set to c->actions[result] on line 825, which is then tested on line 832, but at 846 we jump back to 'redo', which immediately sets it back to -1 again. So the condition on 806 _looks_ redundant? It wasn't there in the v2.2.0 test: https://github.com/FreeRADIUS/freeradius-server/blob/release_2_2_0/src/main/... 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>
Matthew Newton wrote:
Is there any way that condition (priority <= 0) can ever be false?
I'll take a look.
It's subsequenty set to c->actions[result] on line 825, which is then tested on line 832, but at 846 we jump back to 'redo', which immediately sets it back to -1 again.
So the condition on 806 _looks_ redundant? It wasn't there in the v2.2.0 test:
The code has changed for the better. But there may be cruft left over from intermediate steps. Alan DeKok.
On Mon, Nov 04, 2013 at 02:02:32PM -0800, Alan DeKok wrote:
The code has changed for the better. But there may be cruft left over from intermediate steps.
Agreed. It was much easier parsing the new code. Another change is that the priority test has changed from >= to >, but I can't see that will matter much. It will potentially change behaviour if someone has set several codes to the same priority for some reason (previously the last result will be taken, now it will be the first). 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>
Matthew Newton wrote:
Another change is that the priority test has changed from >= to >, but I can't see that will matter much.
It matters a lot, actually. The use of >= was a bug.
It will potentially change behaviour if someone has set several codes to the same priority for some reason (previously the last result will be taken, now it will be the first).
Yes, well... Alan DeKok.
participants (3)
-
Alan DeKok -
Jakob Hirsch -
Matthew Newton