PATCH: Sanitise reply from check_spamhelo
Not totally appropriate for a commercial mail server, IMO.
Gordon
--- plugins/check_spamhelo.orig 2005-07-02 17:07:43.0 +1000
+++ plugins/check_spamhelo 2005-07-02 17:08:56.0 +1000
@@ -29,7 +29,7 @@
for my $bad ($self-qp-config('badhelo')) {
if ($host eq lc $bad) {
$self-log(LOGDEBUG, Denying HELO from host claiming to be $bad);
- return (DENY, Uh-huh. You're $host, and I'm a boil on the
bottom of the Marquess of Queensbury's great-aunt.);
+ return (DENY, Sorry, I don't believe that you are $host.);
}
}
return DECLINED;
Re: Changing the Plugin APIs
In terms of backwards compatibility - we can maintain it by having the
(current) register subroutine create the appropriate subroutine or
alias.
Or the other way around - if the hook_* functions exist, have
register_hook called magically behind the scenes when you lookup whether
it can do the hooks.
Good idea. This is cleaner. Patch below.
Interesting idea. Since many plugins use register() as an opportunity
to parse their config line options and initialize private storage,
your new scheme would require some other way of accomplishing that
task, like an init() function which is called when loading the plugin
(supplying the command line args, which is why BEGIN {} wouldn't
work).
register() doesn't have to go away -- it can still be used for parsing
command line options and initializing private storage.
Other than that, it sounds good! As you say, it's going to slightly
annoying to loop over the available hook subs for each plugin; perhaps
the loop should shortcut if the plugin _has_ a register() sub already
(old-style, in other words).
I'm not sure if this is worth it. 20 $class-can's can't be that slow.
Do we want to draw a line and say old style has register() and new
style has init()? (And use that to differentiate?) Backwards
compatibility is preserved either way.
Anyway, here's a patch for eyeballing. I'll commit it if nobody
screams. Then, after 0.30 goes out, I'll modify all the core hooks to
use this model.
One issue is some of our hook names aren't valid subroutine names.
I've handled that with the s/\W/_/g;
Further below is an example of a child plugin. Works quite nicely.
-R
=== lib/Qpsmtpd/Plugin.pm
==
--- lib/Qpsmtpd/Plugin.pm (revision 454)
+++ lib/Qpsmtpd/Plugin.pm (local)
@@ -16,7 +16,7 @@
sub register_hook {
my ($plugin, $hook, $method, $unshift) = @_;
-
+
die $plugin-plugin_name . : Invalid hook: $hook unless $hooks{$hook};
# I can't quite decide if it's better to parse this code ref or if
@@ -33,6 +33,7 @@
my $qp = shift;
local $self-{_qp} = $qp;
$self-register($qp, @_);
+ $self-register_standard_hooks($qp, @_);
}
sub qp {
@@ -86,7 +87,7 @@
return if defined {${newPackage}::register};
- Qpsmtpd::_compile($self-plugin_name . _isa,
+ $self-compile($self-plugin_name . _isa,
$newPackage,
plugins/$parent); # assumes Cwd is qpsmtpd root
@@ -141,4 +142,16 @@
die eval $@ if $@;
}
+sub register_standard_hooks {
+ my ($plugin, $qp) = @_;
+
+ for my $hook (keys %hooks) {
+my $hooksub = hook_$hook;
+$hooksub =~ s/\W/_/g;
+$plugin-register_hook( $hook, $hooksub )
+ if ($plugin-can($hooksub));
+ }
+}
+
+
#- --- example child plugin
# -*- perl -*-
sub register {
my ($self, $qp) = @_;
$self-isa_plugin('rcpt_ok');
}
sub hook_rcpt {
my ($self, $transaction, $recipient) = @_;
warn uh.. don't really care, just calling child\n;
$self-SUPER::hook_rcpt( $transaction, $recipient );
}
sub hook_helo {
return DECLINED;
}
PATCH: Check relayclient in rhsbl
--- plugins/rhsbl.orig 2005-07-02 17:15:11.0 +1000
+++ plugins/rhsbl 2005-07-02 17:15:29.0 +1000
@@ -9,6 +9,8 @@
sub mail_handler {
my ($self, $transaction, $sender) = @_;
+ return (DECLINED) if $self-qp-connection-relay_client();
+
my $res = new Net::DNS::Resolver;
my $sel = IO::Select-new();
my %rhsbl_zones_map = ();
@@ -45,6 +47,8 @@
my $host = $transaction-sender-host;
my $hello = $self-qp-connection-hello_host;
+ return (DECLINED) if $self-qp-connection-relay_client();
+
my $result = $self-process_sockets;
if ($result defined($self-{_rhsbl_zones_map}{$result})) {
if ($result =~ /^$host\./ ) {
PATCH: config.sample for badrcptto_patterns
--- config.sample/badrcptto_patterns.orig 1970-01-01 10:00:00.0 +1000 +++ config.sample/badrcptto_patterns2005-07-02 17:24:07.0 +1000 @@ -0,0 +1,5 @@ +# Format is pattern\s+Response +# Don't forget to anchor the pattern if required +! Sorry, bang paths not accepted here [EMAIL PROTECTED]@Sorry, multiple at signs not accepted here +% Sorry, percent hack not accepted here
Re: Changing the Plugin APIs
Robert Spier wrote:
+sub register_standard_hooks {
+ my ($plugin, $qp) = @_;
+
+ for my $hook (keys %hooks) {
+my $hooksub = hook_$hook;
+$hooksub =~ s/\W/_/g;
+$plugin-register_hook( $hook, $hooksub )
+ if ($plugin-can($hooksub));
+ }
+}
Looks good, except it might be useful to log that at the debug level, e.g.
if ($plugin-can($hooksub)) {
# can't call -log() directly because it assumes it's inside a hook
$plugin-varlog(LOGDEBUG, $self-plugin_name, hooking , $hook);
$plugin-register_hook( $hook, $hooksub );
}
since it is a useful bit of information to grep for all plugins hooking a
particular hook, and with the current plugin loading scheme, it only happens
once (as long as you are not running under tcpserver).
sub hook_rcpt {
my ($self, $transaction, $recipient) = @_;
- warn uh.. don't really care, just calling child\n;
+ $self-log(LOGDEBUG, uh.. don't really care, just calling child);
$self-SUPER::hook_rcpt( $transaction, $recipient );
}
I'd prefer, if you are actually adding the sample plugin, that it be good code
that someone can use to write a new plugin. I've tried to eliminate all
instances of warn() from the core (I see there are a couple more I need to get)
except from within a logging plugin, so that all output from the code is controlled.
Otherwise, You may fire when ready, Gridley! [1]
John
1) http://www.americaslibrary.gov/cgi-bin/page.cgi/jb/reform/dewey_1
Re: PATCH: Sanitise reply from check_spamhelo
Gordon Rowell wrote: Not totally appropriate for a commercial mail server, IMO. Sadly, I have to agree. On very rare occasions, some admin will set Exchange to show the actual error message (it's been documented in the wild) and the sometimes pithy comments that qpsmtpd throws will be relayed back to the user, who may be someone Important like the president of the company. I've already had to deal with this once... Committed, thanks! John
Re: PATCH: Check relayclient in ...
John Peacock wrote: [...] These three I'm not going to commit (without discussion), because I don't like the code duplication involved. Rather than patching every plugin to respect the check_relay() setting (and yes, I know, I have done this recently), and remember to add that code to any *new* plugin, I'd rather have a relay_ok() plugin which hooked mail and rcpt and just shortcircuits the tests, thus making it is site preference to do this rather than global. [...] Sounds good to me. I didn't like patching each of them either. I did this when testing SMTP AUTH, which can change a non-relay address into a relayclient. I'll keep them as local patches for now. Thanks, Gordon
Re: Changing the Plugin APIs
On 2 Jul 2005, at 03:14, Robert Spier wrote: Or the other way around - if the hook_* functions exist, have register_hook called magically behind the scenes when you lookup whether it can do the hooks. Good idea. This is cleaner. Patch below. Should we wait until after 0.30? I know it's not a very invasive patch but we did go to RC already. Matt.
Re: PATCH: Don't reveal version in SMTP greeting
On Jul 2, 2005, at 5:06 AM, Ask Bjørn Hansen wrote: ... Why? If it's for security, will it really make a difference? Does it give any information out that an attacker can use? If there ever is a security problem in qpsmtpd (unlikely, but I suppose possible), wouldn't the attacker just hit SMTP servers at random for it anyway? Or if doing a more targeted attack, surely they'll try no matter what the version string says or doesn't say. ... Although not a technical reason, many companies that do security vulnerability assessments (such as those from Cisco) count points off if you reveal version numbers. And managers don't like to see points taken off. :) dig @ns1.cisco.com version.bind chaos txt They even turn off the Bind versions. -- /chris/ smime.p7s Description: S/MIME cryptographic signature
Re: Changing the Plugin APIs
Should we wait until after 0.30? I know it's not a very invasive patch but we did go to RC already. Yes.
Re: PATCH: config.sample for badrcptto_patterns
Great. Now I have to change all my config files from using | as a reason seperator to using whitespace. ;) -- Bryan Gordon Rowell wrote: --- config.sample/badrcptto_patterns.orig 1970-01-01 10:00:00.0 +1000 +++ config.sample/badrcptto_patterns2005-07-02 17:24:07.0 +1000 @@ -0,0 +1,5 @@ +# Format is pattern\s+Response +# Don't forget to anchor the pattern if required +! Sorry, bang paths not accepted here [EMAIL PROTECTED]@Sorry, multiple at signs not accepted here +% Sorry, percent hack not accepted here
