Post-Auth-Type REJECT "broken" in 3.1.x
Unlike previous behaviour, post auth { Post-Auth-Type REJECT { ... } } does not get found and called in 3.1.x - you get (1) ERROR: No Auth-Type found: rejecting the user via Post-Auth-Type = Reject (1) Failed to authenticate the user (1) Using Post-Auth-Type Reject (1) Post-Auth-Type Reject sub-section not found. Using default return values. If it's changed to Post-Auth-Type Reject { then all is fine. This broke in commit 0b2e4da0c574dedd9309c9dbbdf4f8cb9d869d27 Author: Alan T. DeKok <aland@freeradius.org> Date: Tue May 24 17:00:29 2016 -0400 Use CONF_SECTIONs directly in unlang_interpret The easiest fix is to change the default config to be "Reject" rather than "REJECT", but that will seemingly unnecessarily break configs when people upgrade from 3.0 so I'm not sure that's the best solution. Matthew -- Matthew Newton, Ph.D. <mcn4@leicester.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 Jun 22, 2016, at 7:32 PM, Matthew Newton <mcn4@leicester.ac.uk> wrote: Unlike previous behaviour,
post auth { Post-Auth-Type REJECT { ... } }
does not get found and called in 3.1.x - you get
(1) ERROR: No Auth-Type found: rejecting the user via Post-Auth-Type = Reject (1) Failed to authenticate the user (1) Using Post-Auth-Type Reject (1) Post-Auth-Type Reject sub-section not found. Using default return values.
Arg...
If it's changed to
Post-Auth-Type Reject {
then all is fine.
That's annoying.
This broke in
commit 0b2e4da0c574dedd9309c9dbbdf4f8cb9d869d27 Author: Alan T. DeKok <aland@freeradius.org> Date: Tue May 24 17:00:29 2016 -0400
Use CONF_SECTIONs directly in unlang_interpret
The easiest fix is to change the default config to be "Reject" rather than "REJECT", but that will seemingly unnecessarily break configs when people upgrade from 3.0 so I'm not sure that's the best solution.
TBH, we could just revert that commit. I was making changes in v3.1.x branch before they became too invasive. I rolled back many of the changes, and went on to re-do them in a newly created v4.0.x branch. If reverting that commit works, just go ahead and do it. At this point, I won't be touching v3.0.x or v3.1.x for anything other than bug fixes. Alan DeKok.
Hi,
The easiest fix is to change the default config to be "Reject" rather than "REJECT", but that will seemingly unnecessarily break configs when people upgrade from 3.0 so I'm not sure that's the best solution.
IIRC upgrading from 3.0.x to 3.1.x wont work with 3.0.x configs anyway due to several symantec changes with variable/option names anyway. (which is one reason I advise people to use 3.1.x rather than 3.0.x when moving from 2.x anyway - as they'll be ready for 3.2.x release....whereas if you've migrated config to 3.0.x you'll have to migrate again for 3.2.x ;-) alan
On Jun 23, 2016, at 9:38 AM, A.L.M.Buxey@lboro.ac.uk wrote:
IIRC upgrading from 3.0.x to 3.1.x wont work with 3.0.x configs anyway due to several symantec changes with variable/option names anyway.
Still, minimal surprises are nice.
(which is one reason I advise people to use 3.1.x rather than 3.0.x when moving from 2.x anyway - as they'll be ready for 3.2.x release....whereas if you've migrated config to 3.0.x you'll have to migrate again for 3.2.x ;-)
If only we could get people to upgrade from 2.1.12. <sigh> The larger problem for 4.0 is that there will be many, many, more changes to the configuration. Practices of 18 years will unfortunately be changed. The good news is that those changes are required to add massive new functionality. The whole server becomes simpler to maintain (for the coders), and simpler to understand for the administrators. Alan DeKok.
On Thu, Jun 23, 2016 at 09:41:27AM -0400, Alan DeKok wrote:
On Jun 23, 2016, at 9:38 AM, A.L.M.Buxey@lboro.ac.uk wrote:
IIRC upgrading from 3.0.x to 3.1.x wont work with 3.0.x configs anyway due to several symantec changes with variable/option names anyway.
Still, minimal surprises are nice.
That's what I thought. I'll take a look at it.
(which is one reason I advise people to use 3.1.x rather than 3.0.x when moving from 2.x anyway - as they'll be ready for 3.2.x release....whereas if you've migrated config to 3.0.x you'll have to migrate again for 3.2.x ;-)
If only we could get people to upgrade from 2.1.12. <sigh>
:(
The larger problem for 4.0 is that there will be many, many, more changes to the configuration. Practices of 18 years will unfortunately be changed.
I guess 4.0.x might now be a good time to consider several things that have been around for a long time, but may no longer be particularly common or required. Looking at e.g. preprocess, there are a shedload of hacks for things that look pretty old. How many are still useful, or could be written as unlang policies instead? With unlang, are hints and huntgroups still worth keeping? Is preprocess still needed at all? Is there a more generic way rlm_files could work to cover the same thing? Things like mschap NT domain hack... Talk a while back about renameing authorize{}, post-auth{} etc. Though I don't think there were any conclusive arguments. I don't have any idea whether people are still using any of this stuff... Matthew -- Matthew Newton, Ph.D. <mcn4@leicester.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 Jun 23, 2016, at 10:54 AM, Matthew Newton <mcn4@leicester.ac.uk> wrote:
I'll take a look at it.
Thanks.
I guess 4.0.x might now be a good time to consider several things that have been around for a long time, but may no longer be particularly common or required.
Yes.
Looking at e.g. preprocess, there are a shedload of hacks for things that look pretty old. How many are still useful, or could be written as unlang policies instead?
Many could be written as policies. My only concern is performance. While unlang is flexible, it's not particularly fast. So having policies in C is often useful.
With unlang, are hints and huntgroups still worth keeping? Is preprocess still needed at all? Is there a more generic way rlm_files could work to cover the same thing?
The files module might be able to do something similar.
Things like mschap NT domain hack...
Talk a while back about renameing authorize{}, post-auth{} etc. Though I don't think there were any conclusive arguments.
It's started in 4.0. :( The argument is simple: keeping the existing methods is RADIUS-specific, confusing, etc. Q: what happens when the server sends an Access-Reject? A: it runs the post-auth section, and for that, the Post-Auth-Type Reject subsection Q: Huh? What kind of crack are you guys on anyways? A: :( In 4.0, it's now: send Access-Reject { ... } Simple. Very, very, simple. Also, the non-RADIUS protocols are currently hacked together by running them through "authorize" or "post-auth" sections. That's terrible. In 4.0, it's: recv DHCP-Foo { } send DHCP-Bar { } Which is again much simpler. See src/modules/proto_radius/proto_radius_acct.c for details
I don't have any idea whether people are still using any of this stuff...
There are occasional questions about hunt groups, nothing about hints. TBH, hints could go today. The hunt groups could be removed, and replaced with "use a database". Many of the other NAS-specific hacks can be removed. They've been there for 18 years, and I don't think any of that equipment is still running. If it is, people can just write "unlang" for it. Alan DeKok.
On Thu, Jun 23, 2016 at 11:02:28AM -0400, Alan DeKok wrote:
On Jun 23, 2016, at 10:54 AM, Matthew Newton <mcn4@leicester.ac.uk> wrote:
Looking at e.g. preprocess, there are a shedload of hacks for things that look pretty old. How many are still useful, or could be written as unlang policies instead?
Many could be written as policies. My only concern is performance. While unlang is flexible, it's not particularly fast. So having policies in C is often useful.
Yes. I was mainly thinking that if hardly anyone uses some feature, then an unlang policy might be a tidier option even though it might not be quite as quick.
Things like mschap NT domain hack...
Talk a while back about renameing authorize{}, post-auth{} etc. Though I don't think there were any conclusive arguments.
It's started in 4.0. :(
That's ":)" surely? :)
The argument is simple: keeping the existing methods is RADIUS-specific, confusing, etc.
Q: what happens when the server sends an Access-Reject?
A: it runs the post-auth section, and for that, the Post-Auth-Type Reject subsection
Yeah.
Q: Huh? What kind of crack are you guys on anyways?
Not good enough quality crack, obviously. (Maybe just "cracked", happy to include myself...)
In 4.0, it's now:
send Access-Reject { ... }
Simple. Very, very, simple.
That looks nicer.
Also, the non-RADIUS protocols are currently hacked together by running them through "authorize" or "post-auth" sections. That's terrible.
It always seemed rather messy, and it's obvious they were an afterthought.
I don't have any idea whether people are still using any of this stuff...
There are occasional questions about hunt groups, nothing about hints.
There was a question about hints a couple of days ago, which was what got me thinking about it. It was the first question I remember in a very long time.
TBH, hints could go today. The hunt groups could be removed, and replaced with "use a database".
I wonder if FR should actually use a sqlite database by default, given some tools to manage the data in it. Though I wouldn't want to see 'files' go away entirely. I must admit though I've never really liked the standard FR SQL schema though. But I've also not ever had time to look at it and see if there is something more sensible these days. Again, it smells of 'legacy'.
Many of the other NAS-specific hacks can be removed. They've been there for 18 years, and I don't think any of that equipment is still running. If it is, people can just write "unlang" for it.
Agreed. Matthew -- Matthew Newton, Ph.D. <mcn4@leicester.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 Thu, Jun 23, 2016 at 09:33:52AM -0400, Alan DeKok wrote:
This broke in
commit 0b2e4da0c574dedd9309c9dbbdf4f8cb9d869d27
The easiest fix is to change the default config to be "Reject" rather than "REJECT", but that will seemingly unnecessarily break configs when people upgrade from 3.0 so I'm not sure that's the best solution.
TBH, we could just revert that commit. I was making changes in v3.1.x branch before they became too invasive. I rolled back many of the changes, and went on to re-do them in a newly created v4.0.x branch.
If reverting that commit works, just go ahead and do it. At this point, I won't be touching v3.0.x or v3.1.x for anything other than bug fixes.
After a brief go at this it looks like it's a whole chunk of commits after that as well. I've got 20 or so at the moment but still not enough to revert cleanly. So it's probably pretty much roll back to before that commit and apply the few other things that are unrelated, or fix it another way. Which way would you prefer? I'm not familiar with that code so I can try and come up with another fix but it may take a while. Thanks, Matthew -- Matthew Newton, Ph.D. <mcn4@leicester.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 Thu, Jun 23, 2016 at 11:03:32PM +0100, Matthew Newton wrote:
On Thu, Jun 23, 2016 at 09:33:52AM -0400, Alan DeKok wrote:
This broke in
commit 0b2e4da0c574dedd9309c9dbbdf4f8cb9d869d27
TBH, we could just revert that commit. I was making changes in v3.1.x branch before they became too invasive. I rolled back many of the changes, and went on to re-do them in a newly created v4.0.x branch.
After a brief go at this it looks like it's a whole chunk of commits after that as well. I've got 20 or so at the moment but
OK, this *seems* to be the commits that need to be reverted: 0e3dbd8 Move prototypes for public consumption 155cce8 try this... c3c11c1 move pointer from typedef to struct entry? 159c2d4 Put "component" and "actions" into compile context b9ba753 rename function to be more consistent b4dcc39 one more default action compile 708f4b3 compile_module should use compile_defaultactions() 5039b29 use compile_default_actions everywhere 7323bb7 dump_tree needs a name, not a component 9aa50a9 modcall does not need rlm_component_t 0ca5d62 Remove modcall.h, it was only 4 lines long 9cd1e70 rename modcall_compile_section() to unlang_compile() 2bed61e more functions use group_allocate 613bf40 Remove all knowledge of components from the interpreter. 7d6df32 always set request->server_cs 65c65ac make unlang_interpret() take a default action 4df0f89 Set request->server_cs if not already set 12217ed remove indexed_modcall and virtual_server_t 0b2e4da Use CONF_SECTIONs directly in unlang_interpret which is any changes to src/main/modules.c, src/include/interpreter.h, src/main/interpreter.c and src/main/modcall.c since 0b2e4da. i.e. git log --pretty='%h' 0b2e4da^..4e131ebd -- src/main/modules.c src/include/interpreter.h src/main/interpreter.c src/main/modcall.c | xargs git revert -n I've pushed it to https://github.com/mcnewton/freeradius-server/commits/postauthreject It builds and the unit tests all complete OK. Let me know if you're happy to go with that? Thanks Matthew -- Matthew Newton, Ph.D. <mcn4@leicester.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>
participants (3)
-
A.L.M.Buxey＠lboro.ac.uk -
Alan DeKok -
Matthew Newton