auth.c and rlm_pap tidying patches

Alan DeKok aland at deployingradius.com
Fri Sep 28 10:17:00 CEST 2012


Matthew Newton wrote:
> Been hacking on (mainly) rad_check_password in auth.c. It's got
> sections marked as FIXME, and lots of it is duplicated in rlm_pap
> and rlm_chap.

  Yeah.  I meant to clean that up before 3.0.  The code is pretty horrific.

> There are a series of patches here:
> 
>   https://github.com/mcnewton/freeradius-server/commits/auth_tidy
> 
> In order, they are:
> 
> 5843221b3 -
>   remove the large chunk of code that finds Crypt-Password, or
>   User-Password or Cleartext-Password, sets Auth-Type to 'Crypt'
>   or 'Local', and checks auth for these. It's completely
>   duplicated in rlm_pap and rlm_chap, so unnecessary here. Still
>   throws up warnings telling the user to fix their config to use
>   pap or chap if Auth-Type has not been set (but
>   Cleartext-Password or User-Password are set) before failing
>   auth.

  Sounds good.

> e12867d57 -
>   no longer copy User-Password to Cleartext-Password if the admin
>   configured it wrong. a) it's unnecessary because the login
>   checks here have been removed, and b) rlm_pap will use both
>   anyway, so no need to copy. Still throws up the big !!! warning !!!
>   about using Cleartext-Password instead.

  Sounds good.

> 0cb1cb3cd -
>   move the !!! warning !!! about User-Password from auth.c into
>   rlm_pap.c, which is where it is checked. There's no reason for
>   it to clutter up rad_check_password any more really.

  Which means people don't see it if they edit the configs to remove
rlm_pap.  Oh well.  I'm fine with that.  I've been gradually trying to
remove RADIUS functionality from the core for years.

> c5350ba22 -
>   tidy up comments in rad_check_password

  OK.

> fdd53ce23 -
>   updates to rlm_pap.c - the (undocumented) encryption_scheme
>   option was completely broken, so change 'scheme' to 'inst->sch'
>   to fix that, and update it so that any forced encryption type
>   with this option must compare with Cleartext-Password, not
>   User-Password. Warn and fail if this is not the case.

  Hmm... the encryption_scheme is a holdover from 1.1.x, I think.

> The next step would be either to document the PAP encryption_scheme
> option correctly, or to remove it entirely - I'm not sure of its
> history, whether it has ever worked, and if it is wanted or not.
> It seems a potentially useful option, but maybe {type} has
> superseeded it. If it is removed, the rlm_pap.c could potentially
> be tidied up quite a lot.

  It should be removed.  The {crypt} etc. method is superior, and widely
used.

  Send a pull request && I'll get it in.

  Alan DeKok.


More information about the Freeradius-Devel mailing list