/
Bugzilla – Bug 2922
Auth API allows one auth module to authenticate user data provided by a different auth module
Last modified: 2008-05-13 11:04:05 UTC
Due to the way the FTP protocol requires separate USER and PASS commands, ProFTPD has to have separate lookups for the user data named by USER, and for the authentication of that user when PASS is received. This, combined with the fact that ProFTPD allows multiple simultaneous auth modules to exist (e.g. mod_auth_unix, mod_sql, mod_ldap), makes it possible for one of those auth modules to provide user data (e.g. mod_auth_unix) but then another module to authenticate that user data (e.g. mod_sql). This is very much a problem when an auth module, e.g. mod_sql, is configured to use a less strict authentication policy, such as: SQLAuthTypes Plaintext In this particular example, it means that mod_sql's "authenticate" handler will use a simple string comparison. Combine that with user data supplied from, say, /etc/passwd, and allowing users to login that shouldn't be able to login becomes possible. While such configurations should be heartily discouraged, this also demonstrates an interaction within ProFTPD's Auth API, by various auth modules, that is not clear or expected. To fix this, the Auth API should remember which auth module supplied information for a given user, and then later, when asked to authenticate that user, *only* that module should be called.
Created attachment 2595 [details] Fixes bug This patch adds a specific sort of caching to the Auth API. Whenever user data is retrieved using the pr_auth_getpwnam() function, the Auth API will store the module which returned the requested user information in a table, keyed by user name. Later, when either pr_auth_authenticate() or pr_auth_check() are called, the Auth API will check to see if that table contains an entry for the user being authenticated. If so, *only* that module's "authenticate" or "check" handlers will be used -- no other auth modules. In effect, this means that the Auth API remembers which module provided data for a user, will then use only that module for authenticating that data. Note that calls to pr_auth_endpwent() will clear the table of user name/supplier module associations.
Note that now auth.h and mod_auth.c declare both auth_sess_init() as static and non static, causing build failure.
Created attachment 2597 [details] Fixes bug Updates previous patch to avoid build warning/failure.
Created attachment 2598 [details] Fixes bug Make sure the cache table is not used outside of a session, i.e. that it is not used when proftpd is first starting up.
Patch committed to CVS.
This patch seems to be for 1.3.1rc. Would it be possible to have a backported version to 1.3.0a?
This patch breaks PAM logins. See http://bugs.gentoo.org/show_bug.cgi?id=178866 and http://bugs.gentoo.org/show_bug.cgi?id=175082
Ugh. Thanks.
Confirmed. See http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=421818
Created attachment 2634 [details] Adds check using PAM, if present This patch, to be used in addition to the prior patch, should re-add the check using PAM, prior to the stashed auth module that provided the user info, during authentication.
User reported the patch works
PAM check patch committed to CVS.
(In reply to comment #12) This patch breaks mod_ldap and mod_authfile when compiled with --enable-auth-pam=yes. It may break other auth modules as well, but I haven't tested them.
Which patch, specifically? There were two patches applied: http://bugs.proftpd.org/attachment.cgi?id=2598 http://bugs.proftpd.org/attachment.cgi?id=2634 If there is an issue, after applying *both* patches, please provide logging information, configuration files, etc.
Created attachment 2643 [details] Fixes typo in Auth API, for PAM check This patch fixes a typo in this patch: http://bugs.proftpd.org/attachment.cgi?id=2634 and fixes the Trace logging to be more accurate.
(In reply to comment #15) > Created an attachment (id=2643) [details] > Fixes typo in Auth API, for PAM check Tests ok for me. Thanks TJ.
Sorry to ask the same question again, but since 1.3.0a is still the release considered "stable", would it be possible to have a backport of this patch for it? This bug has been assigned CVE-2007-2165 and a "Medium" severity. It's always a bit of a problem to have to choose between a release with known security issues and a release that the developers haven't dared tag as "final", usually for a good reason.
Resolved in 1.3.1rc3.
Created attachment 2816 [details] Above 3 patches as one, for 1.3.0 branch As requested: 'backport'. Above 3 patches rolled into 1, with a slight change for 1.3.0. (I don't think you'll need it anymore, but since I was rolling it for myself anyway...)
Created attachment 2817 [details] Fixed patch to 1.3.0 OK, now everyone scream "try and use your own patches before uploading, stupid!"