Bug#933948: Config value incorrectly parsed in init.d script

2020-12-27 Thread Norbert Harrer

Hi Markus.

I replaced my init scripts with the versions from your merge. And tried 
with various combinations of spaces and quotes. Works very well. Great 
regex/escape-fu.


Best Regads,
Norbert.

On 27.12.2020 14:32, Markus Wanner wrote:

Control: -1 severity grave
Control: -1 tags +pending

Hi,

On 11/21/20 3:46 PM, Norbert Harrer wrote:
> But in /etc/init.d/courier-mta-ssl it is parsed with sed:
>
> DO_START=$(sed -ne 's/^ESMTPDSSLSTART=\([^[:space:]]*\)/\1/p'
> /etc/courier/esmtpd-ssl | tr "A-Z" "a-z")

thanks a lot for this fine analysis.  I propose to tweak the regular 
expressions to be more resilient to quotes (single and double) as well 
as spaces (which deviates from how the shell would interpret it, though).


Please review the following merge request:
https://salsa.debian.org/debian/courier/-/merge_requests/8

Note 2: Another thing is curious: Not all courier services use the 
init.d

scripts, some have their own systemd config in /lib/systemd/system/
which bypass the init.d script


Yeah, this is unfortunate and should be addressed as well.  Thanks for 
pointing it out.


Best Regards

Markus


--
Dipl.Ing. Norbert Harrer | Software Engineer | (+43) 0699 10926825 
|nhar...@equilaris.at



Bug#933948: Config value incorrectly parsed in init.d script

2020-12-27 Thread Markus Wanner

Control: -1 severity grave
Control: -1 tags +pending

Hi,

On 11/21/20 3:46 PM, Norbert Harrer wrote:
> But in /etc/init.d/courier-mta-ssl it is parsed with sed:
>
> DO_START=$(sed -ne 's/^ESMTPDSSLSTART=\([^[:space:]]*\)/\1/p'
> /etc/courier/esmtpd-ssl | tr "A-Z" "a-z")

thanks a lot for this fine analysis.  I propose to tweak the regular 
expressions to be more resilient to quotes (single and double) as well 
as spaces (which deviates from how the shell would interpret it, though).


Please review the following merge request:
https://salsa.debian.org/debian/courier/-/merge_requests/8


Note 2: Another thing is curious: Not all courier services use the init.d
scripts, some have their own systemd config in /lib/systemd/system/
which bypass the init.d script


Yeah, this is unfortunate and should be addressed as well.  Thanks for 
pointing it out.


Best Regards

Markus



Bug#933948: Config value incorrectly parsed in init.d script

2020-11-21 Thread Norbert Harrer

I don't think it's systemd's fault. There is a sub-optimal sed regex in
most courier init.d scripts. Here is a scenario with courier-mta-ssl as
an example (though, it also happens to other parts like courier-imap-ssl):

Usually, the config file /etc/courier/esmtpd-ssl contains:

ESMTPDSSLSTART=YES

And everything works fine. However, when you save the config through the
admin webpage, it changes it to:

ESMTPDSSLSTART="YES"

This should be fine as well, since the shell command executed by the
admin page parses it just the same:

> /usr/sbin/esmtpd-ssl stop ; . /etc/courier/esmtpd-ssl ; test 
"$ESMTPDSSLSTART" != YES || /usr/sbin/esmtpd-ssl start


But in /etc/init.d/courier-mta-ssl it is parsed with sed:

DO_START=$(sed -ne 's/^ESMTPDSSLSTART=\([^[:space:]]*\)/\1/p' 
/etc/courier/esmtpd-ssl | tr "A-Z" "a-z")


Which leads to DO_START containing the value "yes" including the quotes.
Further down the line in /usr/lib/courier/init-d-script-courier this
causes the script to exit quietly, if DO_START is anything other then yes.
After replacing the line with:

DO_START=$(sed -ne 's/^ESMTPDSSLSTART=\W*\(\w\+\)\W*/\1/p' 
/etc/courier/esmtpd-ssl | tr "A-Z" "a-z")


"systemctl restart courier-mta-ssl" works again.


Note 1: The ugly thing is, that when you make the changes through the web
interface, everything continues to work fine, because it parses the
config files correctly during restart. Then some time later, when you
restart the machine or restart the service with systemctl, it doesn't
come up again with no hints at all in the log files. I also think it
happens when courier crashes, and systemd tries to restart the service.

Note 2: Another thing is curious: Not all courier services use the init.d
scripts, some have their own systemd config in /lib/systemd/system/
which bypass the init.d script:

> systemctl status `systemctl -t service | grep courier | awk '{ print 
$1; }'` | grep Loaded


   Loaded: loaded (/lib/systemd/system/courier-authdaemon.service; 
enabled; vendor preset: enabled)

   Loaded: loaded (/etc/init.d/courier-imap-ssl; generated)
   Loaded: loaded (/lib/systemd/system/courier-imap.service; enabled; 
vendor preset: enabled)

   Loaded: loaded (/etc/init.d/courier-msa; generated)
   Loaded: loaded (/etc/init.d/courier-mta-ssl; generated)
   Loaded: loaded (/etc/init.d/courier-pop-ssl; generated)
   Loaded: loaded (/lib/systemd/system/courier-pop.service; enabled; 
vendor preset: enabled)

   Loaded: loaded (/etc/init.d/courier; generated)
   Loaded: loaded (/etc/init.d/courierfilter; generated)

So only the services

  courier-imap-ssl
  courier-msa
  courier-mta-ssl
  courier-pop-ssl

are prone to this bug, which gives the whole thing an even more sporadic
character.

Regards,
Norbert