Re: [exim-dev] Test suite status for 4.77

On 2011-10-09 at 23:55 -0400, Phil Pennock wrote:
> On 2011-10-09 at 23:44 -0400, Phil Pennock wrote:
> > On balance, I'm not holding up the 4.77 release for 0152, just 0437
> > until I understand what is happening.
>
> > * Basic/0437 queue run: close lookups before delivery
> > ----------------------------8< cut here >8------------------------------
> > ** Command 1 ("exim", starting at line 2)
> > ** Return code 1 (expected 0)
> > show stdErr, show stdOut, Continue (without file comparison), or Quit? [Q] e
> > 2011-10-10 02:35:23 failed to expand spool_directory "[...]/${lookup{spool}lsearch{[...]/aux-fixed/0437.ls}}": unknown lookup type "lsearch"
> > ----------------------------8< cut here >8------------------------------

Isolated. So:
* this is a bug
Yes, the test-suite caught a real regression!
And we've only now noticed. :(
* need to consider security ramifications carefully
* it was broken in 4.76 too
* so proceeding with release

The reason the bug disappeared with debugging turned on is that
show_whats_supported() calls init_lookup_list() and does so _before_ the
call to readconf_main().

In the non-debugging case, we call init_lookup_list() after dropping all
privileges.

3447 /* Read the main runtime configuration data; this gives up if there
3448 is a failure. It leaves the configuration file open so that the subsequent
3449 configuration data for delivery can be read if needed. */
3450
3451 readconf_main();

3635 /* Initialise lookup_list
3636 If debugging, already called above via version reporting.
3637 This does mean that debugging causes the list to be initialised while root.
3638 This *should* be harmless -- all modules are loaded from a fixed dir and
3639 it's code that would, if not a module, be part of Exim already. */
3640 init_lookup_list();

That comment comes to us courtesy of me, in 6545de78 on 2011-01-21.
Reported as Exim 4.76 PP/02. At that point, I had added the
init_lookup_list() call to the debugging output. The position of the
regular init_lookup_list() did not change in that commit.

That position comes from David's change of 2011-01-05 in commit
e6d225ae, which added loadable lookup modules.

I *believe* that my comment at 3635 is correct and we can safely move
the init_lookup_list() call to before readconf_main(), so that
expandable options used in the base config can use lookups too. I'm not
seeing a security impact in doing so. We always used to have lookups
usable from the very start, and we do carefully check permissions on the
directory from which we load lookup modules.

Nonetheless, it's just risky enough that I'm not making the change now.
I'm going to proceed with cutting 4.77 with this known issue, while we
investigate and decide on the correct course of action for lookups like
this.

Note that this does mean that nobody using 4.76 was caught by this and,
presumably, none of the people who've been using packages where the
module support was maintained out-of-tree have ever been caught by it.

This affects:
* spool_directory
* log_file_path
* pid_file_path
* localhost_number
* uid lists and gid lists
all from readconf.c; exim.c does not expand any strings between
readconf_main() and init_lookup_list().

The likelihood of any sane configuration using lookups to set the values
of those files is somewhat low. The closest I see to being likely is
someone using the hostname to key into a file for looking up
localhost_number, in a cluster.

I suspect that we'll just move init_lookup_list() to before
readconf_main() before 4.78.

-Phil