Bug#931085: mailavenger: drops TCP immediately after receiving 'QUIT', without sending the 221 response

2019-06-26 Thread Philip Hands
I thought I had a working autopkgtest in the branch I referred to in the
bug report, but it turns out that it wouldn't work in the salsa-ci
docker environment.

That being the case, I've set up a couple of new branches:

  salsa-ci -- changes needed to get salsa-ci working
  bug-931085-fix -- that plus the bug fix

Here's the original code failing with the unexpected connection closure:

  https://salsa.debian.org/philh/mailavenger/-/jobs/204437

whereas this is it running successfully with the bugfix:

  https://salsa.debian.org/philh/mailavenger/-/jobs/204443

Meanwhile I got in touch with David Mazieres (upstream) referring to
this bug -- given that asmptd is single threaded, my patch is not
actually good enough because the flush will wait for the other end,
which may have already gone away.  The existing code when run on BSD
sends the last packet but doesn't hang around if it doesn't get
acknowledged.  David intends to investigate how to get Linux to do
something similar.  I'd imagine he'd appreciate suggestions.

Now that the two new git branches exist, and are a bit cleaner than the
old ones had become after my attempts to get CI to work, I'll discard
the old branches, so the original links in the report will break -- all
the information is in the new branches anyway.

BTW while doing that, I noticed that the package's systemd postinst
attempts to activate the daemon immediately.  The salsa-ci branch
includes changes to stop this being the case, because as with init.d
this should not occur until the admin has a chance to configure things.
Also, it was breaking the autopkgtest.  My changes regarding than are
not ready for release though.  I should report a new bug regarding this bit.

Cheers, Phil.
-- 
|)|  Philip Hands  [+44 (0)20 8530 9560]  HANDS.COM Ltd.
|-|  http://www.hands.com/http://ftp.uk.debian.org/
|(|  Hugo-Klemm-Strasse 34,   21075 Hamburg,GERMANY



Bug#931085: mailavenger: drops TCP immediately after receiving 'QUIT', without sending the 221 response

2019-06-25 Thread Philip Hands
Package: mailavenger
Version: 0.8.5-1
Severity: normal

Hi Dererk,

It seems that mailavenger does not actually send the '221 {hostname}' message
that it is supposed to (despite showing it doing so in its debugging output).

This appears to make some SMTP clients decide that the email was not sent
(despite having received the '250 ok' after the data, resulting in them
sending the same mail repeatedly.

One can easily confirm this with swaks, thus:

  swaks --from h...@example.com --to th...@example.com --server=my.avenger.host

Which will give you a result which ends with something like this:

=-=-=-=-
...
 -> This is a test mailing
 -> 
 -> 
 -> .
<-  250 ok
 -> QUIT
*** Remote host closed connection unexpectedly.
=-=-=-=-

The thing that makes this "unexpected" is that the 221 response is missing.

This would seem to be because the IO is not being flushed when the socket
is closed.  This can be fixed by adding a flush, as you can see in this
git branch:

  https://salsa.debian.org/philh/mailavenger/tree/premature-close-bug-fixed

Specifically in this patch file:

  
https://salsa.debian.org/philh/mailavenger/blob/premature-close-bug-fixed/debian/patches/flush-aio-to-force-sending-of-farewell-p.patch

(I've attached that patch to this report, just for completeness)

It would not surprise me if there's a better way to do this, but this
does at least work.  The reason I suspect that there's a better way is
that presumably there are other places where the connection is being
dropped with unsent messages in the buffers, so it probably ought to be
fixed in the code responsible for bringing the connection down.

The result of building with this patch is currently running on my smtp
server, so one can demonstrate the correct behaviour that this results
in with telnet, thus:

=-=-=-=-
phil@whist  ~ % telnet mail.hands.com 25
Trying 78.129.164.123...
Connected to mail.hands.com.
Escape character is '^]'.
220 free.hands.com ESMTP
quit
221 free.hands.com
Connection closed by foreign host.
=-=-=-=-

Note the line "221 free.hands.com" -- this line is missing when running the 
0.8.5-1 version.

BTW also in that salsa repo, I've added a configuration to run salsa-CI,
as well as the beginnings of an autopkgtest test.  The autopkgtest does
work when run locally, but seems not to work within docker as in the
salsa-CI setup, presumably because networking is different within the
container -- I'll fix that if I can work out what's up.

The following system information is from my laptop, which exhibits the
same symptoms as the server, but may well have more recent packages.
Feel free to ask for the info from the server if you think it's important.

Cheers, Phil.

-- System Information:
Debian Release: 10.0
  APT prefers testing
  APT policy: (500, 'testing'), (500, 'stable'), (300, 'unstable')
Architecture: amd64 (x86_64)

Kernel: Linux 4.19.0-5-amd64 (SMP w/4 CPU cores)
Kernel taint flags: TAINT_DIE, TAINT_WARN, TAINT_OOT_MODULE, 
TAINT_UNSIGNED_MODULE
Locale: LANG=en_GB.utf8, LC_CTYPE=en_GB.utf8 (charmap=UTF-8), 
LANGUAGE=en_GB.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages mailavenger depends on:
ii  adduser 3.118
ii  dma [mail-transport-agent]  0.11-1+b1
ii  libc6   2.28-10
ii  libdb5.35.3.28+dfsg1-0.5
ii  libgcc1 1:8.3.0-6
ii  libpcap0.8  1.8.1-6
ii  libssl1.1   1.1.1c-1
ii  libstdc++6  8.3.0-6
ii  lsb-base10.2019051400

mailavenger recommends no packages.

mailavenger suggests no packages.

-- no debconf information
From: Philip Hands 
Date: Tue, 25 Jun 2019 12:32:00 +0200
X-Dgit-Generated: 0.8.5-1.1~hands1 aeafef4798f48117120eb6e9eef9d11c77ee2dfb
Subject: flush aio, to force sending of farewell packet

I suspect that there's a better way to do this, as it seems
to me that the library should know to flush all IO when the
socket is closed.  However this is a trivial change, and it
does work.

---

--- mailavenger-0.8.5.orig/asmtpd/smtpd.C
+++ mailavenger-0.8.5/asmtpd/smtpd.C
@@ -889,6 +889,7 @@ smtpd::getcmd (str line, int err)
   }
   else if (cmd == "quit") {
 aio << "221 " << opt->hostname << "\r\n";
+aio->flush ();
 delete this;
   }
   else if (cmd == "noop")