diff --git a/keys.c b/keys.c index a7c53f4..1305e82 100644 --- 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 7b7f361..eacbacd 100644 --- 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 4eb3674..4013672 100644 --- 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 7a70ff4..a70f6c6 100644 --- 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 6519791..634ccb0 100644 --- 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 91434f2..4a99205 100644 --- 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 677a142..0a24879 100644 --- 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 9cfff5f..21f88f6 100644 --- 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++)