Any objection to deleting support for "clients"?

classic Classic list List threaded Threaded
33 messages Options
12
| Threaded
Open this post in threaded view
|

Any objection to deleting support for "clients"?

Alan DeKok
  It's been deprecated forever...

  Alan DeKok.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features (was: Any objection to deleting support for "clients"?)

Nicolas Baradakis
Alan DeKok wrote:

>   It's been deprecated forever...

I agree. And I think we could perhaps take a moment to look at other
deprecated features.

* post_proxy_authorize

  This option provides backward compatibility with FreeRADIUS 0.7.
  It is obsoleted with the post-proxy section. We might make the
  default to "post_proxy_authorize = no" or perhaps completely remove
  the option from the documentation.

* authorize in rlm_attr_filter

  This module should be used in {pre,post}-proxy sections only.
  There is a warning at module startup since a long time, nobody
  should be using it in authorize section anymore.

* usercollide

  I never used this, but the doc says it doesn't work so well (with a
  big warning), and there is another way of achieving the same goal.

* bind_address and port

  The "listen" directive can do the same, and it's much more flexible.
  We might remove the old directives from the documentation.

What do you think? This list has to be completed, of course.

--
Nicolas Baradakis

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features (was: Any objection to deleting support for "clients"?)

Alan DeKok
Nicolas Baradakis <[hidden email]> wrote:
> I agree. And I think we could perhaps take a moment to look at other
> deprecated features.
>
> * post_proxy_authorize

  We can delete it.

> * authorize in rlm_attr_filter

  Delete it.  We should also note that the "rlm_protocol_filter"
module does related things.

> * usercollide
>
>   I never used this, but the doc says it doesn't work so well (with a
>   big warning), and there is another way of achieving the same goal.

  It's a hack.  Nuke it.

  Also, the "lower_user" and "nospace_user" directives are atrocious.
They should be deleted, and replaced with something else.

> * bind_address and port
>
>   The "listen" directive can do the same, and it's much more flexible.
>   We might remove the old directives from the documentation.

  Sounds reasonable.

> What do you think? This list has to be completed, of course.

  I think that the CVS head has changed enough from the 1.0.x branch
that we should call it 2.0, and not 1.1.x.  Since it's a 2.0, we
should fix everything we can find.

  e.g. I've hacked up rlm_unix, so "Auth-Type := System" doesn't
exist.

  Also, the modules should have a header ala Apache, which allows us
to do better sanity checking on them.

  Alan DeKok.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features (was: Any objection to deleting support for"clients"?)

Thor Spruyt
In reply to this post by Nicolas Baradakis
Nicolas Baradakis wrote:
> I agree. And I think we could perhaps take a moment to look at other
> deprecated features.
>
> * post_proxy_authorize
>
>   This option provides backward compatibility with FreeRADIUS 0.7.
>   It is obsoleted with the post-proxy section. We might make the
>   default to "post_proxy_authorize = no" or perhaps completely remove
>   the option from the documentation.

Huh... my current v1.0.1 production servers have this set to "yes" and
probably a lot of other people still have this set to "yes", since that's
the default.
Regularly the advice of "only change the defaults if needed" is given.
Therfore, I wouldn't just delete this configuration parameter or
funtionality, but just change the default, so that users don't use it
anymore over time!

--
Groeten, Regards, Salutations,

Thor Spruyt
M: +32 (0)475 67 22 65
E: [hidden email]
W: www.thor-spruyt.com

www.salesguide.be
www.telenethotspot.be

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features (was: Any objection to deleting support for"clients"?)

Thor Spruyt
In reply to this post by Alan DeKok
Alan DeKok wrote:
>> * post_proxy_authorize
>
>   We can delete it.

Better change the default to "no" first.
Since the default is still "yes" even in 1.0.4, I guess a lot of people
still use "yes"!

>   I think that the CVS head has changed enough from the 1.0.x branch
> that we should call it 2.0, and not 1.1.x.  Since it's a 2.0, we
> should fix everything we can find.

Would you include a fix for Exec-Program-Wait (see below)? If needed, I can
submit it on bugs.freeradius.org


--- src/main/auth.c.orig        2005-07-18 14:17:40.000000000 +0000
+++ src/main/auth.c     2005-07-18 14:31:31.000000000 +0000
@@ -895,24 +895,35 @@
                pairmove(&request->reply->vps, &tmp);
                pairfree(&tmp);

-               if (r != 0) {
+               if (r < 0) {
                        /*
                         *      Error. radius_exec_program() returns -1 on
-                        *      fork/exec errors, or >0 if the exec'ed
program
-                        *      had a non-zero exit status.
+                        *      fork/exec errors.
                         */
-                       if (umsg[0] == '\0') {
-                               user_msg = "\r\nAccess denied (external
check failed).";
-                       } else {
-                               user_msg = &umsg[0];
-                       }

-                       request->reply->code = PW_AUTHENTICATION_REJECT;
+                       user_msg = "Access denied (external check failed)";
                        tmp = pairmake("Reply-Message", user_msg, T_OP_SET);
-
                        pairadd(&request->reply->vps, tmp);
+
+                       request->reply->code = PW_AUTHENTICATION_REJECT;
+
                        rad_authlog("Login incorrect (external check
failed)",
-                                       request, 0);
+                                       request, 1);
+
+                       rad_postauth_reject(request);
+
+                       return RLM_MODULE_REJECT;
+               }
+               if (r > 0) {
+                       /*
+                        *      Reject. radius_exec_program() returns or >0
+                        *      if the exec'ed program had a non-zero exit
status.
+                        */
+
+                       request->reply->code = PW_AUTHENTICATION_REJECT;
+
+                       rad_authlog("Login incorrect (external check said
so)",
+                                       request, 1);

                        rad_postauth_reject(request);


--
Groeten, Regards, Salutations,

Thor Spruyt
M: +32 (0)475 67 22 65
E: [hidden email]
W: www.thor-spruyt.com

www.salesguide.be
www.telenethotspot.be

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features (was: Any objection to deleting support for"clients"?)

Alan DeKok
In reply to this post by Thor Spruyt
"Thor Spruyt" <[hidden email]> wrote:
> > * post_proxy_authorize
...
> Huh... my current v1.0.1 production servers have this set to "yes" and
> probably a lot of other people still have this set to "yes", since that's
> the default.

  That's OK.

> Regularly the advice of "only change the defaults if needed" is given.
> Therfore, I wouldn't just delete this configuration parameter or
> funtionality, but just change the default, so that users don't use it
> anymore over time!

  I'd say change both.  I'd like to update the server core to allow
more configurable calling of sections.  So the "post-proxy-authorize"
can morph into "when receiving a reply from a home server, use section
foo, and call the modules authorize function for each module listed in
that section"

  That kind of flexibility also allows you to automagically do
different things based on packet reply code, in a more flexible way
that the current "post-auth-type REJECT".

  Alan DeKok.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features (was: Any objection to deleting support for"clients"?)

Nicolas Baradakis
Alan DeKok wrote:

> "Thor Spruyt" <[hidden email]> wrote:
>
> > Regularly the advice of "only change the defaults if needed" is given.
> > Therfore, I wouldn't just delete this configuration parameter or
> > funtionality, but just change the default, so that users don't use it
> > anymore over time!
>
>   I'd say change both.  I'd like to update the server core to allow
> more configurable calling of sections.  So the "post-proxy-authorize"
> can morph into "when receiving a reply from a home server, use section
> foo, and call the modules authorize function for each module listed in
> that section"

In CVS head we have the stanza 'Post-Proxy-Type' which allows to run
different modules groups when receiving a reply from a home server.
Perhaps I didn't understand what you'd like to do, but I'd prefer not
running modules from authorize section a second time. (and moving all
the modules to the section post-proxy)

>   That kind of flexibility also allows you to automagically do
> different things based on packet reply code, in a more flexible way
> that the current "post-auth-type REJECT".

It could be easy to have a "Post-Proxy-Type REJECT", too.

--
Nicolas Baradakis

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features (was: Any objection to deleting supportfor"clients"?)

Thor Spruyt
Nicolas Baradakis wrote:

> Alan DeKok wrote:
>
>> "Thor Spruyt" <[hidden email]> wrote:
>>
>>> Regularly the advice of "only change the defaults if needed" is
>>> given. Therfore, I wouldn't just delete this configuration
>>> parameter or funtionality, but just change the default, so that
>>> users don't use it anymore over time!
>>
>>   I'd say change both.  I'd like to update the server core to allow
>> more configurable calling of sections.  So the "post-proxy-authorize"
>> can morph into "when receiving a reply from a home server, use
>> section foo, and call the modules authorize function for each module
>> listed in that section"
>
> In CVS head we have the stanza 'Post-Proxy-Type' which allows to run
> different modules groups when receiving a reply from a home server.
> Perhaps I didn't understand what you'd like to do, but I'd prefer not
> running modules from authorize section a second time. (and moving all
> the modules to the section post-proxy)
>
>>   That kind of flexibility also allows you to automagically do
>> different things based on packet reply code, in a more flexible way
>> that the current "post-auth-type REJECT".
>
> It could be easy to have a "Post-Proxy-Type REJECT", too.

I want to log replies sent to my NAS, whether it's an Accept or Reject or
whether the request was proxied or not.
I haven't been able to do this with 1.0.1, but I'll be upgrading to 1.0.4
with which I managed to do so.
As long as that's possible, everything is fine for me :)

--
Groeten, Regards, Salutations,

Thor Spruyt
M: +32 (0)475 67 22 65
E: [hidden email]
W: www.thor-spruyt.com

www.salesguide.be
www.telenethotspot.be

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features (was: Any objection to deleting support for"clients"?)

Alan DeKok
In reply to this post by Nicolas Baradakis
Nicolas Baradakis <[hidden email]> wrote:
> In CVS head we have the stanza 'Post-Proxy-Type' which allows to run
> different modules groups when receiving a reply from a home server.
> Perhaps I didn't understand what you'd like to do, but I'd prefer not
> running modules from authorize section a second time. (and moving all
> the modules to the section post-proxy)

  I agree with the last bit.

  As for what I'm trying to do, I'm not exactly sure.  Maybe in
"post-auth", we need to have sub-sections, to make it clear what's
run, and where:

post-auth {
          Access-Accept {
          ...
          }
          Access-Challenge {
                ...
          }
          Access-Reject {
          ...
          }
}

  That would be obvious, at least.

  Alan DeKok.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features (was: Any objection to deleting supportfor"clients"?)

Nicolas Baradakis
In reply to this post by Thor Spruyt
Thor Spruyt wrote:

> I want to log replies sent to my NAS, whether it's an Accept or Reject or
> whether the request was proxied or not.
> I haven't been able to do this with 1.0.1, but I'll be upgrading to 1.0.4
> with which I managed to do so.
> As long as that's possible, everything is fine for me :)

Logging replies sent to the NAS should be done in post-auth and has
nothing to do with the authorize section. Changing the default for
post_proxy_authorize will never break that.

--
Nicolas Baradakis

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Exec-Program-Wait return value (was: Deprecated features)

Nicolas Baradakis
In reply to this post by Thor Spruyt
Thor Spruyt wrote:

> Would you include a fix for Exec-Program-Wait (see below)? If needed, I can
> submit it on bugs.freeradius.org

Comments below..

> --- src/main/auth.c.orig        2005-07-18 14:17:40.000000000 +0000
> +++ src/main/auth.c     2005-07-18 14:31:31.000000000 +0000
> @@ -895,24 +895,35 @@
>                 pairmove(&request->reply->vps, &tmp);
>                 pairfree(&tmp);
>
> -               if (r != 0) {
> +               if (r < 0) {
>                         /*
>                          *      Error. radius_exec_program() returns -1 on
> -                        *      fork/exec errors, or >0 if the exec'ed
> program
> -                        *      had a non-zero exit status.
> +                        *      fork/exec errors.
>                          */
> -                       if (umsg[0] == '\0') {
> -                               user_msg = "\r\nAccess denied (external
> check failed).";
> -                       } else {
> -                               user_msg = &umsg[0];
> -                       }

Is it correct to forget the message in umsg ? I don't know this part of
the code, what sort of message is in the string umsg ?

If it isn't usefull, we could pass NULL to the function radius_exec_program().

> -                       request->reply->code = PW_AUTHENTICATION_REJECT;
> +                       user_msg = "Access denied (external check failed)";
>                         tmp = pairmake("Reply-Message", user_msg, T_OP_SET);
> -
>                         pairadd(&request->reply->vps, tmp);
> +
> +                       request->reply->code = PW_AUTHENTICATION_REJECT;
> +
>                         rad_authlog("Login incorrect (external check
> failed)",
> -                                       request, 0);
> +                                       request, 1);
> +
> +                       rad_postauth_reject(request);
> +
> +                       return RLM_MODULE_REJECT;
> +               }
> +               if (r > 0) {
> +                       /*
> +                        *      Reject. radius_exec_program() returns or >0
> +                        *      if the exec'ed program had a non-zero exit
> status.
> +                        */

In CVS head a return code >0 has a different meaning for the rlm_exec
module. Perhaps we may want something similar with Exec-Program-Wait,
what do you think?

See the rlm_exec examples in
http://www.freeradius.org/cgi-bin/cvsweb.cgi/~checkout~/radiusd/raddb/radiusd.conf.in?rev=1.216

> +                       request->reply->code = PW_AUTHENTICATION_REJECT;
> +
> +                       rad_authlog("Login incorrect (external check said
> so)",
> +                                       request, 1);
>
>                         rad_postauth_reject(request);

In the case (r > 0) you didn't add a "Reply-Message" in request->reply->vps.

--
Nicolas Baradakis

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: post-auth and post-proxy subsections (was: Deprecated features)

Nicolas Baradakis
In reply to this post by Alan DeKok
Alan DeKok wrote:

>   I agree with the last bit.
>
>   As for what I'm trying to do, I'm not exactly sure.  Maybe in
> "post-auth", we need to have sub-sections, to make it clear what's
> run, and where:
>
> post-auth {
>  Access-Accept {
>   ...
>  }
>  Access-Challenge {
> ...
>  }
>  Access-Reject {
>   ...
>  }
> }
>
>   That would be obvious, at least.

But what should we do when the administrator wants different Post-Auth-Type
stanzas for each realm on a multi-realm server?

I'm not sure about about it either. Perhaps this approach could be
possible: if a check item 'Post-Auth-Type' already exists, we can look
for a stanza named %{check:Post-Auth-Type}.%{reply:Packet-Type}. If we
found such a stanza, we run the modules we found inside. Otherwise we
run the modules in the stanza named %{check:Post-Auth-Type}. (fallback
to the current behaviour)

--
Nicolas Baradakis

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features

Paul "TBBle" Hampson
In reply to this post by Thor Spruyt
On Wed, Jul 27, 2005 at 07:25:54PM +0200, Thor Spruyt wrote:
> Alan DeKok wrote:
> >   I think that the CVS head has changed enough from the 1.0.x branch
> > that we should call it 2.0, and not 1.1.x.  Since it's a 2.0, we
> > should fix everything we can find.

Definately. ^_^

> Would you include a fix for Exec-Program-Wait (see below)? If needed, I can
> submit it on bugs.freeradius.org

Can we instead nuke Exec-Program and Exec-Program-Wait? Are there any
more cases they handle that rlm_exec does not?

I didn't port the rlm_exec return values changes to Exec-Program-Wait,
because they're to do with the configurable failover, and the
Exec-Program calls are done after that's no longer relevant.

And because I'd like to see Exec-Program nuked. ^_^

--
Paul "TBBle" Hampson, on an alternate email client.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: post-auth and post-proxy subsections (was: Deprecated features)

Emile van Bergen
In reply to this post by Nicolas Baradakis
Hi,

On Thu, Jul 28, 2005 at 11:53:56AM +0200, Nicolas Baradakis wrote:

> Alan DeKok wrote:
>
> >   I agree with the last bit.
> >
> >   As for what I'm trying to do, I'm not exactly sure.  Maybe in
> > "post-auth", we need to have sub-sections, to make it clear what's
> > run, and where:
> >
> > post-auth {
> >  Access-Accept {
> >   ...
> >  }
> >  Access-Challenge {
> > ...
> >  }
> >  Access-Reject {
> >   ...
> >  }
> > }
> >
> >   That would be obvious, at least.
>
> But what should we do when the administrator wants different Post-Auth-Type
> stanzas for each realm on a multi-realm server?
>
> I'm not sure about about it either. Perhaps this approach could be
> possible: if a check item 'Post-Auth-Type' already exists, we can look
> for a stanza named %{check:Post-Auth-Type}.%{reply:Packet-Type}. If we
> found such a stanza, we run the modules we found inside. Otherwise we
> run the modules in the stanza named %{check:Post-Auth-Type}. (fallback
> to the current behaviour)

<tongue loosely in cheeck>

I see I definitely did choose the right approach in OpenRADIUS to leave
that sort of decision wholly to the administrator, and to put that sort
of logic on a layer well above the server code.

Cheers,


Emile

--
E-Advies - Emile van Bergen           [hidden email]      
tel. +31 (0)70 3906153           http://www.e-advies.nl   
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: post-auth and post-proxy subsections (was: Deprecated features)

Alan DeKok
In reply to this post by Nicolas Baradakis
Nicolas Baradakis <[hidden email]> wrote:
> But what should we do when the administrator wants different Post-Auth-Type
> stanzas for each realm on a multi-realm server?

  Hmm...

> I'm not sure about about it either. Perhaps this approach could be
> possible:
....

  Maybe we should just move to a completely separate way of handling things.

  Alan DeKok.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features

Alan DeKok
In reply to this post by Paul "TBBle" Hampson
[hidden email] (Paul Hampson) wrote:
> Can we instead nuke Exec-Program and Exec-Program-Wait? Are there any
> more cases they handle that rlm_exec does not?

  Not really.  They should be nuked.  They're hacks.

  Alan DeKok.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Exec-Program-Wait return value (was: Deprecated features)

Thor Spruyt
In reply to this post by Nicolas Baradakis
Nicolas Baradakis wrote:
> Is it correct to forget the message in umsg ? I don't know this part
> of
> the code, what sort of message is in the string umsg ?

If the external program fails, it won't be sending a message :-)
So we can just add our own message indicating that the external program
failed.
I even wouldn't add that message, since the user shouldn't know something
failed, but it should be logged!

> In CVS head a return code >0 has a different meaning for the rlm_exec
> module. Perhaps we may want something similar with Exec-Program-Wait,
> what do you think?

Sure, but I'm not that good in coding :)

> In the case (r > 0) you didn't add a "Reply-Message" in
> request->reply->vps.

No, since the external program didn't fail, I think it's up to the external
program to supply a message if it whishes to do so.
The Reply-Message is an attribute that should contain a message readable by
the user, so it shouldn't be something like "(external check failed)", but
it should be like "Your account had expired" or whatever the external
program whishes.

--
Groeten, Regards, Salutations,

Thor Spruyt
M: +32 (0)475 67 22 65
E: [hidden email]
W: www.thor-spruyt.com

www.salesguide.be
www.telenethotspot.be

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features

Thor Spruyt
In reply to this post by Paul "TBBle" Hampson
Paul Hampson wrote:

>> Would you include a fix for Exec-Program-Wait (see below)? If
>> needed, I can submit it on bugs.freeradius.org
>
> Can we instead nuke Exec-Program and Exec-Program-Wait? Are there any
> more cases they handle that rlm_exec does not?
>
> I didn't port the rlm_exec return values changes to Exec-Program-Wait,
> because they're to do with the configurable failover, and the
> Exec-Program calls are done after that's no longer relevant.
>
> And because I'd like to see Exec-Program nuked. ^_^

Well, I experimented with rlm_exec half a year ago, and I didn't found it
suitable for my needs, where Exec-Program and Exec-Program-Wait did!
I would suggest deprecating them, but not removing, so to give people time
to move to rlm_exec or rlm_perl (when is that finaly going to be stable?).
When I have time, I'll try rlm_exec again to see if things have changed in
the right direction.

--
Groeten, Regards, Salutations,

Thor Spruyt
M: +32 (0)475 67 22 65
E: [hidden email]
W: www.thor-spruyt.com

www.salesguide.be
www.telenethotspot.be

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: post-auth and post-proxy subsections (was: Deprecated features)

Alan DeKok
In reply to this post by Emile van Bergen
Emile van Bergen <[hidden email]> wrote:
> I see I definitely did choose the right approach in OpenRADIUS to leave
> that sort of decision wholly to the administrator, and to put that sort
> of logic on a layer well above the server code.

  FreeRADIUS is structured that way, too, it's just not obvious.  The
"track radius request" code is independent of the modules, which are
independent of how the modules are put together in lists.

  There's really nothing preventing FreeRADIUS from doing something
similar to OpenRADIUS, other than time.

  Alan DeKok.

-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
| Threaded
Open this post in threaded view
|

Re: Deprecated features

Alan DeKok
In reply to this post by Thor Spruyt
"Thor Spruyt" <[hidden email]> wrote:
> Well, I experimented with rlm_exec half a year ago, and I didn't found it
> suitable for my needs, where Exec-Program and Exec-Program-Wait did!

  What were the differences?

> I would suggest deprecating them, but not removing, so to give people time
> to move to rlm_exec or rlm_perl (when is that finaly going to be stable?).

  rlm_perl will be marked "stable" in 2.0.

  Alan DeKok.
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
12