faraphel/chrony
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

ntp: drop support for long NTPv4 MACs

Browse source 
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.
This commit is contained in:
Miroslav Lichvar 2020-09-10 10:22:27 +02:00
parent 2bb88b45c6
commit fdfcabd79b
3 changed files with 20 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
ntp_auth.c
View file

@ -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;

12
ntp_core.c
View file

@ -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,

21
test/unit/ntp_core.c
View file

@ -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);