Yahoo Groups archive

Milter-greylist

Index last updated: 2026-04-28 23:32 UTC

Message

Re: [milter-greylist] [PATCH] New feature and threading bug fix

2006-08-22 by AIDA Shinra

> > 1. may_be_forged ACL condition
> > http://www.j10n.org/files/milter-greylist-3.0a2-step1.patch
> > 
> > When a client has bogus reverse DNS, that is, IP -> PTR -> A != IP,
> > sendmail sets {client_resolve} macro to FORGED. This patch implements
> > ACL condition to take advantage of it. You need to add
> > {client_resolve} into Milter.macros.connect. Example:
> > acl blacklist domain /.*\.info/ may_be_forged
> > acl greylist may_be_forged
> 
> Some thoughts:
> - may_be_forged name is a bit long to read in ACL. What about just
> forged?

First, most of their reverse DNS are not malicious forgeries but their
ISP's misconfigurations. I feel "forged" misleading. Second, sendmail
records them as "(may be forged)" to its log. It is a good hint for
users.

> - Changes to documentation for the new feature would be nice :-)

http://www.j10n.org/files/milter-greylist-3.0a2-doc.patch

> - Do you catch a lot of spam with that?

I get 5% to 10% of spam from such zombie hosts. Approximately an half
of them are not registered to SORBS DNSBL, and part of them have
static IP addresses. Conclusion: may_be_forged test improves zombie
probe in some percent.

False positives will be not significant as long as it is applied to
greylisting rather than blacklisting, because legitimate MTA runners
will probably find and report their ISP's DNS misconfiguration. My
test can also become positive due to a temporary lookup failure, but
temporary false positives do not harm in greylisting.

> > 2. Threading bug fix
> > http://www.j10n.org/files/milter-greylist-3.0a2-step2.patch
> > 
> > This patch resolvs number of race conditions in threading
> > code. It also includes some minor modifications. Since my changes
> > affect almost everywhere, code reviews are welcome.
> 
> There are huge changes. Could you explain what was wrong?

The conf and dump_dirty are clear examples. They are not protected by
any lock. Another example is sync_sender(). If something is added to
sync queue exactly before pthread_cond_wait(), it will not be sent to
peers until another item is added.

But now I understand nothing is wrong in the autowhite.c. *I* was
wrong. The pending.c is also correct except that:
 - pending_del_addr() needs writelocking.
 - the pending list should be really sorted or pending_timeout()
   should tranverse to the end.

> > 3. Binary search tree
> > http://www.j10n.org/files/milter-greylist-3.0a2-step3.patch
> > 
> > This patch introduces red-black tree structure to manage the greylist
> > and autowhitelist. It is just a pedantic change. No realistic
> > improvement is expected.
> 
> I'm a bit reluctant to make changes that buy nothing. What's the
> advantage of adding this?

Ignore it if you don't like.

Attachments

Move to quarantaine

This moves the raw source file on disk only. The archive index is not changed automatically, so you still need to run a manual refresh afterward.