ntp: refactor NTP_Packet structure for extension fields

diff --git a/keys.c b/keys.c
index a7c53f4eeb12312a29dcf1eca74273a9d246bfe7..1305e8212a306c5321f2855e49b0401f12313b7d 100644 (file)
--- a/keys.c
+++ b/keys.c
@@ -135,8 +135,9 @@ determine_hash_delay(uint32_t key_id)
 
   for (i = 0; i < 10; i++) {
     LCL_ReadRawTime(&before);
-    KEY_GenerateAuth(key_id, (unsigned char *)&pkt, NTP_NORMAL_PACKET_LENGTH,
-        (unsigned char *)&pkt.auth_data, sizeof (pkt.auth_data));
+    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);

diff --git a/ntp.h b/ntp.h
index 7b7f361c3f635f81406a8223c21c73a5638cc316..eacbacd54189e0334fd55d9810ef18e98bf24172 100644 (file)
--- a/ntp.h
+++ b/ntp.h
@@ -47,18 +47,18 @@ typedef uint32_t NTP_int32;
 /* Maximum stratum number (infinity) */
 #define NTP_MAX_STRATUM 16
 
-/* The minimum valid length of an extension field */
-#define NTP_MIN_EXTENSION_LENGTH 16
-
-/* The maximum assumed length of all extension fields in received
-   packets (RFC 5905 doesn't specify a limit on length or number of
-   extension fields in one packet) */
-#define NTP_MAX_EXTENSIONS_LENGTH 1024
-
 /* The minimum and maximum supported length of MAC */
 #define NTP_MIN_MAC_LENGTH (4 + 16)
 #define NTP_MAX_MAC_LENGTH (4 + MAX_HASH_LENGTH)
 
+/* The minimum valid length of an extension field */
+#define NTP_MIN_EF_LENGTH 16
+
+/* The maximum assumed length of all extension fields in an NTP packet,
+   including a MAC (RFC 5905 doesn't specify a limit on length or number of
+   extension fields in one packet) */
+#define NTP_MAX_EXTENSIONS_LENGTH (1024 + NTP_MAX_MAC_LENGTH)
+
 /* The maximum length of MAC in NTPv4 packets which allows deterministic
    parsing of extension fields (RFC 7822) */
 #define NTP_MAX_V4_MAC_LENGTH (4 + 20)
@@ -93,21 +93,10 @@ typedef struct {
   NTP_int64 receive_ts;
   NTP_int64 transmit_ts;
 
-  /* Optional extension fields, we don't send packets with them yet */
-  /* uint8_t extensions[] */
-
-  /* Optional message authentication code (MAC) */
-  NTP_int32 auth_keyid;
-  uint8_t auth_data[NTP_MAX_MAC_LENGTH - 4];
+  uint8_t extensions[NTP_MAX_EXTENSIONS_LENGTH];
 } NTP_Packet;
 
-#define NTP_NORMAL_PACKET_LENGTH (int)offsetof(NTP_Packet, auth_keyid)
-
-/* The buffer used to hold a datagram read from the network */
-typedef struct {
-  NTP_Packet ntp_pkt;
-  uint8_t extensions[NTP_MAX_EXTENSIONS_LENGTH];
-} NTP_Receive_Buffer;
+#define NTP_HEADER_LENGTH (int)offsetof(NTP_Packet, extensions)
 
 /* Macros to work with the lvm field */
 #define NTP_LVM_TO_LEAP(lvm) (((lvm) >> 6) & 0x3)

diff --git a/ntp_core.c b/ntp_core.c
index 4eb36744012aaf5b9ac4791106e9f0ab54d2bc58..4013672b88b62015de37b64e2d11bd3582ce4342 100644 (file)
--- a/ntp_core.c
+++ b/ntp_core.c
@@ -1067,7 +1067,7 @@ transmit_packet(NTP_Mode my_mode, /* The mode this machine wants to be */
     if (smooth_time)
       UTI_AddDoubleToTimespec(&local_transmit, smooth_offset, &local_transmit);
 
-    length = NTP_NORMAL_PACKET_LENGTH;
+    length = NTP_HEADER_LENGTH;
 
     /* Authenticate the packet */
 
@@ -1083,19 +1083,18 @@ transmit_packet(NTP_Mode my_mode, /* The mode this machine wants to be */
       if (auth_mode == AUTH_SYMMETRIC) {
         /* Truncate long MACs in NTPv4 packets to allow deterministic parsing
            of extension fields (RFC 7822) */
-        max_auth_len = version == 4 ?
-                       NTP_MAX_V4_MAC_LENGTH - 4 : sizeof (message.auth_data);
+        max_auth_len = (version == 4 ?
+                        NTP_MAX_V4_MAC_LENGTH : (sizeof (message) - length)) - 4;
 
-        auth_len = KEY_GenerateAuth(key_id, (unsigned char *) &message,
-                                    offsetof(NTP_Packet, auth_keyid),
-                                    (unsigned char *)&message.auth_data, max_auth_len);
+        auth_len = KEY_GenerateAuth(key_id, (unsigned char *)&message, length,
+                                    (unsigned char *)&message + length + 4, max_auth_len);
         if (!auth_len) {
           DEBUG_LOG("Could not generate auth data with key %"PRIu32, key_id);
           return 0;
         }
 
-        message.auth_keyid = htonl(key_id);
-        length += sizeof (message.auth_keyid) + auth_len;
+        *(uint32_t *)((unsigned char *)&message + length) = htonl(key_id);
+        length += 4 + auth_len;
       } else if (auth_mode == AUTH_MSSNTP) {
         /* MS-SNTP packets are signed (asynchronously) by ntp_signd */
         return NSD_SignAndSendPacket(key_id, &message, where_to, from, length);
@@ -1294,7 +1293,7 @@ check_packet_format(NTP_Packet *message, int length)
     return 0;
   } 
 
-  if (length < NTP_NORMAL_PACKET_LENGTH || (unsigned int)length % 4) {
+  if (length < NTP_HEADER_LENGTH || (unsigned int)length % 4) {
     DEBUG_LOG("NTP packet has invalid length %d", length);
     return 0;
   }
@@ -1331,7 +1330,7 @@ check_packet_auth(NTP_Packet *pkt, int length,
   /* Go through extension fields and see if there is a valid MAC */
 
   version = NTP_LVM_TO_VERSION(pkt->lvm);
-  i = NTP_NORMAL_PACKET_LENGTH;
+  i = NTP_HEADER_LENGTH;
   data = (void *)pkt;
 
   while (1) {
@@ -1355,7 +1354,7 @@ check_packet_auth(NTP_Packet *pkt, int length,
 
         /* If it's an NTPv4 packet with long MAC and no extension fields,
            rewrite the version in the packet to respond with long MAC too */
-        if (version == 4 && NTP_NORMAL_PACKET_LENGTH + remainder == length &&
+        if (version == 4 && NTP_HEADER_LENGTH + remainder == length &&
             remainder > NTP_MAX_V4_MAC_LENGTH)
           pkt->lvm = NTP_LVM(NTP_LVM_TO_LEAP(pkt->lvm), 3, NTP_LVM_TO_MODE(pkt->lvm));
 
@@ -1365,9 +1364,9 @@ check_packet_auth(NTP_Packet *pkt, int length,
 
     /* Check if this is a valid NTPv4 extension field and skip it.  It should
        have a 16-bit type, 16-bit length, and data padded to 32 bits. */
-    if (version == 4 && remainder >= NTP_MIN_EXTENSION_LENGTH) {
+    if (version == 4 && remainder >= NTP_MIN_EF_LENGTH) {
       ext_length = ntohs(*(uint16_t *)(data + i + 2));
-      if (ext_length >= NTP_MIN_EXTENSION_LENGTH &&
+      if (ext_length >= NTP_MIN_EF_LENGTH &&
           ext_length <= remainder && ext_length % 4 == 0) {
         i += ext_length;
         continue;

diff --git a/ntp_io.c b/ntp_io.c
index 7a70ff4a1404ceeb20c5ce2cfb4d2542b9e6cda5..a70f6c64e1ba460e051fc9190bf3873284e14639 100644 (file)
--- a/ntp_io.c
+++ b/ntp_io.c
@@ -391,8 +391,7 @@ process_message(SCK_Message *message, int sock_fd, int event)
               UTI_DiffTimespecsToDouble(&sched_ts, &local_ts.ts), local_ts.source);
 
   /* Just ignore the packet if it's not of a recognized length */
-  if (message->length < NTP_NORMAL_PACKET_LENGTH ||
-      message->length > sizeof (NTP_Receive_Buffer)) {
+  if (message->length < NTP_HEADER_LENGTH || message->length > sizeof (NTP_Packet)) {
     DEBUG_LOG("Unexpected length");
     return;
   }

diff --git a/ntp_io_linux.c b/ntp_io_linux.c
index 6519791fc0b468d2e3547c1e024853facde74b95..634ccb0807951f553b1731c754fee00ab2533194 100644 (file)
--- a/ntp_io_linux.c
+++ b/ntp_io_linux.c
@@ -775,7 +775,7 @@ NIO_Linux_ProcessMessage(SCK_Message *message, NTP_Local_Address *local_addr,
     return 1;
   }
 
-  if (message->length < NTP_NORMAL_PACKET_LENGTH)
+  if (message->length < NTP_HEADER_LENGTH)
     return 1;
 
   NSR_ProcessTx(&message->remote_addr.ip, local_addr, local_ts, message->data, message->length);

diff --git a/ntp_signd.c b/ntp_signd.c
index 91434f222740a119e78c956716f87127632e6abf..4a99205277763daadf557a01ffc434dea3010e36 100644 (file)
--- a/ntp_signd.c
+++ b/ntp_signd.c
@@ -323,7 +323,7 @@ NSD_SignAndSendPacket(uint32_t key_id, NTP_Packet *packet, NTP_Remote_Address *r
     return 0;
   }
 
-  if (length != NTP_NORMAL_PACKET_LENGTH) {
+  if (length != NTP_HEADER_LENGTH) {
     DEBUG_LOG("Invalid packet length");
     return 0;
   }

diff --git a/socket.c b/socket.c
index 677a1425ea884b83ba65ac2a0b8b72821b78d382..0a248797cacc87d94766dcafe56cd7b86327e5f0 100644 (file)
--- a/socket.c
+++ b/socket.c
@@ -59,7 +59,7 @@ struct Message {
   struct iovec iov;
   /* Buffer of sufficient length for all expected messages */
   union {
-    NTP_Receive_Buffer ntp_msg;
+    NTP_Packet ntp_msg;
     CMD_Request cmd_request;
     CMD_Reply cmd_reply;
   } msg_buf;

diff --git a/test/unit/ntp_core.c b/test/unit/ntp_core.c
index 9cfff5fbec630f003bc12eff46ab844d095f1724..21f88f668ef3886ec33440707cfca969f765ff40 100644 (file)
--- a/test/unit/ntp_core.c
+++ b/test/unit/ntp_core.c
@@ -31,7 +31,7 @@
 #ifdef FEAT_NTP
 
 static struct timespec current_time;
-static NTP_Receive_Buffer req_buffer, res_buffer;
+static NTP_Packet req_buffer, res_buffer;
 static int req_length, res_length;
 
 #define NIO_OpenServerSocket(addr) ((addr)->ip_addr.family != IPADDR_UNSPEC ? 100 : 0)
@@ -101,7 +101,7 @@ send_request(NCR_Instance inst)
     local_ts.err = 0.0;
     local_ts.source = NTP_TS_KERNEL;
 
-    NCR_ProcessTxKnown(inst, &local_addr, &local_ts, &req_buffer.ntp_pkt, req_length);
+    NCR_ProcessTxKnown(inst, &local_addr, &local_ts, &req_buffer, req_length);
   }
 }
 
@@ -120,7 +120,7 @@ process_request(NTP_Remote_Address *remote_addr)
 
   res_length = 0;
   NCR_ProcessRxUnknown(remote_addr, &local_addr, &local_ts,
-                       &req_buffer.ntp_pkt, req_length);
+                       &req_buffer, req_length);
   res_length = req_length;
   res_buffer = req_buffer;
 
@@ -129,7 +129,7 @@ process_request(NTP_Remote_Address *remote_addr)
   if (random() % 2) {
     local_ts.ts = current_time;
     NCR_ProcessTxUnknown(remote_addr, &local_addr, &local_ts,
-                         &res_buffer.ntp_pkt, res_length);
+                         &res_buffer, res_length);
   }
 }
 
@@ -138,11 +138,12 @@ send_response(int interleaved, int authenticated, int allow_update, int valid_ts
 {
   NTP_Packet *req, *res;
   int auth_len = 0;
+  uint32_t key_id;
 
-  req = &req_buffer.ntp_pkt;
-  res = &res_buffer.ntp_pkt;
+  req = &req_buffer;
+  res = &res_buffer;
 
-  TEST_CHECK(req_length >= NTP_NORMAL_PACKET_LENGTH);
+  TEST_CHECK(req_length >= NTP_HEADER_LENGTH);
 
   res->lvm = NTP_LVM(LEAP_Normal, NTP_LVM_TO_VERSION(req->lvm),
                      NTP_LVM_TO_MODE(req->lvm) == MODE_CLIENT ? MODE_SERVER : MODE_ACTIVE);
@@ -184,18 +185,20 @@ send_response(int interleaved, int authenticated, int allow_update, int valid_ts
   }
 
   if (authenticated) {
-    res->auth_keyid = req->auth_keyid ? req->auth_keyid : htonl(get_random_key_id());
-    auth_len = KEY_GetAuthLength(ntohl(res->auth_keyid));
+    key_id = ntohl(*(uint32_t *)req->extensions);
+    if (key_id == 0)
+      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)
       auth_len = MIN(auth_len, NTP_MAX_V4_MAC_LENGTH - 4);
 
-    if (KEY_GenerateAuth(ntohl(res->auth_keyid), (unsigned char *)res,
-                         NTP_NORMAL_PACKET_LENGTH, res->auth_data, auth_len) != auth_len)
+    if (KEY_GenerateAuth(key_id, (unsigned char *)res, NTP_HEADER_LENGTH,
+                         res->extensions + 4, auth_len) != auth_len)
       assert(0);
-    res_length = NTP_NORMAL_PACKET_LENGTH + 4 + auth_len;
+    res_length = NTP_HEADER_LENGTH + 4 + auth_len;
   } else {
-    res_length = NTP_NORMAL_PACKET_LENGTH;
+    res_length = NTP_HEADER_LENGTH;
   }
 
   if (!valid_auth && authenticated) {
@@ -203,27 +206,29 @@ send_response(int interleaved, int authenticated, int allow_update, int valid_ts
 
     switch (random() % 4) {
       case 0:
-        res->auth_keyid = htonl(ntohl(res->auth_keyid) + 1);
+        key_id++;
         break;
       case 1:
-        res->auth_keyid = htonl(ntohl(res->auth_keyid) ^ 1);
-        if (KEY_GenerateAuth(ntohl(res->auth_keyid), (unsigned char *)res,
-                             NTP_NORMAL_PACKET_LENGTH, res->auth_data, auth_len) != auth_len)
+        key_id ^= 1;
+        if (KEY_GenerateAuth(key_id, (unsigned char *)res, NTP_HEADER_LENGTH,
+                             res->extensions + 4, auth_len) != auth_len)
           assert(0);
         break;
       case 2:
-        res->auth_data[random() % auth_len]++;
+        res->extensions[4 + random() % auth_len]++;
         break;
       case 3:
-        res_length = NTP_NORMAL_PACKET_LENGTH + 4 * (random() % ((4 + auth_len) / 4));
+        res_length = NTP_HEADER_LENGTH + 4 * (random() % ((4 + auth_len) / 4));
         if (NTP_LVM_TO_VERSION(res->lvm) == 4 &&
-            res_length == NTP_NORMAL_PACKET_LENGTH + NTP_MAX_V4_MAC_LENGTH)
+            res_length == NTP_HEADER_LENGTH + NTP_MAX_V4_MAC_LENGTH)
           res_length -= 4;
         break;
       default:
         assert(0);
     }
   }
+
+  *(uint32_t *)res->extensions = htonl(key_id);
 }
 
 static void
@@ -236,7 +241,7 @@ process_response(NCR_Instance inst, int good, int valid, int updated_sync, int u
   struct timespec prev_rx_ts, prev_init_rx_ts;
   int prev_open_socket, ret;
 
-  res = &res_buffer.ntp_pkt;
+  res = &res_buffer;
 
   local_addr.ip_addr.family = IPADDR_UNSPEC;
   local_addr.if_index = INVALID_IF_INDEX;
@@ -285,13 +290,12 @@ process_response(NCR_Instance inst, int good, int valid, int updated_sync, int u
 }
 
 static void
-process_replay(NCR_Instance inst, NTP_Receive_Buffer *packet_queue,
+process_replay(NCR_Instance inst, NTP_Packet *packet_queue,
                int queue_length, int updated_init)
 {
   do {
     res_buffer = packet_queue[random() % queue_length];
-  } while (!UTI_CompareNtp64(&res_buffer.ntp_pkt.transmit_ts,
-                             &inst->remote_ntp_tx));
+  } while (!UTI_CompareNtp64(&res_buffer.transmit_ts, &inst->remote_ntp_tx));
   process_response(inst, 0, 0, 0, updated_init);
   advance_time(1e-6);
 }
@@ -312,7 +316,7 @@ test_unit(void)
   CPS_NTP_Source source;
   NTP_Remote_Address remote_addr;
   NCR_Instance inst1, inst2;
-  NTP_Receive_Buffer packet_queue[PACKET_QUEUE_LENGTH];
+  NTP_Packet packet_queue[PACKET_QUEUE_LENGTH];
 
   CNF_Initialise(0, 0);
   for (i = 0; i < sizeof conf / sizeof conf[0]; i++)