/ Bug 2922 – Auth API allows one auth module to authenticate user data provided by a different auth module
Bug 2922 - Auth API allows one auth module to authenticate user data provided by a different auth module
: Auth API allows one auth module to authenticate user data provided by a diffe...
Status: CLOSED FIXED
Product: ProFTPD
core
: 1.2.10
: All All
: P2 major
Assigned To: proftpd development group
: http://bugs.debian.org/cgi-bin/bugrep...
:
:
:
  Show dependency treegraph
 
Reported: 2007-04-16 20:45 UTC by TJ Saunders
Modified: 2008-05-13 11:04 UTC (History)
4 users (show)

See Also:


Attachments
Fixes bug (10.74 KB, patch)
2007-04-16 20:52 UTC, TJ Saunders
Details
Fixes bug (10.72 KB, patch)
2007-04-17 11:16 UTC, TJ Saunders
Details
Fixes bug (10.75 KB, patch)
2007-04-17 17:22 UTC, TJ Saunders
Details
Adds check using PAM, if present (1.88 KB, patch)
2007-05-18 11:03 UTC, TJ Saunders
Details
Fixes typo in Auth API, for PAM check (1.25 KB, patch)
2007-06-12 14:56 UTC, TJ Saunders
Details
Above 3 patches as one, for 1.3.0 branch (11.17 KB, patch)
2008-05-13 10:01 UTC, Roderik Muit
Details
Fixed patch to 1.3.0 (11.08 KB, application/octet-stream)
2008-05-13 11:04 UTC, Roderik Muit
Details

Note You need to log in before you can comment on or make changes to this bug.
Description TJ Saunders 2007-04-16 20:45:50 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.
Comment 1 TJ Saunders 2007-04-16 20:52:38 UTC
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.
Comment 2 Francesco Paolo Lovergine 2007-04-17 05:25:43 UTC
Note that now auth.h and mod_auth.c declare both auth_sess_init() as static and
non static, causing build failure.
Comment 3 TJ Saunders 2007-04-17 11:16:30 UTC
Created attachment 2597 [details]
Fixes bug

Updates previous patch to avoid build warning/failure.
Comment 4 TJ Saunders 2007-04-17 17:22:08 UTC
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.
Comment 5 TJ Saunders 2007-04-17 17:34:34 UTC
Patch committed to CVS.
Comment 6 Matthias Saou 2007-05-04 09:31:07 UTC
This patch seems to be for 1.3.1rc. Would it be possible to have a backported
version to 1.3.0a?
Comment 7 SchAmane 2007-05-17 11:11:41 UTC
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
Comment 8 TJ Saunders 2007-05-17 11:39:07 UTC
Ugh.  Thanks.
Comment 9 Francesco Paolo Lovergine 2007-05-18 04:56:05 UTC
Confirmed.
See http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=421818
Comment 10 TJ Saunders 2007-05-18 11:03:30 UTC
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.
Comment 11 Francesco Paolo Lovergine 2007-05-19 13:09:07 UTC
User reported the patch works
Comment 12 TJ Saunders 2007-05-21 11:20:04 UTC
PAM check patch committed to CVS.
Comment 13 Andrew Roberts 2007-06-12 13:26:14 UTC
(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.
Comment 14 TJ Saunders 2007-06-12 13:29:43 UTC
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. 
Comment 15 TJ Saunders 2007-06-12 14:56:08 UTC
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.
Comment 16 Andrew Roberts 2007-06-12 15:39:14 UTC
(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.
Comment 17 Matthias Saou 2007-06-19 14:03:28 UTC
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.
Comment 18 TJ Saunders 2007-07-05 13:07:03 UTC
Resolved in 1.3.1rc3.
Comment 19 Roderik Muit 2008-05-13 10:01:48 UTC
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...)
Comment 20 Roderik Muit 2008-05-13 11:04:05 UTC
Created attachment 2817 [details]
Fixed patch to 1.3.0

OK, now everyone scream "try and use your own patches before uploading,
stupid!"