From: Danny Mayer <mayer@ntp.org>
Date: Fri, 5 Feb 2016 04:29:08 +0000 (+0000)
Subject: Validate crypto-NAK's
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2f8f8acfd13aa5a773f550b7f81f87300ae8ab3c;p=thirdparty%2Fntp.git

Validate crypto-NAK's

bk: 56b42514rgZyUCPCWq2Uyhw4BWUpSg
---

diff --git a/ChangeLog b/ChangeLog
index c70fe8fc5..3bb8f305a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,4 @@
+* [Bug 3007] Validate crypto-NAKs
 ---
 
 * [Bug 2994] Systems with HAVE_SIGNALED_IO fail to compile. perlinger@ntp.org
diff --git a/include/ntp.h b/include/ntp.h
index 6a4e9aa6f..9d7407d7c 100644
--- a/include/ntp.h
+++ b/include/ntp.h
@@ -391,6 +391,7 @@ struct peer {
 	u_long	received;	/* packets received */
 	u_long	processed;	/* packets processed */
 	u_long	badauth;	/* bad authentication (TEST5) */
+	u_long	badNAK;		/* invalid crypto-NAK */
 	u_long	bogusorg;	/* bogus origin (TEST2, TEST3) */
 	u_long	oldpkt;		/* old duplicate (TEST1) */
 	u_long	seldisptoolarge; /* bad header (TEST6, TEST7) */
@@ -545,6 +546,7 @@ struct pkt {
 	l_fp	rec;		/* receive time stamp */
 	l_fp	xmt;		/* transmit time stamp */
 
+#define	MIN_V4_PKT_LEN	(12 * sizeof(u_int32)) /* min header length */
 #define	LEN_PKT_NOMAC	(12 * sizeof(u_int32)) /* min header length */
 #define MIN_MAC_LEN	(1 * sizeof(u_int32))	/* crypto_NAK */
 #define MAX_MD5_LEN	(5 * sizeof(u_int32))	/* MD5 */
diff --git a/ntpd/ntp_proto.c b/ntpd/ntp_proto.c
index ad454099f..6d6658b31 100644
--- a/ntpd/ntp_proto.c
+++ b/ntpd/ntp_proto.c
@@ -50,6 +50,12 @@ enum kiss_codes {
 	UNKNOWNKISS			/* Unknown Kiss Code */
 };
 
+enum nak_error_codes {
+	NONAK,				/* No NAK seen */
+	INVALIDNAK,			/* NAK cannot be used */
+	VALIDNAK			/* NAK is valid */
+};
+
 /*
  * traffic shaping parameters
  */
@@ -252,6 +258,63 @@ kiss_code_check(
 	}
 }
 
+/* 
+ * Check that NAK is valid
+ */
+BOOL
+valid_NAK(
+	  struct peer *peer,
+	  struct recvbuf *rbufp,
+	  u_char hismode
+	  )
+{
+	int base_packet_length = MIN_V4_PKT_LEN;
+	int remainder_size;
+	struct pkt *rpkt;
+	int keyid;
+
+	/*
+	 * Check to see if there is something beyond the basic packet
+	 */
+	if (rbufp->recv_length == base_packet_length) {
+		return NONAK;
+	}
+
+	remainder_size = rbufp->recv_length - base_packet_length;
+	/*
+	 * Is this a potential NAK?
+	 */
+	if (remainder_size != 4) {
+		return NONAK;
+	}
+
+	/*
+	 * Only server responses can contain NAK's
+	 */
+
+	if (hismode != MODE_SERVER) {
+		return (INVALIDNAK);
+	}
+
+	/* 
+	 * Make sure that the extra field in the packet is all zeros
+	 */
+	rpkt = &rbufp->recv_pkt;
+	keyid = ntohl(((u_int32 *)rpkt)[base_packet_length / 4]);
+	if (keyid != 0) {
+		return (INVALIDNAK);
+	}
+
+	/* 
+	 * Only valid if peer uses a key
+	 */
+	if (peer->keyid > 0 || peer->flags & FLAG_SKEY) {
+		return (VALIDNAK);
+	}
+	else {
+		return (INVALIDNAK);
+	}
+}
 
 /*
  * transmit - transmit procedure called by poll timeout
@@ -493,6 +556,7 @@ receive(
 	int	has_mac;		/* length of MAC field */
 	int	authlen;		/* offset of MAC field */
 	int	is_authentic = 0;	/* cryptosum ok */
+	int	crypto_nak_test;	/* result of crypto-NAK check */
 	int	retcode = AM_NOMATCH;	/* match code */
 	keyid_t	skeyid = 0;		/* key IDs */
 	u_int32	opcode = 0;		/* extension field opcode */
@@ -617,6 +681,7 @@ receive(
 	 * extension field is present, so we subtract the length of the
 	 * field and go around again.
 	 */
+
 	authlen = LEN_PKT_NOMAC;
 	has_mac = rbufp->recv_length - authlen;
 	while (has_mac > 0) {
@@ -767,6 +832,17 @@ receive(
 	 * is zero, acceptable outcomes of y are NONE and OK. If x is
 	 * one, the only acceptable outcome of y is OK.
 	 */
+	crypto_nak_test = valid_NAK(peer, rbufp, hismode);
+
+	/*
+	 * Drop any invalid crypto-NAKs
+	 */
+	if (crypto_nak_test == INVALIDNAK) {
+		peer->badNAK++;
+		msyslog(LOG_ERR, "crypto-NAK error at %ld %s<-%s", 
+			current_time, stoa(dstadr_sin), stoa(&rbufp->recv_srcadr));
+		return;
+	}
 
 	if (has_mac == 0) {
 		restrict_mask &= ~RES_MSSNTP;
@@ -777,7 +853,7 @@ receive(
 			    authlen,
 			    ntohl(pkt->org.l_ui), ntohl(pkt->org.l_uf),
 			    ntohl(pkt->xmt.l_ui), ntohl(pkt->xmt.l_uf)));
-	} else if (has_mac == 4) {
+	} else if (crypto_nak_test == VALIDNAK) {
 		restrict_mask &= ~RES_MSSNTP;
 		is_authentic = AUTH_CRYPTO; /* crypto-NAK */
 		DPRINTF(2, ("receive: at %ld %s<-%s mode %d/%s:%s keyid %08x len %d auth %d org %#010x.%08x xmt %#010x.%08x MAC4\n",
@@ -1507,7 +1583,7 @@ receive(
 	 * client packet. The server might have just changed keys. Clear
 	 * the association and restart the protocol.
 	 */
-	if (is_authentic == AUTH_CRYPTO) {
+	if (crypto_nak_test == VALIDNAK) {
 		report_event(PEVNT_AUTH, peer, "crypto_NAK");
 		peer->flash |= TEST5;		/* bad auth */
 		peer->badauth++;