Milter-greylist

Hi

Here is our beta7 of milter-greylist 2.0
http://ftp.espci.fr/pub/milter-greylist/milter-greylist-2.0b7.tgz
MD5 (milter-greylist-2.0b7.tgz) = 1ca9d0871e18558621f0bee4436b5888

I think we are close to 2.0. If you contributed something, please 
check that you are in the contributor section of the man page and bug
me if you are missing.
-- 
Emmanuel Dreyfus
manu@...

Hi,

>>>>> On Wed, 11 May 2005 14:35:59 +0000
>>>>> Emmanuel Dreyfus <manu@...> said:

manu> Here is our beta7 of milter-greylist 2.0
manu> http://ftp.espci.fr/pub/milter-greylist/milter-greylist-2.0b7.tgz
manu> MD5 (milter-greylist-2.0b7.tgz) = 1ca9d0871e18558621f0bee4436b5888

manu> I think we are close to 2.0. If you contributed something, please 
manu> check that you are in the contributor section of the man page and bug
manu> me if you are missing.

Sorry but I found possible memory leak in my code.  Fortunately, it
should not happen.  However, it should be fixed.  Sorry for the mess.

Index: pending.c
diff -u -p pending.c.orig pending.c
--- pending.c.orig	Wed Apr 20 01:57:47 2005
+++ pending.c	Wed May 11 13:00:11 2005
@@ -531,8 +531,10 @@ ipfromstring(str, sa, salen, family)
 	hints.ai_socktype = SOCK_STREAM;
 	if (getaddrinfo(str, "0", &hints, &res) != 0)
 		return 0;
-	if (*salen < res->ai_addrlen)
+	if (*salen < res->ai_addrlen) {
+		freeaddrinfo(res);
 		return -1;
+	}
 	memcpy(sa, res->ai_addr, res->ai_addrlen);
 	*salen = res->ai_addrlen;
 	freeaddrinfo(res);


Sincerely,

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

On Wed, 11 May 2005, Emmanuel Dreyfus wrote:
> Here is our beta7 of milter-greylist 2.0
> http://ftp.espci.fr/pub/milter-greylist/milter-greylist-2.0b7.tgz
> MD5 (milter-greylist-2.0b7.tgz) = 1ca9d0871e18558621f0bee4436b5888
> I think we are close to 2.0. If you contributed something, please 
> check that you are in the contributor section of the man page and bug
> me if you are missing.

milter-greylist.spec needs to be fixed:
Requires: sendmail >= 8.12
Requires: sendmail-cf >= 8.12
BuildRequires: sendmail-devel >= 8.12

Not true. Sendmail 8.11 will do just fine.

-Dan

On Wed, May 11, 2005 at 12:54:58PM -0700, Dan Hollis wrote:
> On Wed, 11 May 2005, Emmanuel Dreyfus wrote:
> > Here is our beta7 of milter-greylist 2.0
> > http://ftp.espci.fr/pub/milter-greylist/milter-greylist-2.0b7.tgz
> > MD5 (milter-greylist-2.0b7.tgz) = 1ca9d0871e18558621f0bee4436b5888
> > I think we are close to 2.0. If you contributed something, please 
> > check that you are in the contributor section of the man page and bug
> > me if you are missing.
> 
> milter-greylist.spec needs to be fixed:
> Requires: sendmail >= 8.12
> Requires: sendmail-cf >= 8.12
> BuildRequires: sendmail-devel >= 8.12
> 
> Not true. Sendmail 8.11 will do just fine.
> 
> -Dan

Dan, 

sendmail 8.11 is way old and offer limited Milter support. 
The 2 last sendmail series are 8.12.x and 8.13.x . 
Users should pick only versions in these 2 series  : more exactly 
8.12.10 or 8.12.11, or 8.13.x if they want stability.

IMHO If not, people should upgrade ....

SL/

On Thu, 12 May 2005, Stephane Lentz wrote:
> On Wed, May 11, 2005 at 12:54:58PM -0700, Dan Hollis wrote:
> > milter-greylist.spec needs to be fixed:
> > Requires: sendmail >= 8.12
> > Requires: sendmail-cf >= 8.12
> > BuildRequires: sendmail-devel >= 8.12
> > Not true. Sendmail 8.11 will do just fine.
> sendmail 8.11 is way old and offer limited Milter support. 
> The 2 last sendmail series are 8.12.x and 8.13.x . 
> Users should pick only versions in these 2 series  : more exactly 
> 8.12.10 or 8.12.11, or 8.13.x if they want stability.
> IMHO If not, people should upgrade ....

This is not a sufficient reason for the RPM specfile to claim it 
_requires_ >= 8.12, since 8.11 works very fine. We use 8.11 and 
milter-greylist on several servers with many thousands of users each 
and it performs just perfectly.

And the 8.11 milter support is perfectly fine and adequate for full 
complete functioning of milter-greylist.

So, this is no good reason to make the RPM specfile _fail_ on 8.11 when 
milter-greylist itself builds and runs just fine against 8.11 with 0 
errors.

-Dan

On Wed, May 11, 2005 at 07:59:24PM -0700, Dan Hollis wrote:
> This is not a sufficient reason for the RPM specfile to claim it 
> _requires_ >= 8.12, since 8.11 works very fine. We use 8.11 and 
> milter-greylist on several servers with many thousands of users each 
> and it performs just perfectly.

I agree with Dan: it's not the job of the package maintainer to dictate
when administrators have to upgrade. If it's possible to issue a warning, 
go for it, but the RPM should not prevent someone who knows what he is
doing to use an older sendmail.

-- 
Emmanuel Dreyfus
manu@...

On Wed, May 11, 2005 at 02:35:59PM +0000, Emmanuel Dreyfus wrote:
> 
> Here is our beta7 of milter-greylist 2.0
> http://ftp.espci.fr/pub/milter-greylist/milter-greylist-2.0b7.tgz

Would it make sense to include conf_yacc.c, conf_lex.c, dump_yacc.c,
and dump_lex.c in the milter-greylist distribution ? They don't 
need to be generated every time on the system where it is compiled,
and you could remove the dependency on flex/yacc/bison from the
README.

At least under Solaris 9 flex is not included in the core operating
system. While Hans Mayer might say that is included on the companion
CD - the packages there are *optional*, and not installed on all
Solaris machines.

I've seen comments like "flex/bison are only required to compile
if you want to modify the *.l/*.y files" in the READMEs and 
installation instructions for other Open Source packages, so it's not
uncommon to include the *.c files right away.

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

--- In milter-greylist@yahoogroups.com, Martin Paul <martin@p...>
wrote:
> On Wed, May 11, 2005 at 02:35:59PM +0000, Emmanuel Dreyfus wrote:
> > 
> > Here is our beta7 of milter-greylist 2.0
> > http://ftp.espci.fr/pub/milter-greylist/milter-greylist-2.0b7.tgz
> 
> Would it make sense to include conf_yacc.c, conf_lex.c, dump_yacc.c,
> and dump_lex.c in the milter-greylist distribution ? They don't 
> need to be generated every time on the system where it is compiled,
> and you could remove the dependency on flex/yacc/bison from the
> README.

I haven't checked for sure, but usually flex and lex like to link
against libfl.a or libl.a. So you'd need the *fitting* version of that
anyway.

Shipping files *might* work for bison though.

> At least under Solaris 9 flex is not included in the core operating
> system. While Hans Mayer might say that is included on the companion
> CD - the packages there are *optional*, and not installed on all
> Solaris machines.

Well, if you build stuff from sources, you should really be prepared
to install some prerequisits. And the prereqs of milter-greylist are
really harmless!


    Elrond

On Thu, May 12, 2005 at 11:45:54AM -0000, Elrond wrote:
> I haven't checked for sure, but usually flex and lex like to link
> against libfl.a or libl.a. So you'd need the *fitting* version of that
> anyway.

I think this only applies if you want to create an executable from
just lex source, which isn't the case with milter-greylist. Checking
the Makefile and output of make, it doesn't pull in libl or libfl
anywhere.

> Well, if you build stuff from sources, you should really be prepared
> to install some prerequisits. And the prereqs of milter-greylist are
> really harmless!

I agree on both, and I like milter-greylist especially for the short
list of prerequisites. Shouldn't stop one from making the short
list even shorter, if the only downside is a plus 50kb on download
size.

mp.

--- In milter-greylist@yahoogroups.com, Martin Paul <martin@p...>
wrote:
[...]
> I think this only applies if you want to create an executable from
> just lex source, which isn't the case with milter-greylist. Checking
> the Makefile and output of make, it doesn't pull in libl or libfl
> anywhere.

Hmmm, okay.

So just shipping them should stop make from trying to rebuild them (as
long, as they're newer as the depedencies). Should work.


[...]
> I agree on both, and I like milter-greylist especially for the short
> list of prerequisites. Shouldn't stop one from making the short
> list even shorter, if the only downside is a plus 50kb on download
> size.

Well, if we wanted to save a bit on shipped size, the CVS dir
shouldn't be there ;) (debian's packagaging "lint" tool doesn't like
it either.)

And most autoconf'd packages don't ship Makefile.

Just a few random thoughts.


    Elrond

Elrond <yahoo.com@...> wrote:

> Well, if we wanted to save a bit on shipped size, the CVS dir
> shouldn't be there ;) (debian's packagaging "lint" tool doesn't like
> it either.)

Yep, I'll think about doing cvs ex instead of cvs co next time.
 
> And most autoconf'd packages don't ship Makefile.

The makefiles are there so that you can tweak them to get the thing
building by yourself if configure fails.

-- 
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@...

Hello,

I would change the distribution root directory permission from 744 to 755.

Bests,
Attila

I upgraded to rc1 this morning and I'm seeing a small bug in the logging,
the trailing > on from addresses is being lost during the processing of a
message.  I've narrowed it down to this call in milter-greylist.c

     /*
      * Check the ACL
      */
     if ((priv->priv_whitelist = acl_filter(SA(&priv->priv_addr),
         priv->priv_addrlen, priv->priv_hostname, priv->priv_from,
         rcpt, priv->priv_queueid)) & EXF_WHITELIST) {
         priv->priv_elapsed = 0;
         return SMFIS_CONTINUE;
     }

   The priv->priv_from value is fine before the acl_filter() call and
is truncated by one character after returning from that call.  I've
looked through that function and don't see anything obvious.  Is anybody
else seeing this?

   Example from debugging added around the call:

May 19 13:40:36 mxa milter-greylist: j4JKeZRA032115: addr = 69.6.174.155, 
from = <>, rcpt = <nea58npz@...>
May 19 13:40:36 mxa milter-greylist: j4JKeZRA032115: addr = 69.6.174.155, 
from = <, rcpt = <nea58npz@...>

>   I upgraded to rc1 this morning and I'm seeing a small bug in the logging,
>the trailing > on from addresses is being lost during the processing of a
>message.  I've narrowed it down to this call in milter-greylist.c
>
>     /*
>      * Check the ACL
>      */
>     if ((priv->priv_whitelist = acl_filter(SA(&priv->priv_addr),
>         priv->priv_addrlen, priv->priv_hostname, priv->priv_from,
>         rcpt, priv->priv_queueid)) & EXF_WHITELIST) {
>         priv->priv_elapsed = 0;
>         return SMFIS_CONTINUE;
>     }
>
>   The priv->priv_from value is fine before the acl_filter() call and
>is truncated by one character after returning from that call.  I've
>looked through that function and don't see anything obvious.  Is anybody
>else seeing this?

   BTW, the bug is a bit more severe then I first thought.  It appears that
any acl rule testing against "from" now fails if the rule was written as
the examples are, with a trailing >.  I reverted back to b2 because our
local whitelistings were all broken.

At 09:20 AM 05/20/2005, Brian W. Antoine wrote:
>   BTW, the bug is a bit more severe then I first thought.  It appears that
>any acl rule testing against "from" now fails if the rule was written as
>the examples are, with a trailing >.  I reverted back to b2 because our
>local whitelistings were all broken.

   Ok, I've tracked the problem to the emailcmp() routine in acl.c, which
somebody modified to strip leading and trailing characters from recently.
The problem is, they strip them from the real strings, not temporary copies
that are then used for the comparison, and while they simply step over the
leading characters they don't want, they actually truncate the string to
get rid of the extras.

   Stripping leading and trailing whitespace is probably OK, but I think the
< and > should be left in place.  Opinions?

Brian W. Antoine <briana@...> wrote:

>    Ok, I've tracked the problem to the emailcmp() routine in acl.c, which
> somebody modified to strip leading and trailing characters from recently.
> The problem is, they strip them from the real strings, not temporary copies
> that are then used for the comparison, and while they simply step over the
> leading characters they don't want, they actually truncate the string to
> get rid of the extras.
> 
>    Stripping leading and trailing whitespace is probably OK, but I think the
> < and > should be left in place.  Opinions?

My idea was to copy the string before touching it. Would it be alright?

-- 
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@...

At 11:06 AM 5/21/05, manu@... wrote:
>Brian W. Antoine <briana@...> wrote:
>
>>    Ok, I've tracked the problem to the emailcmp() routine in acl.c, which
>> somebody modified to strip leading and trailing characters from recently.
>> The problem is, they strip them from the real strings, not temporary copies
>> that are then used for the comparison, and while they simply step over the
>> leading characters they don't want, they actually truncate the string to
>> get rid of the extras.
>> 
>>    Stripping leading and trailing whitespace is probably OK, but I think the
>> < and > should be left in place.  Opinions?
>
>My idea was to copy the string before touching it. Would it be alright?

  That would solve it also, though at the expense of copying the data around
a couple of more times.  Those printf's that are still in there are probably
responsible for more wasted cycles than the data copy would be.

  I just ifdef'd out the whole stripping section so that the routine resembles
the b2 version's behavior, for the moment at least.  I can understand the
logic behind removing the whitespace, make sure the strings stand the best
chance of testing similar data, I'd have left it at that though and not
touched the <> characters.

Brian W. Antoine <briana@...> wrote:

>   That would solve it also, though at the expense of copying the data around
> a couple of more times.  Those printf's that are still in there are probably
> responsible for more wasted cycles than the data copy would be.

Woops, yes, I did not noticed the debug printf there.
 
>   I just ifdef'd out the whole stripping section so that the routine resembles
> the b2 version's behavior, for the moment at least.  I can understand the
> logic behind removing the whitespace, make sure the strings stand the best
> chance of testing similar data, I'd have left it at that though and not
> touched the <> characters.

Can you test that the following patch fixes everything?
http://ftp.espci.fr/shadow/manu/emailcmp.patch

-- 
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@...

At 01:02 PM 5/21/05, you wrote:
>Brian W. Antoine <briana@...> wrote:
>
>>   That would solve it also, though at the expense of copying the data around
>> a couple of more times.  Those printf's that are still in there are probably
>> responsible for more wasted cycles than the data copy would be.
>
>Woops, yes, I did not noticed the debug printf there.

  I believe I was a few more of them scattered around also.
 
>>   I just ifdef'd out the whole stripping section so that the routine resembles
>> the b2 version's behavior, for the moment at least.  I can understand the
>> logic behind removing the whitespace, make sure the strings stand the best
>> chance of testing similar data, I'd have left it at that though and not
>> touched the <> characters.
>
>Can you test that the following patch fixes everything?
>http://ftp.espci.fr/shadow/manu/emailcmp.patch

  It will be Monday before I can install and test it, but looking
over the patch, that should take care of the problem.

Brian W. Antoine <briana@...> wrote:
 
>   It will be Monday before I can install and test it, but looking
> over the patch, that should take care of the problem.

So, is it alright?

-- 
Emmanuel Dreyfus
Un bouquin en français sur BSD:
http://www.eyrolles.com/Informatique/Livre/9782212114638/livre-bsd.php
manu@...

At 11:27 AM 05/23/2005, manu@... wrote:
>Brian W. Antoine <briana@...> wrote:
>
> >   It will be Monday before I can install and test it, but looking
> > over the patch, that should take care of the problem.
>
>So, is it alright?

   I was just installing it when I got your message.  Nope, it core dumps
someplace and kills the milter.  I've reverted back to an earlier version
of that function to get running again.

Here is milter-greylist 2.0rc2:
http://ftp.espci.fr/pub/milter-greylist/milter-greylist-2.0rc2.tgz  
MD5 (milter-greylist-2.0rc2.tgz) = 1b5dc6139f0f3c3345a5c6fc9679e3bc

Changce since 2.0rc1: just a bug fix in e-mail address comparison code.

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