Bug#822067: courier-mta: init scripts completely broken

[J Mo]

Package: courier-mta
Version: 0.75.0-18
Severity: grave
Justification: renders package unusable

Recently I decided to upgrade courier (mta and imap) on one of my mail servers. It was a disaster. The quality of these packages is abysmal and dangerous. This is one of the many serious, grave, and critical bugs I ran into during that process.



I have the following courier-related init scritps on my system:

courier-mta: /etc/init.d/courier
courier-authdaemon: /etc/init.d/courier-authdaemon
courier-mta: /etc/init.d/courierfilter
courier-imap: /etc/init.d/courier-imap
courier-imap: /etc/init.d/courier-imap-ssl
courier-mta: /etc/init.d/courier-msa
courier-mta: /etc/init.d/courier-mta
courier-mta: /etc/init.d/courier-mta-ssl



Nearly all of these init scripts are broken in many ways. Almost nothing works. I would be shocked if any of this passes requirements of the Debian policy.

There is no stdout or stderr provided from the init script. Failures result in no output. Success results in no output. There is no error/sanity checking.

stop|status|restart|try-restart|force-reload are all broken.

start is broken on multiple init scripts.

The status argument fails and is not implemented on any of them that I can see.

The stop argument fails to stop running daemons/programs.

The only courier init script which behaves somewhat reasonably is /etc/init.d/courier-authdaemon



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

Kernel: Linux 4.3.0-1-amd64 (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)

Versions of packages courier-mta depends on:
ii courier-authlib 0.66.4-7
ii courier-base 0.75.0-18
ii debconf [debconf-2.0] 1.5.59
ii libc6 2.22-7
ii libcourier-unicode1 1.4-2
ii libgcc1 1:5.3.1-14
ii libgdbm3 1.8.3-13.1
ii libidn11 1.32-3
ii libnet-cidr-perl 0.17-1
ii libperl5.22 5.22.1-10
ii libstdc++6 5.3.1-14
ii sysvinit-utils 2.88dsf-59.3

courier-mta recommends no packages.

Versions of packages courier-mta suggests:
ii bsd-mailx [mail-reader] 8.1.2-0.20160123cvs-2
ii courier-doc 0.75.0-18
ii courier-filter-perl 0.200+ds-4
pn couriergrey <none>
ii emacs24 [mail-reader] 24.5+1-6+b2
ii mutt [mail-reader] 1.5.24-1+b1
ii s-nail [mail-reader] 14.8.8-1

-- Configuration Files:
/etc/courier/aliases/system [Errno 13] Permission denied: u'/etc/courier/aliases/system'
/etc/courier/courierd changed [not included]
/etc/courier/dsnheader.txt changed [not included]
/etc/courier/esmtpauthclient [Errno 13] Permission denied: u'/etc/courier/esmtpauthclient'
/etc/courier/esmtpd changed [not included]
/etc/courier/esmtpd-msa changed [not included]
/etc/courier/esmtpd-ssl changed [not included]
/etc/courier/esmtpd.cnf [Errno 13] Permission denied: u'/etc/courier/esmtpd.cnf'
/etc/courier/smtpaccess/default [Errno 13] Permission denied: u'/etc/courier/smtpaccess/default'
/etc/init.d/courier-mta changed [not included]
/etc/init.d/courier-mta-ssl changed [not included]

-- debconf information excluded

[J Mo]

Ondrej

You are misinterpreting the severity of the situation with a personal attack or some hyperbolic raving on my part.

If an auto mechanic gives back an owner's car and it's on fire with the engine and doors missing, it's not unreasonable for the mechanic to get yelled at, no matter what his intentions were.

There is no engine in these init scripts. The bug is that the code is missing. Most of these scritps are four short lines long beyond the common header. There is no code to provide any of the required functions like status, stop, restart, etc. Start works for some of them, but that's it.

Please see the following:

https://wiki.debian.org/LSBInitScripts

https://www.debian.org/doc/debian-policy/ch-opersys.html#s9.3.2



Also all of the man pages have various errors. Man courier and grep for "man.base.url.for.relative.links" in the NOTES section down at the bottom. All of the man pages I looked at had various errors like this.

On 4/21/16 00:40, Ondřej Surý wrote:

Control: tags -1 +moreinfo
Control: severity -1 important

J, you probably should not fill bugs when you are pissed.

This bug report is not a bug report, but a complaint. If you have
anything you want to fix, you should say it. You are also more then
welcome to send patches or join the effort to package Courier MTA in
Debian as you clearly use it and understand the problems with the
packages.

O.

[Ondřej Surý]

Please read /lib/init/init-d-script (and
/usr/lib/courier/init-d-script-courier) first and then come back again.

--
Ondřej Surý <ond...@sury.org>
Knot DNS (https://www.knot-dns.cz/) – a high-performance DNS server

[Ondřej Surý]

> If an auto mechanic gives back an owner's car and it's on fire with the
> engine and doors missing, it's not unreasonable for the mechanic to get
> yelled at, no matter what his intentions were.

And this part is seriously wrong.

O.

--
Ondřej Surý <ond...@sury.org>
Knot DNS (https://www.knot-dns.cz/) – a high-performance DNS server

On Thu, Apr 21, 2016, at 11:20, J Mo wrote:
>

[J Mo]

Ondrej, I have some questions

Why were the old courier init scripts discarded in favor of using the
init-d-script method/system? Were there some bugs related to them which
needed to get fixed? And, was there a particular reason why the
init-d-script method was used over writing stand-alone init scripts?

Historically, Courier uses it's environment to determine configuration
parameters, which, admittedly, is weird/questionable/ugly. All of those
config files in /etc/courier are just shell fragments specifying
parameters. The old init scripts sourced those parameters into the
environment before execution and passed them down. Additionally, daemons
were customarily executed as part of a pipeline involving courierlogger
and couriertcpd. The new init scripts don't do this. Was that on
purpose, or were you unaware of any of this?

Before I propose any changes, I'd like to find out what your ideas were.

[Ondřej Surý]

Hi J,

On Wed, Apr 27, 2016, at 06:04, J Mo wrote:
> Ondrej, I have some questions
>
> Why were the old courier init scripts discarded in favor of using the init-d-script method/system?

The sysvrc script update were done as part of updating courier with
systemd service files. I understand you are using sysvinit-core on your
system.

> Were there some bugs related to them which needed to get fixed?

Just the need to keep the two init systems in sync and accumulated
baggage in the scripts.

> And, was there a particular reason why the init-d-script method was used over writing stand-alone init scripts?

init-d-script method is almost "declarative", so it was chosen for
overall simplicity.

> Historically, Courier uses it's environment to determine configuration
> parameters, which, admittedly, is weird/questionable/ugly.

There are even more horrible things in the courier upstream package like
having configure.ac in every freaking subdirectory.

> All of those config files in /etc/courier are just shell fragments specifying
> parameters. The old init scripts sourced those parameters into the
> environment before execution and passed them down.

That can still be done, but from what I recall those are sourced by the
shell wrappers around every daemon?

/usr/sbin/esmtpd-msa is launched from /etc/init.d/courier-msa and
sources:

. ${sysconfdir}/esmtpd
. ${sysconfdir}/esmtpd-msa

So I believe this should be covered.

> Additionally, daemons were customarily executed as part of a pipeline involving courierlogger
> and couriertcpd. The new init scripts don't do this. Was that on purpose, or were you unaware of any of this?

Again this is done via upstream wrapper scripts, in
/usr/sbin/esmtpd-msa, I see:

echo ${sbindir}/couriertcpd $TCPDOPTS $PORT \
${sbindir}/courieresmtpd'>/dev/null 2>&1 </dev/null'

So I believe this is taken care by upstream shell wrapper scripts.

> Before I propose any changes, I'd like to find out what your ideas were.

Thank you very much for helping making courier packages better.

If you are going to propose any changes, please try to make them both in
.init and .service files at the same time. Or at least explain, so I can
keep it in sync. Also you know the courier* packages could use a
co-maintainer who runs the packages in the production.

I can definitely help with the packaging bits, but as I don't run
courier myself, and was just asked by some courier users, to help
improving this, I am at severe disadvantage here.

Cheers,

[J Mo]

Most of the problems that I've found so far are actually bugs in
init-d-script. The rest are between init-d-script and
init-d-script-courier _override() functions not replicating needed
functionality.



Before I started looking into this, I had never heard of
/lib/init/init-d-script before.

The first thing I did was to find out who else was using it in their
init scripts. The answer for me turned out to be almost nobody. Of the
nine different hosts I looked at, I could find only one other package
which used it: apache2's apache-htcacheclean, for cleaning up after
mod_cache_disk. And even there it's not really comparable because of the
very limited function it performs. The main apache2 init script doesn't
use init-d-script.

And since nobody apparently uses it, it didn't surprise me when I
started finding bugs. See bug #822674 for our current most significant
issue. Nothing will work until that gets fixed or worked around.

It does not help that PIDFILE is missing from all of the init scripts,
except for courier-authdaemon, where it's wrongly declared as
PIDFILE="/run/courier/pid". Thus, init-d-script is looking for the
PIDFILES in all the wrong locations. This will have to get fixed too,
but won't make a difference because of the above mentioned bug.

The "exit 0" at the end of init-d-script is probably also a bug as it
destroys almost all return codes. I have like five other issues which
might get turned into bugs once I've had a second look at it tomorrow.



Using /etc/init.d/skeleton as a template may have seemed like a
reasonable place to start to write a new init script, but I don't know
if it's worth working around the issues I've found so far. I'm already
using my own init script replacements so things will work.

The Courier suite, being as weird as it is, also makes it a poor
candidate to fit into a per-defined structure like init-d-script provides.



Most of our other issues are that functionality normally expected isn't
in the init-d-script-courier _override() funcs. There's no start/stop
logging, no stdout, etc. That's all fixable though.



I will look at this more later in the week and send you some proposed
changes. I'm not sure if init-d-script should be ditched or not yet. I
like the idea of a uniform init script, but the issues I've found so far
explains why nobody is using it.

On 04/21/2016 02:38 AM, Ondřej Surý wrote:

[Ondřej Surý]

Just a quick reaction - I am fine with rewriting the scripts using
/etc/init.d/skeleton if it's a better fit.

Or we can simply add a custom:

do_status_override() {
status_of_proc "$DAEMON" "$NAME" -p "$PIDFILE"
}

that would implement the missing things.

And I think I found the most obvious error -> I intended to override
do_start_cmd and do_stop_cmd instead of do_start and do_stop functions
in the init-d-script-courier, and that would fix the missing logging and
probably the error codes as well.

Cheers,

--
Ondřej Surý <ond...@sury.org>
Knot DNS (https://www.knot-dns.cz/) – a high-performance DNS server

[J Mo]

Hi

Sorry I've been silent. My linux desktop motherboard died about a week
ago and I had to deal with issues surrounding that.

Here are the init scripts which I am currently using. These work for me:

http://jmomo.net/files/courier-init/

If you find any problems let me know and we can work it out. Keep an eye
out for any bash-isms I might have missed.



Notes/thoughts:

I am not using any other parts of the courier suite. I would not mind
looking at all of those and doing it but I'm not sure I have the time
right now. I'll add it to my TODO list and let you know if I get around
to it.

These don't use the init-d-script method because bugs #822753 and
#822674 break it for us. I don't think init-d-script is usable. I like
the idea but it has multiple serious bugs and is abandoned by it's
authors as far as I can tell. Fixing and testing would take a lot of
work and it's pretty obvious that basic LSB/Debian policy testing was
never done.

I'm not sure what the original motive was for splitting up courier-mta
into courier-mta, courier-msa, courier, and courier-filter, but I kept
it that way. However, this is a behavior change which is worthy of a
NEWS item by itself. People who expect configuration to be reloaded by
using courier-mta are not getting what they used to get.

courierd itself does not create a PID file, so getting status on it is
non-ideal, but it works. This is an upstream issue.

courier-authdaemon status can not be obtained by a non-privileged user
due to directory permissions. This is consistent with prior behavior.

Those init scripts may need to consider if upstart or systemd is in use
on the system and behave appropriately. I don't know what official
policy is on this one. I need to look it up. Added it to my TODO list.

I called the main courier daemon a "scheduler" since that's mostly what
it does but you might want to call it something else? I don't know.

[Ondřej Surý]

I looked at your new init-d scripts and the referenced bugs, and I am
not convinced we need to go to shell spaghetti again, as:

- Re #822753, see #208010 - there's no Debian policy on init.d script
exit codes unfortunately, so init-d-script is compliant.

- Re #822674 - this is easy to fix, by adding do_status_override() in
init-d-script-courier.

So in the end, I think this patch is all that's needed to make
do_status() work:

https://anonscm.debian.org/git/collab-maint/courier.git/commit/?id=0d998590ca2e02a263de14a0147e1fcf6ba871bd

And I added NEWS.Debian to courier-mta package about the split:

https://anonscm.debian.org/git/collab-maint/courier.git/commit/?id=d6aa0d670db053782ffab9c94b6882881ada773d

Please open individual bugs about the init scripts if you find that
something doesn't work as it should as I think the currents script
mirror the behavior of the scripts you sent 1:1.

Cheers,
--
Ondřej Surý <ond...@sury.org>
Knot DNS (https://www.knot-dns.cz/) – a high-performance DNS server

Vše pro chleba (https://vseprochleba.cz) – Potřeby pro pečení chleba
všeho druhu