Milter-greylist

I made this patch so that I could send messages out of my PDA using SMTP after POP/IMAP auth without the greylist delay.  Emmanuel Dreyfus suggested sending

Hi,

>>>>> On Sun, 27 Nov 2005 22:51:30 -0800
>>>>> Romain Kang <romain@...> said:

romain> I made this patch so that I could send messages out of my PDA using
romain> "SMTP after POP/IMAP" auth without the greylist delay.  Emmanuel
romain> Dreyfus suggested sending it to the mailing list for review.
romain> Perhaps this will be useful to someone else.

DRAC doesn't support an IPv6, and it seems your patch doesn't take
care that `addrstr' may hold a string form of an IPv6 address.  So, it
is better to prevent check_drac() from taking an IPv6 address as its
argument, IMHO.

Index: milter-greylist.c
diff -u -p milter-greylist.c.orig milter-greylist.c
--- milter-greylist.c.orig	Mon Nov 28 16:00:36 2005
+++ milter-greylist.c	Mon Nov 28 16:14:12 2005
@@ -300,7 +300,8 @@ mlfi_envrcpt(ctx, envrcpt)
 		    priv->priv_queueid, addrstr, priv->priv_from, *envrcpt);
 
 #ifdef USE_DRAC
-	if (check_drac(addrstr)) {
+	if (SA(&priv->priv_addr)->sa_family == AF_INET &&
+	    check_drac(addrstr)) {
 		if (conf.c_debug)
 			syslog(LOG_DEBUG, "%s found in DRAC", addrstr);
 		return SMFIS_CONTINUE;


Sincerely,

--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
ume@...  ume@{,jp.}FreeBSD.org
http://www.imasy.org/~ume/

On Sun, Nov 27, 2005 at 10:51:30PM -0800, Romain Kang wrote:
> I made this patch so that I could send messages out of my PDA using
> "SMTP after POP/IMAP" auth without the greylist delay.  Emmanuel
> Dreyfus suggested sending it to the mailing list for review.
> Perhaps this will be useful to someone else.

Some improvements that would have to be done:

1) Cope with IPv6: Hajimu UMEMOTO proposed a workaround.

2) Add a configure flag to enable USE_DRAC and set the location of the
database. We also need to check for Berkeley DB location. It would be nice
if you could do it, but if you are unfamiliar with autotools and don't have
time to waste, don't worry, I'll do it.

3) When check_drac returns true, don't pass the message immediatly, 
set priv->priv_whitelist = EXF_DRAC instead. Everywhere in the file, 
duplicate the EXF_SPF case and turn it into a EXF_DRAC. This will make
the whitelisting reason appear in the log file and the header.
Again I can do it, but don't hesitate to propose an upgraded patch.

4) As I understand, milter-greylist is only a consumer of the DB 
database. Anotehr process will feed it. What happens if we read while
the other process is writing? Shouldn't we open the file with option
O_SHLOCK to prevent concurent access? Even with O_SHLOCK, there 
might still be a problem if the DB file is on a NFS filesystem, and this
should be documented.

5) Documentation: do we have any? What does DRAC means, BTW?

-- 
Emmanuel Dreyfus
manu@...

On Mon, 28 Nov 2005, Emmanuel Dreyfus wrote:
> 5) Documentation: do we have any? What does DRAC means, BTW?

http://mail.cc.umanitoba.ca/drac/

-Dan

Thanks for taking time to review my patch.  It sounds like your notes
about EXF_SPF could be the basis of "So you want to add <blah> support
to milter-greylist"...  However, I'm merely a user of milter-greylist,
not a hacker of it, so I've only done enough to make my own pain go
away.  If the consensus is that having DRAC support is useful to
others, then I'd appreciate having someone more conversant in
milter-greylist integrate it in a manner consistent with the rest of
the code.

--- In milter-greylist@yahoogroups.com, Emmanuel Dreyfus <manu@n...>
wrote:

> 4) As I understand, milter-greylist is only a consumer of the DB 
> database. Anotehr process will feed it. What happens if we read while
> the other process is writing?

It looks like sendmail protects itself by using Berkeley style locking
if available, fcntl locking otherwise.  That looks like another
autotools check to do it in milter-greylist.  Not sure whether
sendmail opens its DB files every time or just flips the lock on and
off as needed.  

Romain

On Mon, Nov 28, 2005 at 04:54:45PM -0000, Romain Kang wrote:
> Thanks for taking time to review my patch.  It sounds like your notes
> about EXF_SPF could be the basis of "So you want to add <blah> support
> to milter-greylist"...

No, that's not my point. It's just that milter-greylist attempts to 
add a header in each mail telling why it was greylisted. This is done
through these EXF_* flags. If you don't add one (and add the header
later by looking at the EXF_* flags), then the header will not be there.

> > 4) As I understand, milter-greylist is only a consumer of the DB 
> > database. Anotehr process will feed it. What happens if we read while
> > the other process is writing?
> 
> It looks like sendmail protects itself by using Berkeley style locking
> if available, fcntl locking otherwise.  That looks like another
> autotools check to do it in milter-greylist.  Not sure whether
> sendmail opens its DB files every time or just flips the lock on and
> off as needed.  

Bo basically we won't be able to open it if it's been written? Then we need
to loop until we can open it, don't we?

-- 
Emmanuel Dreyfus
manu@...

Hi,

I found that we didn't copy helo string for SPF2.  There is a patch.
It may better to always copy helo string regardless of HAVE_SPF*
definition to avoid such mistake.

Index: milter-greylist.c
diff -u -p milter-greylist.c.orig milter-greylist.c
--- milter-greylist.c.orig	Mon Nov 28 16:00:36 2005
+++ milter-greylist.c	Tue Nov 29 15:22:49 2005
@@ -166,7 +166,8 @@ mlfi_helo(ctx, helostr)
 
 	priv = (struct mlfi_priv *) smfi_getpriv(ctx);
 
-#if (defined(HAVE_SPF) || defined(HAVE_SPF_ALT)) 
+#if defined(HAVE_SPF) || defined(HAVE_SPF_ALT) || defined(HAVE_SPF2_10) || \
+    defined(HAVE_SPF2)
 	strncpy_rmsp(priv->priv_helo, helostr, ADDRLEN);
 	priv->priv_helo[ADDRLEN] = '\0';
 #endif


Sincerely,

--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
ume@...  ume@{,jp.}FreeBSD.org
http://www.imasy.org/~ume/

On Sun, Nov 27, 2005 at 10:51:30PM -0800, Romain Kang wrote:
> I made this patch so that I could send messages out of my PDA using
> "SMTP after POP/IMAP" auth without the greylist delay.  

Here is an improved patch. It includes your original contribution, the IPv6 
fix and three new config file directives: drac timeout, drac db and nodrac.

Enable DRAC at build time by configure --enable-drac

drac db "path" is used to specify the DRAC DB location
drac timeout 60s is used to tell a custom  timeout
nodrac disables DRAC if it was built in.

Can you please test the patch?

Any volunteer for updating the docs (greylist.conf.5 for the new options,
README to tell about DRAC)


-- 
Emmanuel Dreyfus
manu@...

On Tue, 29 Nov 2005, Emmanuel Dreyfus wrote:
> Here is an improved patch. It includes your original contribution, the IPv6
> fix and three new config file directives: drac timeout, drac db and nodrac.

no patch was sent.

-Dan

... Sorry, here it is -- Emmanuel Dreyfus manu@netbsd.org Index: acl.h =================================================================== RCS file:

Thanks, the improved patch is informative.

I had to leave this a while, but after looking at sendmail some more,
I determined that it's opening up the DB maps on demand.  I believe
that using dbopen() with O_SHLOCK is the way to go.  If the dbopen()
fails, I think it's OK to let the usual greylist logic take over.
In this mode, I think the "drac timeout" parameter can be removed.

I can't seem to get a clean patch out of the Yahoos Group message
archive, so here's the resulting check_drac().

Strictly speaking, the data retrieved by the get() operation is the
expiration time of the DRAC authorization, but rpc.dracd deletes
old entries on its own, so I punt on this point.

static int
check_drac(char *dotted_ip)
{
	DB *ddb;
	DBT key, data;
	char ipkey[32];
	int rc;

	ddb = dbopen(dracdb, (O_RDONLY | O_SHLOCK), 0666, DB_BTREE, NULL);
	if (ddb == NULL)
		return 0;

	key.data = strncpy(ipkey, dotted_ip, sizeof(ipkey));
	key.size = strlen(ipkey);
	rc = ddb->get(ddb, &key, &data, 0);
	ddb->close(ddb);

	switch (rc) {
	    case 0:
#ifdef TEST
		printf("key.data=%.*s (len=%d)  data.data=%.*s (len=%d)\n",
			key.size, key.data, key.size,
			data.size, data.data, data.size);
#endif /* TEST */
		return 1;
		/*NOTREACHED*/

	    case 1:
		return 0;
		/*NOTREACHED*/

	    case -1:
		fprintf(stderr, "errno=%d\n", errno);
		break;
	}

	return 0;
}

On Wed, Nov 30, 2005 at 01:20:27AM -0800, Romain Kang wrote:
> I had to leave this a while, but after looking at sendmail some more,
> I determined that it's opening up the DB maps on demand.  I believe
> that using dbopen() with O_SHLOCK is the way to go.

Do we have another software using DRAC? I'd like to be sure that 
it's the right way.

-- 
Emmanuel Dreyfus
manu@...

--- In milter-greylist@yahoogroups.com, Emmanuel Dreyfus <manu@n...>
wrote:
> Do we have another software using DRAC? I'd like to be sure that 
> it's the right way.

The only writer of the file is rpc.dracd.  AFAIK, sendmail is the only
other reader of dracd.db, though it seems likely that DB btree files
are used for similar purposes in other contexts.

Romain

On Wed, Nov 30, 2005 at 07:36:13PM -0000, Romain Kang wrote:
> The only writer of the file is rpc.dracd.  AFAIK, sendmail is the only
> other reader of dracd.db, though it seems likely that DB btree files
> are used for similar purposes in other contexts.

Could you grep sendmail source for DRAC usage? Just to be sure we are
to open the file forever with a shared lock...

-- 
Emmanuel Dreyfus
manu@...

On Wed, Nov 30, 2005 at 10:00:57PM +0000, Emmanuel Dreyfus wrote:
> Could you grep sendmail source for DRAC usage? Just to be sure we are
> to open the file forever with a shared lock...

The reference to drac is made through the .cf file, rather than the
sendmail program itself.  The .mc says something like this:
    LOCAL_CONFIG
    # dynamic relay authorization control map
    Kdrac btree -o /usr/local/etc/dracd
    ...
    SLocal_check_relay
    # allow recent POP/IMAP mail clients to relay
    R$*				$: $&{client_addr}
    R$+				$: $(drac $1 $: ? $)
    R?				$@ ?
    R$+				$@ $#OK

Running lsof on an idle sendmail daemon shows none of the map files
are held open (e.g., dracd.db, aliases.db).  They are only opened
when reference is made to a particular map.  The sendmail map.c
code acquires and releases the lock with the sequence
    lockfile(..., LOCK_SH);
    ...
    lockfile(..., LOCK_UN);
In turn, lockfile() uses BSD style flock() when available, or fcntl()
style locks otherwise.

The check_drac() function that I posted last night no longer holds
the map file open.  Instead, the sequence goes like this:
    - open the DB file with the O_SHLOCK flag set
    - attempt to get() the item as an existence check
    - close the file (implicitly clearing the shared lock)
So if there are systems which don't support O_SHLOCK or only have
fcntl() locks, there could be a possible hole, but this risk seems
negligible.  Moreover, since the DB is merely used as an existence
check, the content is not that significant.

Romain

... Right, I overlooked that. Here is an updated patch with the documentation. If it s okay, I ll release the newer version with it. -- Emmanuel Dreyfus 

The new patch seems to be fine on my server.  I made an adjustment
to conf.h and added some more description to the README.

Thanks,
Romain

--- conf.h-	Wed Nov 30 16:03:12 2005
+++ conf.h	Wed Nov 30 16:17:55 2005
@@ -54,5 +54,5 @@
 
 #ifndef DRACDB
-#define DRACDB "/usr/local/etc/dracdb.db"
+#define DRACDB "/usr/local/etc/dracd.db"
 #endif
 
--- README-	Wed Nov 30 16:03:12 2005
+++ README	Wed Nov 30 16:27:06 2005
@@ -274,4 +274,15 @@
  ============
 
+
+Briefly, Dynamic Relay Authorization Control provides "POP/IMAP
+before SMTP" authorization.  If an IP address has recently been a
+client of some POP or IMAP server affiliated with the SMTP server,
+the address is considered to be a legitimate user when submitting
+email via SMTP.  Enabling DRAC support effectively whitelists the
+address for some period following the most recent read operation,
+typically for 30 minutes until the DRAC daemon revokes its
+authorization.  More information is available from the DRAC website,
+<http://mail.cc.umanitoba.ca/drac/>.
+
 milter-greylist can be built with DRAC support, by giving the
 --enable-drac flag to configure. Location of the DRAC DB file can