ntp: drop precompensation of TX timestamp

The daemon transmit timestamps are precompensated for the time it takes
to generate a MAC using a symmetric key (as measured on chronyd start)
and also an average round-trip time of the Samba signing of MS-SNTP
responses. This improves accuracy of the transmit timestamp, but it
has some issues.

The correction has a random error which is changing over time due to
variable CPU frequency, system load, migration to a different machine,
etc. If the measured delay is too large, the correction may cause the
transmit timestamp to be later than the actual transmission. Also, the
delay is measured for a packet of a minimal length with no extension
fields, and there is no support for NTS.

Drop the precompensation in favor of the interleaved mode, which now
avoids the authentication delay even when no kernel/hardware timestamps
are available.

diff --git a/keys.c b/keys.c
index 2600c8fcf8fe07b31cbb8c6f283f0b56db4c3e4d..a08af973e716fd29229a6b8706dc69517d8dff42 100644 (file)
--- a/keys.c
+++ b/keys.c
@@ -60,7 +60,6 @@ typedef struct {
     } ntp_mac;
     CMC_Instance cmac;
   } data;
-  int auth_delay;
 } Key;
 
 static ARR_Instance keys;
@@ -122,38 +121,6 @@ get_key(unsigned int index)
   return ((Key *)ARR_GetElements(keys)) + index;
 }
 
-/* ================================================== */
-
-static int
-determine_hash_delay(uint32_t key_id)
-{
-  NTP_Packet pkt;
-  struct timespec before, after;
-  double diff, min_diff;
-  int i, nsecs;
-
-  memset(&pkt, 0, sizeof (pkt));
-
-  for (i = 0; i < 10; i++) {
-    LCL_ReadRawTime(&before);
-    KEY_GenerateAuth(key_id, (unsigned char *)&pkt, NTP_HEADER_LENGTH,
-                     (unsigned char *)&pkt + NTP_HEADER_LENGTH,
-                     sizeof (pkt) - NTP_HEADER_LENGTH);
-    LCL_ReadRawTime(&after);
-
-    diff = UTI_DiffTimespecsToDouble(&after, &before);
-
-    if (i == 0 || min_diff > diff)
-      min_diff = diff;
-  }
-
-  nsecs = 1.0e9 * min_diff;
-
-  DEBUG_LOG("authentication delay for key %"PRIu32": %d nsecs", key_id, nsecs);
-
-  return nsecs;
-}
-
 /* ================================================== */
 /* Decode key encoded in ASCII or HEX */
 
@@ -296,9 +263,6 @@ KEY_Reload(void)
 
   /* Erase any passwords from stack */
   memset(line, 0, sizeof (line));
-
-  for (i = 0; i < ARR_GetSize(keys); i++)
-    get_key(i)->auth_delay = determine_hash_delay(get_key(i)->id);
 }
 
 /* ================================================== */
@@ -354,21 +318,6 @@ KEY_KeyKnown(uint32_t key_id)
 
 /* ================================================== */
 
-int
-KEY_GetAuthDelay(uint32_t key_id)
-{
-  Key *key;
-
-  key = get_key_by_id(key_id);
-
-  if (!key)
-    return 0;
-
-  return key->auth_delay;
-}
-
-/* ================================================== */
-
 int
 KEY_GetAuthLength(uint32_t key_id)
 {

diff --git a/keys.h b/keys.h
index 99a8fdd0cfd64005fb5118f56ad0fdb87f0b72bb..706459071ee39cfce13d7e55fa05b6261c941f82 100644 (file)
--- a/keys.h
+++ b/keys.h
@@ -35,7 +35,6 @@ extern void KEY_Finalise(void);
 extern void KEY_Reload(void);
 
 extern int KEY_KeyKnown(uint32_t key_id);
-extern int KEY_GetAuthDelay(uint32_t key_id);
 extern int KEY_GetAuthLength(uint32_t key_id);
 extern int KEY_CheckKeyLength(uint32_t key_id);
 extern int KEY_GetKeyInfo(uint32_t key_id, int *type, int *bits);

diff --git a/ntp_auth.c b/ntp_auth.c
index 18c9ea9d05fc23d0d64b8e127cbe1255510d5c46..f7c31c4f270fe2cd39b4e42fbff66aa1a0266eec 100644 (file)
--- a/ntp_auth.c
+++ b/ntp_auth.c
@@ -96,24 +96,6 @@ check_symmetric_auth(NTP_Packet *packet, NTP_PacketInfo *info)
 
 /* ================================================== */
 
-static void
-adjust_timestamp(NTP_AuthMode mode, uint32_t key_id, struct timespec *ts)
-{
-  switch (mode) {
-    case NTP_AUTH_SYMMETRIC:
-      ts->tv_nsec += KEY_GetAuthDelay(key_id);
-      UTI_NormaliseTimespec(ts);
-      break;
-    case NTP_AUTH_MSSNTP:
-      ts->tv_nsec += NSD_GetAuthDelay(key_id);
-      UTI_NormaliseTimespec(ts);
-    default:
-      break;
-  }
-}
-
-/* ================================================== */
-
 static int
 is_zero_data(unsigned char *data, int length)
 {
@@ -230,14 +212,6 @@ NAU_PrepareRequestAuth(NAU_Instance instance)
 
 /* ================================================== */
 
-void
-NAU_AdjustRequestTimestamp(NAU_Instance instance, struct timespec *ts)
-{
-  adjust_timestamp(instance->mode, instance->key_id, ts);
-}
-
-/* ================================================== */
-
 int
 NAU_GenerateRequestAuth(NAU_Instance instance, NTP_Packet *request, NTP_PacketInfo *info)
 {
@@ -397,14 +371,6 @@ NAU_CheckRequestAuth(NTP_Packet *request, NTP_PacketInfo *info, uint32_t *kod)
 
 /* ================================================== */
 
-void
-NAU_AdjustResponseTimestamp(NTP_Packet *request, NTP_PacketInfo *info, struct timespec *ts)
-{
-  adjust_timestamp(info->auth.mode, info->auth.mac.key_id, ts);
-}
-
-/* ================================================== */
-
 int
 NAU_GenerateResponseAuth(NTP_Packet *request, NTP_PacketInfo *request_info,
                          NTP_Packet *response, NTP_PacketInfo *response_info,

diff --git a/ntp_auth.h b/ntp_auth.h
index d336b5553e061f57ea71b29d5f3c8795189948a8..4a5deb4e95e13551651001c482a185777e5e885c 100644 (file)
--- a/ntp_auth.h
+++ b/ntp_auth.h
@@ -51,10 +51,6 @@ extern int NAU_GetSuggestedNtpVersion(NAU_Instance instance);
 /* Perform operations necessary for NAU_GenerateRequestAuth() */
 extern int NAU_PrepareRequestAuth(NAU_Instance instance);
 
-/* Adjust a transmit timestamp for an estimated minimum time it takes to call
-   NAU_GenerateRequestAuth() */
-extern void NAU_AdjustRequestTimestamp(NAU_Instance instance, struct timespec *ts);
-
 /* Extend a request with data required by the authentication mode */
 extern int NAU_GenerateRequestAuth(NAU_Instance instance, NTP_Packet *request,
                                    NTP_PacketInfo *info);
@@ -66,11 +62,6 @@ extern int NAU_ParsePacket(NTP_Packet *packet, NTP_PacketInfo *info);
    kod code is returned, a KoD response should be sent back. */
 extern int NAU_CheckRequestAuth(NTP_Packet *request, NTP_PacketInfo *info, uint32_t *kod);
 
-/* Adjust a transmit timestamp for an estimated minimum time it takes to call
-   NAU_GenerateResponseAuth() */
-extern void NAU_AdjustResponseTimestamp(NTP_Packet *request, NTP_PacketInfo *info,
-                                        struct timespec *ts);
-
 /* Extend a response with data required by the authentication mode.  This
    function can be called only if the previous call of NAU_CheckRequestAuth()
    was on the same request. */

diff --git a/ntp_core.c b/ntp_core.c
index ecd5f25476c6e54c71da6ca797a03d451c94ea91..8c817a60b07b484b35256a475f954fe9b6901a99 100644 (file)
--- a/ntp_core.c
+++ b/ntp_core.c
@@ -1068,13 +1068,6 @@ transmit_packet(NTP_Mode my_mode, /* The mode this machine wants to be */
       LCL_ReadCookedTime(&local_transmit, &local_transmit_err);
       if (smooth_time)
         UTI_AddDoubleToTimespec(&local_transmit, smooth_offset, &local_transmit);
-
-      /* Pre-compensate the transmit time by approximately how long it will
-         take to generate the authentication data */
-      if (auth)
-        NAU_AdjustRequestTimestamp(auth, &local_transmit);
-      else
-        NAU_AdjustResponseTimestamp(request, request_info, &local_transmit);
     }
 
     UTI_TimespecToNtp64(interleaved ? &local_tx->ts : &local_transmit,

diff --git a/ntp_signd.c b/ntp_signd.c
index 6647bf8df49c61d7a6af4dce12a48439ff5af152..77b3249a6d1b68867dc9ded073b2f42cefa0f36b 100644 (file)
--- a/ntp_signd.c
+++ b/ntp_signd.c
@@ -96,14 +96,6 @@ static unsigned int queue_tail;
 /* Unix domain socket connected to ntp_signd */
 static int sock_fd;
 
-#define MIN_AUTH_DELAY 1.0e-5
-#define MAX_AUTH_DELAY 1.0e-2
-
-/* Average time needed for signing one packet.  This is used to adjust the
-   transmit timestamp in NTP packets.  The timestamp won't be very accurate as
-   the delay is variable, but it should be good enough for MS-SNTP clients. */
-static double auth_delay;
-
 /* Flag indicating if the MS-SNTP authentication is enabled */
 static int enabled;
 
@@ -183,10 +175,6 @@ process_response(SignInstance *inst)
   NIO_SendPacket(&inst->response.signed_packet, &inst->remote_addr, &inst->local_addr,
                  ntohl(inst->response.length) + sizeof (inst->response.length) -
                  offsetof(SigndResponse, signed_packet), 0);
-
-  /* Update exponential moving average of the authentication delay */
-  delay = CLAMP(MIN_AUTH_DELAY, delay, MAX_AUTH_DELAY);
-  auth_delay += 0.1 * (delay - auth_delay);
 }
 
 /* ================================================== */
@@ -274,7 +262,6 @@ void
 NSD_Initialise()
 {
   sock_fd = INVALID_SOCK_FD;
-  auth_delay = MIN_AUTH_DELAY;
   enabled = CNF_GetNtpSigndSocket() && CNF_GetNtpSigndSocket()[0];
 
   if (!enabled)
@@ -301,13 +288,6 @@ NSD_Finalise()
 
 /* ================================================== */
 
-extern int NSD_GetAuthDelay(uint32_t key_id)
-{
-  return 1.0e9 * auth_delay;
-}
-
-/* ================================================== */
-
 int
 NSD_SignAndSendPacket(uint32_t key_id, NTP_Packet *packet, NTP_PacketInfo *info,
                       NTP_Remote_Address *remote_addr, NTP_Local_Address *local_addr)

diff --git a/ntp_signd.h b/ntp_signd.h
index 985cf20562a6cfc5950aa690722331b8f83ce83d..d333c9a8423088178d3829ba2fcf1797120a78ab 100644 (file)
--- a/ntp_signd.h
+++ b/ntp_signd.h
@@ -35,9 +35,6 @@ extern void NSD_Initialise(void);
 /* Finalisation function */
 extern void NSD_Finalise(void);
 
-/* Function to get an estimate of delay due to signing */
-extern int NSD_GetAuthDelay(uint32_t key_id);
-
 /* Function to sign an NTP packet and send it */
 extern int NSD_SignAndSendPacket(uint32_t key_id, NTP_Packet *packet, NTP_PacketInfo *info,
                                  NTP_Remote_Address *remote_addr, NTP_Local_Address *local_addr);

diff --git a/stubs.c b/stubs.c
index 0158b3b7e6cff3511247f2c0ffa3bc0fb18e450b..73b9a95ea983a39b0383e7809293532cc4f797ef 100644 (file)
--- a/stubs.c
+++ b/stubs.c
@@ -427,12 +427,6 @@ NSD_Finalise(void)
 {
 }
 
-int
-NSD_GetAuthDelay(uint32_t key_id)
-{
-  return 0;
-}
-
 int
 NSD_SignAndSendPacket(uint32_t key_id, NTP_Packet *packet, NTP_PacketInfo *info,
                       NTP_Remote_Address *remote_addr, NTP_Local_Address *local_addr)

diff --git a/test/unit/keys.c b/test/unit/keys.c
index 0f674792180cd5e1a2f72e00644a3a7f3ed67a8e..aa5e6491f78d34202d5edb50c370fbed09d7777e 100644 (file)
--- a/test/unit/keys.c
+++ b/test/unit/keys.c
@@ -125,7 +125,6 @@ test_unit(void)
 
     for (j = 0; j < KEYS; j++) {
       TEST_CHECK(KEY_KeyKnown(keys[j]));
-      TEST_CHECK(KEY_GetAuthDelay(keys[j]) >= 0);
       TEST_CHECK(KEY_GetAuthLength(keys[j]) >= 16);
 
       data_len = random() % (sizeof (data) + 1);