[ISC BIND 9.10.2-P1 and older] "flawed" zone file modification check

[Milos Ivanovic]

Hi bind-users,

I've encountered an edge case that was not considered while developing
the method that BIND uses to check if a zone file has been modified. I
will immediately state that this is an extreme edge case, but
nonetheless one that should (and can) be avoided with minimal change to
the source code.

First I will list steps to reproduce, and then I will explain how I
encountered this scenario and how it may occur for another user, finally
ending with a workaround and suggested fix.

To reproduce:
1. Set the hardware clock to some time in the future
2. Boot the system, including BIND
3. Let NTP fix the time, or fix the time manually
4. Edit a zone, finishing by increasing its serial
5. run `rndc reload yourzone.example.com'
6. receive `zone reload up-to-date',
   or if you have debugging enabled `skipping load: master file older
   than last load' will appear in the logs

As you can see, BIND does not account for clock drift. This will be an
issue for users that have specific (bogus) /etc/adjtime file offsets or
are changing timezones such that it would ultimately result in moving
the system time backwards after having started BIND.

Understandably, the former issue is not exactly BIND's fault; in fact
neither of them are. It's also trivial to work around this scenario:
simply restart BIND and it will no longer have impaired judgement of
time as it will load with the correct system time from the very start.

That being said, this edge case is also trivial to avoid. Instead of
storing the time when each zone was loaded as per line 1794 of dns/zone.c:

    /*
     * Store the current time before the zone is loaded, so that if the
     * file changes between the time of the load and the time that
     * zone->loadtime is set, then the file will still be reloaded
     * the next time dns_zone_load is called.
     */
    TIME_NOW(&loadtime);

    /*
     * Don't do the load if the file that stores the zone is older
     * than the last time the zone was loaded.  If the zone has not
     * been loaded yet, zone->loadtime will be the epoch.
     */
    if (zone->masterfile != NULL) {
        ...

it would be wiser to store the mtime of each zone file instead, and
simply compare the stored mtime with the current mtime when a reload is
requested, alleviating the need to rely on the system time at all. This
gives more certainty that a reload will be granted if a file is touched,
even if that file was modified "in the past" in terms of BIND's start time.

Of course, the best way to verify zone changes is to actually check the
serials, but I understand the current implementation is the way it is
for performance reasons - namely, to avoid parsing all zone files when a
`rndc reload' is issued by older versions or those who do not realise
that this command can be avoided when editing single zones in favour of
the newer `rndc reload yourzone.example.com'.

Thoughts?


--
Milos Ivanovic