Milter-greylist

On Mon, Oct 11, 2004 at 03:21:03PM +0200, Klas Heggemann wrote:
> we received approx 47000 letter a day. Now this is down to 18000 a 
> day...

After you starte greylisting, you usually still get a lot of spam
sent from regular mail servers. Removal requests and/or blacklisting
cures it.

> The major thing is that when a autowhitelisted entry is renewed, this
> information does notseem to  propagate to the other mx-host which will 
> loose this entry when it times out.

True. I never thought about it, but we can call it a bug.

> A config file that can be  shared between the mx-hosts, i e you
> would be permitted to have a 'peer'-line that includes
> the machine on which the milter is running. This would be detected
> at startup and no sync information should be sent that way.

Yes, that makes sense. I use a workaround for this problem:
On the machines I run milter-greylist on, I build the config file
from a common part and a local part. The local part contains the
peer statements. 

> The database should be dumped when the milter is stopped/killed.

I recall implementing that.

-- 
Emmanuel Dreyfus
manu@...

Emmanuel Dreyfus wrote:


> On Mon, Oct 11, 2004 at 03:21:03PM +0200, Klas Heggemann wrote:
>> we received approx 47000 letter a day. Now this is down to 18000 a
>> day...
>
> After you starte greylisting, you usually still get a lot of spam
> sent from regular mail servers. Removal requests and/or blacklisting
> cures it.

I think a lot of this is legitimate letters (to our approx 15000 users 
...)
I myself receive very little spam after the greylisting was implemented.
People here wish there was a Noble Prize for spam fighting ....

>
>> The major thing is that when a autowhitelisted entry is renewed, this
>> information does notseem to  propagate to the other mx-host which will
>> loose this entry when it times out.
>
> True. I never thought about it, but we can call it a bug.

Tnx.


>
>> A config file that can be  shared between the mx-hosts, i e you
>> would be permitted to have a 'peer'-line that includes
>> the machine on which the milter is running. This would be detected
>> at startup and no sync information should be sent that way.
>
> Yes, that makes sense. I use a workaround for this problem:
> On the machines I run milter-greylist on, I build the config file
> from a common part and a local part. The local part contains the
> peer statements.


Yeah we do approx the same here (just commenting the relevent lines).


>>
>> The database should be dumped when the milter is stopped/killed.
>
> I recall implementing that.

Yes, I can se this now in the code. It does not seem to work here (on 
Solaris 9)


>
> -- 
> Emmanuel Dreyfus
> manu@...
>


Really nice work.  Both the users  here + administrators are realy 
happy...


/klas

Klas Heggemann <klas@...> wrote:

> Yes, I can se this now in the code. It does not seem to work here (on
> Solaris 9)

It's done through atexit(). Is there some known oddities with atexit()
on Solaris?

-- 
Emmanuel Dreyfus
Il y a 10 sortes de personnes dans le monde: ceux qui comprennent 
le binaire et ceux qui ne le comprennent pas.
manu@...

On Mon, Oct 11, 2004 at 10:34:45PM +0200, Emmanuel Dreyfus wrote:
> It's done through atexit(). Is there some known oddities with atexit()
> on Solaris?

No. But function registered that way will of cause only be called if
the program return from main() or exists via exit(3C). Being killed
by signal won't do that.

"rc-solaris.sh.in" in the 1.4 distribution uses "pkill" which sends SIGTERM.
And I cannot see where "milter-greylist" would try to catch that signal.

	Kind regards

-- 
Matthias Scheler                                  http://scheler.de/~matthias/

On Mon, Oct 11, 2004 at 02:34:36PM +0000, Emmanuel Dreyfus wrote:
> > A config file that can be  shared between the mx-hosts, i e you
> > would be permitted to have a 'peer'-line that includes
> > the machine on which the milter is running. This would be detected
> > at startup and no sync information should be sent that way.
> Yes, that makes sense.

An easy and portable way to implement that feature would be trying to bind
a UDP socket to the address of every peer (requesting an arbitrary port).
If that doesn't fail it is a local address and can be ignored.

	Kind regards

-- 
Matthias Scheler                                  http://scheler.de/~matthias/

Matthias Scheler <tron@...> wrote:

> No. But function registered that way will of cause only be called if
> the program return from main() or exists via exit(3C). Being killed
> by signal won't do that.

That's bad since we will never call exit()
 
> "rc-solaris.sh.in" in the 1.4 distribution uses "pkill" which sends SIGTERM.
> And I cannot see where "milter-greylist" would try to catch that signal.

It cannot. libmilter takes care of the signals, there is no way of
catching them from a milter.
 
-- 
Emmanuel Dreyfus
Il y a 10 sortes de personnes dans le monde: ceux qui comprennent 
le binaire et ceux qui ne le comprennent pas.
manu@...

Matthias Scheler <tron@...> wrote:

> An easy and portable way to implement that feature would be trying to bind
> a UDP socket to the address of every peer (requesting an arbitrary port).
> If that doesn't fail it is a local address and can be ignored.

But that kills backward compatibility.
We could also check that the address is non local. Is there a way of
doing that?

-- 
Emmanuel Dreyfus
Il y a 10 sortes de personnes dans le monde: ceux qui comprennent 
le binaire et ceux qui ne le comprennent pas.
manu@...

On Tue, Oct 12, 2004 at 09:53:26AM +0200, Emmanuel Dreyfus wrote:
> But that kills backward compatibility.

Why? I don't say you should use the UDP socket except for checking
if you can bind to the IP address of a specific peer which means
it is a local address.

> We could also check that the address is non local.

That's exactly what I'm suggesting, isn't it?

The implementation could look like this:

int
IsLocalAddress(const struct sockaddr *sa)
{
	struct sockaddr_in	sin;
	struct sockaddr_in6	sin6;
	socklen_t		length;
	int			sfd, islocal;

	switch (sa->sa_family) {
	case AF_INET:
		length = sizeof (sin);
		(void) memcpy(&sin, sa, sizeof (sin));
		sin.sin_port = 0;
		sa = (struct sockaddr *)&sin;
		break;
	case AF_INET6:
		length = sizeof (sin6);
		(void) memcpy(&sin, sa, sizeof (sin));
		sin6.sin6_port = 0;
		sin6.sin6_flowinfo = 0;
		sin6.sin6_scope_id = 0;
		sa = (struct sockaddr *)&sin6;
		break;
	default:
		/* error handling */
		[...]
	}

	if ((sfd = socket(sa->sa_family, SOCK_DGRAM, IPPROTO_UDP)) < 0) {
		/* error handling */
		[...]
	}

	if (bind(sfd, sa, length) < 0) {
		if (errno != EADDRNOTAVAIL) {
			/* error handling */
			[...]
		}
		islocal = 0;
	} else {
		islocal = 1;
	}

	(void) close(sfd);

	return (islocal);
}

	Kind regards

-- 
Matthias Scheler                                  http://scheler.de/~matthias/

On Tue, Oct 12, 2004 at 09:53:20AM +0200, Emmanuel Dreyfus wrote:
> > No. But function registered that way will of cause only be called if
> > the program return from main() or exists via exit(3C). Being killed
> > by signal won't do that.
> That's bad since we will never call exit()

Registering functions via atexit() is pointless in that case.

> It cannot. libmilter takes care of the signals, there is no way of
> catching them from a milter.

How does libmilter handle SIGTERM? I would be surprised if it doesn't
call exit(3C).

	Kind regards

-- 
Matthias Scheler                                  http://scheler.de/~matthias/

On Tue, Oct 12, 2004 at 12:47:53PM +0200, Matthias Scheler wrote:
> > But that kills backward compatibility.
> Why? I don't say you should use the UDP socket except for checking
> if you can bind to the IP address of a specific peer which means
> it is a local address.

Ok, I thought you meant to replace the sync mecanism by something based
on UDP. I'll implement what you suggest.

-- 
Emmanuel Dreyfus
manu@...

On Mon, Oct 11, 2004 at 03:21:03PM +0200, Klas Heggemann wrote:
> So far I've identified a few things that can be considered as small 
> glitches.
(snip)

> A config file that can be  shared between the mx-hosts, i e you
> would be permitted to have a 'peer'-line that includes
> the machine on which the milter is running. This would be detected
> at startup and no sync information should be sent that way.

Could you test the following patch?
ftp://ftp.espci.fr/shadow/manu/local.patch

It should mark a local peer as such and avoind sending sync information 
to it. I wrote it quickly from Mathias suggestion, but I have not tested it 
at all.

If it works, I'll commit it.

-- 
Emmanuel Dreyfus
manu@...

Emmanuel Dreyfus:

> On Mon, Oct 11, 2004 at 03:21:03PM +0200, Klas Heggemann wrote:
>> So far I've identified a few things that can be considered as small
>> glitches.
> (snip)
>
>> A config file that can be  shared between the mx-hosts, i e you
>> would be permitted to have a 'peer'-line that includes
>> the machine on which the milter is running. This would be detected
>> at startup and no sync information should be sent that way.
>
> Could you test the following patch?
> ftp://ftp.espci.fr/shadow/manu/local.patch

To which release is this? We run 1.5.3 and it does not seem to
apply to this version.


>
> It should mark a local peer as such and avoind sending sync information
> to it. I wrote it quickly from Mathias suggestion, but I have not 
> tested it
> at all.
>
> If it works, I'll commit it.
>

In order to get the database saved when the process gets killed, I've
added a few lines to catch SIGTERM (which the start/stop-script sends)
and call exit (which dumps the database via onexit()). You can take
a look at it here:

	ftp://ftp.nada.kth.se/Sysgr/milter.signal.patch

Only tested on Solaris 9....


>
>
> -- 
> Emmanuel Dreyfus
> manu@...
>


/klas

On Tue, Oct 12, 2004 at 03:30:43PM +0200, Klas Heggemann wrote:
> > Could you test the following patch?
> > ftp://ftp.espci.fr/shadow/manu/local.patch
> To which release is this? We run 1.5.3 and it does not seem to
> apply to this version.

Latest: 1.5.8

> In order to get the database saved when the process gets killed, I've
> added a few lines to catch SIGTERM (which the start/stop-script sends)
> and call exit (which dumps the database via onexit()). You can take
> a look at it here:
> 
> 	ftp://ftp.nada.kth.se/Sysgr/milter.signal.patch

I'm not sure we can do that: libmilter documentation explicitely says
it takes care of all signals. Catching the signal may break the library API
in the future.

-- 
Emmanuel Dreyfus
manu@...

Hi,

>>>>> On Tue, 12 Oct 2004 12:56:22 +0000
>>>>> Emmanuel Dreyfus <manu@...> said:

manu> Could you test the following patch?
manu> ftp://ftp.espci.fr/shadow/manu/local.patch

+	case AF_INET6:
+		len = sizeof(sin6);
+		memcpy(&sin6, sa, len);
+		sin6.sin6_port = 0;
+		sin6.sin6_flowinfo = 0;
+		sin6.sin6_scope_id = 0;
+		sa = (struct sockaddr *)&sin6;
+		break;	

Why do you set sin6_flowinfo and sin6_scope_id to zero?  It breaks
scoped address case.  These should not be touched.

Sincerely,

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

Hajimu UMEMOTO <ume@...> wrote:

> Why do you set sin6_flowinfo and sin6_scope_id to zero?  It breaks
> scoped address case.  These should not be touched.

I just copied Mathias' sample code. I'll remove these allocations.

-- 
Emmanuel Dreyfus
Il y a 10 sortes de personnes dans le monde: ceux qui comprennent 
le binaire et ceux qui ne le comprennent pas.
manu@...

On Wed, Oct 13, 2004 at 03:47:30AM +0900, Hajimu UMEMOTO wrote:
> manu> Could you test the following patch?
> manu> ftp://ftp.espci.fr/shadow/manu/local.patch
> 
> +	case AF_INET6:
> +		len = sizeof(sin6);
> +		memcpy(&sin6, sa, len);
> +		sin6.sin6_port = 0;
> +		sin6.sin6_flowinfo = 0;
> +		sin6.sin6_scope_id = 0;
> +		sa = (struct sockaddr *)&sin6;
> +		break;	
> 
> Why do you set sin6_flowinfo and sin6_scope_id to zero?

My fault. I remebered incorrectly that you have to clear these fields
for arguments to socket related system calls. But they are of cause
allowed to be non-zero for scoped addresses.

> These should not be touched.

Correct.

	Kind regards

-- 
Matthias Scheler                                  http://scheler.de/~matthias/