ntp: drop support for long NTPv4 MACs

Don't accept NTPv4 packets which have a MAC longer than 24 octets to
strictly follow RFC 7822, which specifies the maximum length of a MAC
and the minimum length of the last extension field to avoid an ambiguity
in parsing of the packet.

This removes an ugly hack that was needed to accept packets that
contained one or more extension fields without a MAC, before RFC 7822
was written and NTP implementations started using truncated MACs.

The long MACs were used by chrony in versions 2.x when configured to
authenticate a server or peer with a key using a 256-bit or longer hash
(e.g. SHA256). For compatibility with chrony >= 4.0, these clients/peers
will need to have "version 3" added to the server/peer line in
chrony.conf.

diff --git a/ntp_auth.c b/ntp_auth.c
index 50b78a8032fce841454c926e0fd96894163726ca..58c89883ea358b5fd85967491d9f7048bf506f0b 100644 (file)
--- a/ntp_auth.c
+++ b/ntp_auth.c
@@ -300,21 +300,9 @@ NAU_ParsePacket(NTP_Packet *packet, NTP_PacketInfo *info)
     if (remainder >= NTP_MIN_MAC_LENGTH && remainder <= NTP_MAX_V4_MAC_LENGTH)
       break;
 
-    /* The NTPv4-specific limit for MAC length enables deterministic parsing of
-       packets with extension fields (RFC 7822), but we support longer MACs in
-       packets with no extension fields for compatibility with older chrony
-       clients.  Check if the longer MAC would authenticate the packet before
-       trying to parse the data as an extension field. */
-    if (parsed == NTP_HEADER_LENGTH &&
-        remainder > NTP_MAX_V4_MAC_LENGTH && remainder <= NTP_MAX_MAC_LENGTH &&
-        KEY_CheckAuth(ntohl(*(uint32_t *)(data + parsed)), data, parsed,
-                      data + parsed + 4, remainder - 4, NTP_MAX_MAC_LENGTH - 4))
-      break;
-
     /* Check if this is a valid NTPv4 extension field and skip it */
     if (!NEF_ParseField(packet, info->length, parsed, &ef_length, &ef_type, NULL, NULL)) {
-      /* Invalid MAC or format error */
-      DEBUG_LOG("Invalid format or MAC");
+      DEBUG_LOG("Invalid format");
       return 0;
     }
 
@@ -340,9 +328,6 @@ NAU_ParsePacket(NTP_Packet *packet, NTP_PacketInfo *info)
     /* No MAC */
     return 1;
   } else if (remainder >= NTP_MIN_MAC_LENGTH) {
-    /* This is not 100% reliable as a MAC could fail to authenticate and could
-       pass as an extension field, leaving reminder smaller than the minimum MAC
-       length */
     info->auth.mode = NTP_AUTH_SYMMETRIC;
     info->auth.mac.start = parsed;
     info->auth.mac.length = remainder;

diff --git a/ntp_core.c b/ntp_core.c
index c306b03e5a471840ffbd742030ab1b9b35ef75c5..60c030357c011373b2da464a2bd99f1edb1d8cee 100644 (file)
--- a/ntp_core.c
+++ b/ntp_core.c
@@ -2095,15 +2095,6 @@ NCR_ProcessRxUnknown(NTP_Remote_Address *remote_addr, NTP_Local_Address *local_a
     CLG_LogAuthNtpRequest();
   }
 
-  /* If it is an NTPv4 packet with a long MAC and no extension fields,
-     respond with an NTPv3 packet to avoid breaking RFC 7822 and keep
-     the length symmetric.  Otherwise, respond with the same version. */
-  if (info.version == 4 && info.ext_fields == 0 && info.auth.mode == NTP_AUTH_SYMMETRIC &&
-      info.auth.mac.length > NTP_MAX_V4_MAC_LENGTH)
-    version = 3;
-  else
-    version = info.version;
-
   local_ntp_rx = local_ntp_tx = NULL;
   tx_ts = NULL;
   interleaved = 0;
@@ -2133,6 +2124,9 @@ NCR_ProcessRxUnknown(NTP_Remote_Address *remote_addr, NTP_Local_Address *local_a
   poll = CLG_GetNtpMinPoll();
   poll = MAX(poll, message->poll);
 
+  /* Respond with the same version */
+  version = info.version;
+
   /* Send a reply */
   transmit_packet(my_mode, interleaved, poll, version, kod, NULL,
                   &message->receive_ts, &message->transmit_ts,

diff --git a/test/unit/ntp_core.c b/test/unit/ntp_core.c
index 63b2d55b4379a7ab510eb10d0501ba21d51264e9..a7f89ee8f89fdf7cca0369b72b979f3bdabbe8df 100644 (file)
--- a/test/unit/ntp_core.c
+++ b/test/unit/ntp_core.c
@@ -190,7 +190,7 @@ send_response(int interleaved, int authenticated, int allow_update, int valid_ts
       key_id = get_random_key_id();
     auth_len = KEY_GetAuthLength(key_id);
     assert(auth_len);
-    if (NTP_LVM_TO_VERSION(res->lvm) == 4 && random() % 2)
+    if (NTP_LVM_TO_VERSION(res->lvm) == 4)
       auth_len = MIN(auth_len, NTP_MAX_V4_MAC_LENGTH - 4);
 
     if (KEY_GenerateAuth(key_id, (unsigned char *)res, NTP_HEADER_LENGTH,
@@ -204,7 +204,7 @@ send_response(int interleaved, int authenticated, int allow_update, int valid_ts
   if (!valid_auth && authenticated) {
     assert(auth_len);
 
-    switch (random() % 4) {
+    switch (random() % 5) {
       case 0:
         key_id++;
         break;
@@ -219,9 +219,20 @@ send_response(int interleaved, int authenticated, int allow_update, int valid_ts
         break;
       case 3:
         res_length = NTP_HEADER_LENGTH + 4 * (random() % ((4 + auth_len) / 4));
-        if (NTP_LVM_TO_VERSION(res->lvm) == 4 &&
-            res_length == NTP_HEADER_LENGTH + NTP_MAX_V4_MAC_LENGTH)
-          res_length -= 4;
+        break;
+      case 4:
+        if (NTP_LVM_TO_VERSION(res->lvm) == 4 && random() % 2 &&
+            KEY_GetAuthLength(key_id) > NTP_MAX_V4_MAC_LENGTH - 4) {
+          auth_len += 4 + 4 * (random() %
+                               ((KEY_GetAuthLength(key_id) - NTP_MAX_V4_MAC_LENGTH - 4) / 4));
+          if (KEY_GenerateAuth(key_id, (unsigned char *)res, NTP_HEADER_LENGTH,
+                               res->extensions + 4, auth_len) != auth_len)
+              assert(0);
+          res_length = NTP_HEADER_LENGTH + 4 + auth_len;
+        } else {
+          memset((unsigned char *)res + res_length, 0, 4);
+          res_length += 4;
+        }
         break;
       default:
         assert(0);