doc: update NEWS

When writing the pidfile, open the file with the O_CREAT|O_EXCL flags
to avoid following a symlink and writing the PID to an unexpected file,
when chronyd still has the root privileges.

The Linux open(2) man page warns about O_EXCL not working as expected on
NFS versions before 3 and Linux versions before 2.6. Saving pidfiles on
a distributed filesystem like NFS is not generally expected, but if
there is a reason to do that, these old kernel and NFS versions are not
considered to be supported for saving files by chronyd.

This is a minimal backport specific to this issue of the following
commits:
- commit 2fc8edacb8 ("use PATH_MAX")
- commit f4c6a00b2a ("logging: call exit() in LOG_Message()")
- commit 7a4c396bba ("util: add functions for common file operations")
- commit e18903a6b5 ("switch to new util file functions")

Reported-by: Matthias Gerstner <mgerstner@suse.de>

NEWS

@@ -1,9 +1,18 @@
+New in version 3.5.1
+====================
+
+Security fixes
+--------------
+* Create new file when writing pidfile (CVE-2020-14367)
+
 New in version 3.5
 ==================
 
 Enhancements
 ------------
 * Add support for more accurate reading of PHC on Linux 5.0
+* Add support for hardware timestamping on interfaces with read-only
+  timestamping configuration
 * Add support for memory locking and real-time priority on FreeBSD,
   NetBSD, Solaris
 * Update seccomp filter to work on more architectures

client.c

@@ -3159,7 +3159,7 @@ static void
 display_gpl(void)
 {
     printf("chrony version %s\n"
-           "Copyright (C) 1997-2003, 2007, 2009-2018 Richard P. Curnow and others\n"
+           "Copyright (C) 1997-2003, 2007, 2009-2019 Richard P. Curnow and others\n"
            "chrony comes with ABSOLUTELY NO WARRANTY.  This is free software, and\n"
            "you are welcome to redistribute it under certain conditions.  See the\n"
            "GNU General Public License version 2 for details.\n\n",

configure

@@ -6,6 +6,7 @@
 # Copyright (C) Richard P. Curnow  1997-2003
 # Copyright (C) Bryan Christianson  2016
 # Copyright (C) Miroslav Lichvar  2009, 2012-2018
+# Copyright (C) Stefan R. Filipek  2019
 #
 # =======================================================================
 

doc/faq.adoc

@@ -267,6 +267,10 @@ maxchange 1000 1 1
 maxclockerror 15
 ----
 
+Note that increasing `minsamples` may cause the offsets in the `tracking` and
+`sourcestats` reports/logs to be significantly smaller than the actual offsets
+and be unsuitable for monitoring.
+
 === What happened to the `commandkey` and `generatecommandkey` directives?
 
 They were removed in version 2.2. Authentication is no longer supported in the

examples/chronyd.service

@@ -7,7 +7,7 @@ ConditionCapability=CAP_SYS_TIME
 
 [Service]
 Type=forking
-PIDFile=/var/run/chrony/chronyd.pid
+PIDFile=/run/chrony/chronyd.pid
 EnvironmentFile=-/etc/sysconfig/chronyd
 ExecStart=/usr/sbin/chronyd $OPTIONS
 PrivateTmp=yes

logging.c

@@ -171,6 +171,7 @@ void LOG_Message(LOG_Severity severity,
         system_log = 0;
         log_message(1, severity, buf);
       }
+      exit(1);
       break;
     default:
       assert(0);

main.c

@@ -281,13 +281,9 @@ write_pidfile(void)
   if (!pidfile[0])
     return;
 
-  out = fopen(pidfile, "w");
+  out = UTI_OpenFile(NULL, pidfile, NULL, 'W', 0644);
-  if (!out) {
+  fprintf(out, "%d\n", (int)getpid());
-    LOG_FATAL("Could not open %s : %s", pidfile, strerror(errno));
+  fclose(out);
-  } else {
-    fprintf(out, "%d\n", (int)getpid());
-    fclose(out);
-  }
 }
 
 /* ================================================== */

ntp_io_linux.c

@@ -2,7 +2,7 @@
   chronyd/chronyc - Programs for keeping computer clocks accurate.
 
  **********************************************************************
- * Copyright (C) Miroslav Lichvar  2016-2018
+ * Copyright (C) Miroslav Lichvar  2016-2019
  * 
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of version 2 of the GNU General Public License as
@@ -123,7 +123,7 @@ add_interface(CNF_HwTsInterface *conf_iface)
   struct ethtool_ts_info ts_info;
   struct hwtstamp_config ts_config;
   struct ifreq req;
-  int sock_fd, if_index, phc_fd, req_hwts_flags;
+  int sock_fd, if_index, phc_fd, req_hwts_flags, rx_filter;
   unsigned int i;
   struct Interface *iface;
 
@@ -177,40 +177,51 @@ add_interface(CNF_HwTsInterface *conf_iface)
     return 0;
   }
 
-  ts_config.flags = 0;
-  ts_config.tx_type = HWTSTAMP_TX_ON;
-
   switch (conf_iface->rxfilter) {
     case CNF_HWTS_RXFILTER_ANY:
 #ifdef HAVE_LINUX_TIMESTAMPING_RXFILTER_NTP
       if (ts_info.rx_filters & (1 << HWTSTAMP_FILTER_NTP_ALL))
-        ts_config.rx_filter = HWTSTAMP_FILTER_NTP_ALL;
+        rx_filter = HWTSTAMP_FILTER_NTP_ALL;
       else
 #endif
       if (ts_info.rx_filters & (1 << HWTSTAMP_FILTER_ALL))
-        ts_config.rx_filter = HWTSTAMP_FILTER_ALL;
+        rx_filter = HWTSTAMP_FILTER_ALL;
       else
-        ts_config.rx_filter = HWTSTAMP_FILTER_NONE;
+        rx_filter = HWTSTAMP_FILTER_NONE;
       break;
     case CNF_HWTS_RXFILTER_NONE:
-      ts_config.rx_filter = HWTSTAMP_FILTER_NONE;
+      rx_filter = HWTSTAMP_FILTER_NONE;
       break;
 #ifdef HAVE_LINUX_TIMESTAMPING_RXFILTER_NTP
     case CNF_HWTS_RXFILTER_NTP:
-      ts_config.rx_filter = HWTSTAMP_FILTER_NTP_ALL;
+      rx_filter = HWTSTAMP_FILTER_NTP_ALL;
       break;
 #endif
     default:
-      ts_config.rx_filter = HWTSTAMP_FILTER_ALL;
+      rx_filter = HWTSTAMP_FILTER_ALL;
       break;
   }
 
+  ts_config.flags = 0;
+  ts_config.tx_type = HWTSTAMP_TX_ON;
+  ts_config.rx_filter = rx_filter;
   req.ifr_data = (char *)&ts_config;
 
   if (ioctl(sock_fd, SIOCSHWTSTAMP, &req)) {
     DEBUG_LOG("ioctl(%s) failed : %s", "SIOCSHWTSTAMP", strerror(errno));
-    close(sock_fd);
+
-    return 0;
+    /* Check the current timestamping configuration in case this interface
+       allows only reading of the configuration and it was already configured
+       as requested */
+    req.ifr_data = (char *)&ts_config;
+#ifdef SIOCGHWTSTAMP
+    if (ioctl(sock_fd, SIOCGHWTSTAMP, &req) ||
+        ts_config.tx_type != HWTSTAMP_TX_ON || ts_config.rx_filter != rx_filter)
+#endif
+    {
+      close(sock_fd);
+      return 0;
+    }
   }
 
   close(sock_fd);
@@ -845,7 +856,12 @@ NIO_Linux_RequestTxTimestamp(struct msghdr *msg, int cmsglen, int sock_fd)
   /* Add control message that will enable TX timestamping for this message.
      Don't use CMSG_NXTHDR as the one in glibc is buggy for creating new
      control messages. */
-  cmsg = (struct cmsghdr *)((char *)CMSG_FIRSTHDR(msg) + cmsglen);
+
+  cmsg = CMSG_FIRSTHDR(msg);
+  if (!cmsg || cmsglen + CMSG_SPACE(sizeof (ts_tx_flags)) > msg->msg_controllen)
+    return cmsglen;
+
+  cmsg = (struct cmsghdr *)((char *)cmsg + cmsglen);
   memset(cmsg, 0, CMSG_SPACE(sizeof (ts_tx_flags)));
   cmsglen += CMSG_SPACE(sizeof (ts_tx_flags));
 

refclock.c

@@ -2,7 +2,7 @@
   chronyd/chronyc - Programs for keeping computer clocks accurate.
 
  **********************************************************************
- * Copyright (C) Miroslav Lichvar  2009-2011, 2013-2014, 2016-2018
+ * Copyright (C) Miroslav Lichvar  2009-2011, 2013-2014, 2016-2019
  * 
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of version 2 of the GNU General Public License as
@@ -350,7 +350,7 @@ RCL_CheckDriverOptions(RCL_Instance instance, const char **options)
        option = get_next_driver_option(instance, option)) {
     for (i = 0; options && options[i]; i++) {
       len = strlen(options[i]);
-      if (!strncmp(options[i], option, strlen(options[i])) &&
+      if (!strncmp(options[i], option, len) &&
           (option[len] == '=' || option[len] == '\0'))
         break;
     }

sysincl.h

@@ -37,6 +37,7 @@
 #include <glob.h>
 #include <grp.h>
 #include <inttypes.h>
+#include <limits.h>
 #include <math.h>
 #include <netinet/in.h>
 #include <pwd.h>

test/simulation/133-hwtimestamp

@@ -13,22 +13,25 @@ refclock_offset=10.0
 min_sync_time=4
 max_sync_time=20
 limit=200
-client_conf="hwtimestamp eth0"
+server_conf="hwtimestamp eth0"
-client_server_options="minpoll 0 maxpoll 0 minsamples 32"
+client_server_options="minpoll 0 maxpoll 0 minsamples 32 xleave"
 client_chronyd_options="-d"
 
-run_test || test_fail
+for client_conf in "hwtimestamp eth0" "hwtimestamp eth0
-check_chronyd_exit || test_fail
+acquisitionport 123"; do
-check_source_selection || test_fail
+	run_test || test_fail
-check_sync || test_fail
+	check_chronyd_exit || test_fail
+	check_source_selection || test_fail
+	check_sync || test_fail
 
-if check_config_h 'FEAT_DEBUG 1'; then
+	if check_config_h 'FEAT_DEBUG 1'; then
-	check_log_messages "HW clock samples" 190 200 || test_fail
+		check_log_messages "HW clock samples" 190 200 || test_fail
-	check_log_messages "HW clock reset" 0 0 || test_fail
+		check_log_messages "HW clock reset" 0 0 || test_fail
-	check_log_messages "Received.*tss=1" 1 1 || test_fail
+		check_log_messages "Received.*tss=1" 1 1 || test_fail
-	check_log_messages "Received.*tss=2" 390 400 || test_fail
+		check_log_messages "Received.*tss=2" 390 400 || test_fail
-	check_log_messages "update_tx_timestamp.*Updated" 50 140 || test_fail
+		check_log_messages "update_tx_timestamp.*Updated" 50 140 || test_fail
-	check_log_messages "update_tx_timestamp.*Unacceptable" 50 140 || test_fail
+		check_log_messages "update_tx_timestamp.*Unacceptable" 50 140 || test_fail
-fi
+	fi
+done
 
 test_pass

util.c

@@ -1179,6 +1179,101 @@ UTI_CheckDirPermissions(const char *path, mode_t perm, uid_t uid, gid_t gid)
 
 /* ================================================== */
 
+static int
+join_path(const char *basedir, const char *name, const char *suffix,
+          char *buffer, size_t length, LOG_Severity severity)
+{
+  const char *sep;
+
+  if (!basedir) {
+    basedir = "";
+    sep = "";
+  } else {
+    sep = "/";
+  }
+
+  if (!suffix)
+    suffix = "";
+
+  if (snprintf(buffer, length, "%s%s%s%s", basedir, sep, name, suffix) >= length) {
+    LOG(severity, "File path %s%s%s%s too long", basedir, sep, name, suffix);
+    return 0;
+  }
+
+  return 1;
+}
+
+/* ================================================== */
+
+FILE *
+UTI_OpenFile(const char *basedir, const char *name, const char *suffix,
+             char mode, mode_t perm)
+{
+  const char *file_mode;
+  char path[PATH_MAX];
+  LOG_Severity severity;
+  int fd, flags;
+  FILE *file;
+
+  severity = mode >= 'A' && mode <= 'Z' ? LOGS_FATAL : LOGS_ERR;
+
+  if (!join_path(basedir, name, suffix, path, sizeof (path), severity))
+    return NULL;
+
+  switch (mode) {
+    case 'r':
+    case 'R':
+      flags = O_RDONLY;
+      file_mode = "r";
+      if (severity != LOGS_FATAL)
+        severity = LOGS_DEBUG;
+      break;
+    case 'w':
+    case 'W':
+      flags = O_WRONLY | O_CREAT | O_EXCL;
+      file_mode = "w";
+      break;
+    case 'a':
+    case 'A':
+      flags = O_WRONLY | O_CREAT | O_APPEND;
+      file_mode = "a";
+      break;
+    default:
+      assert(0);
+      return NULL;
+  }
+
+try_again:
+  fd = open(path, flags, perm);
+  if (fd < 0) {
+    if (errno == EEXIST) {
+      if (unlink(path) < 0) {
+        LOG(severity, "Could not remove %s : %s", path, strerror(errno));
+        return NULL;
+      }
+      DEBUG_LOG("Removed %s", path);
+      goto try_again;
+    }
+    LOG(severity, "Could not open %s : %s", path, strerror(errno));
+    return NULL;
+  }
+
+  UTI_FdSetCloexec(fd);
+
+  file = fdopen(fd, file_mode);
+  if (!file) {
+    LOG(severity, "Could not open %s : %s", path, strerror(errno));
+    close(fd);
+    return NULL;
+  }
+
+  DEBUG_LOG("Opened %s fd=%d mode=%c", path, fd, mode);
+
+  return file;
+}
+
+/* ================================================== */
+
 void
 UTI_DropRoot(uid_t uid, gid_t gid)
 {

util.h

@@ -176,6 +176,17 @@ extern int UTI_CreateDirAndParents(const char *path, mode_t mode, uid_t uid, gid
    permissions and its uid/gid must match the specified values. */
 extern int UTI_CheckDirPermissions(const char *path, mode_t perm, uid_t uid, gid_t gid);
 
+/* Open a file.  The full path of the file is constructed from the basedir
+   (may be NULL), '/' (if basedir is not NULL), name, and suffix (may be NULL).
+   Created files have specified permissions (umasked).  Returns NULL on error.
+   The following modes are supported (if the mode is an uppercase character,
+   errors are fatal):
+   r/R - open an existing file for reading
+   w/W - open a new file for writing (remove existing file)
+   a/A - open an existing file for appending (create if does not exist) */
+extern FILE *UTI_OpenFile(const char *basedir, const char *name, const char *suffix,
+                          char mode, mode_t perm);
+
 /* Set process user/group IDs and drop supplementary groups */
 extern void UTI_DropRoot(uid_t uid, gid_t gid);