Re: Bugs in qpsmtpd-prefork + some features + patches
On Mon, 03 Sep 2007 08:24:51 +0200 Stefan Priebe <[EMAIL PROTECTED]> wrote: > That's bad :-( > > Why you don't use hook_quit ? If any plugin with hook_quit() returns DONE the hook_disconnect() is not called. If you have a plugin which which hooks 'reset_transaction', the final hook_reset_transaction() will not be called if you exit in hook_disconnect(). The only way not to call the post-connection hook in -prefork (and -forkserver) is to kill the process. > Hanno Hecker schrieb: > > For now it seems like the only solution is to exit the process if SSL > > and non-SSL connections are used on the same port (STARTTLS), > > prefereably not in hook_disconnect() but in hook_post_connection(), see > > attached diff. With SSL-only connections on port 465 everything works > > fine for me. Hanno
Re: Bugs in qpsmtpd-prefork + some features + patches
Hello! What's with number two? It is more important than the other ones. And it works perfectly. Stefan
Re: Bugs in qpsmtpd-prefork + some features + patches
Hello! That's bad :-( Why you don't use hook_quit ? Stefan Hanno Hecker schrieb: On Fri, 31 Aug 2007 13:58:07 +0200 Stefan Priebe <[EMAIL PROTECTED]> wrote: To 4.) perhaps the new instance also solves this problem. I don't remember the failure - i only know that it was TLS related. And the connection fails after TLS command. It does not solve it, it seems to be a problem with the SSL socket. We can't close it properly with $ssl_sock->close(SSL_no_shutdown => 1, SSL_ctx_free => 1); The child would have no socket to talk to the next client... Downgrading to clear text doesn't seem to bee supported by IO::Socket::SSL. I guess that -async would have the same problem (if SSL would be possible). For now it seems like the only solution is to exit the process if SSL and non-SSL connections are used on the same port (STARTTLS), prefereably not in hook_disconnect() but in hook_post_connection(), see attached diff. With SSL-only connections on port 465 everything works fine for me. Hanno
Re: Bugs in qpsmtpd-prefork + some features + patches
On Fri, 31 Aug 2007 13:58:07 +0200
Stefan Priebe <[EMAIL PROTECTED]> wrote:
> To 4.) perhaps the new instance also solves this problem. I don't
> remember the failure - i only know that it was TLS related. And the
> connection fails after TLS command.
It does not solve it, it seems to be a problem with the SSL socket. We
can't close it properly with
$ssl_sock->close(SSL_no_shutdown => 1, SSL_ctx_free => 1);
The child would have no socket to talk to the next client...
Downgrading to clear text doesn't seem to bee supported by
IO::Socket::SSL. I guess that -async would have the same problem (if
SSL would be possible).
For now it seems like the only solution is to exit the process if SSL
and non-SSL connections are used on the same port (STARTTLS),
prefereably not in hook_disconnect() but in hook_post_connection(), see
attached diff. With SSL-only connections on port 465 everything works
fine for me.
Hanno
Index: plugins/tls
===
--- plugins/tls (revision 784)
+++ plugins/tls (working copy)
@@ -151,6 +151,23 @@
return DECLINED;
}
+# work-around for failed connections in -prefork after STARTTLS connection:
+sub hook_post_connection {
+my $self = shift;
+
+return (DECLINED)
+ unless $self->qp->isa('Qpsmtpd::SMTP::Prefork');
+
+return (DECLINED)
+ if $self->qp->connection->local_port == 465;
+
+if ($self->connection->notes('tls_enabled')) {
+$self->log(LOGWARN, "Exiting, because 'tls_enabled' was true.");
+exit;
+}
+return(DECLINED);
+}
+
sub _convert_to_ssl {
my ($self) = @_;
Re: Bugs in qpsmtpd-prefork + some features + patches
On Sep 1, 2007, at 12:22 AM, Hanno Hecker wrote: Attached are two diffs: the first (prefork-newinst.diff) does it right. I'm not sure if everything works as expected with the second one (prefork-cloneinst.diff), but from the first tests, everything looks ok. Oh, the second one is a bit faster .) Apart from an objection that it doesn't have any tests then either one is okay as far as I'm concerned. - ask -- http://develooper.com/ - http://askask.com/
Re: Bugs in qpsmtpd-prefork + some features + patches
Hello! I think the second one is the better one. Cause the system has not to create a new object after each connection. Stefan Hanno Hecker schrieb: On Fri, 31 Aug 2007 13:58:26 +0200 Stefan Priebe <[EMAIL PROTECTED]> wrote: To 1.) i'm not shure but it works perfectly with only deleting the _auth. The main question is - what gives us MORE performance (delete the values or create a new instance). The clean solution is, to create a new instance - that's right. Ok, here we go... ;-) Attached are two diffs: the first (prefork-newinst.diff) does it right. I'm not sure if everything works as expected with the second one (prefork-cloneinst.diff), but from the first tests, everything looks ok. Oh, the second one is a bit faster .) Hanno
Re: Bugs in qpsmtpd-prefork + some features + patches
On Fri, 31 Aug 2007 13:58:26 +0200
Stefan Priebe <[EMAIL PROTECTED]> wrote:
> To 1.) i'm not shure but it works perfectly with only deleting the
> _auth. The main question is - what gives us MORE performance (delete the
> values or create a new instance). The clean solution is, to create a new
> instance - that's right.
Ok, here we go... ;-)
Attached are two diffs: the first (prefork-newinst.diff) does it right.
I'm not sure if everything works as expected with the second one
(prefork-cloneinst.diff), but from the first tests, everything looks
ok. Oh, the second one is a bit faster .)
Hanno
Index: qpsmtpd-prefork
===
--- qpsmtpd-prefork (revision 783)
+++ qpsmtpd-prefork (working copy)
@@ -222,7 +222,7 @@
};
# setup qpsmtpd_instance
-$qpsmtpd = qpmsptd_instance();
+$qpsmtpd = qpsmtpd_instance();
# child reaper
$SIG{CHLD} = \&reaper;
@@ -367,6 +367,9 @@
or die
"failed to create new object - $!"; # wait here until client connects
info("connect from: " . $client->peerhost . ":" . $client->peerport);
+
+# clear a previously running instance by creating a new:
+$qpsmtpd = qpsmtpd_instance();
# set STDIN/STDOUT and autoflush
POSIX::dup2(fileno($client), 0)
@@ -410,7 +413,7 @@
# qpsmtpd_instance: setup qpsmtpd instance
# arg0: void
# ret0: ref to qpsmtpd_instance
-sub qpmsptd_instance {
+sub qpsmtpd_instance {
my $qpsmtpd = Qpsmtpd::TcpServer::Prefork->new();
$qpsmtpd->load_plugins;
$qpsmtpd->spool_dir;
Index: qpsmtpd-prefork
===
--- qpsmtpd-prefork (revision 783)
+++ qpsmtpd-prefork (working copy)
@@ -35,7 +35,7 @@
my $VERSION = "1.0";
# qpsmtpd instance
-my $qpsmtpd;
+my ($qpsmtpd, $qpsmtpd_base);
# cmd's needed by IPC
my $ipcrm = '/usr/bin/ipcrm';
@@ -222,7 +222,7 @@
};
# setup qpsmtpd_instance
-$qpsmtpd = qpmsptd_instance();
+$qpsmtpd = $qpsmtpd_base = qpsmtpd_instance();
# child reaper
$SIG{CHLD} = \&reaper;
@@ -367,6 +367,9 @@
or die
"failed to create new object - $!"; # wait here until client connects
info("connect from: " . $client->peerhost . ":" . $client->peerport);
+
+# clear a previously running instance by cloning the base:
+$qpsmtpd = $qpsmtpd_base;
# set STDIN/STDOUT and autoflush
POSIX::dup2(fileno($client), 0)
@@ -410,7 +413,7 @@
# qpsmtpd_instance: setup qpsmtpd instance
# arg0: void
# ret0: ref to qpsmtpd_instance
-sub qpmsptd_instance {
+sub qpsmtpd_instance {
my $qpsmtpd = Qpsmtpd::TcpServer::Prefork->new();
$qpsmtpd->load_plugins;
$qpsmtpd->spool_dir;
Re: Bugs in qpsmtpd-prefork + some features + patches
OK - that's right. Stefan Johan Almqvist schrieb: Hanno Hecker wrote: To 1.) i'm not shure but it works perfectly with only deleting the _auth. The main question is - what gives us MORE performance (delete the values or create a new instance). The clean solution is, to create a new instance - that's right. Performance aside, a new instance is the way to go - it may spare us a lot of complications in the future. -Johan
Re: Bugs in qpsmtpd-prefork + some features + patches
Hanno Hecker wrote: To 1.) i'm not shure but it works perfectly with only deleting the _auth. The main question is - what gives us MORE performance (delete the values or create a new instance). The clean solution is, to create a new instance - that's right. Performance aside, a new instance is the way to go - it may spare us a lot of complications in the future. -Johan -- Johan Almqvist Christies gate 34 A 0557 Oslo Norway Mobile: +47 40 04 68 21 E-mail: [EMAIL PROTECTED]
Re: Bugs in qpsmtpd-prefork + some features + patches
Hi Stefan, [Cc: to ML again] On Fri, 31 Aug 2007 13:58:07 +0200 Stefan Priebe <[EMAIL PROTECTED]> wrote: > To 1.) i'm not shure but it works perfectly with only deleting the > _auth. The main question is - what gives us MORE performance (delete the > values or create a new instance). The clean solution is, to create a new > instance - that's right. > What is about 2.) you've nothing written about it. This is one of the > most important things. Haven't looked at it, yet :) If someone else says it's ok... > To 3.) OK => wonderful. > > To 4.) perhaps the new instance also solves this problem. I don't > remember the failure - i only know that it was TLS related. And the > connection fails after TLS command. > > And what do you think about 5.) ? We should find out what the majority thinks about this change. Hanno
Re: Bugs in qpsmtpd-prefork + some features + patches
Hello! I'm very happy, that someone reads my post :-) To 1.) i'm not shure but it works perfectly with only deleting the _auth. The main question is - what gives us MORE performance (delete the values or create a new instance). The clean solution is, to create a new instance - that's right. What is about 2.) you've nothing written about it. This is one of the most important things. To 3.) OK => wonderful. To 4.) perhaps the new instance also solves this problem. I don't remember the failure - i only know that it was TLS related. And the connection fails after TLS command. And what do you think about 5.) ? Stefan
Re: Bugs in qpsmtpd-prefork + some features + patches
On Thu, 30 Aug 2007 08:50:40 +0200
Stefan Priebe <[EMAIL PROTECTED]> wrote:
> So here are some problems and the solutions ( the patches may not apply
> cleanly to the actual 0.40 version):
>
>
> 1.) You get some errors with the auth mechanism if an old process has
> used auth the value seems not to be destroyed. So add
> "delete($qpsmtpd->{_auth});"
> in function new_child before my ($client, $iinfo) = $d->accept().
> otherwise some other hosts are authorized even if they don't authourize
> at all.
What about $qpsmtpd->{_auth_user}, $qpsmtpd->{_auth_mechanism}?
Do we want to reset the $qpsmtpd->command_counter, too (AKA
$qpsmtpd->{_counter})?
I'd rather not fix it this way if possible. Better would be to create a
new instance of $qpsmtpd when a new client connects. Else we'll be hunting bugs
like these if something like $qpsmtpd->{_auth} is added to the core.
[...]
> 3.) If you would like to run more than one qpsmtpd-prefork on a single
> machine you get a problem with the shared memory so my patch:
> # setup shared memory
> -$chld_shmem = shmem("qpsmtpd", 1);
> +$chld_shmem = shmem($d_port."qpsmtpd", 1);
> untie $chld_shmem;
>
> @@ -470,7 +485,7 @@ sub shmem_opt {
>
> my ($chld_shmem, $chld_busy);
> eval {
> -$chld_shmem = &shmem("qpsmtpd", 0);#connect to shared
> memory hash
> +$chld_shmem = &shmem($d_port."qpsmtpd", 0);#connect to
> shared memory hash
>
> if (tied %{$chld_shmem}) {
> # perform options
Seems to be ok, if noone speaks against it, I'll commit tomorrow. There
should be a note in the docs then, that you can't use two ports like
10025 and 10026:
The association between variables in distinct processes is
provided by GLUE. This is an integer number or 4 character
string[1] that serves as a common identifier for data across
process space.
> 4.) The only last problem i have is, that if i don't exit the child if
> it has done a TLS connection a second or third TLS connection mostly fail.
Any (verbose) logs about the failed connections?
