[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#969349: buster-pu: package chrony/3.4-4+deb10u1



Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian.org@packages.debian.org
Usertags: pu

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hi,

[ Reason ]
chrony versions prior to 3.5.1 are vulnerable to a symlink race when 
creating the PID file. CVE-2020-14367 has been assigned to this 
vulnerability.

In accordance with Salvatore Bonaccorso from the security team, no DSA 
has been released.

[ Impact ]
Data loss and a denial of service due to the path traversal are possible 
in some cases.
While that sounds worrisome, this vulnerabilily can’t be exploited using 
the default configuration provided by chrony on Debian, that’s why the 
security team marked it as “unimportant”.

[ Tests ]
I manually tested the proposed update to ensure that chronyd still runs 
fine using the default PID file location and an alternative one where 
the vulnerability could be exploited. I can confirm that the issue is 
fixed by the proposed patch and that no regression appeared while 
testing.

[ Risks ]
Most of the other major distributions provide this patch now with no 
apparent problems, so the risks seem quite low.

[ Checklist ]
  [X] *all* changes are documented in the d/changelog
  [X] I reviewed all changes and I approve them
  [X] attach debdiff against the package in (old)stable
  [X] the issue is verified as fixed in unstable

[ Changes ]
Switch from fopen() to the open() function 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.

[ Other info ]
I also took the oportunity to fix the autopkgtest of chrony which was 
failing on Buster since quite a while.

Cheers,
Vincent

-----BEGIN PGP SIGNATURE-----

iQJLBAEBCgA1FiEE/VQBlxWoTJPh4vI5ipzudlpxp4AFAl9NUNgXHHZpbmNlbnQu
ZGViaWFuQGZyZWUuZnIACgkQipzudlpxp4CUhA//drGMsuRybyVv99bmgdtEzxZu
uTsxCF2YL6W7aGxTJnKMfVYJ/PAtAbzjXWUZyYNkmL/vSsf+432slB2IYsDNzs5M
sAXUZkMl/G5BhQ8F25LwNQs7Xnplq0p5MZEcNF97P1G9RFIBjW4+7Rj5OHQDbxHr
odWcsgl9kSAQ/l7A8jJQODWd+n7/saeyUL3UHEiLiZUw/PWuOdxv/ekBtU6a4kzM
YWjCcuvzvRuoBZNIihWYCQcJfRdMdSGe/eehf3mfwAfTJB/PdapATiUGN7rFrnnM
jhjRfS9QlKBrOxKzsGdSaGz4JcmSc3VqOs2AmguQ3a11dWQ6kjc4PyQ+Un5aZ71M
9Qv/zG/P+YX8L56jYxQk+qBYsD/qP2/a2apVgMeEl146ECx1WdAm+vyBSJxgZYdM
sNmCDJfpR+2xg8CDcWQZFGmusbaDfgvD/wgU+STOWrvKkjo0M8pH7+aETmAEk7Wc
btyvooG44zL0OmQm0CAIISgMZpXXvFLyeLJcfv9HZSoc0GqcL2lrhrgsWHw7F8T0
VAOMC3IiAriuSJuIIfCZI7ZDVklCr/xZK4S1RwSIJXSY4RmWVqFh5mDTZFVQ2ZIl
2vJcQQJhnHy8q1N14nHEcBsNffEB/O5caerl7DBs8MpCgLyo+vMJ7ifUa/4xqpae
e8KuReDpa8ZtwLuDTCQ=
=lYFE
-----END PGP SIGNATURE-----
diff -Nru chrony-3.4/debian/changelog chrony-3.4/debian/changelog
--- chrony-3.4/debian/changelog	2019-03-18 19:35:34.000000000 +0100
+++ chrony-3.4/debian/changelog	2020-08-29 20:13:04.000000000 +0200
@@ -1,3 +1,15 @@
+chrony (3.4-4+deb10u1) buster; urgency=medium
+
+  * debian/patches/:
+    - Add create-new-file-when-writing-pidfile.patch to prevent symlink race
+    when writing to PID file (CVE-2020-14367).
+
+  * debian/tests/:
+    - Fix a regression when running upstream-simulation-test-suite autopkgtest
+    on Buster.
+
+ -- Vincent Blut <vincent.debian@free.fr>  Sat, 29 Aug 2020 20:13:04 +0200
+
 chrony (3.4-4) unstable; urgency=medium
 
   * debian/patches/*:
diff -Nru chrony-3.4/debian/.gitlab-ci.yml chrony-3.4/debian/.gitlab-ci.yml
--- chrony-3.4/debian/.gitlab-ci.yml	2019-03-18 19:35:34.000000000 +0100
+++ chrony-3.4/debian/.gitlab-ci.yml	2020-08-26 18:41:29.000000000 +0200
@@ -1,20 +1,7 @@
-include: https://salsa.debian.org/salsa-ci-team/pipeline/raw/master/salsa-ci.yml
+include:
+  - https://salsa.debian.org/salsa-ci-team/pipeline/raw/master/salsa-ci.yml
+  - https://salsa.debian.org/salsa-ci-team/pipeline/raw/master/pipeline-jobs.yml
 
-build:
-    extends: .build-unstable
 
-reprotest:
-    extends: .test-reprotest
-
-lintian:
-    extends: .test-lintian
-
-autopkgtest:
-    extends: .test-autopkgtest
-    allow_failure: true
-
-piuparts:
-    extends: .test-piuparts
-
-blhc:
-    extends: .test-blhc
+variables:
+  RELEASE: 'buster'
diff -Nru chrony-3.4/debian/patches/create-new-file-when-writing-pidfile.patch chrony-3.4/debian/patches/create-new-file-when-writing-pidfile.patch
--- chrony-3.4/debian/patches/create-new-file-when-writing-pidfile.patch	1970-01-01 01:00:00.000000000 +0100
+++ chrony-3.4/debian/patches/create-new-file-when-writing-pidfile.patch	2020-08-26 18:41:29.000000000 +0200
@@ -0,0 +1,187 @@
+From f00fed20092b6a42283f29c6ee1f58244d74b545 Mon Sep 17 00:00:00 2001
+From: Miroslav Lichvar <mlichvar@redhat.com>
+Date: Thu, 6 Aug 2020 09:31:11 +0200
+Subject: main: create new file when writing pidfile
+
+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 2fc8edacb810 ("use PATH_MAX")
+- commit f4c6a00b2a11 ("logging: call exit() in LOG_Message()")
+- commit 7a4c396bba8f ("util: add functions for common file operations")
+- commit e18903a6b563 ("switch to new util file functions")
+
+Reported-by: Matthias Gerstner <mgerstner@suse.de>
+
+--- a/logging.c
++++ b/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);
+--- a/main.c
++++ b/main.c
+@@ -281,13 +281,9 @@ write_pidfile(void)
+   if (!pidfile[0])
+     return;
+ 
+-  out = fopen(pidfile, "w");
+-  if (!out) {
+-    LOG_FATAL("Could not open %s : %s", pidfile, strerror(errno));
+-  } else {
+-    fprintf(out, "%d\n", (int)getpid());
+-    fclose(out);
+-  }
++  out = UTI_OpenFile(NULL, pidfile, NULL, 'W', 0644);
++  fprintf(out, "%d\n", (int)getpid());
++  fclose(out);
+ }
+ 
+ /* ================================================== */
+--- a/sysincl.h
++++ b/sysincl.h
+@@ -37,6 +37,7 @@
+ #include <glob.h>
+ #include <grp.h>
+ #include <inttypes.h>
++#include <limits.h>
+ #include <math.h>
+ #include <netdb.h>
+ #include <netinet/in.h>
+--- a/util.c
++++ b/util.c
+@@ -1179,6 +1179,101 @@ UTI_CheckDirPermissions(const char *path
+ 
+ /* ================================================== */
+ 
++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)
+ {
+--- a/util.h
++++ b/util.h
+@@ -176,6 +176,17 @@ extern int UTI_CreateDirAndParents(const
+    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);
+ 
diff -Nru chrony-3.4/debian/patches/series chrony-3.4/debian/patches/series
--- chrony-3.4/debian/patches/series	2019-03-18 19:35:34.000000000 +0100
+++ chrony-3.4/debian/patches/series	2020-08-26 18:41:29.000000000 +0200
@@ -2,3 +2,4 @@
 allow-waitpid-in-seccomp-filter.patch
 allow-recv-send-in-seccomp-filter.patch
 allow-further-syscalls-in-seccomp-filter.patch
+create-new-file-when-writing-pidfile.patch
diff -Nru chrony-3.4/debian/tests/upstream-simulation-test-suite chrony-3.4/debian/tests/upstream-simulation-test-suite
--- chrony-3.4/debian/tests/upstream-simulation-test-suite	2019-03-18 19:35:34.000000000 +0100
+++ chrony-3.4/debian/tests/upstream-simulation-test-suite	2020-08-26 18:59:17.000000000 +0200
@@ -14,11 +14,13 @@
 cd test/simulation
 
 if [ ! -d clknetsim ]; then
-    git clone https://github.com/mlichvar/clknetsim
+    if git clone https://github.com/mlichvar/clknetsim; then
+        cd clknetsim && git checkout 58c5e8b
+    fi
 fi
 
-if [ ! -x "clknetsim/clknetsim" ] && [ ! -e "clknetsim/clknetsim.so" ]; then
-    make -C clknetsim
+if [ ! -x "clknetsim" ] && [ ! -e "clknetsim.so" ]; then
+    make
 fi
 
-./run -i 20 -m 2
+cd - && ./run -i 20 -m 2

Reply to: