On Thursday, October 24, 2002, at 04:30 PM, Nathan Neulinger wrote:
Yup, you're right. This is pretty stupid. I'm surprised no one noticed before.As near as I can tell from the courier auth*vchkpw.c code, it only triggers either the vset_lastauth or the open_smtp_relay() routines BEFORE authentication.What good is that? Two problems - 1. The way it calls vset_lastauth in the pre* code means that you can have a denial of service race since it explicity sets the remote_ip field to "imap". If it already had a useful value in it, it's lost. As above - this is done before checking the password, so any putz that tries to fake a login can dork the contents of the table. Plus - it never updates the lastauth with a real ip, so that table is essentially useless when used with courier. 2. The open_smtp_relay() call is also done prior to login, so it's not actually protecting anything. Seems like the current code implements "Last time someone TRIED to log in from this IP", as opposed to "last successful auth from this ip". Anyone have a patch to courier to fix this completely useless/broken behavior?
I have a couple of ideas for fixing it:
1. move the open_smtp_relay() and vset_lastauth() stuff to a new vchkpw_post() function that is called AFTER authentication is verified. The downside is that this will be a second lookup in the vchkpw auth module (the authinfo struct doesn't have a method to store the vpopmail gid field), which would double auth traffic for anyone that uses these functions.
2. Modify auth_vchkpw_pre() to include the password provided. But I don't fully understand all the components of Sam's auth module structure, so I'm not sure of any ramifications of doing this. This is probably the simplest method, though.
Thoughts?
Regarding the logging of "service" in the lastauth table, probably the cleanest thing would be to update the vset_lastauth() function to include both service and IP. In the short term, though, you could always replace "servcice" with getenv("TCPREMOTEIP").
Regards,
Bill Shupp
