diff --git a/ntp_auth.c b/ntp_auth.c index 50b78a8..58c8988 100644 --- 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 c306b03..60c0303 100644 --- 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 63b2d55..a7f89ee 100644 --- 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);