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,20 @@
+New in version 1.31.2
+=====================
+
+Security fixes
+--------------
+* Restrict authentication of NTP server/peer to specified key (CVE-2016-1567)
+
+New in version 1.31.1
+=====================
+
+Security fixes
+--------------
+* Protect authenticated symmetric NTP associations against DoS attacks
+  (CVE-2015-1853)
+* Fix access configuration with subnet size indivisible by 4 (CVE-2015-1821)
+* Fix initialization of reply slots for authenticated commands (CVE-2015-1822)
+
 New in version 1.31
 ===================
 
@@ -11,7 +28,10 @@ Enhancements
 * Use NTP packets instead of UDP echo for presend
 * Don't adjust polling interval when sending fails
 * Allow binding to addresses that don't exist yet
+* Ignore measurements around leap second
 * Improve detection of unexpected time jumps
+* Include example of logrotate configuration, systemd services and
+  NetworkManager dispatcher script
 
 Bug fixes
 ---------

addrfilt.c

@@ -199,7 +199,10 @@ set_subnet(TableNode *start_node,
 
       /* How many subnet entries to set : 1->8, 2->4, 3->2 */
       N = 1 << (NBITS-bits_to_go);
-      subnet = get_subnet(ip, bits_consumed);
+
+      subnet = get_subnet(ip, bits_consumed) & ~(N - 1);
+      assert(subnet + N <= TABLE_SIZE);
+
       if (!(node->extended)) {
         open_node(node);
       }

chrony.texi.in

@@ -873,9 +873,9 @@ For the @file{@SYSCONFDIR@/chrony.conf} file, the following can be used as an
 example.
 
 @example
-server 0.pool.ntp.org minpoll 5 maxpoll 10 maxdelay 0.4 offline
+server 0.pool.ntp.org maxdelay 0.4 offline
-server 1.pool.ntp.org minpoll 5 maxpoll 10 maxdelay 0.4 offline
+server 1.pool.ntp.org maxdelay 0.4 offline
-server 2.pool.ntp.org minpoll 5 maxpoll 10 maxdelay 0.4 offline
+server 2.pool.ntp.org maxdelay 0.4 offline
 logdir /var/log/chrony
 log statistics measurements tracking
 driftfile @CHRONYVARDIR@/drift
@@ -1114,7 +1114,8 @@ specified with a command line option.
 
 Each command in the configuration file is placed on a separate line.
 The following sections describe each of the commands in turn.  The
-directives can occur in any order in the file.
+directives can occur in any order in the file and they are not
+case-sensitive.
 
 The configuration commands can also be specified directly on the
 @code{chronyd} command line, each argument is parsed as a line and
@@ -2459,6 +2460,24 @@ be reported using the @code{clients} command in @code{chronyc}.
 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
+with random timestamps to host A with source address of B which will set the
+NTP state variables on A to the values sent by the attacker.  Host A will then
+send on its next poll to B a packet with originate timestamp that doesn't match
+the transmit timestamp of B and the packet will be dropped.  If the attacker
+does this periodically for both hosts, they won't be able to synchronize to
+each other.
+
+This attack can be prevented by enabling authentication with the key option, or
+using the @code{server} directive on both sides to specify the other host as a
+server instead of peer, the only drawback is that it will double the network
+traffic between the two hosts.
 @c }}}
 @c {{{ pidfile
 @node pidfile directive
@@ -4628,6 +4647,43 @@ On Linux, if @code{chronyd} is compiled with support for Linux capabilities
 the `-u' option or @code{user} directive in the @file{chrony.conf} file to drop
 root privileges after start.  The configure option @code{--with-user} can be
 used to drop the privileges by default.
+
+@subsection How can I improve the accuracy of the system clock with NTP sources?
+Select NTP servers that are well synchronised, stable and close to your network.
+It's better to use more than one server, three or four is usually recommended as
+the minimum, so @code{chronyd} can detect falsetickers and combine measurements
+from multiple sources.
+
+There are also useful options which can be set in the @code{server} directive,
+they are @code{minpoll}, @code{maxpoll}, @code{polltarget}, @code{maxdelay},
+@code{maxdelayratio} and @code{maxdelaydevratio}.
+
+The first three options set the minimum and maximum allowed polling interval,
+and how should be the actual interval adjusted in the specified range.  Their
+default values are suitable for public NTP servers, which normally don't allow
+too frequent polling, but if you run your own NTP servers or have permission to
+poll the servers frequently, setting the options for shorter polling intervals
+may significantly improve the accuracy of the system clock.
+
+The optimal polling interval depends on many factors, this includes the ratio
+between the wander of the clock and the network jitter (sometimes expressed in
+NTP documents as the Allan intercept), the temperature sensitivity of the
+crystal oscillator and the maximum rate of change of the temperature.  An
+example of the directive for a server located in the same LAN could be
+
+@example
+server ntp.local minpoll 2 maxpoll 4 polltarget 30
+@end example
+
+The maxdelay options are useful to ignore measurements with larger delay (e.g.
+due to congestion in the network) and improve the stability of the
+synchronisation.  The @code{maxdelaydevratio} option could be added to the
+previous example
+
+@example
+server ntp.local minpoll 2 maxpoll 4 polltarget 30 maxdelaydevratio 2
+@end example
+
 @c }}}
 @c {{{ S:Computer is not synchronising
 @node Computer is not synchronising

client.c

@@ -2616,7 +2616,7 @@ authenticate_from_config(const char *filename)
 
   in = fopen(filename, "r");
   if (!in) {
-    fprintf(stderr, "Could not open file %s\n", filename);
+    fprintf(stderr, "Could not open file %s : %s\n", filename, strerror(errno));
     return 0;
   }
 
@@ -2641,7 +2641,7 @@ authenticate_from_config(const char *filename)
 
   in = fopen(keyfile, "r");
   if (!in) {
-    fprintf(stderr, "Could not open keyfile %s\n", keyfile);
+    fprintf(stderr, "Could not open keyfile %s : %s\n", keyfile, strerror(errno));
     return 0;
   }
 

cmdmon.c

@@ -566,6 +566,7 @@ get_more_replies(void)
     for (i=1; i<REPLY_EXTEND_QUANTUM; i++) {
       new_replies[i-1].next = new_replies + i;
     }
+    new_replies[REPLY_EXTEND_QUANTUM - 1].next = NULL;
     free_replies = new_replies;
   }
 }

cmdparse.c

@@ -204,7 +204,7 @@ CPS_NormalizeLine(char *line)
 
   /* Remove white-space at beginning and replace white-spaces with space char */
   for (p = q = line; *p; p++) {
-    if (isspace(*p)) {
+    if (isspace((unsigned char)*p)) {
       if (!space)
         *q++ = ' ';
       space = 1;
@@ -234,15 +234,15 @@ CPS_SplitWord(char *line)
   char *p = line, *q = line;
 
   /* Skip white-space before the word */
-  while (*q && isspace(*q))
+  while (*q && isspace((unsigned char)*q))
     q++;
 
   /* Move the word to the beginning */
-  while (*q && !isspace(*q))
+  while (*q && !isspace((unsigned char)*q))
     *p++ = *q++;
 
   /* Find the next word */
-  while (*q && isspace(*q))
+  while (*q && isspace((unsigned char)*q))
     q++;
 
   *p = '\0';

configure

@@ -160,6 +160,13 @@ add_def () {
   fi
 }
 #}}}
+#{{{ pkg_config
+pkg_config () {
+  type pkg-config > /dev/null 2> /dev/null || return 1
+
+  pkg-config $@ 2> /dev/null
+}
+#}}}
 
 # ======================================================================
 
@@ -604,8 +611,8 @@ HASH_COMPILE=""
 HASH_LINK=""
 
 if [ $try_nss = "1" ]; then
-  test_cflags="`pkg-config --cflags nss 2> /dev/null`"
+  test_cflags="`pkg_config --cflags nss`"
-  test_link="`pkg-config --libs-only-L nss 2> /dev/null` -lfreebl3"
+  test_link="`pkg_config --libs-only-L nss` -lfreebl3"
   if test_code 'NSS' 'nss.h hasht.h nsslowhash.h' \
     "$test_cflags" "$test_link" \
     'NSSLOWHASH_Begin(NSSLOWHASH_NewContext(NSSLOW_Init(), HASH_AlgSHA512));'

examples/chrony-wait.service

@@ -0,0 +1,17 @@
+[Unit]
+Description=Wait for chrony to synchronize system clock
+After=chronyd.service
+Requires=chronyd.service
+Before=time-sync.target
+Wants=time-sync.target
+
+[Service]
+Type=oneshot
+# Wait up to ~10 minutes for chronyd to synchronize and the remaining
+# clock correction to be less than 0.1 seconds
+ExecStart=/usr/bin/chronyc waitsync 60 0.1
+RemainAfterExit=yes
+StandardOutput=null
+
+[Install]
+WantedBy=multi-user.target

examples/chrony.logrotate

@@ -0,0 +1,8 @@
+/var/log/chrony/*.log {
+    missingok
+    nocreate
+    sharedscripts
+    postrotate
+        /usr/bin/chronyc -a cyclelogs > /dev/null 2>&1 || true
+    endscript
+}

examples/chrony.nm-dispatcher

@@ -0,0 +1,17 @@
+#!/bin/sh
+# This is a NetworkManager dispatcher script for chronyd to set its NTP sources
+# online/offline when a default route is configured/removed on the system.
+
+export LC_ALL=C
+
+if [ "$2" = "up" ]; then
+	/sbin/ip route list dev "$1" | grep -q '^default' &&
+		/usr/bin/chronyc -a online > /dev/null 2>&1
+fi
+
+if [ "$2" = "down" ]; then
+	/sbin/ip route list | grep -q '^default' ||
+		/usr/bin/chronyc -a offline > /dev/null 2>&1
+fi
+
+exit 0

examples/chronyd.service

@@ -0,0 +1,13 @@
+[Unit]
+Description=NTP client/server
+After=ntpdate.service sntp.service ntpd.service
+Conflicts=ntpd.service systemd-timesyncd.service
+
+[Service]
+Type=forking
+PIDFile=/var/run/chronyd.pid
+EnvironmentFile=-/etc/sysconfig/chronyd
+ExecStart=/usr/sbin/chronyd $OPTIONS
+
+[Install]
+WantedBy=multi-user.target

getdate.c

@@ -2532,7 +2532,7 @@ LookupWord (buff)
   /* Make it lowercase. */
   for (p = buff; *p; p++)
     if (ISUPPER ((unsigned char) *p))
-      *p = tolower (*p);
+      *p = tolower ((unsigned char) *p);
 
   if (strcmp (buff, "am") == 0 || strcmp (buff, "a.m.") == 0)
     {

getdate.y

@@ -711,7 +711,7 @@ LookupWord (buff)
   /* Make it lowercase. */
   for (p = buff; *p; p++)
     if (ISUPPER ((unsigned char) *p))
-      *p = tolower (*p);
+      *p = tolower ((unsigned char) *p);
 
   if (strcmp (buff, "am") == 0 || strcmp (buff, "a.m.") == 0)
     {

logging.h

@@ -29,6 +29,8 @@
 #ifndef GOT_LOGGING_H
 #define GOT_LOGGING_H
 
+#include "sysincl.h"
+
 /* Flag indicating whether debug messages are logged */
 extern int log_debug_enabled;
 

make_release

@@ -37,7 +37,7 @@ cd RELEASES/$subdir || exit 1
 
 echo $version > version.txt
 
-sed -e "s%@@VERSION@@%${version}%" < chrony.spec.sample > chrony.spec
+sed -i -e "s%@@VERSION@@%${version}%" examples/chrony.spec
 
 for m in chrony.1 chronyc.1.in chrony.conf.5.in chronyd.8.in; do
   sed -e "s%@VERSION@%${version}%;s%@MAN_DATE@%${mandate}%" \
@@ -69,7 +69,7 @@ if [ $(wc -l < FAQ) -gt 400 -o $(wc -l < FAQ) -lt 200 ]; then
   exit 3
 fi
 
-rm -f config.h config.log faqgen.pl make_release chrony.spec.sample .gitignore
+rm -f config.h config.log make_release .gitignore
 
 cd ..
 tar cv --owner root --group root $subdir | gzip -9 > ${subdir}.tar.gz

ntp_core.c

@@ -262,6 +262,9 @@ do_size_checks(void)
 static void
 do_time_checks(void)
 {
+  struct timeval now;
+  time_t warning_advance = 3600 * 24 * 365 * 10; /* 10 years */
+
 #ifdef HAVE_LONG_TIME_T
   /* Check that time before NTP_ERA_SPLIT underflows correctly */
 
@@ -278,6 +281,16 @@ do_time_checks(void)
       tv1.tv_sec + (1ULL << 32) - 1 == tv2.tv_sec;
 
   assert(r);
+
+  LCL_ReadRawTime(&now);
+  if (tv2.tv_sec - now.tv_sec < warning_advance)
+    LOG(LOGS_WARN, LOGF_NtpCore, "Assumed NTP time ends at %s!",
+        UTI_TimeToLogForm(tv2.tv_sec));
+#else
+  LCL_ReadRawTime(&now);
+  if (now.tv_sec > 0x7fffffff - warning_advance)
+    LOG(LOGS_WARN, LOGF_NtpCore, "System time ends at %s!",
+        UTI_TimeToLogForm(0x7fffffff));
 #endif
 }
 
@@ -992,9 +1005,6 @@ receive_packet(NTP_Packet *message, struct timeval *now, double now_err, NCR_Ins
 
   /* ==================== */
 
-  /* Save local receive timestamp */
-  inst->local_rx = *now;
-
   pkt_leap = (message->lvm >> 6) & 0x3;
   if (pkt_leap == 0x3) {
     source_is_synchronized = 0;
@@ -1026,14 +1036,6 @@ receive_packet(NTP_Packet *message, struct timeval *now, double now_err, NCR_Ins
     test2 = 1; /* Success */
   }
 
-  /* Regardless of any validity checks we apply, we are required to
-     save this field from the packet into the ntp source
-     instance record.  See RFC1305 section 3.4.4, peer.org <- pkt.xmt
-     & peer.peerpoll <- pkt.poll.  Note we can't do this assignment
-     before test1 has been carried out!! */
-
-  inst->remote_orig = message->transmit_ts;
-
   /* Test 3 requires that pkt.org != 0 and pkt.rec != 0.  If
      either of these are true it means the association is not properly
      'up'. */
@@ -1138,7 +1140,8 @@ receive_packet(NTP_Packet *message, struct timeval *now, double now_err, NCR_Ins
   if (inst->do_auth) {
     if (auth_len > 0) {
       auth_key_id = ntohl(message->auth_keyid);
-      test5 = check_packet_auth(message, auth_key_id, auth_len);
+      test5 = check_packet_auth(message, auth_key_id, auth_len) &&
+              auth_key_id == inst->auth_key_id;
     } else {
       /* If we expect authenticated info from this peer/server and the packet
          doesn't have it, it's got to fail */
@@ -1206,6 +1209,14 @@ receive_packet(NTP_Packet *message, struct timeval *now, double now_err, NCR_Ins
         kod_rate = 1;
   }
 
+  /* The transmit timestamp and local receive timestamp must not be saved when
+     the authentication test failed to prevent denial-of-service attacks on
+     symmetric associations using authentication */
+  if (test5) {
+    inst->remote_orig = message->transmit_ts;
+    inst->local_rx = *now;
+  }
+
   valid_kod = test1 && test2 && test5;
 
   valid_data = test1 && test2 && test3 && test4 && test4a && test4b;

reference.c

@@ -680,7 +680,7 @@ update_leap_status(NTP_Leap leap, time_t now)
     }
   }
   
-  if (leap_sec != our_leap_sec) {
+  if (leap_sec != our_leap_sec && !REF_IsLeapSecondClose()) {
     LCL_SetLeap(leap_sec);
     our_leap_sec = leap_sec;
   }
@@ -1150,6 +1150,31 @@ REF_IsLocalActive(void)
 
 /* ================================================== */
 
+#define LEAP_SECOND_CLOSE 5
+
+int REF_IsLeapSecondClose(void)
+{
+  struct timeval now, now_raw;
+  time_t t;
+
+  if (!our_leap_sec)
+    return 0;
+
+  SCH_GetLastEventTime(&now, NULL, &now_raw);
+
+  t = now.tv_sec > 0 ? now.tv_sec : -now.tv_sec;
+  if ((t + LEAP_SECOND_CLOSE) % (24 * 3600) < 2 * LEAP_SECOND_CLOSE)
+    return 1;
+
+  t = now_raw.tv_sec > 0 ? now_raw.tv_sec : -now_raw.tv_sec;
+  if ((t + LEAP_SECOND_CLOSE) % (24 * 3600) < 2 * LEAP_SECOND_CLOSE)
+    return 1;
+
+  return 0;
+}
+
+/* ================================================== */
+
 void
 REF_GetTrackingReport(RPT_TrackingReport *rep)
 {

reference.h

@@ -161,6 +161,10 @@ extern void REF_EnableLocal(int stratum);
 extern void REF_DisableLocal(void);
 extern int REF_IsLocalActive(void);
 
+/* Check if current raw or cooked time is close to a leap second
+   and is better to discard any measurements */
+extern int REF_IsLeapSecondClose(void);
+
 extern void REF_GetTrackingReport(RPT_TrackingReport *rep);
 
 #endif /* GOT_REFERENCE_H */

sched.c

@@ -618,6 +618,8 @@ SCH_MainLoop(void)
 
     } else {
       ptv = NULL;
+      /* This is needed to fix a compiler warning */
+      saved_tv.tv_sec = 0;
     }
 
     /* if there are no file descriptors being waited on and no

sources.c

@@ -312,6 +312,11 @@ void SRC_AccumulateSample
   DEBUG_LOG(LOGF_Sources, "ip=[%s] t=%s ofs=%f del=%f disp=%f str=%d",
       source_to_string(inst), UTI_TimevalToString(sample_time), -offset, root_delay, root_dispersion, stratum);
 
+  if (REF_IsLeapSecondClose()) {
+    LOG(LOGS_INFO, LOGF_Sources, "Dropping sample around leap second");
+    return;
+  }
+
   /* WE HAVE TO NEGATE OFFSET IN THIS CALL, IT IS HERE THAT THE SENSE OF OFFSET
      IS FLIPPED */
   SST_AccumulateSample(inst->stats, sample_time, -offset, peer_delay, peer_dispersion, root_delay, root_dispersion, stratum);

sys_linux.c

@@ -367,7 +367,7 @@ SYS_Linux_DropRoot(char *user)
   }
 
   if (prctl(PR_SET_KEEPCAPS, 1)) {
-    LOG_FATAL(LOGF_SysLinux, "prcap() failed");
+    LOG_FATAL(LOGF_SysLinux, "prctl() failed");
   }
   
   if (setgroups(0, NULL)) {

sys_netbsd.c

@@ -281,7 +281,6 @@ SYS_NetBSD_Initialise(void)
   };
 
   kvm_t *kt;
-  FILE *fp;
 
   kt = kvm_open(NULL, NULL, NULL, O_RDONLY, NULL);
   if (!kt) {

util.c

@@ -550,13 +550,15 @@ UTI_Int64ToTimeval(NTP_int64 *src,
 void
 UTI_TimevalNetworkToHost(Timeval *src, struct timeval *dest)
 {
-  uint32_t sec_low, sec_high;
+  uint32_t sec_low;
+#ifdef HAVE_LONG_TIME_T
+  uint32_t sec_high;
+#endif
 
   dest->tv_usec = ntohl(src->tv_nsec) / 1000;
-  sec_high = ntohl(src->tv_sec_high);
   sec_low = ntohl(src->tv_sec_low);
-
 #ifdef HAVE_LONG_TIME_T
+  sec_high = ntohl(src->tv_sec_high);
   if (sec_high == TV_NOHIGHSEC)
     sec_high = 0;