On Thursday, October 24, 2002, at 04:30  PM, Nathan Neulinger wrote:

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
Yup, you're right. This is pretty stupid. I'm surprised no one noticed before.

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.


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").


Bill Shupp

Reply via email to