formats icon indicating copy to clipboard operation
formats copied to clipboard

tai64: wrong results -- 10s is not right offset and doesn't account for past or future leap seconds in SystemTime conversion

Open programmerjake opened this issue 2 years ago • 9 comments

The offset between UTC time and TAI time isn't currently 10s, it's 37s and may change to something other than 37s in the future.

The current code assumes the offset is always 10s.

programmerjake avatar Jun 17 '22 06:06 programmerjake

comparison on my system (after installing ntp so linux has the correct offset): note how the result from linux and Tai64N::now() differ -- linux: 1655618465 vs. tai64: 1655618438 -- SystemTime: 1655618428 code:

use tai64::Tai64N;
use std::time::SystemTime;
use libc::{clock_gettime, CLOCK_TAI, timespec};

fn now_tai() -> std::io::Result<timespec> {
    let mut retval = timespec { tv_sec: 0, tv_nsec: 0 };
    let r = unsafe { clock_gettime(CLOCK_TAI, &mut retval) };
    if r != 0 {
        Err(std::io::Error::last_os_error())
    } else {
        Ok(retval)
    }
}

fn main() {
    let t = dbg!(Tai64N::now());
    let t = (t.0.0 as i64).wrapping_sub(1 << 62);
    dbg!(t);
    dbg!(SystemTime::now());
    let tai = now_tai().unwrap();
    dbg!(tai.tv_sec);
    dbg!(tai.tv_nsec);
}

output:

[src/main.rs:16] Tai64N::now() = Tai64N(
    Tai64(
        4611686020083006342,
    ),
    259280636,
)
[src/main.rs:18] t = 1655618438
[src/main.rs:19] SystemTime::now() = SystemTime {
    tv_sec: 1655618428,
    tv_nsec: 259298279,
}
[src/main.rs:21] tai.tv_sec = 1655618465
[src/main.rs:22] tai.tv_nsec = 259305372

programmerjake avatar Jun 19 '22 06:06 programmerjake

See also: https://lists.zx2c4.com/pipermail/wireguard/2022-June/007657.html

programmerjake avatar Jun 19 '22 06:06 programmerjake

The implementation is indeed naive/broken. I thought we had an open tracking issue for this, but apparently not.

I'm not sure the best way to handle this, especially given the existing published crates.

tarcieri avatar Jun 20 '22 14:06 tarcieri

how about downloading the official leap-second table and converting it to rust in a build.rs? you could have that enabled by an enabled-by-default feature.

apparently an official source is https://www.ietf.org/timezones/data/leap-seconds.list

programmerjake avatar Jun 20 '22 16:06 programmerjake

I'd probably suggest against making network connections from a build script, especially in any security-related package (indeed there are renewed efforts underway to run build scripts under WASM).

However, I think it would be okay to use AOT conversion of the table, leveraging the build script as a sort of "time bomb" which makes the crate fail to compile after the table expires.

Or we could add some sort of automation to yank the crate releases containing expired tables.

tarcieri avatar Jun 20 '22 16:06 tarcieri

for wireguard compatibility, you could add a disabled-by-default feature assume-incorrect-10s-offset and release a new major version, or maybe instead just add a now_with_assumed_offset function (and similar for other UTC-based interfaces).

programmerjake avatar Jun 20 '22 16:06 programmerjake

I'd probably suggest against making network connections from a build script, especially in any security-related package (indeed there are renewed efforts underway to run build scripts under WASM).

ok, at least on linux you can usually get an up-to-date table from the tzdata package which is usually installed by default: /usr/share/zoneinfo/leap-seconds.list

programmerjake avatar Jun 20 '22 16:06 programmerjake

apparently recent versions of windows store the leap second table in the registry: https://github.com/microsoft/STL/blob/7f04137880e1c3df5125c1baf808f16f399dee2e/stl/inc/chrono#L2040 https://github.com/microsoft/STL/blob/b9505b9818c8bd53148922730fc2a426e8f38620/stl/src/tzdb.cpp#L556

programmerjake avatar Jun 20 '22 16:06 programmerjake

Perhaps it could try to query OS facilities to find the table, and if it can't, fall back on an internal table.

tarcieri avatar Jun 20 '22 16:06 tarcieri