timedatectl: does not notice corrupted zoneinfo files

[mbiebl]

Submission type

• Request for enhancement (RFE)

systemd version the issue has been seen with

Originally filed against v215 but still seems valid with v238

Used distribution

Debian

From the downstream bug report at
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=767593

Due to local problems, my zoneinfo file was corrupted (content replaced
with garbage). Setting the timezone with timedatectl set-timezone lead
to the timezone being set to UTC, but using the correct name.

root@zion:/etc# timedatectl # correct here.
      Local time: Sam 2014-11-01 11:54:45 CET
  Universal time: Sam 2014-11-01 10:54:45 UTC
        RTC time: Sam 2014-11-01 10:54:45
       Time zone: CET (CET, +0100)
     NTP enabled: yes
NTP synchronized: yes
 RTC in local TZ: no
      DST active: no
 Last DST change: DST ended at
                  Son 2014-10-26 02:59:59 CEST
                  Son 2014-10-26 02:00:00 CET
 Next DST change: DST begins (the clock jumps one hour forward) at
                  Son 2015-03-29 01:59:59 CET
                  Son 2015-03-29 03:00:00 CEST
root@zion:/etc# ls -la timezone localtime
lrwxrwxrwx 1 root root 25 Nov  1 11:50 localtime -> 
../usr/share/zoneinfo/CET
-rw-r--r-- 1 root root  4 Nov  1 11:50 timezone
root@zion:/etc# timedatectl set-timezone Europe/Vienna
root@zion:/etc# date
Sam Nov  1 10:54:56 UTC 2014
root@zion:/etc# timedatectl # "valid", but not what's intended
      Local time: Sam 2014-11-01 10:55:01 UTC
  Universal time: Sam 2014-11-01 10:55:01 UTC
        RTC time: Sam 2014-11-01 10:55:01
       Time zone: Europe/Vienna (UTC, +0000)
     NTP enabled: yes
NTP synchronized: yes
 RTC in local TZ: no
      DST active: n/a
root@zion:/etc# ls -la timezone localtime

Given that zoneinfo files have an obvious magic number and the garbage
file hadn't one, it'd be really great if timedatectl could do basic
sanity checks before loading the file and creating an "invalid"
configuration.

[yuwata]

@mbiebl Could you explain what kind of check should we do? Also, could you provide the broken zoneinfo? I cannot reproduce this on Fedora 28 (tzdata-2018d). The following is the output of timedatectl on recent snapshot. It seems fine for me.

$ sudo timedatectl set-timezone Europe/Vienna
$ timedatectl
               Local time: Wed 2018-05-09 15:53:19 CEST
           Universal time: Wed 2018-05-09 13:53:19 UTC
                 RTC time: Wed 2018-05-09 13:53:18
                Time zone: Europe/Vienna (CEST, +0200)
System clock synchronized: yes
              NTP service: active
          RTC in local TZ: no

[mbiebl]

@yuwata (keep in mind that I'm only forwarding this bug report, I'm not the original bug reporter)

It is suggested in the bug report, that zoneinfo files have a magic number, which timedatectl/timedated could check for, before applying any (incorrect) changes.

[mbiebl]

to create a garbage file, it should be as simple as running

echo $RANDOM > /usr/share/zoneinfo/Europe/Vienna

and the rerun your test above (make a copy of the correct file beforehand)

[mbiebl]

As for the magic number, see
http://man7.org/linux/man-pages/man5/tzfile.5.html

       The timezone information files used by tzset(3) are typically found
       under a directory with a name like /usr/share/zoneinfo.  These files
       begin with a 44-byte header containing the following fields:

       * The magic four-byte ASCII sequence “TZif” identifies the file as a
         timezone information file.

[yuwata]

@mbiebl Thank you for the clarification. But, now I am skeptical that we should really support such a case... The original bug report seems not ours.

[mbiebl]

Huh, I don't understand, can you elaborate?

[poettering]

timedatectl and timedated do not parse the timezone data, and we generally assume that data in /usr is in a good state. I mean, we use all kinds of files all over the place in /usr and do not validate it first, under the assumption that /usr is vendor supplied and hence in a good state.

If your package manager corrupts files in /usr then this appears to be something to fix in the package manager, no?

I mean, it's basically as if you are asking us before we run a shell script we go and open the script first, read the shebang line, validate that it refers to a valid ELF binary and so on. But this is not how we generally do stuff on Linux.

I am quite puzzled about this bug report, really, and don't grok what this is about?

[poettering]

And to make this clear: it's glibc that parses the zone files, not systemd. If you think the parser is not good in glibc, please file a bug in glibc.

timedatectl ultimately just shows the data glibc's tzset() makes available, that's all. If tzset() is broken, then please work with glibc to fix it.

[floppym]

I suspect the glibc time functions already do a magic number check and default to UTC if the data is bad. That seems consistent with the output timedatectl produces in this instance.

[mbiebl]

I'm puzzled by the responses. I don't think anywhere in the bug report it was mentioned, that the package manager did corrupt the timezone data files (If I might guess, it's more likely a file system issue).
I also don't get the remark regarding the timezone parser.

Remember, that I'm passing this bug report along. But for me it was quite obvious:
Before timedatectl set-timezone bla sets a new timezone, it should do some basic sanity checks, before potentially setting it to a broken configuration. Such a sanity check could be, to look for the magic byte marker in the timezone file.

[mbiebl]

I mean, we already do some basic sanity checks, like:

# timedatectl set-timezone Europe/Buxtehude
Failed to set time zone: Invalid time zone 'Europe/Buxtehude'

This would just be another safe-guard.

[floppym]

I was going to submit a patch to implement this, but I think I have decided against it.

The current code in timezone_is_valid basically just stats /usr/share/zoneinfo/Europe/Buxtehude. It would mainly catch typos.

Opening the file and checking its contents to ensure the file isn't corrupted is kind of a leap. I think the file format is specific to glibc, and its format does change occasionally. It's unlikely the magic number would change, but checking that doesn't even guarantee the file is valid; the rest of the file could very well be garbage.

If glibc provides some function that could be used to validate the timezone, I think that would make a lot more sense.

[keszybz]

Agreed, we shouldn't try to second-guess glibc and parse the format ourselves.
A quick search does not show up any glibc function to do the check.
Let's close this. If glibc grows and api to do the check, we can revisit that.

[poettering]

Interestingly though there's now a manpage tzfile(5) that formally documents the format... (or maybe that was always there?)

[keszybz]

There's "version 1 format", "version 2 format", "version 3 format". Hence, "version 4 format" might appear in the future. So even if we encountered a file that we don't understand, I don't think we could reject it. If glibc gave us a verification function, things would be different.

[mbiebl]

@keszybz are you concerned that a potential future version 4 format might drop the 4 byte magic sequence?
That is a possibility, but seems highly unlikely, given that the tzfile format is formally documented and glibc is usually pretty conservative when it comes to breaking backwards compat.
Again, I would only check for the 4 byte magic sequence on the beginning of the file and not do any further validation. While an explicit libc API for that would be nice, it seems we make the perfect the enemy of the good here.

[keszybz]

Sure, we can do that. If you feel it's worth it... Protecting against random corruptions in the file system is rather futile imho, but a check like this probably wouldn't hurt.