Merge branch '2.2-security'

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
 ==================
 

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

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
+     from this peer/server and the packet doesn't have it, the authentication
-     is bad, it's got to fail.  If the peer or server sends us an authenticated
+     is bad, or it's authenticated with a different key than expected, it's got
-     frame, but we're not bothered about whether he authenticates or not, just
+     to fail.  If we don't expect the packet to be authenticated, just ignore
-     ignore the test. */
+     the test. */
-  test5 = inst->do_auth ? check_packet_auth(message, length, NULL, NULL) : 1;
+  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 &&

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

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