Radu Greab | 4 Sep 2008 14:15
Picon

Re: PATCH: qpsmtpd-prefork


Diego d'Ambra wrote:
> 
> Attached is proposed reworked version of daemon qpsmtpd-prefork.
> 
> Due to the number of changes I've included 2 files - one is the full 
> perl file (if somebody finds that more easy to use) the other is diff 
> against svn revision 936.

I've split your change into smaller pieces, as per your changelog, and
committed them with the following modifications:

>   * Fixed taint issue with argument [--interface].

Since the opinions I've seen suggest that trusting the config files (and
command line parameters) is OK, I've opted to just untaint the value and
not validate it with Data::Validate::IP. The value will be validated
when attempting the bind. Change #940.

>   * Fixed issue with slow or missing adjustment of children pool size on 
> very busy system.

For this I changed the main loop to not sleep when, after some children
have exited, it should check and adjust the pool size. Change #939.

I don't know if it is enough. If not, please give more details about the
comparison you make in your patch, where you recheck the pool if parent
slept less than two seconds.

>   * Fixed shutdown of parent (and children) on very busy system.
(Continue reading)

Aurélien Gazagne | 15 Sep 2008 22:22

problem with dnsbl

Hi,

I have a problem with the plugin dnsbl

I've always got "RBLSMTPD not set for ip" in my log.

My config/plugins is

hosts_allow
require_resolvable_fromhost
dnsbl disconnect
uribl deny
whitelist_soft
greylisting
count_unrecognized_commands 4
check_relay
check_badmailfrom
check_badrcptto
check_spamhelo
sender_permitted_from spf_deny 1
check_earlytalker wait 0.12 action deny
check_basicheaders 9
check_loop 80
# this plugin needs to run after all other "rcpt" plugins
rcpt_ok

# we'll turn this on later
#spamassassin
#virus/clamdscan deny_viruses yes clamd_socket /home/smtpd/qpsmtpd/tmp/clamd

(Continue reading)

Matthew Horsfall | 15 Sep 2008 22:35

Re: problem with dnsbl

This is just a debug message - you can ignore it.

Per the dnsbl documentation:

=item RBLSMTPD is set and non-empty

The contents are used as the SMTP conversation error.
Use this for forcibly blocking sites you don't like

=item RBLSMTPD is set, but empty

In this case no RBL checks are made.
This can be used for local addresses.

=item RBLSMTPD is not set

All RBL checks will be made.
This is the setting for remote sites that you want to check against RBL.

=back

For you, RBLSMTPD is not set, so all checks are being made. (The last 
option)

--

-- 
Matthew Horsfall
Dynamic Network Services, Inc.
http://www.dyndns.com

Aurélien Gazagne wrote:
(Continue reading)

Chris Lewis | 15 Sep 2008 22:40
Favicon

$transaction->body_filename;

According to the documentation, when you call
$transaction->body_filename, you get a temporary file name that points
at a file that contains the message.  If you examine body_filename, it
has no headers.

The clamdscan plugin uses body_filename to hand off to clamdscan.  Which
means that ClamAV doesn't get to see the headers.  Which is important to
some ClamAV detections (eg: the ClamAV self-test email is _not_ caught
by the clamdscan plugin).

[In contrast, the spamassassin plugin carefully spits out
header->as_string and then the body into spamd.]

Is this working as intended?

Matt Sergeant | 15 Sep 2008 23:01
Favicon

Re: $transaction->body_filename;

On Mon, 15 Sep 2008 16:40:24 -0400, Chris Lewis wrote:
> According to the documentation, when you call
> $transaction->body_filename, you get a temporary file name that points
> at a file that contains the message.  If you examine body_filename, it
> has no headers.
> 
> The clamdscan plugin uses body_filename to hand off to clamdscan.  Which
> means that ClamAV doesn't get to see the headers.  Which is important to
> some ClamAV detections (eg: the ClamAV self-test email is _not_ caught
> by the clamdscan plugin).
> 
> [In contrast, the spamassassin plugin carefully spits out
> header->as_string and then the body into spamd.]
> 
> Is this working as intended?

My gut instinct is to say no. But I worry a little bit that we might 
break something else by fixing it...

Chris Lewis | 15 Sep 2008 23:39
Favicon

Re: $transaction->body_filename;

Matt Sergeant wrote:
> On Mon, 15 Sep 2008 16:40:24 -0400, Chris Lewis wrote:
>> According to the documentation, when you call
>> $transaction->body_filename, you get a temporary file name that points
>> at a file that contains the message.  If you examine body_filename, it
>> has no headers.
>>
>> The clamdscan plugin uses body_filename to hand off to clamdscan.  Which
>> means that ClamAV doesn't get to see the headers.  Which is important to
>> some ClamAV detections (eg: the ClamAV self-test email is _not_ caught
>> by the clamdscan plugin).
>>
>> [In contrast, the spamassassin plugin carefully spits out
>> header->as_string and then the body into spamd.]
>>
>> Is this working as intended?
> 
> My gut instinct is to say no. But I worry a little bit that we might 
> break something else by fixing it...

Would it be worth considering have a data_filename() call, that does
exactly the same thing as body_filename, but includes the headers too?
Then we can fix the clamdscan plugin without breaking anything else.

Matt Sergeant | 16 Sep 2008 01:30
Favicon

Re: $transaction->body_filename;

On Mon, 15 Sep 2008 17:39:33 -0400, Chris Lewis wrote:
> Matt Sergeant wrote:
>> On Mon, 15 Sep 2008 16:40:24 -0400, Chris Lewis wrote:
>>> According to the documentation, when you call
>>> $transaction->body_filename, you get a temporary file name that points
>>> at a file that contains the message.  If you examine body_filename, it
>>> has no headers.
>>> 
>>> The clamdscan plugin uses body_filename to hand off to clamdscan.  Which
>>> means that ClamAV doesn't get to see the headers.  Which is important to
>>> some ClamAV detections (eg: the ClamAV self-test email is _not_ caught
>>> by the clamdscan plugin).
>>> 
>>> [In contrast, the spamassassin plugin carefully spits out
>>> header->as_string and then the body into spamd.]
>>> 
>>> Is this working as intended?
>> 
>> My gut instinct is to say no. But I worry a little bit that we might 
>> break something else by fixing it...
> 
> Would it be worth considering have a data_filename() call, that does
> exactly the same thing as body_filename, but includes the headers too?
> Then we can fix the clamdscan plugin without breaking anything else.

I thought about that, but you'd have to write to a file twice then :-/

Matt Sergeant | 16 Sep 2008 01:46
Favicon

Re: $transaction->body_filename;

On Mon, 15 Sep 2008 19:30:11 -0400, Matt Sergeant wrote:
>> Would it be worth considering have a data_filename() call, that does
>> exactly the same thing as body_filename, but includes the headers too?
>> Then we can fix the clamdscan plugin without breaking anything else.
> 
> I thought about that, but you'd have to write to a file twice then :-/

So I think we should just fix the bug, and see if anything breaks.

Ask Bjørn Hansen | 16 Sep 2008 02:35
Gravatar

Re: $transaction->body_filename;


On Sep 15, 2008, at 13:40, Chris Lewis wrote:

> According to the documentation, when you call
> $transaction->body_filename, you get a temporary file name that points
> at a file that contains the message.  If you examine body_filename, it
> has no headers.

It was made that way first because the headers can change, so we  
needed to keep those in memory.

If we put them into the file we have to document that they are there  
as received from the client which isn't necessarily the same as what  
they are now.   Also any plugins (queue plugins) that use the body but  
need the correct headers will have to know to skip the headers (maybe  
a method to give a filehandle starting at the right place?).

  - ask

--

-- 
http://develooper.com/ - http://askask.com/

Matt Sergeant | 16 Sep 2008 04:03
Favicon

Re: $transaction->body_filename;

On Mon, 15 Sep 2008 17:35:31 -0700, Ask Bjørn Hansen wrote:
> 
> On Sep 15, 2008, at 13:40, Chris Lewis wrote:
> 
>> According to the documentation, when you call
>> $transaction->body_filename, you get a temporary file name that points
>> at a file that contains the message.  If you examine body_filename, it
>> has no headers.
> 
> 
> It was made that way first because the headers can change, so we 
> needed to keep those in memory.
> 
> If we put them into the file we have to document that they are there 
> as received from the client which isn't necessarily the same as what 
> they are now.   Also any plugins (queue plugins) that use the body 
> but need the correct headers will have to know to skip the headers 
> (maybe a method to give a filehandle starting at the right place?).

We already have that, with body_start().


Gmane