doc: update NEWS

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.

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
 ==================
 
@@ -12,6 +19,7 @@ Enhancements
 * Add dynamic drift removal on Mac OS X
 * Add support for setting real-time priority on Mac OS X
 * Add maxdistance directive to limit source selection by root distance
+  (3 seconds by default)
 * Add refresh command to get new addresses of NTP sources
 * Allow wildcard patterns in include directive
 * Restore time from driftfile with -s option if later than RTC time
@@ -23,7 +31,7 @@ Enhancements
 
 Bug fixes
 ---------
-* Fix building on Solaris
+* Fix building on NetBSD, Solaris
 * Restore time from driftfile with -s option if reading RTC failed
 
 Removed features

chrony.texi.in

@@ -3,7 +3,7 @@
 @afourwide
 @paragraphindent 0
 @setfilename chrony.info
-@settitle User guide for the chrony suite
+@settitle User guide for the chrony suite version @CHRONY_VERSION@
 @c @setchapternewpage off
 
 @ifinfo
@@ -49,7 +49,6 @@ Copyright @copyright{} 2009-2015 Miroslav Lichvar
 * Other time synchronisation packages::  Comparision with other software
 * Distribution and warranty::   There is no warranty
 * Bug reporting::               How to report bugs and make suggestions
-* Contributing::                Areas where contributions are particularly welcome
 @end menu
 @c }}}
 @c {{{ S:Overview
@@ -138,9 +137,9 @@ The `reference' implementation of the Network Time Protocol is the
 program @code{ntpd}, available via
 @uref{http://www.ntp.org/, The NTP home page}.
 
-One of the main differences between @code{ntpd} and @code{chronyd} is in
-the algorithms used to control the computer's clock. Things
-@code{chronyd} can do better than @code{ntpd}:
+One of the main differences between @code{ntpd} and @code{chronyd} is in how
+they control the computer's clock.  Things @code{chronyd} can do better than
+@code{ntpd}:
 
 @itemize @bullet
 @item
@@ -160,13 +159,16 @@ longer periods of time.
 @item
 @code{chronyd} in the default configuration never steps the time to not
 upset other running programs.  @code{ntpd} can be configured to never
-step the time too, but it has to use a different means of adjusting the
-clock, which has some
-disadvantages.
+step the time too, but in that case it has to use a different means of
+adjusting the clock (daemon loop instead of kernel discipline), which may
+have a negative effect on accuracy of the clock.
 @item
 @code{chronyd} can adjust the rate of the clock in a larger range, which
 allows it to operate even on machines with broken or unstable clock
 (e.g. in some virtual machines).
+@item
+@code{chronyd} is smaller, it uses less memory and it wakes up the CPU only
+when necessary, which is better for power saving.
 @end itemize
 
 Things @code{chronyd} can do that @code{ntpd} can't:
@@ -192,21 +194,36 @@ Things @code{ntpd} can do that @code{chronyd} can't:
 
 @itemize @bullet
 @item
-@code{ntpd} supports all operating modes from RFC 5905, including
-broadcast, multicast and manycast client / server.  It supports the
-orphan mode and it also supports authentication based on public-key
-cryptography described in RFC 5906.
+@code{ntpd} supports all operating modes from RFC 5905, including broadcast,
+multicast, and manycast server/client.  However, the broadcast and multicast
+modes are inherently less accurate and less secure (even with authentication)
+than the ordinary server/client mode and should generally be avoided.
 
 @item
-@code{ntpd} has been ported to more types of computer / operating
-system.
+@code{ntpd} supports the Autokey protocol (RFC 5906) to authenticate servers
+with public-key cryptography.  Note that the protocol has been shown to be
+insecure and it will be probably replaced with an implementation of the Network
+Time Security (NTS) specification.
 
 @item
-@code{ntpd} includes drivers for many reference clocks. @code{chronyd}
-relies on other programs (e.g. gpsd) to access the data from the
-reference clocks.
+@code{ntpd} supports the orphan mode, which allows synchronisation to a common
+timescale in isolated networks with multiple servers.  With @code{chronyd}
+there can be only one master and all other computers have to be directly or
+indirectly synchronised to it.
+
+@item
+@code{ntpd} has been ported to more operating systems.
+
+@item
+@code{ntpd} includes a large number of reference clock drivers. @code{chronyd}
+relies on other programs (e.g. @code{gpsd}) to access the timing data via the
+@code{SHM} or @code{SOCK} driver.
 @end itemize
 
+A comparison of NTP implementations that includes more features and also
+their performance is on the @uref{http://chrony.tuxfamily.org/comparison.html,
+chrony comparison} page.
+
 @node Comparison with timed
 @subsection timed
 @code{timed} is a program that is part of the BSD networking suite.  It
@@ -272,39 +289,6 @@ pin-point the problem in some cases.  Please be patient and plan for this!
 Of course, if you can debug the problem yourself and send us a source code
 patch to fix it, we will be very grateful!
 
-@c }}}
-@c {{{ S:Contributions
-@node Contributing
-@section Contributions
-
-Although chrony is now a fairly mature and established project, there are still
-areas that could be improved.  If you can program in C and have some expertise
-in these areas, you might be able to fill the gaps.
-
-Particular areas that need addressing are :
-
-@enumerate
-@item Porting to other Unices
-
-This involves creating equivalents of sys_solaris.c, sys_linux.c etc for the
-new system.
-
-@item Porting to Windows NT
-
-A small amount of work on this was done under Cygwin.  Only the sorting
-out of the include files has really been achieved so far.  The two main
-areas still to address are
-
-@enumerate
-@item The system clock driver.
-@item How to make chronyd into an NT service (i.e. what to replace fork(),
-setsid() etc with so that chronyd can be automatically started in the system
-bootstrap.
-@end enumerate
-
-@item More drivers for reference clock support
-
-@end enumerate
 @c }}}
 @c }}}
 @c {{{ Ch:Installation
@@ -2394,7 +2378,11 @@ includes the accumulated dispersion, which may be large when the source is no
 longer synchronised, and half of the total round-trip delay to the primary
 source.
 
-By default, the maximum distance is 3 seconds.
+By default, the maximum root distance is 3 seconds.
+
+Setting @code{maxdistance} to a larger value can be useful to allow
+synchronisation with a server that only has a very infrequent connection to its
+sources and can accumulate a large dispersion between updates of its clock.
 
 The syntax is
 
@@ -2520,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

client.c

@@ -1366,6 +1366,7 @@ submit_request(CMD_Request *request, CMD_Reply *reply)
       if (recv_status < 0) {
         /* If we get connrefused here, it suggests the sendto is
            going to a dead port */
+        DEBUG_LOG(LOGF_Client, "Could not receive : %s", strerror(errno));
 
         n_attempts++;
         if (n_attempts > max_retries) {

configure

@@ -95,7 +95,7 @@ For better control, use the options below.
   --disable-rtc          Don't include RTC even on Linux
   --disable-privdrop     Disable support for dropping root privileges
   --without-libcap       Don't use libcap even if it is available
-  --disable-scfilter     Disable support for system call filtering
+  --enable-scfilter      Enable support for system call filtering
   --without-seccomp      Don't use seccomp even if it is available
   --disable-asyncdns     Disable asynchronous name resolving
   --disable-forcednsretry Don't retry on permanent DNS error
@@ -200,7 +200,7 @@ try_rtc=0
 feat_droproot=1
 try_libcap=-1
 try_clockctl=0
-feat_scfilter=1
+feat_scfilter=0
 try_seccomp=-1
 readline_lib=""
 readline_inc=""
@@ -305,6 +305,9 @@ do
     --without-libcap|--disable-linuxcaps)
       try_libcap=0
     ;;
+    --enable-scfilter)
+      feat_scfilter=1
+    ;;
     --disable-scfilter)
       feat_scfilter=0
     ;;
@@ -519,6 +522,13 @@ if test_code '<inttypes.h>' 'inttypes.h' '' '' ''; then
   add_def HAVE_INTTYPES_H
 fi
 
+if test_code 'struct in_pktinfo' 'sys/socket.h netinet/in.h' '' '' '
+  struct in_pktinfo ipi;
+  return sizeof (ipi.ipi_spec_dst.s_addr) + IP_PKTINFO;'
+then
+  add_def HAVE_IN_PKTINFO
+fi
+
 if [ $feat_ipv6 = "1" ] && \
   test_code 'IPv6 support' 'arpa/inet.h sys/socket.h netinet/in.h' '' "$EXTRA_LIBS" '
     struct sockaddr_in6 n;
@@ -527,13 +537,13 @@ if [ $feat_ipv6 = "1" ] && \
     return !inet_ntop(AF_INET6, &n.sin6_addr.s6_addr, p, sizeof(p));'
 then
   add_def FEAT_IPV6
-  if test_code 'in6_pktinfo' 'sys/socket.h netinet/in.h' '' '' '
-    return sizeof(struct in6_pktinfo);'
+  if test_code 'struct in6_pktinfo' 'sys/socket.h netinet/in.h' '' '' '
+    return sizeof (struct in6_pktinfo) + IPV6_PKTINFO;'
   then
     add_def HAVE_IN6_PKTINFO
   else
-    if test_code 'in6_pktinfo with _GNU_SOURCE' 'sys/socket.h netinet/in.h' \
-      '-D_GNU_SOURCE' '' 'return sizeof(struct in6_pktinfo);'
+    if test_code 'struct in6_pktinfo with _GNU_SOURCE' 'sys/socket.h netinet/in.h' \
+      '-D_GNU_SOURCE' '' 'return sizeof (struct in6_pktinfo) + IPV6_PKTINFO;'
     then
       add_def _GNU_SOURCE
       add_def HAVE_IN6_PKTINFO
@@ -802,11 +812,13 @@ add_def CHRONYD_FEATURES "\"$chronyd_features $common_features\""
 echo "Features : $chronyd_features $chronyc_features $common_features"
 
 if [ -f version.txt ]; then
-  add_def CHRONY_VERSION "\"`cat version.txt`\""
+  CHRONY_VERSION="`cat version.txt`"
 else
-  add_def CHRONY_VERSION "\"DEVELOPMENT\""
+  CHRONY_VERSION="DEVELOPMENT"
 fi
 
+add_def CHRONY_VERSION "\"${CHRONY_VERSION}\""
+
 for f in Makefile chrony.conf.5 chrony.texi chronyc.1 chronyd.8
 do
   echo Creating $f
@@ -831,7 +843,8 @@ do
           s%@CHRONYSOCKDIR@%${CHRONYSOCKDIR}%;\
           s%@CHRONYVARDIR@%${CHRONYVARDIR}%;\
           s%@DEFAULT_HWCLOCK_FILE@%${default_hwclockfile}%;\
-          s%@DEFAULT_USER@%${default_user}%;"\
+          s%@DEFAULT_USER@%${default_user}%;\
+          s%@CHRONY_VERSION@%${CHRONY_VERSION}%;" \
           < ${f}.in > $f
 done
 

doc/faq.adoc

@@ -49,6 +49,44 @@ added to +chrony+ to deal with this.
 
 == Configuration issues
 
+=== What is the minimum recommended configuration for an NTP client?
+
+First, the client needs to know which NTP servers it should ask for the current
+time.  They are specified by the +server+ or +pool+ directive.  The +pool+
+directive can be used for names that resolve to multiple addresses.  For good
+reliability the client should have at least three servers.  The +iburst+ option
+speeds up the initial synchronisation.
+
+To stabilize the initial synchronisation on the next start, the estimated drift
+of the system clock is saved by adding the +driftfile+ directive.
+
+If the system clock can be far from the true time after boot for any reason,
++chronyd+ should be allowed to correct it quickly by stepping instead of
+slewing, which would take a very long time.  The +makestep+ directive does
+that.
+
+In order to keep the real-time clock (RTC) close to the true time on Linux, so
+the system time is reasonably close to the true time when it's initialized on
+the next boot from the RTC, the +rtcsync+ directive enables a kernel mode in
+which the system time is copied to the RTC every 11 minutes.
+
+If you want to use public NTP servers from the
+http://www.pool.ntp.org/[pool.ntp.org] project, the minimal 'chrony.conf' file
+could be:
+
+----
+pool pool.ntp.org iburst
+driftfile /var/lib/chrony/drift
+makestep 1 3
+rtcsync
+----
+
+=== How do I make an NTP server from an NTP client?
+
+You need to add an +allow+ directive to the 'chrony.conf' file in order to open
+the NTP port and allow +chronyd+ to reply to client requests.  +allow+ with no
+specified subnet allows all IPv4 and IPv6 addresses.
+
 === I have several computers on a LAN. Should be all clients of an external server?
 
 The best configuration is usually to make one computer the master, with

logging.h

@@ -47,7 +47,7 @@ extern int log_debug_enabled;
 
 #if DEBUG > 0
 #define LOG_MESSAGE(severity, facility, ...) \
-  LOG_Message(LOGS_DEBUG, facility, __LINE__, __FILE__, FUNCTION_NAME, __VA_ARGS__);
+  LOG_Message(severity, facility, __LINE__, __FILE__, FUNCTION_NAME, __VA_ARGS__);
 #else
 #define LOG_MESSAGE(severity, facility, ...) \
   LOG_Message(severity, __VA_ARGS__);

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

ntp_io.c

@@ -189,7 +189,7 @@ prepare_socket(int family, int port_number, int client_only)
 #endif
 
   if (family == AF_INET) {
-#ifdef IP_PKTINFO
+#ifdef HAVE_IN_PKTINFO
     /* We want the local IP info on server sockets */
     if (setsockopt(sock_fd, IPPROTO_IP, IP_PKTINFO, (char *)&on_off, sizeof(on_off)) < 0) {
       LOG(LOGS_ERR, LOGF_NtpIO, "Could not set packet info socket option");
@@ -206,14 +206,16 @@ prepare_socket(int family, int port_number, int client_only)
     }
 #endif
 
+#ifdef HAVE_IN6_PKTINFO
 #ifdef IPV6_RECVPKTINFO
     if (setsockopt(sock_fd, IPPROTO_IPV6, IPV6_RECVPKTINFO, (char *)&on_off, sizeof(on_off)) < 0) {
       LOG(LOGS_ERR, LOGF_NtpIO, "Could not set IPv6 packet info socket option");
     }
-#elif defined(IPV6_PKTINFO)
+#else
     if (setsockopt(sock_fd, IPPROTO_IPV6, IPV6_PKTINFO, (char *)&on_off, sizeof(on_off)) < 0) {
       LOG(LOGS_ERR, LOGF_NtpIO, "Could not set IPv6 packet info socket option");
     }
+#endif
 #endif
   }
 #endif
@@ -531,7 +533,7 @@ read_from_socket(void *anything)
     local_addr.sock_fd = sock_fd;
 
     for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
-#ifdef IP_PKTINFO
+#ifdef HAVE_IN_PKTINFO
       if (cmsg->cmsg_level == IPPROTO_IP && cmsg->cmsg_type == IP_PKTINFO) {
         struct in_pktinfo ipi;
 
@@ -541,7 +543,7 @@ read_from_socket(void *anything)
       }
 #endif
 
-#if defined(IPV6_PKTINFO) && defined(HAVE_IN6_PKTINFO)
+#ifdef HAVE_IN6_PKTINFO
       if (cmsg->cmsg_level == IPPROTO_IPV6 && cmsg->cmsg_type == IPV6_PKTINFO) {
         struct in6_pktinfo ipi;
 
@@ -629,7 +631,7 @@ send_packet(void *packet, int packetlen, NTP_Remote_Address *remote_addr, NTP_Lo
   msg.msg_flags = 0;
   cmsglen = 0;
 
-#ifdef IP_PKTINFO
+#ifdef HAVE_IN_PKTINFO
   if (local_addr->ip_addr.family == IPADDR_INET4) {
     struct cmsghdr *cmsg;
     struct in_pktinfo *ipi;
@@ -647,7 +649,7 @@ send_packet(void *packet, int packetlen, NTP_Remote_Address *remote_addr, NTP_Lo
   }
 #endif
 
-#if defined(IPV6_PKTINFO) && defined(HAVE_IN6_PKTINFO)
+#ifdef HAVE_IN6_PKTINFO
   if (local_addr->ip_addr.family == IPADDR_INET6) {
     struct cmsghdr *cmsg;
     struct in6_pktinfo *ipi;

test/compilation/001-features

@@ -6,12 +6,12 @@ cd ../..
 
 for opts in \
 	"--enable-debug" \
+	"--enable-scfilter" \
 	"--disable-asyncdns" \
 	"--disable-ipv6" \
 	"--disable-privdrop" \
 	"--disable-readline" \
 	"--disable-rtc" \
-	"--disable-scfilter" \
 	"--disable-sechash" \
 	"--disable-cmdmon" \
 	"--disable-ntp" \

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