From: David Venhoek <david@tweedegolf.com>
Date: Fri, 26 Jan 2024 09:40:03 +0000 (+0100)
Subject: timesyncd: make the transmit timestamp in requests fully random
X-Git-Tag: v256-rc1~1025
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=678bd12cfc1a7f3f0d074ac9c52f0b06ec601618;p=thirdparty%2Fsystemd.git

timesyncd: make the transmit timestamp in requests fully random

This improves security against off-path attackers, and avoids leaking
the current system time.
---

diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
index bb37de9f253..8e0eda07973 100644
--- a/src/timesync/timesyncd-manager.c
+++ b/src/timesync/timesyncd-manager.c
@@ -27,6 +27,7 @@
 #include "network-util.h"
 #include "ratelimit.h"
 #include "resolve-private.h"
+#include "random-util.h"
 #include "socket-util.h"
 #include "string-util.h"
 #include "strv.h"
@@ -78,13 +79,6 @@ static double ts_to_d(const struct timespec *ts) {
         return ts->tv_sec + (1.0e-9 * ts->tv_nsec);
 }
 
-static uint32_t graceful_add_offset_1900_1970(time_t t) {
-        /* Adds OFFSET_1900_1970 to t and returns it as 32-bit value. This is handles overflows
-         * gracefully in a deterministic and well-defined way by cutting off the top bits. */
-        uint64_t a = (uint64_t) t + OFFSET_1900_1970;
-        return (uint32_t) (a & UINT64_C(0xFFFFFFFF));
-}
-
 static int manager_timeout(sd_event_source *source, usec_t usec, void *userdata) {
         _cleanup_free_ char *pretty = NULL;
         Manager *m = ASSERT_PTR(userdata);
@@ -126,20 +120,22 @@ static int manager_send_request(Manager *m) {
         }
 
         /*
-         * Set transmit timestamp, remember it; the server will send that back
-         * as the origin timestamp and we have an indication that this is the
-         * matching answer to our request.
-         *
-         * The actual value does not matter, We do not care about the correct
-         * NTP UINT_MAX fraction; we just pass the plain nanosecond value.
+         * Generate a random number as transmit timestamp, to ensure we get
+         * a full 64 bits of entropy to make it hard for off-path attackers
+         * to inject random time to us.
          */
-        assert_se(clock_gettime(CLOCK_BOOTTIME, &m->trans_time_mon) >= 0);
-        assert_se(clock_gettime(CLOCK_REALTIME, &m->trans_time) >= 0);
-        ntpmsg.trans_time.sec = htobe32(graceful_add_offset_1900_1970(m->trans_time.tv_sec));
-        ntpmsg.trans_time.frac = htobe32(m->trans_time.tv_nsec);
+        random_bytes(&m->request_nonce, sizeof(m->request_nonce));
+        ntpmsg.trans_time = m->request_nonce;
 
         server_address_pretty(m->current_server_address, &pretty);
 
+        /*
+         * Record the transmit timestamp. This should be as close as possible to
+         * the send-to to ensure the timestamp is reasonably accurate
+         */
+        assert_se(clock_gettime(CLOCK_BOOTTIME, &m->trans_time_mon) >= 0);
+        assert_se(clock_gettime(CLOCK_REALTIME, &m->trans_time) >= 0);
+
         len = sendto(m->server_socket, &ntpmsg, sizeof(ntpmsg), MSG_DONTWAIT, &m->current_server_address->sockaddr.sa, m->current_server_address->socklen);
         if (len == sizeof(ntpmsg)) {
                 m->pending = true;
@@ -457,9 +453,8 @@ static int manager_receive_response(sd_event_source *source, int fd, uint32_t re
 
         m->missed_replies = 0;
 
-        /* check our "time cookie" (we just stored nanoseconds in the fraction field) */
-        if (be32toh(ntpmsg.origin_time.sec) != graceful_add_offset_1900_1970(m->trans_time.tv_sec) ||
-            be32toh(ntpmsg.origin_time.frac) != (unsigned long) m->trans_time.tv_nsec) {
+        /* check the transmit request nonce was properly returned in the origin_time field */
+        if (ntpmsg.origin_time.sec != m->request_nonce.sec || ntpmsg.origin_time.frac != m->request_nonce.frac) {
                 log_debug("Invalid reply; not our transmit time. Ignoring.");
                 return 0;
         }
diff --git a/src/timesync/timesyncd-manager.h b/src/timesync/timesyncd-manager.h
index 8cbb91d9075..f4447874897 100644
--- a/src/timesync/timesyncd-manager.h
+++ b/src/timesync/timesyncd-manager.h
@@ -71,6 +71,7 @@ struct Manager {
         /* last sent packet */
         struct timespec trans_time_mon;
         struct timespec trans_time;
+        struct ntp_ts request_nonce;
         usec_t retry_interval;
         usec_t connection_retry_usec;
         bool pending;