[Opendnssec-develop] Making ODS immune for localtime changes

[Rick van Rein]

Hello,

When I asked whether OpenDNSSEC was immune for time warps such as the change
between summer time and winter time, I did not get a conclusive answer.  I've
always been weary about the plethory of time-related functions in the UNIX API,
and expect others are too, so I decided to dive into it.  Please expand or
correct as you see fit.


First, aside from neutral time interval types, the types that matter are
time_t and struct tm.  The first is seconds-since-epoch, and is UTC-only,
the latter can hold timezone-specific information.  So, anything that maps
from time_t to struct tm is suspicious.  I made a list of all the functions
related to time that I could find in the UNIX API, and ended up with

asctime                 NEUTRAL
asctime_r               NEUTRAL
ctime                   BAD
ctime_r                 BAD
difftime                GOOD
gettimeofday            BAD
gmtime                  GOOD
gmtime_r                GOOD
localtime               BAD
localtime_r             BAD
mktime                  SILLY
settimeofday            BAD
strftime                NEUTRAL
strptime                NEUTRAL
time                    GOOD
timegm                  GOOD
timelocal               BAD
tzset                   BAD
utimes                  UNRELATED
utime                   UNRELATED

I scored them as
 * BAD if a function introduces time zone information
 * GOOD if a function does not introduce time zone information
 * SILLY if it does not sound like we'd want to use it in OpenDNSSEC
 * NEUTRAL if they did nothing with time zone information
 * UNRELATED if the function deals with other things than absolute time

After doing this, I did a grep for BAD and SILLY functions:

/usr/local/src/opendnssec-1.4.0b1# rgrep '\<ctime\|ctime_r\|gettimeofday\|localtime\|localtime_r\|mktime\|settimeofday\|timelocal\|tzset\>' * | grep '[^:]*\.c:'
enforcer/test/cunit/test_datetime.c:    (void) localtime_r(&curtime, &time1);
enforcer/test/cunit/test_datetime.c:    (void) localtime_r(&curtime, &time2);
enforcer/test/cunit/test_datetime.c:    (void) localtime_r(&curtime, &time1);
enforcer/test/cunit/test_datetime.c:    (void) localtime_r(&curtime, &time2);
enforcer/ksm/datetime.c:    ptr = localtime_r(&curtime, datetime);
enforcer/ksm/datetime.c:                t1 = mktime(&tm1);
enforcer/ksm/datetime.c:                t2 = mktime(&tm2);
enforcer/enforcerd/enforcer.c:  if (gettimeofday(&tv, NULL)) {
enforcer/enforcerd/enforcer.c:    if (gettimeofday(&tv, NULL)) {
libhsm/src/bin/hsmspeed.c:    gettimeofday(&start, NULL);
libhsm/src/bin/hsmspeed.c:    gettimeofday(&end, NULL);
signer/src/wire/netio.c: * Retrieve the current time (using gettimeofday(2)).
signer/src/wire/netio.c:        if (gettimeofday(&current_timeval, NULL) == -1) {
signer/src/wire/netio.c:                "gettimeofday() failed (%s)", netio_str,
signer/src/shared/duration.c:    tmp = localtime(&t);
signer/src/shared/duration.c:        ods_log_error("[%s] time_datestamp: localtime() failed", duration_str);
signer/src/shared/locks.c:#include <sys/time.h> /* gettimeofday() */
signer/src/shared/locks.c:#include <time.h> /* gettimeofday() */
signer/src/shared/locks.c:    if (gettimeofday(&tv, NULL) != 0) {
signer/src/shared/log.c:    (void) ctime_r(&now, nowstr);
signer/src/scheduler/task.c:        strtime = ctime(&task->when);
signer/src/scheduler/task.c:        strtime = ctime(&task->when);
signer/src/scheduler/task.c:        strtime = ctime(&task->when);
signer/src/daemon/engine.c:    tzset(); /* for portability */
signer/src/daemon/cmdhandler.c:    strtime = ctime(&now);

Note that I did not incorporate dependent libraries, but I could if this
is thought useful.



If we wanted to clear out our timezone-dependency completely, then the
following (fairly straightforward) replacements would have to be made:


localtime -> gmtime
localtime_r -> gmtime_r
	Trivial, literal replacements.

ctime (t) -> asctime (gmtime (t))
ctime_r (t, b) -> asctime_r (gmtime (t), b)
	Trivial, literal replacements.

mktime -> ???
	enforcer/ksm/datetime.c: Used to differentiate time in DtDateDiff()
		which is harmless if we can rest assured that such times
		are always in UTC.  With the changes above, this should be
		the case, right?

tzset -> ???
	I hope we can drop it if we stick to UTC.

gettimeofday -> ???
	libhsm/src/bin/hsmspeed.c: You'd be silly to speed test accross time warps!
	signer/src/wire/netio.c: Harmless, used for difference of times
	signer/src/shared/locks.c / enforcer/enforcerd/enforcer.c:
		Is the alternate clock_gettime() not omnipresent?
		The time is used for a wait, and might cause hickups when time
		warps at an unhandy time in the functions ods_thread_wait() or
		enforcer_worker_timed_dequeue() -- I cannot tell if this could never
		lead to trouble?  (IOW, what happens if either of these functions
		waits 1h too long, or 1h too short?)


As you can guess, I'd like to propose making OpenDNSSEC independent of timezone
by making the above amendments.  The InceptionOffset will often catch any time
warps due to changes of summer/winter time, but its actual purpose will not be
fulfilled, or be fulfilled with 1h less impact, at time warps.  Also, with the
InceptionOffset less than 1h bad things could happen, like a prepublished ZSK
getting published too late for its new signatures to be accepted by validators.

Note that a similar thing might happen too if an admin was to correct the time
zone setting of a machine running OpenDNSSEC.  Although I'll admit not having
traced the uses of localtime to potential clashes, it can easily be avoided;
scripts around OpenDNSSEC would be a very good reason to do so, I think.

I always setup servers to run in UTC, but I know that not everybody does that.
And I'll frankly admit that I've not considered time warps when planning our
architecture; it might be that others miss it too.  So... I'd prefer OpenDNSSEC
to avoid that users can shoot themselves in the foot on this one :)



I hope this is helpful,


Cheers,
 -Rick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 274 bytes
Desc: Digital signature
URL: <http://lists.opendnssec.org/pipermail/opendnssec-develop/attachments/20121102/93758d03/attachment.bin>