[Git][NTPsec/ntpsec][master] 10 commits: CVE-2015-7975: Add missing length check to nextvar()

Daniel Fox Franke gitlab at mg.gitlab.com
Thu Jan 21 00:26:18 UTC 2016


Daniel Fox Franke pushed to branch master at NTPsec / ntpsec


Commits:
9bd05b08 by Daniel Fox Franke at 2016-01-20T11:37:31-05:00
CVE-2015-7975: Add missing length check to nextvar()

- - - - -
e31cdda3 by Daniel Fox Franke at 2016-01-20T11:37:59-05:00
CVE-2015-8139: Don't allow rv of expected origin timestamp

- - - - -
bd103818 by Daniel Fox Franke at 2016-01-20T11:38:06-05:00
CVE-2015-8138: fix logic error accepting zero origin timestamps

- - - - -
16277376 by Daniel Fox Franke at 2016-01-20T11:38:16-05:00
CVE-2015-8158: Avoid infinite loop processing responses in ntpq

- - - - -
1b14fb93 by Daniel Fox Franke at 2016-01-20T11:40:16-05:00
CVE-2015-7979: Don't demobilize in response to unauthenticated packet

- - - - -
1eb96e5e by Daniel Fox Franke at 2016-01-20T14:28:23-05:00
Make MAC verification run in constant time

- - - - -
5b4aac16 by Daniel Fox Franke at 2016-01-20T14:38:40-05:00
CVE-2015-7973: prevent replay in authenticated broadcast mode

- - - - -
576d06e9 by Daniel Fox Franke at 2016-01-20T14:57:11-05:00
Make sure saveconfig doesn't get re-enabled by accident

This is dangerous functionality. Make sure it doesn't get re-enabled
by accident while we're deciding whether to repair or remove it.

- - - - -
36ec8063 by Daniel Fox Franke at 2016-01-20T15:24:08-05:00
Add missing length checks to decodearr and outputarr

These function are awful! This commit should make them non-vulnerable,
but they desperately want to be rewritten.

- - - - -
74bd2def by Daniel Fox Franke at 2016-01-20T15:31:31-05:00
Draft release notes for 0.9.1

- - - - -


6 changed files:

- NEWS
- libntp/a_md5encrypt.c
- ntpd/ntp_control.c
- ntpd/ntp_proto.c
- ntpq/ntpq.c
- pylib/configure.py


Changes:

=====================================
NEWS
=====================================
--- a/NEWS
+++ b/NEWS
@@ -6,8 +6,69 @@ Much of the traditional function of a news file is now better addressed
 by browsing the comments in the revision history.  This file will focus
 on user-visible changes.
 
-== 2016-01-19: 0.9.1 (pending) ==
-
+== 2016-01-20: 0.9.1 (pending) ==
+
+Point release for security. Fixes:
+
+* CVE-2015-7973: Replay attack on authenticated broadcast mode
+  (Aanchal Malhotra)
+* CVE-2015-7975: nextvar() missing length check (Jonathan Gardner)
+* CVE-2015-7979: Off-path Denial of Service (DoS) attack on
+  authenticated broadcast and other preemptable modes (Aanchal
+  Malhotra)
+* CVE-2015-8138: Zero Origin Timestamp Bypass (Matthew van Gundy &
+  Jonathan Gardner)
+* CVE-2015-8139: Origin Leak: ntpq and ntpdc Disclose Origin Timestamp
+  to Unauthenticated Clients (Matthew van Gundy)
+* CVE-2015-8158: Potential Infinite Loop in ntpq (Jonathan Gardner)
+* Timing attack on MAC verification (Daniel Franke)
+* Missing length checks in decodearr() and outputarr() (Daniel Franke)
+
+Two additional security issues hve been reported to us for which we
+are not implementing code changes, but the user should be aware of
+their impact.
+
+The first (CVE-2015-8140) pertains to NTP's dynamic reconfiguration
+feature, which permits on-the-fly modification of NTP's configuration
+via ntpq. This feature is rarely used, typically disabled, and can
+only be enabled when authentication is configured. Ntpd has no means
+of detecting that a request to change its configuration is a replay of
+an old packet. Therefore, if an administrator sets ntpd to
+configuration A and then to configuration B, an attacker who captures
+the packets commanding these changes can replay the first one and
+restore ntpd's state to configuration A. This is only a concern when
+the configuration commands are sent over an untrusted
+network. Configuration changes made via localhost are not susceptible.
+
+This is an inherent design flaw in NTP cryptography and in the remote
+reconfiguration protocol, and can be fixed only with a considerable
+reworking and by changing the protocol in a way that is neither
+forward nor backward compatible. This cryptographic rework is on the
+horizon in the form of Network Time Security (currently a draft in the
+IETF network time working group). Given that this vulnerability
+impacts few if any real users, we have chosen to defer fixing it until
+we have tools more suitable to the task. For the mean time, if you
+rely on NTP's reconfiguration support, we recommend either restricting
+its use to localhost or trusted networks, or tunneling through SSH or
+a VPN. The 'nomodify' option to the 'restrict' directive may be used
+to enforce this policy.
+
+The second (CVE-2015-7974) pertains to the fact that when multiple
+trusted keys are configured, no mechanism exists to associate
+particular keys with particular peers or assign particular privileges.
+This is not a bug, per se, but rather a lack of expressiveness in
+NTP's configuration language. We intend to address in a future release
+as part of a larger redesign aimed at giving clearer semantics to the
+configuration language and making it easier to write safe
+configurations.
+
+Note that NTPsec is not impacted by CVE-2015-7976, CVE-2015-7977, or
+CVE-2015-7978. CVE-2015-7977 and CVE-2015-7978 both pertain to mode 7
+packets, support for which was completely removed before NTPsec's
+first beta. CVE-2015-7976 is a feature request to restrict the format
+of filenames used in saveconfig commands. Saveconfig support is
+disabled at compile time in NTPsec and will not be re-enabled without
+much more extensive hardening.
 
 == 2015-11-16: 0.9.0 ==
 


=====================================
libntp/a_md5encrypt.c
=====================================
--- a/libntp/a_md5encrypt.c
+++ b/libntp/a_md5encrypt.c
@@ -4,12 +4,38 @@
 #include <config.h>
 
 #include <string.h>
+#include <stddef.h>
+#include <stdbool.h>
+#include <stdint.h>
 
 #include "ntp_fp.h"
 #include "ntp_stdlib.h"
 #include "ntp.h"
 #include "ntp_md5.h"	/* provides OpenSSL digest API */
 
+/* ctmemeq - test two blocks memory for equality without leaking
+ * timing information.
+ *
+ * Return value: true if the two blocks of memory are equal, false
+ * otherwise.
+ *
+ * TODO: find out if this is useful elsewhere and if so move
+ * it to a more appropriate place and give it a prototype in a
+ * header file.
+ */
+static bool ctmemeq(const void *s1, const void *s2, size_t n) {
+	const uint8_t *a = s1;
+	const uint8_t *b = s2;
+	uint8_t accum = 0;
+        ptrdiff_t i;
+
+	for(i=0; i<n; i++) {
+		accum |= a[i] ^ b[i];
+	}
+
+	return accum == 0;
+}
+
 /*
  * MD5authencrypt - generate message digest
  *
@@ -93,7 +119,7 @@ MD5authdecrypt(
 		    "MAC decrypt: MAC length error");
 		return (0);
 	}
-	return !memcmp(digest, (char *)pkt + length + 4, len);
+	return (int)ctmemeq(digest, (char *)pkt + length + 4, len);
 }
 
 /*


=====================================
ntpd/ntp_control.c
=====================================
--- a/ntpd/ntp_control.c
+++ b/ntpd/ntp_control.c
@@ -251,7 +251,7 @@ static const struct ctl_proc control_codes[] = {
 #define	CP_ROOTDISPERSION	15
 #define	CP_REFID		16
 #define	CP_REFTIME		17
-#define	CP_ORG			18
+/* #define	CP_ORG			18 */
 #define	CP_REC			19
 #define	CP_XMT			20
 #define	CP_REACH		21
@@ -491,7 +491,9 @@ static const struct ctl_var peer_var[] = {
 	{ CP_ROOTDISPERSION, RO, "rootdisp" },	/* 15 */
 	{ CP_REFID,	RO, "refid" },		/* 16 */
 	{ CP_REFTIME,	RO, "reftime" },	/* 17 */
-	{ CP_ORG,	RO, "org" },		/* 18 */
+        /* This variable is disabled because leaking it creates a
+           vulnerability */
+        /*	{ CP_ORG,	RO, "org" }, */	/* 18 */
 	{ CP_REC,	RO, "rec" },		/* 19 */
 	{ CP_XMT,	RO, "xleave" },		/* 20 */
 	{ CP_REACH,	RO, "reach" },		/* 21 */
@@ -2447,10 +2449,6 @@ ctl_putpeer(
 		ctl_putts(peer_var[id].text, &p->reftime);
 		break;
 
-	case CP_ORG:
-		ctl_putts(peer_var[id].text, &p->aorg);
-		break;
-
 	case CP_REC:
 		ctl_putts(peer_var[id].text, &p->dst);
 		break;


=====================================
ntpd/ntp_proto.c
=====================================
--- a/ntpd/ntp_proto.c
+++ b/ntpd/ntp_proto.c
@@ -1249,9 +1249,8 @@ receive(
 		peer->flash |= BOGON3;			/* unsynch */
 
 	/*
-	 * If the transmit timestamp duplicates a previous one, the
-	 * packet is a replay. This prevents the bad guys from replaying
-	 * the most recent packet, authenticated or not.
+	 * If the transmit timestamp duplicates the previous one, the
+	 * packet is a duplicate.
 	 */
 	} else if (L_ISEQU(&peer->xmt, &p_xmt)) {
 		peer->flash |= BOGON1;			/* duplicate */
@@ -1273,13 +1272,33 @@ receive(
 			return;
 		}
 
+		/* In basic (non-interleaved) broadcast mode, origin
+		 * timestamps are zero. This is problematic because,
+		 * when authentication is not enabled, origin timestamp
+		 * checking is our only real line of defense to prevent
+		 * spoofing by off-path attackers. Enabling
+		 * authentication helps, but then we need some means
+		 * of replay detection. Our solution is to reject
+		 * packets whose transmit timestamp is earlier than
+		 * one which was previously seen. This should be enforced
+		 * *only* if authentication is enabled, because otherwise
+		 * it results in an easy DoS by sending a spoofed packet
+		 * with the transmit timestamp far in the future.
+		 */
+		
+		if((restrict_mask & RES_DONTTRUST) &&
+		   L_ISGEQ(&peer->xmt, &p_xmt)) {
+			peer->flash |= BOGON1;
+			peer->oldpkt++;
+			return;
+		}
 	/*
 	 * Check for bogus packet in basic mode. If found, switch to
 	 * interleaved mode and resynchronize, but only after confirming
 	 * the packet is not bogus in symmetric interleaved mode.
 	 */
 	} else if (peer->flip == 0) {
-		if (!L_ISEQU(&p_org, &peer->aorg)) {
+		if (!L_ISEQU(&p_org, &peer->aorg) || L_ISZERO(&p_org)) {
 			peer->bogusorg++;
 			peer->flash |= BOGON2;	/* bogus */
 			if (!L_ISZERO(&peer->dst) && L_ISEQU(&p_org,
@@ -1319,14 +1338,6 @@ receive(
 		report_event(PEVNT_AUTH, peer, "crypto_NAK");
 		peer->flash |= BOGON5;		/* bad auth */
 		peer->badauth++;
-		if (peer->flags & FLAG_PREEMPT) {
-			unpeer(peer);
-			return;
-		}
-#ifdef ENABLE_AUTOKEY
-		if (peer->crypto)
-			peer_clear(peer, "AUTH");
-#endif	/* ENABLE_AUTOKEY */
 		return;
 
 	/*
@@ -1345,10 +1356,6 @@ receive(
 		if (has_mac &&
 		    (hismode == MODE_ACTIVE || hismode == MODE_PASSIVE))
 			fast_xmit(rbufp, MODE_ACTIVE, 0, restrict_mask);
-		if (peer->flags & FLAG_PREEMPT) {
-			unpeer(peer);
-			return;
-		}
 #ifdef ENABLE_AUTOKEY
 		if (peer->crypto)
 			peer_clear(peer, "AUTH");


=====================================
ntpq/ntpq.c
=====================================
--- a/ntpq/ntpq.c
+++ b/ntpq/ntpq.c
@@ -877,6 +877,7 @@ getresponse(
 	fd_set fds;
 	int n;
 	int errcode;
+	int bail = 0;
 
 	/*
 	 * This is pretty tricky.  We may get between 1 and MAXFRAG packets
@@ -901,6 +902,14 @@ getresponse(
 	 */
 	for (;;) {
 
+                /* Discarding various invalid packets can cause us to
+                   loop more than MAXFRAGS times, but enforce a sane bound
+                   on how long we're willing to spend here. */
+		if(bail++ >= (2*MAXFRAGS)) {
+                        warning("too many packets in response; bailing out");
+			return ERR_TOOMUCH;
+                }
+
 		if (numfrags == 0)
 			tvo = tvout;
 		else
@@ -2066,8 +2075,12 @@ decodearr(
 		    break;
 
 		bp = buf;
-		while (!isspace((int)*cp) && *cp != '\0')
+		while (!isspace((int)*cp) && *cp != '\0') {
 		    *bp++ = *cp++;
+		    if(bp >= buf + sizeof buf)
+		        return false;
+		}
+
 		*bp++ = '\0';
 
 		if (!decodetime(buf, lfp))
@@ -2861,6 +2874,8 @@ nextvar(
 	len = srclen;
 	while (len > 0 && isspace((unsigned char)cp[len - 1]))
 		len--;
+        if(len >= sizeof name)
+                return 0;
 	if (len > 0)
 		memcpy(name, cp, len);
 	name[len] = '\0';
@@ -3077,7 +3092,11 @@ outputarr(
 	register char *cp;
 	register int i;
 	register int len;
-	char buf[256];
+	char *buf;
+
+	REQUIRE(narr >= 0 && narr <= MAXVALLEN);
+	buf = malloc(16 + 8*narr);
+	ENSURE(buf != NULL);
 
 	bp = buf;
 	/*
@@ -3105,6 +3124,7 @@ outputarr(
 	}
 	*bp = '\0';
 	output(fp, name, buf);
+	free(buf);
 }
 
 static char *


=====================================
pylib/configure.py
=====================================
--- a/pylib/configure.py
+++ b/pylib/configure.py
@@ -76,9 +76,6 @@ def cmd_configure(ctx):
 	if ctx.options.enable_debug_gdb:
 		ctx.env.CFLAGS += ["-g"]
 
-	if ctx.options.enable_saveconfig:
-		ctx.define("SAVECONFIG", 1)
-
 	if not ctx.options.disable_debug:
 		ctx.define("DEBUG", 1)
 		ctx.env.BISONFLAGS += ["--debug"]



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/bdacafa9bb2adaa14e3738fa24cae631e947c750...74bd2defce5844b89bd80d962ba0294cc3356f35
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ntpsec.org/pipermail/vc/attachments/20160121/1bd28279/attachment.html>


More information about the vc mailing list