Subject: Re: need for end*ent()?
To: Steven M. Bellovin <smb@cs.columbia.edu>
From: John Nemeth <jnemeth@victoria.tc.ca>
List: tech-security
Date: 09/15/2005 02:54:51
On Feb 3, 10:51pm, "Steven M. Bellovin" wrote:
} In message <200509140810.j8E8AEcN028511@vtn1.victoria.tc.ca>, John Nemeth writes:
} >On Feb 3, 10:26pm, "Steven M. Bellovin" wrote:
} >} In message <200509140707.j8E77lfP011272@vtn1.victoria.tc.ca>, John Nemeth writes:
} >} >On Dec 30,  7:52pm, Hubert Feyrer wrote:
} >} >} On Tue, 13 Sep 2005, John Nemeth wrote:
} >} >} >     I am working on libwrap to remove a reference to getgrnam().
} >} >} > Immediately after the use of getgrnam(), it calls endgrent() (there is
} >} >} > also a call to endpwent()).  I'm considering removing these in order to
} >} >} > reduce possible side effects on applications using the library.
} >} >} > However, I'm wondering if they should be left to ensure database
} >} >} > updates are seen in long running daemons as per this paragraph in the
} >} >} > manpage:
} >} >} >
} >} >} >     It is dangerous for long-running programs to keep the file descript
} >ors
} >} >} >     open as the database will become out of date if it is updated while
} > th
} >} >e
} >} >} >     program is running.
} >} >} >
} >} >} > Does anybody else have any thoughts on this issue?
} >} >} 
} >} >} The calls exist and are being used (properly) for the stated reason.
} >} >} Why would you want to remove them?
} >} >
} >} >     Because if the application happens to iterating a database by
} >} >using getgrent() or getpwent() at the time it makes a call to libwrap I
} >} >wouldn't want its pointer to be reset.  I have no idea why an
} >} >application would do this.  However, I don't think it is up to me or
} >} >necessarily up to any particular library to make this choice for it.
} >} 
} >} If that's the case, you're already in trouble, I think -- won't 
} >} getpwnam_r() reset the pointer?  The man page doesn't say (and I 
} >} haven't looked at the code because I'm trying to assess expected 
} >} behavior), but it does say 
} >} 
} >}      Calling getpwent_r() from multiple threads will result in
} >}      each thread reading a disjoint portion of the password database.
} >
} >     getpwent_r() isn't called by the library, just getgrnam() and
} >getpwnam_r() (it will be getgrnam_r() when I'm done).
} 
} I know that getpwent_r() isn't called; that's the ambiguity in the man 
} page that I'm talking about: it doesn't say what happens to the pointer 
} if getpwnam_r() is called.  I quoted the text about getpwent_r() to 
} show that one of the related functions would cause trouble.  Sorry I 
} didn't make that clear.

     Okay, I did some research on this.  I think you asked about what
Posix says about getpwnam().  The Posix definition is here:

http://www.opengroup.org/onlinepubs/009695399/functions/getpwnam.html

It doesn't say anything about the pointer used by getpwent().  As for
getpwent(), it says that the function is only there for backwards
compatibility reasons and that new programs shouldn't use it.  Posix
doesn't appear to define getpwent_r().

     I spent some time looking at the code.  There is a pointer that is
kept for use by getpwent{,_r}().  This pointer isn't touched by
getpw{nam,uid}().  This may be an artifact of using a Berkeley DB to
store passwd information.  The NetBSD implementation of these functions
is located in src/lib/libc/gen/getpwent.c.

     There is a stayopen flag to determine if the passwd database
should be kept open.  The only way to set this flag is by calling
setpassent() with a parameter of anything but 0.

     If the stayopen flag is clear then getpwnam() closes the database
after looking up an entry.  getpwnam_r() closes the database
unconditionally.  Since these are parallel functions and the primary
difference between them is that getpwnam_r() stores the entry in a user
supplied buffer I believe it is a bug that getpwnam_r() closes the
database unconditionally.  I believe its behaviour should be the same
as getpwnam() with the exception of where the data is stored.  Luke,
this is your code.  Any thoughts on this?

     I understand that libwrap is a security library and that given the
original author, Wietse Venema, that we need to carefully consider any
changes to it.  However, I haven't seen any other libraries call
endpwent().  Also, since setting the stayopen flag is a very deliberate
act, I think we should that any application author that does so knows
what they are doing (if they don't, that's their problem).  I don't
think libraries should go around willy nilly changing the state of the
system behind the backs of applications.  After all, this is the whole
purpose of this round of changes.  Therefore, I believe that both the
endpwent() and the endgrent() calls should be removed from libwrap.

}-- End of excerpt from "Steven M. Bellovin"