Mailing List Archive

----- Original Message -----
From: "Alain Knaff" <spf@misc.lka.org.lu>
To: "Mark" <admin@asarian-host.net>
Cc: <spf-devel@v2.listbox.com>; "Meng Weng Wong" <mengwong@dumbo.pobox.com>
Sent: Wednesday, January 28, 2004 11:54 PM
Subject: Start/stop improvements for sendmail-milter, and support for "local
policy"


> The patch appended below makes it easyer to reliably start and stop
> the milter. Moreover it gives access to the "local policy"
> configuration, which may be used if some of your users have set up
> forwardings to their account on your host.
> The patch is made against the 1.21 version, as found on spf.pobox.com

Alain,

A few observations:

1):

Patching file sendmail-milter-spf-1.21.pl using Plan A...
Hunk #1 succeeded at 1 with fuzz 2.
Hunk #2 failed at 20.
Hunk #3 failed at 63.
Hunk #4 failed at 152.
Hunk #5 failed at 416.
Hunk #6 failed at 521.
Hunk #7 failed at 588.
6 out of 7 hunks failed--saving rejects to sendmail-milter-spf-1.21.pl.rej

That does not look promising. :(

> Moreover, I added a #!/usr/bin/perl to the top, to make it more
> easily startable, without needing to explicitly invoke Perl.

That is not a good idea. /usr/bin/perl is the perl in the base-system, and
very rarely will it be thread-enabled. Starting spf-milter will only cause
startup to fail, and cause confusion. You'd have a better chance with
/usr/local/bin/perl; but it is best to leave it the way it was, as the
least-likely location to find a thread-enabled perl will be /usr/bin/.

+# define(`confMILTER_MACROS_ENVFROM', `i, j, {auth_type}, ...

That is not necessary. Sendmail will add a standard set of Milter macros for
the callbacks. And why add the 'i' macro? spf-milter sure does not need it.

+my $pidFile='/var/spf-milter/spf-milter.pid';

Be careful not to treat this as a variable; spf-milter sets all proper
permissions/ownerships, according to the user we run as, based on things
being in /var/spf-milter/. That is necessary, because we drop root
privileges after startup. A pid in /var/run/, for instance, can, after the
demotion, no longer be unlinked from within spf-milter!

+use Getopt::Std;

This is a good change. The command-line parsing was in need of revision.
Thanks. Your other changes seem useful too.

Alain, can you give a working diff? (against
http://spf.pobox.com/sendmail-milter-spf-1.21.pl), so I can test it.

I must admit that people making drastic changes to 'my' milter, makes me a
bit nervous. I went through great trouble ensuring spf-milter runs very
stable; and I like to keep it that way. :) But all is part of the open
source thingy, I guess.

- Mark

System Administrator Asarian-host.org

---
"If you were supposed to understand it,
we wouldn't call it code." - FedEx

-------
To unsubscribe, change your address, or temporarily deactivate your subscription,
please go to http://v2.listbox.com/member/?listname@Ë`Ì{5¤¨wâÇSÓ°)h

begin Thursday 29 January 2004 01:19, Mark quote:
> ----- Original Message -----
> From: "Alain Knaff" <spf@misc.lka.org.lu>
> To: "Mark" <admin@asarian-host.net>
> Cc: <spf-devel@v2.listbox.com>; "Meng Weng Wong" <mengwong@dumbo.pobox.com>
> Sent: Wednesday, January 28, 2004 11:54 PM
> Subject: Start/stop improvements for sendmail-milter, and support for
> "local policy"
>
> > The patch appended below makes it easyer to reliably start and stop
> > the milter. Moreover it gives access to the "local policy"
> > configuration, which may be used if some of your users have set up
> > forwardings to their account on your host.
> > The patch is made against the 1.21 version, as found on spf.pobox.com
>
> Alain,
>
> A few observations:
>
> 1):
>
> Patching file sendmail-milter-spf-1.21.pl using Plan A...
> Hunk #1 succeeded at 1 with fuzz 2.
> Hunk #2 failed at 20.
> Hunk #3 failed at 63.
> Hunk #4 failed at 152.
> Hunk #5 failed at 416.
> Hunk #6 failed at 521.
> Hunk #7 failed at 588.
> 6 out of 7 hunks failed--saving rejects to sendmail-milter-spf-1.21.pl.rej
>
> That does not look promising. :(


Ok, I attached now the complete file (rather than just a diff). I hope
that works better. This version also no longer has the i variable in
the MILTER_MACROS_ENVFROM, which slipped accross from a different test
version

> > Moreover, I added a #!/usr/bin/perl to the top, to make it more
> > easily startable, without needing to explicitly invoke Perl.
>
> That is not a good idea. /usr/bin/perl is the perl in the base-system, and
> very rarely will it be thread-enabled.

Well, as shown by perl -V , both the current SuSE, and the current
Fedora have a thread-enabled Perl.

> Starting spf-milter will only cause
> startup to fail, and cause confusion. You'd have a better chance with
> /usr/local/bin/perl; but it is best to leave it the way it was, as the
> least-likely location to find a thread-enabled perl will be /usr/bin/.

If the user is smart enough to compile his own perl, he'll be smart
enough to fix the #! to point to the correct location for his system.

I was trying to optimize for the common case where the system-supplied
Perl works just fine.

> +# define(`confMILTER_MACROS_ENVFROM', `i, j, {auth_type}, ...
>
> That is not necessary. Sendmail will add a standard set of Milter macros
> for the callbacks. And why add the 'i' macro? spf-milter sure does not need
> it.

"i" was a mistake (hastily copy-pasted from my own sendmail.mc, which
had it because of another milter). I removed it in the attached version.

"j", on the other hand, is useful, and is not the default, as far as I
can see. But possibly, this depends on sendmail version.

> +my $pidFile='/var/spf-milter/spf-milter.pid';
>
> Be careful not to treat this as a variable;

A rather surprising opinion... Most developpers and designers I know
consider it to be a good idea to use constants for path names (and
other items) that are often repeated. The reason for this is twofold:

1. If ever the path (or number, or ...) needs to be changed, it only
needs to be changed in one place in the source, rather than all over
the place.

2. And even if it _doesn't_ need to be changed, it has the advantage
of protecting against typoes: a mistyped path leads to no compile
time error (or rather cperl-check-syntax time error...), whereas a
mistyped constant name gives you an error as soon as you do
cperl-check-syntax.

Heck, point #2 is sufficiently useful that programmers even use
constants for numbers such as Pi, even though Pi would never actually
change.

Regards,

Alain


-------
To unsubscribe, change your address, or temporarily deactivate your subscription,
please go to http://v2.listbox.com/member/?listname@Ë`Ì{5¤¨wâÇSÓ°)h

----- Original Message -----
From: "Alain Knaff" <spf@misc.lka.org.lu>
To: <spf-devel@v2.listbox.com>
Cc: "Meng Weng Wong" <mengwong@dumbo.pobox.com>
Sent: Thursday, January 29, 2004 10:45 AM
Subject: Re: [spf-devel] Re: Start/stop improvements for sendmail-milter,
and support for "local policy"

> > > Moreover, I added a #!/usr/bin/perl to the top, to make it more
> > > easily startable, without needing to explicitly invoke Perl.
> >
> > That is not a good idea. /usr/bin/perl is the perl in the base-system
> > and very rarely will it be thread-enabled.
>
> Well, as shown by perl -V , both the current SuSE, and the current
> Fedora have a thread-enabled Perl.

In the base-system? Must be a new thing. :) It is not a major issue, of
course. Maybe we can just add a comment underneath the shebang line, to
remind people to set it against a thread-enabled Perl.

> "j", on the other hand, is useful, and is not the default, as far as I
> can see. But possibly, this depends on sendmail version.

It is default for my 8.12.11 version. We can define it, of course, if it
does not come default in all 8.12.x versions. And the 'j' macro is used in
spf-milter.

> > +my $pidFile='/var/spf-milter/spf-milter.pid';
> >
> > Be careful not to treat this as a variable;
>
> A rather surprising opinion... Most developpers and designers I know
> consider it to be a good idea to use constants for path names (and
> other items) that are often repeated. The reason for this is twofold:
>
> 1. If ever the path (or number, or ...) needs to be changed, it only
> needs to be changed in one place in the source, rather than all over
> the place.

Yes. Except that you only made the PID file a variable. My warning against
that was because spf-milter sets its permissions/ownership all in
/var/spf-milter/,

if ((not chown $uid, $gid, '/var/spf-milter', glob ('/var/spf-milter/*')) ||
(not chmod 0700, '/var/spf-milter')) {
log_error_and_exit ("Cannot set proper permissions!");
}

So, if people see your variable, and decide to change $pidFile to, say,
'/var/run/spf-milter.pid', things would break.

I think, therefore, the best solution is not to define a single variable for
$pidFile, but rather to set a basedir; like:

$basedir = '/var/spf-milter/';

And then have the rest of the code work with $basedir (so people can change
it to '/var/spool/spf-milter/', or something else to their liking).

I will examine the patch now. :)

- Mark

System Administrator Asarian-host.org

---
"If you were supposed to understand it,
we wouldn't call it code." - FedEx

-------
To unsubscribe, change your address, or temporarily deactivate your subscription,
please go to http://v2.listbox.com/member/?listname@Ë`Ì{5¤¨wâÇSÓ°)h

----- Original Message -----
From: "Alain Knaff" <spf@misc.lka.org.lu>
To: <spf-devel@v2.listbox.com>
Cc: "Meng Weng Wong" <mengwong@dumbo.pobox.com>
Sent: Thursday, January 29, 2004 10:45 AM
Subject: Re: [spf-devel] Re: Start/stop improvements for sendmail-milter,
and support for "local policy"

> Ok, I attached now the complete file (rather than just a diff).
> I hope that works better.

Ok, I reviewed the changes, and am actually quite happy with them. :) I
incorporated them all in this 1.30 version.

I also pressed forward on my idea to provide a basedir variable (in the
config section):

# where do we store pid, sock, and logs? No trailing / please!
# Set it at will, like '/var/spool/spf-milter', as long as it
# ends in "spf-milter". Sanity check in program will check
# that it does!

my $basedir = '/var/spf-milter';

And I replaced all hard-coded paths with $basedir + file/socket name (there
were quite a few), like:

POSIX::strftime ($basedir . "/spflog-%Y%m.log", localtime);
my $pidFile = $basedir . '/spf-milter.pid';
my $sock = $basedir . '/spf-milter.sock';

Etc. And added this now quite necessary sanity check:

# Basic, but vital, sanity-check against $basedir. Since we set
# permissions/ownerships on everything (!) in our $basedir, we
# must avoid disasters, such as setting $basedir to /var/run/.
# Therefore, we require that $basedir ends in "spf-milter".

if (not ($basedir =~ /spf-milter$/i)) {
die '$basedir' . " ('$basedir') must end in /spf-milter!\n";
}

I put a link to the basic INSTALL doc, and added proper accreditation to
Alain Knaff for his work.

This 1.30 version contains no bug-fixes (as no bugs in 1.21 are known), but
enhances the spf-milter functionality nonetheless.

- Mark

System Administrator Asarian-host.org

---
"If you were supposed to understand it,
we wouldn't call it code." - FedEx

-------
To unsubscribe, change your address, or temporarily deactivate your subscription,
please go to http://v2.listbox.com/member/?listname@Ë`Ì{5¤¨wâÇSÓ°)h