[Pkg-netatalk-devel] Bug#1111652: netatalk: Unable to using PAM in centralized authentication scenario

[Hector Rulot]

> Aug 20 16:53:04 netatalkdbg afpd[41840]: DHX2: unknown username

   I have downgraded to netatalk Version: 4.1.2~ds-4 for
   this same reason.

   After recompile and intensive debugging I found the bug,
   but patching it correctly is too complex for me, so
   I downgraded.

   The problem is in the uam_getname() function of
   etc/afpd/uam.c module when calling getpwnam() to get
   the user data.

   The old/classic/working getpwnam() call has been changed
   to its reentrant version getpwnam_r(), even when
   (I think) there is not need as afpd is not threaded.

   But the call to getpwnam_r() has some grave errors;
   using a simplified version we have:

     struct passwd pwent_buf;
     char *buffer = malloc(bufsize);
     getpwnam_r(name, &pwent_buf, buffer, sizeof(buffer), &pwent);
     free(buffer);

   Problems:

   1) sizeof(buffer) is always 2, because buffer is a
   pointer (char *).

   2) The return value of getpwnam_r is not checked, it always
   return "OUT OF RANGE" (insufficient buffer size).

   3) As getpwnam_r returns error, the uam_getname() function
   proceeds to a classic scan to all users using
   "while ((pwent = getpwent()))". This works in the simple
   case of local users, but fails when when the users (as
   in our case) are 350.000 PAM/LDAP entries. The scan last
   minutes and the user generally is not found because
   the download of all users is limited :-(((

   4) Even when a working buffer (using "bufsize" instead
   of "sizeof(buffer)") is used, uam_getname() returns invalid
   data and potentially can break badly, as it returns data that
   is stored in -locally- declared variables that are destroyed
   on function exit,

   getpwnam_r stores its data pointers in "pwent_buf" and
   pwent_buf is -not- static.

   Also, data returned by uam_getname() is stored in "buffer"
   that is dinamically allocated and -freed- on function exit (!).

   5) if "buffer" is not freed, there is a potential memory leak.
   Then correcting the bug is no trivial because uam_getname()
   has to return not only "pwent" but also "buffer" and the calling
   function must free it after use. Also, to be reentrant uam_getname()
   must receive pwent_buf, not declare it itself.

   Hope this helps.