tree 194ba22f5fee67be939a6d08d07b6ac831754ead
parent bcfe23a7ecb212ca07dfbe20423684944d55e086
author Willy Tarreau <w@1wt.eu> 1492593847 +0200
committer Willy Tarreau <w@1wt.eu> 1492595351 +0200
encoding latin1

MEDIUM: config: don't check config validity when there are fatal errors

Overall we do have an issue with the severity of a number of errors. Most
fatal errors are reported with ERR_FATAL (which prevents startup) and not
ERR_ABORT (which stops parsing ASAP), but check_config_validity() is still
called on ERR_FATAL, and will most of the time report bogus errors. This
is what caused smp_resolve_args() to be called on a number of unparsable
ACLs, and it also is what reports incorrect ordering or unresolvable
section names when certain entries could not be properly parsed.

This patch stops this domino effect by simply aborting before trying to
further check and resolve the configuration when it's already know that
there are fatal errors.

A concrete example comes from this config :

  userlist users :
      user foo insecure-password bar

  listen foo
      bind :1234
      mode htttp
      timeout client 10S
      timeout server 10s
      timeout connect 10s
      stats uri /stats
      stats http-request auth unless { http_auth(users) }
      http-request redirect location /index.html if { path / }

It contains a colon after the userlist name, a typo in the client timeout value,
another one in "mode http" which cause some other configuration elements not to
be properly handled.

Previously it would confusingly report :

  [ALERT] 108/114851 (20224) : parsing [err-report.cfg:1] : 'userlist' cannot handle unexpected argument ':'.
  [ALERT] 108/114851 (20224) : parsing [err-report.cfg:6] : unknown proxy mode 'htttp'.
  [ALERT] 108/114851 (20224) : parsing [err-report.cfg:7] : unexpected character 'S' in 'timeout client'
  [ALERT] 108/114851 (20224) : Error(s) found in configuration file : err-report.cfg
  [ALERT] 108/114851 (20224) : parsing [err-report.cfg:11] : unable to find userlist 'users' referenced in arg 1 of ACL keyword 'http_auth' in proxy 'foo'.
  [WARNING] 108/114851 (20224) : config : missing timeouts for proxy 'foo'.
     | While not properly invalid, you will certainly encounter various problems
     | with such a configuration. To fix this, please ensure that all following
     | timeouts are set to a non-zero value: 'client', 'connect', 'server'.
  [WARNING] 108/114851 (20224) : config : 'stats' statement ignored for proxy 'foo' as it requires HTTP mode.
  [WARNING] 108/114851 (20224) : config : 'http-request' rules ignored for proxy 'foo' as they require HTTP mode.
  [ALERT] 108/114851 (20224) : Fatal errors found in configuration.

The "requires HTTP mode" errors are just pollution resulting from the
improper spelling of this mode earlier. The unresolved reference to the
userlist is caused by the extra colon on the declaration, and the warning
regarding the missing timeouts is caused by the wrong character.

Now it more accurately reports :

  [ALERT] 108/114900 (20225) : parsing [err-report.cfg:1] : 'userlist' cannot handle unexpected argument ':'.
  [ALERT] 108/114900 (20225) : parsing [err-report.cfg:6] : unknown proxy mode 'htttp'.
  [ALERT] 108/114900 (20225) : parsing [err-report.cfg:7] : unexpected character 'S' in 'timeout client'
  [ALERT] 108/114900 (20225) : Error(s) found in configuration file : err-report.cfg
  [ALERT] 108/114900 (20225) : Fatal errors found in configuration.

Despite not really a fix, this patch should be backported at least to 1.7,
possibly even 1.6, and 1.5 since it hardens the config parser against
certain bad situations like the recently reported use-after-free and the
last null dereference.