Milter-greylist

Hi,

I'm using milter-greylist 1.6 with sendmail 8.13.2 under Solaris 9
for about 3 weeks now. I'm following log files closely, and for
thousands of messages it works without problems. I've noticed
a strange behaviour with some messages, though (three, until now).

When looking at the logfile entries one can see that a message was
correctly greylisted for 1 minute (as configured). The sending MTA
re-sent the message exactly after 1 minute, and milter-greylist
accepted the message and it was delivered. The strange thing is
that the message triplet was not removed from the greylist and
replaced by an auto-whitelist entry as it should be (and as it
happens correctly with all other messages). In all three cases
where this happened the mail was re-sent exactly after 1 minute.

Ideas, anyone ? 

Here are the logfile entries (sorry for the long lines):

First attempt:

Apr  7 09:21:53 milter-greylist: j377LruE001941: addr 131.130.32.158 from <rudolf.huerner@...> to <brezany@...> delayed for 00:01:00
Apr  7 09:21:53 sm-mta[1941]: j377LruE001941: Milter: to=<brezany@...>, reject=451 4.7.1 Greylisting in action, please come back in 00:01:00
Apr  7 09:21:53 sm-mta[1941]: j377LruE001941: from=<rudolf.huerner@...>, size=0, class=0, nrcpts=0, proto=SMTP, daemon=MTA-v4, relay=einstein.ani.univie.ac.at [131.130.32.158]

Second successfull attempt:

Apr  7 09:22:53 sm-mta[1970]: j377Mr4d001970: from=<rudolf.huerner@...>, size=3193, class=0, nrcpts=1, msgid=<4254DF85.3080701@...>, proto=SMTP, daemon=MTA-v4, relay=einstein.ani.univie.ac.at [131.130.32.158]
Apr  7 09:22:53 sm-mta[1970]: j377Mr4d001970: Milter add: header: X-Greylist: Delayed for 00:01:00 by milter-greylist-1.6 (kim.par.univie.ac.at [131.130.186.100]); Thu, 07 Apr 2005 09:22:53 +0200 (MEST)
Apr  7 09:22:57 sm-mta[1971]: j377Mr4d001970: to=<brezany@...>, delay=00:00:04, xdelay=00:00:04, mailer=local, pri=33556, dsn=2.0.0, stat=Sent

Contents from greylist.db after successfull delivery (wrong, should be AUTO):

# grep 131.130.32.158 /var/milter-greylist/greylist.db 
131.130.32.158  <rudolf.huerner@...>   <brezany@...>     1112858573 # 2005-04-07 09:22:53

mp.
-- 
                         Martin Paul | Systems Administrator
   Institute of Scientific Computing | martin@...
 Nordbergstrasse 15/C/3, A-1090 Wien | Tel: 01 4277 39403
        http://www.par.univie.ac.at/ | Fax: 01 4277 9394

On Fri, Apr 08, 2005 at 02:16:38PM +0200, Martin Paul wrote:
> When looking at the logfile entries one can see that a message was
> correctly greylisted for 1 minute (as configured). The sending MTA
> re-sent the message exactly after 1 minute, and milter-greylist
> accepted the message and it was delivered. The strange thing is
> that the message triplet was not removed from the greylist and
> replaced by an auto-whitelist entry as it should be

That means that in pending.c:pending_check(), we don't match the
if (rest < 0) condition.

rest is time_t. that is signed on my system, is it on yours?
Try running this:

int main(void) {
	size_t x = -1;;

	printf("x = %d\n", x);
	return 0;
}

-- 
Emmanuel Dreyfus
manu@...

Emmanuel Dreyfus wrote:

>That means that in pending.c:pending_check(), we don't match the
>if (rest < 0) condition.
>
>rest is time_t. that is signed on my system, is it on yours?
>Try running this:
>
>int main(void) {
>	size_t x = -1;;
>
>	printf("x = %d\n", x);
>	return 0;
>}
>
>  
>
Emmanuel.. Are you sure that's going to tell you anything useful?

The difference between signed and unsigned is in how the values are
interpreted. You can take -1, cast it as unsigned, and back to signed
and still wind up with -1.

Since %d tells printf to interpret the value as signed int, it should
print -1, even if size_t is unsigned.

$ cat test.c
int main (void)
{
    unsigned x = -1;
    printf ("x = %d",x);
    return 0;
}
$ gcc test.c
$ ./a.out
x = -1

On Fri, Apr 08, 2005 at 11:46:27AM -0400, Matt Kettler wrote:
> Emmanuel.. Are you sure that's going to tell you anything useful?

Yes, you are right, I spoke too fast: that's a wrong test.

That will work better:

int main(void) {
	time_t x = -1;

	if (x < 0)
		printf("time_t is signed\n");
	else
		printf("time_t is unsigned\n");
	return 0;
}
		

-- 
Emmanuel Dreyfus
manu@...

Emmanuel Dreyfus wrote:

>That means that in pending.c:pending_check(), we don't match the
>if (rest < 0) condition.
>
>rest is time_t. that is signed on my system, is it on yours?

Yes, it's signed on my system, so that shouldn't be a problem.
I guess that would result in more severe problems anyway (no
messages accepted at all, as rest will never be < 0).

I've taken a closer look on pending_check(), and maybe I've found
something. After rest is computed as "accepted - now", it's checked
to be < 0. If that's true, it puts the message in the whitelist, and
sets rest=0. Later (goto out), a check for rest==0 decides whether
1 or 0 is returned, which again determines if the message is accepted
or not.

Now if "accepted - now" is exactly 0, it will fail the first check for
<0, so the message will not be whitelisted. Later it's checked for
==0, and the message is accepted. 

Could that be the problem ? And if so, would a viable fix be to
change "if (rest < 0) {" to "if (rest <= 0) {" in pending_check()
(line 305) ?

mp.
-- 
                         Martin Paul | Systems Administrator
   Institute of Scientific Computing | martin@...
 Nordbergstrasse 15/C/3, A-1090 Wien | Tel: 01 4277 39403
        http://www.par.univie.ac.at/ | Fax: 01 4277 9394

On Mon, Apr 11, 2005 at 09:50:09AM +0200, Martin Paul wrote:
> Could that be the problem ? And if so, would a viable fix be to
> change "if (rest < 0) {" to "if (rest <= 0) {" in pending_check()
> (line 305) ?

Give it a try and let us know...

-- 
Emmanuel Dreyfus
manu@...

--- In milter-greylist@yahoogroups.com, Emmanuel Dreyfus <manu@n...>
wrote:
> On Mon, Apr 11, 2005 at 09:50:09AM +0200, Martin Paul wrote:
> > Could that be the problem ? And if so, would a viable fix be to
> > change "if (rest < 0) {" to "if (rest <= 0) {" in pending_check()
> > (line 305) ?
> 
> Give it a try and let us know...

Hardly possible, as I would need a test message that's re-sent
after exactly one minute to have rest == 0.

You know the code better than me, does my description of the
bug sound reasonable ?

mp.

mpaul888 <martin@...> wrote:

> Hardly possible, as I would need a test message that's re-sent
> after exactly one minute to have rest == 0.

You had the issue in the past, so there are some chances you have it
again in the future.
 
> You know the code better than me, does my description of the
> bug sound reasonable ?

Sure it does. That's why it would be good to test that the change has no
side effet and if it's alright, we'll have it in the next beta.

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

> From: manu@...
>
> Sure it does. That's why it would be good to test that the change has no
> side effet and if it's alright, we'll have it in the next beta.

Ok, I have recompiled milter-greylist 1.6 with the following change
to pending.c now:

305c305,311
<                       if (rest < 0) {
---
>                       if (rest <= 0) {
>                               if (rest == 0) {
>                                       syslog(LOG_DEBUG, 
>                                           "rest is 0: %s from %s to %s",
>                                           pending->p_addr, pending->p_from,
>                                           pending->p_rcpt);
>                               }

The extra syslog statement has been added to identify problematic
messages with rest==0, and to see whether they are handled correctly
now.

I will run this version from now on, and report any side effects,
and whether it fixes the problem I saw. This can take quite some
time, as I only had 3 of those messages in about 3 weeks.

mp.

Martin Paul <martin@...> wrote:

> I will run this version from now on, and report any side effects,
> and whether it fixes the problem I saw. This can take quite some
> time, as I only had 3 of those messages in about 3 weeks.

No problem, we'll wait one week.

Thank you for taking care of that issue.

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@...

> From: manu@...
>> Martin Paul <martin@...> wrote:
>> 
>> I will run this version from now on, and report any side effects,
>> and whether it fixes the problem I saw. This can take quite some
>> time, as I only had 3 of those messages in about 3 weeks.
> 
> No problem, we'll wait one week.

I have been running the modified version of milter-greylist for one
week now. No ill side-effects showed up during that time, and I had
one message (where rest==0) which was now handled correctly. So I 
recommend adding the following change to pending.c (the files are
equal in version 1.6 and 2.0b5, so the fix can be applied to both):

  $ diff pending.c.orig pending.c
  305c305
  <                       if (rest < 0) {
  ---
  >                       if (rest <= 0) {

mp.
-- 
                         Martin Paul | Systems Administrator
   Institute of Scientific Computing | martin@...
 Nordbergstrasse 15/C/3, A-1090 Wien | Tel: 01 4277 39403
        http://www.par.univie.ac.at/ | Fax: 01 4277 9394

Martin Paul <martin@...> wrote:

>   $ diff pending.c.orig pending.c
>   305c305
>   <                       if (rest < 0) {
>   ---
>   >                       if (rest <= 0) {

I committed it and it will be available in 2.0 beta6. Should I roll it
now, or should I wait for man pages working on Solaris?

-- 
Emmanuel Dreyfus
Le cahier de l'admin BSD 2eme ed. est dans toutes les bonnes librairies
http://www.eyrolles.com/Informatique/Livre/9782212114638/livre-bsd.php
manu@...

On Tue, Apr 19, 2005 at 07:12:32PM +0200, manu@... wrote:
> I committed it and it will be available in 2.0 beta6. Should I roll it
> now, or should I wait for man pages working on Solaris?

As far as I'm concerned, it can wait. Man pages working under
Solaris definitely would be a good thing, too.

mp.