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 ================== diff --git a/chrony.texi.in b/chrony.texi.in index 654071a..e831b08 100644 --- a/chrony.texi.in +++ b/chrony.texi.in @@ -2521,6 +2521,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 3b18f8b..8428642 100644 --- a/ntp_core.c +++ b/ntp_core.c @@ -1152,7 +1152,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; @@ -1243,11 +1243,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 && 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 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