ntp: separate timestamps for restarting symmetric protocol

Save the local receive and remote transmit timestamp needed for
(re)starting the symmetric protocol when no valid reply was received
separately from the timestamps that are used for synchronization of the
local clock.

This extends the interval in which the local NTP state is (partially)
protected against replay attacks in order to complete a measurement
in the interleaved symmetric mode from [last valid RX, next TX] to
[last TX, next TX], i.e. it should be the same as in the basic mode.

diff --git a/ntp_core.c b/ntp_core.c
index d73111fbcf88f0340b7ef6d9308764428431f42d..18e66843ae4c90ba32a8ca4e445c2c19e192228d 100644 (file)
--- a/ntp_core.c
+++ b/ntp_core.c
@@ -149,14 +149,11 @@ struct NCR_Instance_Record {
      be used for synchronisation */
   int valid_timestamps;
 
-  /* Flag indicating the timestamps below were updated since last request */
-  int updated_timestamps;
-
-  /* Receive and transmit timestamps from the last received packet */
+  /* Receive and transmit timestamps from the last valid response */
   NTP_int64 remote_ntp_rx;
   NTP_int64 remote_ntp_tx;
 
-  /* Local timestamp when the last packet was received from the
+  /* Local timestamp when the last valid response was received from the
      source.  We have to be prepared to tinker with this if the local
      clock has its frequency adjusted before we repond.  The value we
      store here is what our own local time was when the same arrived.
@@ -183,6 +180,15 @@ struct NCR_Instance_Record {
   int prev_local_poll;
   unsigned int prev_tx_count;
 
+  /* Flag indicating the two timestamps below were updated since the
+     last transmission */
+  int updated_init_timestamps;
+
+  /* Timestamps used for (re)starting the symmetric protocol, when we
+     need to respond to a packet which is not a valid response */
+  NTP_int64 init_remote_ntp_tx;
+  NTP_Local_Timestamp init_local_rx;
+
   /* The instance record in the main source management module.  This
      performs the statistical analysis on the samples we generate */
 
@@ -652,7 +658,6 @@ NCR_ResetInstance(NCR_Instance instance)
 
   instance->valid_rx = 0;
   instance->valid_timestamps = 0;
-  instance->updated_timestamps = 0;
   UTI_ZeroNtp64(&instance->remote_ntp_rx);
   UTI_ZeroNtp64(&instance->remote_ntp_tx);
   UTI_ZeroNtp64(&instance->local_ntp_rx);
@@ -662,6 +667,10 @@ NCR_ResetInstance(NCR_Instance instance)
   zero_local_timestamp(&instance->prev_local_tx);
   instance->prev_local_poll = 0;
   instance->prev_tx_count = 0;
+
+  instance->updated_init_timestamps = 0;
+  UTI_ZeroNtp64(&instance->init_remote_ntp_tx);
+  zero_local_timestamp(&instance->init_local_rx);
 }
 
 /* ================================================== */
@@ -914,8 +923,8 @@ transmit_packet(NTP_Mode my_mode, /* The mode this machine wants to be */
     version = NTP_VERSION;
   }
 
-  /* Allow interleaved mode only if there was a prior transmission */
-  if (interleaved && (!local_tx || UTI_IsZeroTimespec(&local_tx->ts)))
+  /* Check if the packet can be formed in the interleaved mode */
+  if (interleaved && (!remote_ntp_rx || !local_tx || UTI_IsZeroTimespec(&local_tx->ts)))
     interleaved = 0;
 
   smooth_time = 0;
@@ -1084,7 +1093,7 @@ transmit_timeout(void *arg)
 {
   NCR_Instance inst = (NCR_Instance) arg;
   NTP_Local_Address local_addr;
-  int interleaved, sent;
+  int interleaved, initial, sent;
 
   inst->tx_timeout_id = 0;
 
@@ -1137,6 +1146,19 @@ transmit_timeout(void *arg)
                  (inst->mode == MODE_ACTIVE &&
                   inst->prev_tx_count == 1 && inst->tx_count == 0));
 
+  /* In symmetric mode, if no valid response was received since the previous
+     transmission, respond to the last received packet even if it failed some
+     specific NTP tests.  This is necessary for starting and restarting the
+     protocol, e.g. when a packet was lost. */
+  initial = inst->mode == MODE_ACTIVE && !inst->valid_rx &&
+            !UTI_IsZeroNtp64(&inst->init_remote_ntp_tx);
+
+  /* Prepare for the response */
+  inst->valid_rx = 0;
+  inst->updated_init_timestamps = 0;
+  if (initial)
+    inst->valid_timestamps = 0;
+
   /* Check whether we need to 'warm up' the link to the other end by
      sending an NTP exchange to ensure both ends' ARP caches are
      primed or whether we need to send two packets first to ensure a
@@ -1148,18 +1170,16 @@ transmit_timeout(void *arg)
     inst->presend_done--;
   }
 
-  sent = transmit_packet(inst->mode, interleaved, inst->local_poll,
-                         inst->version,
+  /* Send the request (which may also be a response in the symmetric mode) */
+  sent = transmit_packet(inst->mode, interleaved, inst->local_poll, inst->version,
                          inst->auth_mode, inst->auth_key_id,
-                         &inst->remote_ntp_rx, &inst->remote_ntp_tx,
-                         &inst->local_rx, &inst->local_tx,
-                         &inst->local_ntp_rx, &inst->local_ntp_tx,
-                         &inst->remote_addr,
-                         &local_addr);
+                         initial ? NULL : &inst->remote_ntp_rx,
+                         initial ? &inst->init_remote_ntp_tx : &inst->remote_ntp_tx,
+                         initial ? &inst->init_local_rx : &inst->local_rx,
+                         &inst->local_tx, &inst->local_ntp_rx, &inst->local_ntp_tx,
+                         &inst->remote_addr, &local_addr);
 
   ++inst->tx_count;
-  inst->valid_rx = 0;
-  inst->updated_timestamps = 0;
   if (sent)
     inst->report.total_tx_count++;
 
@@ -1442,6 +1462,7 @@ receive_packet(NCR_Instance inst, NTP_Local_Address *local_addr,
   NTP_Local_Timestamp local_receive, local_transmit;
   double remote_interval, local_interval, response_time;
   double delay_time, precision;
+  int updated_timestamps;
 
   /* ==================== */
 
@@ -1644,21 +1665,34 @@ receive_packet(NCR_Instance inst, NTP_Local_Address *local_addr,
      The authentication test (test5) is required to prevent DoS attacks using
      unauthenticated packets on authenticated symmetric associations. */
   if ((inst->mode == MODE_CLIENT && valid_packet && !inst->valid_rx) ||
-      (inst->mode == MODE_ACTIVE && (valid_packet || !inst->valid_rx) &&
-       test5 && !UTI_IsZeroNtp64(&message->transmit_ts) &&
-       (!inst->updated_timestamps || (valid_packet && !inst->valid_rx) ||
+      (inst->mode == MODE_ACTIVE && valid_packet &&
+       (!inst->valid_rx ||
         UTI_CompareNtp64(&inst->remote_ntp_tx, &message->transmit_ts) < 0))) {
     inst->remote_ntp_rx = message->receive_ts;
     inst->remote_ntp_tx = message->transmit_ts;
     inst->local_rx = *rx_ts;
     inst->valid_timestamps = synced_packet;
-    inst->updated_timestamps = 1;
+
+    UTI_ZeroNtp64(&inst->init_remote_ntp_tx);
+    zero_local_timestamp(&inst->init_local_rx);
+    inst->updated_init_timestamps = 0;
+    updated_timestamps = 2;
 
     /* Don't use the same set of timestamps for the next sample */
     if (interleaved_packet)
       inst->prev_local_tx = inst->local_tx;
     else
       zero_local_timestamp(&inst->prev_local_tx);
+  } else if (inst->mode == MODE_ACTIVE &&
+             test1 && !UTI_IsZeroNtp64(&message->transmit_ts) && test5 &&
+             (!inst->updated_init_timestamps ||
+              UTI_CompareNtp64(&inst->init_remote_ntp_tx, &message->transmit_ts) < 0)) {
+    inst->init_remote_ntp_tx = message->transmit_ts;
+    inst->init_local_rx = *rx_ts;
+    inst->updated_init_timestamps = 1;
+    updated_timestamps = 1;
+  } else {
+    updated_timestamps = 0;
   }
 
   /* Accept at most one response per request.  The NTP specification recommends
@@ -1694,10 +1728,11 @@ receive_packet(NCR_Instance inst, NTP_Local_Address *local_addr,
   DEBUG_LOG("remote_interval=%.9f local_interval=%.9f response_time=%.9f txs=%c rxs=%c",
             remote_interval, local_interval, response_time,
             tss_chars[local_transmit.source], tss_chars[local_receive.source]);
-  DEBUG_LOG("test123=%d%d%d test567=%d%d%d testABCD=%d%d%d%d kod_rate=%d interleaved=%d presend=%d valid=%d good=%d updated=%d",
+  DEBUG_LOG("test123=%d%d%d test567=%d%d%d testABCD=%d%d%d%d kod_rate=%d interleaved=%d"
+            " presend=%d valid=%d good=%d updated=%d",
             test1, test2, test3, test5, test6, test7, testA, testB, testC, testD,
             kod_rate, interleaved_packet, inst->presend_done, valid_packet, good_packet,
-            !UTI_CompareTimespecs(&inst->local_rx.ts, &rx_ts->ts));
+            updated_timestamps);
 
   if (valid_packet) {
     inst->remote_poll = message->poll;
@@ -2189,6 +2224,9 @@ NCR_SlewTimes(NCR_Instance inst, struct timespec *when, double dfreq, double dof
   if (!UTI_IsZeroTimespec(&inst->prev_local_tx.ts))
     UTI_AdjustTimespec(&inst->prev_local_tx.ts, when, &inst->prev_local_tx.ts, &delta, dfreq,
                        doffset);
+  if (!UTI_IsZeroTimespec(&inst->init_local_rx.ts))
+    UTI_AdjustTimespec(&inst->init_local_rx.ts, when, &inst->local_rx.ts, &delta, dfreq,
+                       doffset);
 }
 
 /* ================================================== */

diff --git a/test/unit/ntp_core.c b/test/unit/ntp_core.c
index b227017d5036a1cc70454231d216bd11864ec61e..9ff62cdbc4aa131b285d477b46b2a47dc4634826 100644 (file)
--- a/test/unit/ntp_core.c
+++ b/test/unit/ntp_core.c
@@ -74,7 +74,6 @@ send_request(void)
 
   transmit_timeout(inst);
   TEST_CHECK(!inst->valid_rx);
-  TEST_CHECK(!inst->updated_timestamps);
   TEST_CHECK(prev_tx_count + 1 == inst->report.total_tx_count);
 
   advance_time(1e-4);
@@ -165,13 +164,13 @@ send_response(int interleaved, int authenticated, int allow_update, int valid_ts
 }
 
 static void
-process_response(int valid, int updated)
+process_response(int valid, int updated_sync, int updated_init)
 {
   NTP_Local_Address local_addr;
   NTP_Local_Timestamp local_ts;
   NTP_Packet *res;
   uint32_t prev_rx_count, prev_valid_count;
-  struct timespec prev_rx_ts;
+  struct timespec prev_rx_ts, prev_init_rx_ts;
   int prev_open_socket;
 
   res = &res_buffer.ntp_pkt;
@@ -186,6 +185,7 @@ process_response(int valid, int updated)
   prev_rx_count = inst->report.total_rx_count;
   prev_valid_count = inst->report.total_valid_count;
   prev_rx_ts = inst->local_rx.ts;
+  prev_init_rx_ts = inst->init_local_rx.ts;
   prev_open_socket = inst->local_addr.sock_fd != INVALID_SOCK_FD;
 
   NCR_ProcessRxKnown(inst, &local_addr, &local_ts, res, res_length);
@@ -200,10 +200,20 @@ process_response(int valid, int updated)
   else
     TEST_CHECK(prev_valid_count == inst->report.total_valid_count);
 
-  if (updated)
+  if (updated_sync)
     TEST_CHECK(UTI_CompareTimespecs(&inst->local_rx.ts, &prev_rx_ts));
   else
     TEST_CHECK(!UTI_CompareTimespecs(&inst->local_rx.ts, &prev_rx_ts));
+
+  if (updated_init)
+    TEST_CHECK(UTI_CompareTimespecs(&inst->init_local_rx.ts, &prev_init_rx_ts));
+  else
+    TEST_CHECK(!UTI_CompareTimespecs(&inst->init_local_rx.ts, &prev_init_rx_ts));
+
+  if (valid) {
+    TEST_CHECK(UTI_IsZeroTimespec(&inst->init_local_rx.ts));
+    TEST_CHECK(UTI_IsZeroNtp64(&inst->init_remote_ntp_tx));
+  }
 }
 
 void
@@ -259,30 +269,38 @@ test_unit(void)
       updated = (valid || inst->mode == MODE_ACTIVE) &&
                 (!source.params.authkey || authenticated);
       has_updated = has_updated || updated;
+      if (inst->mode == MODE_CLIENT)
+        updated = 0;
 
       send_request();
 
       send_response(interleaved, authenticated, 1, 0, 1);
-      process_response(0, inst->mode == MODE_CLIENT ? 0 : updated);
+      DEBUG_LOG("response 1");
+      process_response(0, 0, updated);
 
       if (source.params.authkey) {
         send_response(interleaved, authenticated, 1, 1, 0);
-        process_response(0, 0);
+        DEBUG_LOG("response 2");
+        process_response(0, 0, 0);
       }
 
       send_response(interleaved, authenticated, 1, 1, 1);
-      process_response(valid, updated);
-      process_response(0, 0);
+      DEBUG_LOG("response 3");
+      process_response(valid, valid, updated);
+      DEBUG_LOG("response 4");
+      process_response(0, 0, 0);
 
       advance_time(-1.0);
 
       send_response(interleaved, authenticated, 1, 1, 1);
-      process_response(0, 0);
+      DEBUG_LOG("response 5");
+      process_response(0, 0, updated && valid);
 
       advance_time(1.0);
 
       send_response(interleaved, authenticated, 1, 1, 1);
-      process_response(0, inst->mode == MODE_CLIENT ? 0 : updated);
+      DEBUG_LOG("response 6");
+      process_response(0, valid && updated, updated);
     }
 
     NCR_DestroyInstance(inst);