Vacation Functionality - Details?

I'm very surprised... is NO ONE ELSE concerned about the inner workings of this script?

Is no one else concerned about whether or not it responds when it should, but moreimportantly DOES NOT respond when it SHOULDN'T?

As I said in the initial post, I'm not a programmer, and am unable to tell from simply reading the vacation script, what measures it takes to not respond when it shouldn't.

I'm most concerned about the claim that it doesn't reply to the envelope FROM, but instead replies to the From HEADER, which is easily forged.

Please... would someone who knows and understands this script, please respond to the specific details outlined above?

Thanks,

Charles

Hi, I mean to reply to this; but I've been a bit busy to...

Yes, it does appear to rely on the headers in the email to handle to/from; I'm not sure why this is - it certainly _seems_ the wrong behaviour.

Presumably the correct behaviour is to ignore the to/from in any headers, and use the SMTP RCPT TO / FROM fields, which should be passed into the script via $argv?

David.

Just being the squeaky wheel...

I'd really like to get this aspect of Postfixadmin improved so that the vacation behavior conforms to the 'Best practices' for an auto-responder.

Please, anyone who knows how to read this code (and maybe even improve it), can you take some time to analyse my initial message and then the vacation script?

Thanks,

Charles

Thanks for the reminder; I'm keen to improve it - although finding time to seems difficult :-( (I'll try and release postfixadmin 2.2.1 first)

With all of the talk of modularizing postfixadmin, which will require a total rewrite of much of the code, I'd like to again make a call to first, BEFORE doing that kind of thing, please, please, please lets get the Vacation functionality fixed up...

I think it (the ability to easily enable/disable a 'vacation' message) is an extremely important aspect of postfixadmin - in fact, it is one of the main reasons I decided to use it myself - but there are people on the postfix mail list that denigrate postfixadmin solely because of the vacation script functionality 'bugs' - meaning, failure to behave the way a proper vacation script should.

... if I was a coder and could do this myself, I wouldn't hesitate ...

I wrote a (0.0 release, alpha, test) patch to support mouss suggestions. You can find it for some days on http://www.iotti.biz/vacation/vacation.pl.useEnvelope.diff

I try to do the tests on the email address, stripping all other components from the address fields. It seems right for me, but I can accept other ideas.
For now, I did not get rid of the previous tests made on header recipient/sender addresses: I agree with mouss that when in doubt it's better to send nothing.

In master.cf it has to be called this way:
vacation  unix  -       n       n       -       -       pipe
  flags=Rq user=vacation argv=/var/spool/vacation/vacation.pl -f ${sender} -- ${recipient}

The patch is against the version found in PA-2.2.1.1 and contains the 2 patches I submitted 2-3 days ago too (interval notification and CentOS5 DBI compatibility).

I would like to hear feedback. Be warned that it was only very basically tested on my home server for now.

Luigi

Hi Luigi,

Nice patch - you've saved me from doing it this weekend!

I'll try and test it - the patch itself looks fine from a first run through, so thank you!

David.

Yay! Luigi is my here of the day!

Seriously, thanks, this is great...

Would it be possible for you to list, precisely, but in plain english, what each step of the code does?

Example:

1. tests for vacatation enabled

2. if enabled, tests interval against sender/interval db

3. performs following checks:

a) tests for NULL <> sender

etc... you get the idea...

I'd really like to understand exactly how it works. Also, once this is officially integrated, I this it should be well documented on, or just off of the home page, complete with a request for additional checks that could be added to make its behavior even safer...

But regardless... thanks so much to Luigi and GingerDog for making this happen! I'm very excited about upgrading once it is official...

Hmmm, again the Perpetual Documentation Problem. The code here is particularly self-explaining. Below I'll try to tell what is done in reference to the code chunks. But honestly I think you should try to read the code. It's not that hard, me too am not a pro coder. I think that documentation about this topic would quickly become obsolete when changes are made to the script.

So on to the code (comments are after the # sign):
while (<STDIN>) { # reading the message from standard input
...

# if we find the x-spam-flag: or x-spam-status: flag, exit doing nothing; the comparison is not case-sensitive (the /i )
elsif (/^x-spam-(flag|status):\s+yes/i) { exit (0); }

# if we find the precedence: header followed by space (\s+) and one of the bulk, list, junk words, exit doing nothing;
elsif (/^precedence:\s+(bulk|list|junk)/i) { exit (0); }

# if we find the x-loop: header followed by space (\s+) and the string "postfix admin virtual vacation", exit doing nothing. Hey: we check a x-loop header that we never insert: this will be the object of another patch
elsif (/^x-loop:\s+postfix\ admin\ virtual\ vacation/i) { exit (0); }

# if we find Auto-Submitted: no , let's go on
elsif (/^Auto-Submitted:\s+no/i) { next; }

# if we find Auto-Submitted: with anything else, exit doing nothing;
elsif (/^Auto-Submitted:/i) { exit (0); }

# if we find List-Id: or List-Post header, exit doing nothing
elsif (/^List-(Id|Post):/i) { exit (0); }
...

# if any of the to, from, messageid header fields, or envelope sender/recipient are not set, then exit. Maybe it can be discussed if, now that we use evelope sender and recipient, it does make sense checking the header from/to. I think the answer is still yes.
if (!$from || !$to || !$messageid || !$sender || !$recipient) { exit (0); }

# If the envelope sender begins (the ^) with mailer-daemon or listserv or majordomo or owner- or request- or bounces- , exit doing nothing
if ( $sender =~ /^(mailer-daemon|listserv|majordomo|owner-|request-|bounces-)/i) { exit (0); }

# If the envelope sender contains (note the absence of ^) -owner@ or -request@ or -bounces@ , exit doing nothing
if ( $sender =~ /-(owner|request|bounces)\@/i) { exit (0); }

# the strip_address sub strips anything but the mail address so in $ss we have the mail address of the envelope sender
my $ss = strip_address($sender);
my $sr = strip_address($recipient);
my $ssh = strip_address($sndrhdr);
# if the mail address of the evelope sender is the same of the recipient's, then exit
if ($ss eq $sr) { exit(0); }

# here we loop over a list made of the recipients found in the to, cc, and bcc header fields
for (split(/,\s*/, lc($to)), split(/,\s*/, lc($cc)), split(/,\s*/, lc($bcc))) {
        my $destinatario = strip_address($_); # we take only the address of the recipient we're looping on
        if ($ssh eq $destinatario) { exit(0); } # if it's equal to the Sender: header mail address, then exit. Hey! Shouldn't we check $sh too for this?
        if ($sr eq $destinatario) { $recipfound++; } # increment a variable (initially 0) if we found that it was equale to the envelope recipient mail address
}
if (!$recipfound) { exit (0); } # if we did not find any to, cc, or bcc header field values matchng the envelope recipient, then exit

# use a standard module to check if the from header field has a valid dns entry
if (!Email::Valid->address($from,-mxcheck => 1)) { exit(0); }
# the same for the envelope sender mail address
if (!Email::Valid->address($ss,-mxcheck => 1)) { exit(0); }

# some of the checks performed on the from header value
if ($from eq "" || $from =~ /^(owner-|-(?:request|owner)\@|^(?:mailer-daemon|postmaster)\@)/i) { exit (0); }

Correction to my prevous post: the x-loop header is inserted. I missed it.

For the other thing, I think we should add the line
if ($ss eq $destinatario) { do_debug("envelope sender $ss contains recipient $destinatario"); exit(0); }
over or under the one with
if ($ssh eq $destinatario) { do_debug("sender header $sender contains recipient $destinatario"); exit(0); }
to theck again if the sender envelope address is contained in the header recipients. Maybe redundant, but "melius est abundare".

Hi,

Patch merged - I'm working on changing the logging used to use log4perl, which should improve things too.

(*Note: I've not tested the patch, but it's in trunk, which should be warning enough*)

David.

Wow!

This is awesome... very comprehensive, and looks like it contains all of the checks mouss recommended... and I agree with you that we should *also* check the header from/recipients - can't hurt, but I guess the only question (that I cannot answer) is, does it really accomplish anything? Meaning, will it catch anything that the envelope sender/recipient checks won't?

Do you mind if I invite him to this thread for comments/suggestions?

Thank you very, very much for taking the time to do this!

Anyone is welcome to comment/raise tickets/test/submit patches for the codebase; I'm not fussy! If it's an improvement it'll get in :)

> does it really accomplish anything?
> Meaning, will it catch anything that the
> envelope sender/recipient checks won't?

At a first glance, I think the answer is no. BUT it's highly likely I'm missing some particular cases. I just wrote in Perl, as good as I can, what mouss told to do. I think that leaving the checks on the from/to headers or removing them should be the object of some discussion. In the meanwhile, leaving them there will not hurt (I think).

Hi,

Just a quick note - I've merged Lux's patch and am currently rewriting the logging done by the script to use log4perl (this should make it easier for people to understand what it's doing).

I'm almost ready to commit it, but am hoping to write a simple 'test' harness or something to help show that it is (or isn't!) working.

David.

vacation.pl updated with better logging etc and crude tests....

As detailed in my previous post, I think that in the setup instructions we should recomment to use the command line
argv=/var/spool/vacation/vacation.pl -f ${sender} -- ${user}
instead of
argv=/var/spool/vacation/vacation.pl -f ${sender} -- ${recipient}
in a purely virtual mailbox setup (as is with vanilla PostfixAdmin).

And we should add the ! # $ * / ? | ^ { } ` ~ & ' + = characters in the local part of the email address stripped in strip_address.
Looking at the RFC, we should accept quoted local parts too. Maybe there is a perl module to handle this correctly.

Comments before patching?