[Bug 1738412] Re: Init script fails test on reload/restart because of faulty regex
Andreas Hasenack
andreas at canonical.com
Tue Oct 30 13:21:07 UTC 2018
** Description changed:
[Impact]
The squid initscript has a guard against configuration mistakes that prevents the service from being disrupted if the current config has an invalid setting.
This guard relies on the "squid -k parse" command which analyzes the
configuration and, in the case of a fatal problem, outputs the string
"FATAL: <reason>". The initscript parses that output to catch such
errors before further action is taken.
There is a mistake in the expression that is looked for, though: instead
of "FATAL: ", the initscript is looking for "FATAL " (i.e., no ":"). The
consequence is that actions that would reload or restart the service end
up shutting the service down in the case of a configuration error.
The change fixes the expression that is looked for, restoring the
functionality of the guard.
[Test Case]
* install squid:
sudo apt update && sudo apt install squid -y
* confirm the reload action is quick to return and doesn't change the pid of squid, i.e., it's not restarted:
ubuntu at xenial-squid-reload-syntax:~$ pidof squid
2684
ubuntu at xenial-squid-reload-syntax:~$ sudo service squid reload
ubuntu at xenial-squid-reload-syntax:~$ pidof squid
2684
ubuntu at xenial-squid-reload-syntax:~$
* add an invalid setting to the config file:
echo "acl nonsense nonsense nonsense" | sudo tee -a /etc/squid/squid.conf
* reload squid one more time:
sudo service squid reload
* After about 30s, squid should be dead and service squid status should show errors:
ubuntu at xenial-squid-reload-syntax:~$ pidof squid
ubuntu at xenial-squid-reload-syntax:~$
ubuntu at xenial-squid-reload-syntax:~$ sudo service squid status
● squid.service - LSB: Squid HTTP Proxy version 3.x
(...)
Oct 29 19:56:26 xenial-squid-reload-syntax squid[2684]: Squid Parent: (squid-1) process 2881 started
Oct 29 19:56:26 xenial-squid-reload-syntax squid[2684]: Squid Parent: (squid-1) process 2881 exited with status 1
Oct 29 19:56:26 xenial-squid-reload-syntax squid[2684]: Squid Parent: (squid-1) process 2881 will not be restarted due to repeated, frequent failures
Oct 29 19:56:26 xenial-squid-reload-syntax squid[2684]: Exiting due to repeated, frequent failures
With the fixed package, a reload action (for example) will flag the
error, and keep the service running:
ubuntu at xenial-squid-reload-syntax:~$ pidof squid
3801
ubuntu at xenial-squid-reload-syntax:~$ sudo service squid reload
Job for squid.service failed because the control process exited with error code. See "systemctl status squid.service" and "journalctl -xe" for details.
ubuntu at xenial-squid-reload-syntax:~$ echo $?
1
ubuntu at xenial-squid-reload-syntax:~$ pidof squid
3801
And the status message will be much clearer:
ubuntu at xenial-squid-reload-syntax:~$ sudo service squid status
● squid.service - LSB: Squid HTTP Proxy version 3.x
...
Oct 29 20:13:26 xenial-squid-reload-syntax systemd[1]: Reloading LSB: Squid HTTP Proxy version 3.x.
Oct 29 20:13:26 xenial-squid-reload-syntax squid[3920]: * FATAL: Invalid ACL type 'nonsense'
Oct 29 20:13:26 xenial-squid-reload-syntax squid[3920]: FATAL: Bungled /etc/squid/squid.conf line 7897: acl nonsense nonsense nonsense
Oct 29 20:13:26 xenial-squid-reload-syntax systemd[1]: squid.service: Control process exited, code=exited status=3
Oct 29 20:13:26 xenial-squid-reload-syntax systemd[1]: Reload failed for LSB: Squid HTTP Proxy version 3.x.
[Regression Potential]
+ This changes not only the reload action, but also start and consequently restart. The fix makes an existing safety net around those actions actually work, instead of being ignored.
+ Due to the lack of an explicit restart action in systemd, however, the restart guard in the SysV initscript doesn't take effect. If there is a bad config, and squid is restarted, the service will be in a stopped state at the end:
+ - stop will succeed
+ - start will fail: service remains stopped
- * discussion of how regressions are most likely to manifest as a result
- of this change.
+ There is an upstream bug about it:
+ https://github.com/systemd/systemd/issues/2175
- * It is assumed that any SRU candidate patch is well-tested before
- upload and has a low overall risk of regression, but it's important
- to make the effort to think about what ''could'' happen in the
- event of a regression.
+ This is not a regression, however, as it's the same behavior as before.
+ We can't add the safety net around stop, and there is no way that I know
+ of to discover if the stop action being carried out is part of a restart
+ or not.
- * This both shows the SRU team that the risks have been considered,
- and provides guidance to testers in regression-testing the SRU.
[Other Info]
* Anything else you think is useful to include
* Anticipate questions from users, SRU, +1 maintenance, security teams and the Technical Board
* and address these questions in advance
[Original Description]
This is a very serious issue that got fixed upstream in:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=800341
It is also logged in the Ubuntu changelog as fixed in:
squid3 (3.5.12-1) unstable; urgency=medium
[ Mathieu Parent ]
* Fix FATAL parsing before start/reload/restart (Closes: #800341)
But is in fact not fixed.
When I look in the source package I find two init scripts:
squid3.rc and squid.rc. squid3.rc has the patch while squid.rc does not.
The one being included in the package and deployed is the one that does
not have the fix.
I'm including a patch to fix this issue.
Please push this out ASAP.
--
You received this bug notification because you are a member of Ubuntu
Server, which is subscribed to the bug report.
https://bugs.launchpad.net/bugs/1738412
Title:
Init script fails test on reload/restart because of faulty regex
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/squid3/+bug/1738412/+subscriptions
More information about the Ubuntu-server-bugs
mailing list