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

Bug#925265: marked as done (unblock: ntp/1:4.2.8p12+dfsg-4 or ntp/1:4.2.8p13+dfsg-2 (pre-approval))

Your message dated Wed, 17 Apr 2019 08:13:57 +0200
with message-id <8c6df206-94c5-99d7-080b-9a4f5124b615@debian.org>
and subject line Re: unblock: ntp/1:4.2.8p12+dfsg-4 or ntp/1:4.2.8p13+dfsg-2 (pre-approval)
has caused the Debian Bug report #925265,
regarding unblock: ntp/1:4.2.8p12+dfsg-4 or ntp/1:4.2.8p13+dfsg-2 (pre-approval)
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
925265: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=925265
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Hi,

I'd like to inquiry about the feasibility of an updated ntp package being
unblocked for Buster.

On March 7th (a few days after the effective start of the Full Freeze) upstream
released 4.2.8p13 containing

"1 medium-severity security issue in ntpd, and provides 17 other non-security
fixes and 1 improvements over 4.2.8p12."

The security issue is https://security-tracker.debian.org/tracker/CVE-2019-8936
, a null dereference attack.

The upstream changelog lists

---
(4.2.8p13) 2019/03/07 Released by Harlan Stenn <stenn@ntp.org>

* [Sec 3565] Crafted null dereference attack in authenticated
             mode 6 packet <perlinger@ntp.org>
  - reported by Magnus Stubman
* [Bug 3560] Fix build when HAVE_DROPROOT is not defined <perlinger@ntp.org>
  - applied patch by Ian Lepore
* [Bug 3558] Crash and integer size bug <perlinger@ntp.org>
  - isolate and fix linux/windows specific code issue
* [Bug 3556] ntp_loopfilter.c snprintf compilation warnings <perlinger@ntp.org>
  - provide better function for incremental string formatting
* [Bug 3555] Tidy up print alignment of debug output from ntpdate <perlinger@ntp.org>
  - applied patch by Gerry Garvey
* [Bug 3554] config revoke stores incorrect value <perlinger@ntp.org>
  - original finding by Gerry Garvey, additional cleanup needed
* [Bug 3549] Spurious initgroups() error message <perlinger@ntp.org>
  - patch by Christous Zoulas
* [Bug 3548] Signature not verified on windows system <perlinger@ntp.org>
  - finding by Chen Jiabin, plus another one by me
* [Bug 3541] patch to fix STA_NANO struct timex units <perlinger@ntp.org>
  - applied patch by Maciej Szmigiero
* [Bug 3540] Cannot set minsane to 0 anymore <perlinger@ntp.org>
  - applied patch by Andre Charbonneau
* [Bug 3539] work_fork build fails when droproot is not supported <perlinger@ntp.org>
  - applied patch by Baruch Siach
* [Bug 3538] Build fails for no-MMU targets <perlinger@ntp.org>
  - applied patch by Baruch Siach
* [Bug 3535] libparse won't handle GPS week rollover <perlinger@ntp.org>
  - refactored handling of GPS era based on 'tos basedate' for
    parse (TSIP) and JUPITER clocks
* [Bug 3529] Build failures on Mac OS X 10.13 (High Sierra) <perlinger@ntp.org>
  - patch by Daniel J. Luke; this does not fix a potential linker
    regression issue on MacOS.
* [Bug 3527 - Backward Incompatible] mode7 clockinfo fudgeval2 packet
  anomaly <perlinger@ntp.org>, reported by GGarvey.
  - --enable-bug3527-fix support by HStenn
* [Bug 3526] Incorrect poll interval in packet <perlinger@ntp.org>
  - applied patch by Gerry Garvey
* [Bug 3471] Check for openssl/[ch]mac.h.  <perlinger@ntp.org>
  - added missing check, reported by Reinhard Max <perlinger@ntp.org>
* [Bug 1674] runtime crashes and sync problems affecting both x86 and x86_64
  - this is a variant of [bug 3558] and should be fixed with it
* Implement --disable-signalled-io
---

I'm really not that much of a NTP guru so I cannot judge most of these commits,
but generally ntp is not that great for getting fixes backported (they are
solely using BitKeeper, the git mirror has been outdated for years), so I'd
like to be as close to the release as possible. But I fully understand if you
said no, because quite frankly the diff is scary

226 files changed, 18625 insertions(+), 8325 deletions(-)

alas 99% of that is files being regenerated (automake version increased from
1.15 to 1.15.1, copyright updated, documentation regenerated). Oh, and 7760
changes to the CommitLog ...

The other option would be to only backport the fix for CVE-2019-8936 and upload
-4.

While processing this I discovered three small commits sitting in the master
branch that had not been uploaded yet. They could be backed out if you want to.
They fix important Bug#772790, normal Bug#764546 and remove a leftover from the
locking that used to be in place to prevent a race between ntpd and the ntpdate
ifupdown hooks. The ifupdown hooks have been removed since September, so the
locking is not necessary anymore.

Please tell me what you want me to upload

a) 1:4.2.8p13+dfsg-1 currently in experimental (as -2)
b) 1:4.2.8p13+dfsg-1 currently in experimental (as -2), but with some/all not
   previously uploaded changes to debian/ dropped
c) 1:4.2.8p12+dfsg-4 containing only the CVE fix
d) 1:4.2.8p12+dfsg-4 containing the CVE fix and some/all not previously uploaded
   changes to debian/

Thanks,
Bernhard

 debian/changelog                   | 10 ++++++++
 debian/ntp-systemd-wrapper         | 11 +--------
 debian/ntp.cron.daily              |  2 +-
 debian/ntp.init                    |  9 ++-----
 debian/patches/CVE-2019-8936.patch | 50 ++++++++++++++++++++++++++++++++++++++
 debian/patches/series              |  1 +
 6 files changed, 65 insertions(+), 18 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index e18dbb4..53cd77b 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+ntp (1:4.2.8p12+dfsg-4) unstable; urgency=medium
+
+  * CVE-2019-8936: Crafted null dereference attack in authenticated
+    mode 6 packet (Closes: #924228)
+  * ntp: exit 0 from init script if daemon does not exist (Closes: #764546)
+  * Drop locking from ntp initscript/systemd-wrapper
+  * Only delete ntpd statistics files in cronjob (Closes: #772790)
+
+ -- Bernhard Schmidt <berni@debian.org>  Thu, 21 Mar 2019 23:42:36 +0100
+
 ntp (1:4.2.8p12+dfsg-3) unstable; urgency=low
 
   * Treat testsuite errors as non-fatal on some architectures
diff --git a/debian/ntp-systemd-wrapper b/debian/ntp-systemd-wrapper
index 34e4767..8d5d1f2 100755
--- a/debian/ntp-systemd-wrapper
+++ b/debian/ntp-systemd-wrapper
@@ -2,7 +2,6 @@
 
 DAEMON=/usr/sbin/ntpd
 PIDFILE=/var/run/ntpd.pid
-LOCKFILE=/run/lock/ntpdate
 
 if [ -r /etc/default/ntp ]; then
         . /etc/default/ntp
@@ -18,13 +17,5 @@ if test "$(uname -s)" = "Linux"; then
         NTPD_OPTS="$NTPD_OPTS -u $UGID"
 fi
 
-# Protect the service startup against concurrent ntpdate ifup hooks
-(
-	if flock -w 180 9; then
-		exec $DAEMON -p $PIDFILE $NTPD_OPTS
-	else
-		echo "Timeout waiting for $LOCKFILE"
-		exit 1
-	fi
-) 9>$LOCKFILE
+exec $DAEMON -p $PIDFILE $NTPD_OPTS
 
diff --git a/debian/ntp.cron.daily b/debian/ntp.cron.daily
index 5a737fc..1fe0815 100644
--- a/debian/ntp.cron.daily
+++ b/debian/ntp.cron.daily
@@ -13,7 +13,7 @@ if [ -n "$statsdir" ] && [ -d "$statsdir" ]; then
 	# within the directory and do not descend into subdirectories
 	# to avoid security risks on platforms where find is not using
 	# fts-library.
-	find "$statsdir" -maxdepth 1 -type f -mtime +7 -delete
+	find "$statsdir" -maxdepth 1 -type f -name "*stats*" -mtime +7 -delete
 
 	# compress whatever is left to save space but make sure to really
 	# do it only in the expected directory.
diff --git a/debian/ntp.init b/debian/ntp.init
index 9074e5b..0e599ab 100644
--- a/debian/ntp.init
+++ b/debian/ntp.init
@@ -16,7 +16,7 @@ PATH=/sbin:/bin:/usr/sbin:/usr/bin
 DAEMON=/usr/sbin/ntpd
 PIDFILE=/var/run/ntpd.pid
 
-test -x $DAEMON || exit 5
+test -x $DAEMON || exit 0
 
 if [ -r /etc/default/ntp ]; then
 	. /etc/default/ntp
@@ -27,8 +27,6 @@ if [ -e /run/ntp.conf.dhcp ]; then
 fi
 
 
-LOCKFILE=/run/lock/ntpdate
-
 RUNASUSER=ntp
 UGID=$(getent passwd $RUNASUSER | cut -f 3,4 -d:) || true
 if test "$(uname -s)" = "Linux"; then
@@ -42,10 +40,7 @@ case $1 in
 			log_failure_msg "user \"$RUNASUSER\" does not exist"
 			exit 1
 		fi
-		(
-			flock -w 180 9
-			start-stop-daemon --start --quiet --oknodo --pidfile $PIDFILE --startas $DAEMON -- -p $PIDFILE $NTPD_OPTS
-		) 9>$LOCKFILE
+		start-stop-daemon --start --quiet --oknodo --pidfile $PIDFILE --startas $DAEMON -- -p $PIDFILE $NTPD_OPTS
 		log_end_msg $?
   		;;
 	stop)
diff --git a/debian/patches/CVE-2019-8936.patch b/debian/patches/CVE-2019-8936.patch
new file mode 100644
index 0000000..ab4f5b6
--- /dev/null
+++ b/debian/patches/CVE-2019-8936.patch
@@ -0,0 +1,50 @@
+diff --git a/ntpd/ntp_control.c b/ntpd/ntp_control.c
+index 48cd908..49a197e 100644
+--- a/ntpd/ntp_control.c
++++ b/ntpd/ntp_control.c
+@@ -3446,11 +3448,11 @@ write_variables(
+ 	 * Look through the variables. Dump out at the first sign of
+ 	 * trouble.
+ 	 */
+-	while ((v = ctl_getitem(sys_var, &valuep)) != 0) {
++	while ((v = ctl_getitem(sys_var, &valuep)) != NULL) {
+ 		ext_var = 0;
+ 		if (v->flags & EOV) {
+-			if ((v = ctl_getitem(ext_sys_var, &valuep)) !=
+-			    0) {
++			v = ctl_getitem(ext_sys_var, &valuep);
++			if (v != NULL) {
+ 				if (v->flags & EOV) {
+ 					ctl_error(CERR_UNKNOWNVAR);
+ 					return;
+@@ -3464,16 +3466,24 @@ write_variables(
+ 			ctl_error(CERR_PERMISSION);
+ 			return;
+ 		}
+-		if (!ext_var && (*valuep == '\0' || !atoint(valuep,
+-							    &val))) {
++		/* [bug 3565] writing makes sense only if we *have* a
++		 * value in the packet!
++		 */
++		if (valuep == NULL) {
+ 			ctl_error(CERR_BADFMT);
+ 			return;
+ 		}
+-		if (!ext_var && (val & ~LEAP_NOTINSYNC) != 0) {
+-			ctl_error(CERR_BADVALUE);
+-			return;
++		if (!ext_var) {
++			if ( !(*valuep && atoint(valuep, &val))) {
++				ctl_error(CERR_BADFMT);
++				return;
++			}
++			if ((val & ~LEAP_NOTINSYNC) != 0) {
++				ctl_error(CERR_BADVALUE);
++				return;
++			}
+ 		}
+-
++		
+ 		if (ext_var) {
+ 			octets = strlen(v->text) + strlen(valuep) + 2;
+ 			vareqv = emalloc(octets);
diff --git a/debian/patches/series b/debian/patches/series
index d6c2412..95a9d82 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -10,3 +10,4 @@ sntp-sysexits.patch
 debian-locfile.patch
 sntp-kod-location.patch
 disable-test-decodenetnum.patch
+CVE-2019-8936.patch

Attachment: ntp-4.2.8p13.diff.gz
Description: application/gzip


--- End Message ---
--- Begin Message --- 
Am 08.04.19 um 17:15 schrieb Ivo De Decker:

Hi,

> Both option c or d sound ok. Options a and b really don't sound like something
> we should do at this point.
> 
> So please go ahead with the 1:4.2.8p12+dfsg-4 upload and remove the moreinfo
> tag from this bug once the package is in unstable.

Has been uploaded a few days ago and someone thankfully already
unblocked it, has migrated to testing this night.

Thanks,
Bernhard

--- End Message ---
Reply to: