FreeRADIUS / pam_radius

This is the PAM to RADIUS authentication module. It allows any Linux, OSX or Solaris machine to become a RADIUS client for authentication and password change requests.
GNU General Public License v2.0
102 stars 90 forks source link

Documentation of 'try_first_pass' PAM option seems inaccurate #94

Open demern opened 3 months ago

demern commented 3 months ago

In the USAGE file for this repo, the following is written about the try_first_pass auth option:

try_first_pass - Instead of prompting the user for a password, retrieve
                 the password from the previous authentication module.
                 If the password exists, try it, and return success if it
                 passes.
                 If there was no previous password, or the previous password
                 fails authentication, prompt the user with
                 "Enter RADIUS password: ", and ask for another password.
                 Try this password, and return success/failure as appropriate.

                 This is the default for authentication.

I do not believe this is completely accurate. It seems to me, after experimenting with the configuration on my system and inspecting the source, that pam_radius will not prompt again in the case where a non-NULL, invalid password is passed to it from a previous PAM module. It will simply fail to authenticate the user and pass the password down the chain (assuming the password is not empty, IE *password == ‘\0’. If it is an empty string, nothing is passed on because of this line. Not sure if that should be considered a bug or not. To me it would make sense to pass empty-string passwords on to the next layer...why treat them specially compared to any other password string? Since it’s treated specially, you can end up with weird stuff where your auth stack behaves one way for all invalid passwords a user enters except "", and some other way for "". And either way, it doesn’t seem documented. But I digress...).

If the behavior I've described is 'functioning as designed', or at least not going to be changed at this point, should the documentation be updated to reflect it?