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