From a78bf9725a7b481ebff0e0c321294ba767f2c1d8 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Fri, 8 Jan 2016 15:03:09 +0100 Subject: [PATCH 1/4] ntp: restrict authentication of server/peer to specified key When a server/peer was specified with a key number to enable authentication with a symmetric key, packets received from the server/peer were accepted if they were authenticated with any of the keys contained in the key file and not just the specified key. This allowed an attacker who knew one key of a client/peer to modify packets from its servers/peers that were authenticated with other keys in a man-in-the-middle (MITM) attack. For example, in a network where each NTP association had a separate key and all hosts had only keys they needed, a client of a server could not attack other clients of the server, but it could attack the server and also attack its own clients (i.e. modify packets from other servers). To not allow the server/peer to be authenticated with other keys extend the authentication test to check if the key ID in the received packet is equal to the configured key number. As a consequence, it's no longer possible to authenticate two peers to each other with two different keys, both peers have to be configured to use the same key. This issue was discovered by Matt Street of Cisco ASIG. --- chrony.texi.in | 3 +++ ntp_core.c | 14 ++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/chrony.texi.in b/chrony.texi.in index b56fb81..25f807b 100644 --- a/chrony.texi.in +++ b/chrony.texi.in @@ -2508,6 +2508,9 @@ The syntax of this directive is identical to that for the @code{server} directive (@pxref{server directive}), except that it is used to specify an NTP peer rather than an NTP server. +When a key is specified by the @code{key} option to enable authentication, both +peers must be configured to use the same key and the same key number. + Please note that NTP peers that are not configured with a key to enable authentication are vulnerable to a denial-of-service attack. An attacker knowing that NTP hosts A and B are peering with each other can send a packet diff --git a/ntp_core.c b/ntp_core.c index b477666..2843858 100644 --- a/ntp_core.c +++ b/ntp_core.c @@ -1099,7 +1099,7 @@ static int receive_packet(NTP_Packet *message, struct timeval *now, double now_err, NCR_Instance inst, NTP_Local_Address *local_addr, int length) { int pkt_leap; - uint32_t pkt_refid; + uint32_t pkt_refid, pkt_key_id; double pkt_root_delay; double pkt_root_dispersion; @@ -1190,11 +1190,13 @@ receive_packet(NTP_Packet *message, struct timeval *now, double now_err, NCR_Ins function is called only for known sources. */ /* Test 5 checks for authentication failure. If we expect authenticated info - from this peer/server and the packet doesn't have it or the authentication - is bad, it's got to fail. If the peer or server sends us an authenticated - frame, but we're not bothered about whether he authenticates or not, just - ignore the test. */ - test5 = inst->do_auth ? check_packet_auth(message, length, NULL, NULL) : 1; + from this peer/server and the packet doesn't have it, the authentication + is bad, or it's authenticated with a different key than expected, it's got + to fail. If we don't expect the packet to be authenticated, just ignore + the test. */ + test5 = !inst->do_auth || + (check_packet_auth(message, length, NULL, &pkt_key_id) && + pkt_key_id == inst->auth_key_id); /* Test 6 checks for unsynchronised server */ test6 = pkt_leap != LEAP_Unsynchronised && From 05236a4f2339bed473ed9a70b5b869470c791fcc Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Mon, 11 Jan 2016 16:23:07 +0100 Subject: [PATCH 2/4] test: allow setting options for each peer side separately --- test/simulation/test.common | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/simulation/test.common b/test/simulation/test.common index 03476fd..fbde820 100644 --- a/test/simulation/test.common +++ b/test/simulation/test.common @@ -69,7 +69,11 @@ default_client_server_conf="" default_server_server_options="" default_client_server_options="" default_server_peer_options="" +default_server_lpeer_options="" +default_server_rpeer_options="" default_client_peer_options="" +default_client_lpeer_options="" +default_client_rpeer_options="" default_server_conf="" default_client_conf="" default_chronyc_conf="" @@ -189,7 +193,8 @@ get_chronyd_conf() { done for i in $(seq 1 $peers); do [ $i -eq $peer -o $i -gt $servers ] && continue - echo "peer 192.168.123.$[$servers * ($stratum - 1) + $i] $server_peer_options" + echo -n "peer 192.168.123.$[$servers * ($stratum - 1) + $i] $server_peer_options " + [ $i -lt $peer ] && echo "$server_lpeer_options" || echo "$server_rpeer_options" done echo "$server_conf" else @@ -202,7 +207,8 @@ get_chronyd_conf() { fi for i in $(seq 1 $peers); do [ $i -eq $peer -o $i -gt $clients ] && continue - echo "peer 192.168.123.$[$servers * ($stratum - 1) + $i] $client_peer_options" + echo -n "peer 192.168.123.$[$servers * ($stratum - 1) + $i] $client_peer_options " + [ $i -lt $peer ] && echo "$client_lpeer_options" || echo "$client_rpeer_options" done echo "$client_conf" fi From 86c21a3a857655167f57fc597fd51968824aa4ad Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Mon, 11 Jan 2016 16:40:29 +0100 Subject: [PATCH 3/4] test: extend 105-ntpauth to test symmetric mode --- test/simulation/105-ntpauth | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/simulation/105-ntpauth b/test/simulation/105-ntpauth index 66ea2a5..6968e08 100755 --- a/test/simulation/105-ntpauth +++ b/test/simulation/105-ntpauth @@ -39,4 +39,24 @@ check_chronyd_exit || test_fail # This check must fail as the client doesn't know the key check_sync && test_fail check_packet_interval || test_fail + +client_conf="keyfile tmp/keys" +clients=2 +peers=2 +max_sync_time=300 +base_delay="$default_base_delay (* -1 (equal 0.1 from 3) (equal 0.1 to 1))" +client_lpeer_options="key 1" +client_rpeer_options="key 1" + +run_test || test_fail +check_chronyd_exit || test_fail +check_sync || test_fail + +client_rpeer_options="key 2" + +run_test || test_fail +check_chronyd_exit || test_fail +# This check must fail as the peers are using different keys" +check_sync && test_fail + test_pass From beb275a769968217733a449988d189e503f0bfbc Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Mon, 11 Jan 2016 15:42:36 +0100 Subject: [PATCH 4/4] doc: update NEWS --- NEWS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS b/NEWS index a4e891c..671c762 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,10 @@ +New in version 2.2.1 +==================== + +Security fixes +-------------- +* Restrict authentication of NTP server/peer to specified key (CVE-2016-1567) + New in version 2.2 ==================